Opened 5 years ago

Last modified 19 months ago

#10089 new feature request

feature: warn about unused data definitions (with typeclass instances)

Reported by: slyfox Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.8.4
Keywords: Instances Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect warning at compile-time Test Case:
Blocked By: Blocking:
Related Tickets: #3221 Differential Rev(s):
Wiki Page:

Description

Let's consider an example:

-- M.hs:
module M () where

-- ghc knows about uselessness of those
data D = D

-- but doesn't about this:
data E = E deriving (Show, Read)

ghc warns about useless D, but not E:

$ ghc-7.8.4 -Wall -c M.hs -fforce-recomp

M.hs:5:1: Warning:
    Defined but not used: type constructor or class ‘D’

M.hs:5:10: Warning: Defined but not used: data constructor ‘D’

There is no way to refer to E type outside or call it's unstances, correct?

I would expect no code should be generated for this module.

ghc could also warn about defined, but not used instances for types, that are not exported.

Some background on where those stray types come from:

  1. programmer usually adds a set of data types into their program (say, 10-15 types to represent an AST)
  2. then annotates them with 'deriving (Read, Show)' convenience instances
  3. (months later) amends AST representation a bit and leaves leftover data types not used anywhere

Does that make sense?

Thanks!

Change History (9)

comment:1 Changed 5 years ago by simonpj

Good idea. It would take a little work to implement, though.

A bit of background. Unused defintions are reported by RnNames.reportUnusedNames, which in turn consults the accumulated defs (definitions) and uses in the global tcg_dus. This in turn is set mainly by RnSource.rnSrcDecls.

The defs/uses field is of type NameSet.DefUses, a type synonym for [DefUse]. However, the list is supposed to be in dependency order, as you can see from NameSet.findUses.

Instance declarations are currently treated as defining nothing, but having a bunch of uses. This is a (Nothing, uses) pair in the DefUse terminology. But that means that anything mentioned in an instance is unconditionally treated as "used", with the effect you observe.

To do it right we'd really want to give the instance decl some "defs" too. Consider

instance Show T where
   show = g

It could get a DefUse like this: (Just {Show,T}, {g}). This expresses the idea that if Show or T were referenced somewhere else (or exported) then the instance declaration (and all the things it refers to, namely g) would be treated as used.

Actually you'd really want "and" rather than "or": if Show and T are both referenced somewhere else then the instance declarationis used.

The trouble is that we can't then give the [DefUse] in dependency order: an instance declaration might mention a function, which refers to another type, which is in the head of another instance declaration. The obvious thing to do would be to give up on the dependency-order claim of [DefUse] and just take its transitive closure as a relation. Not too hard.

So:

  • Treat DefUses as an un-ordered relation, and use transitive closure to find uses.
  • Give an instance decl I a DefUse like (used in head of I, used elsewhere in I).
  • Provide "and" semantics as well as "or" semantics for DefUse.

Not a big project, and worthwhile. Any volunteers?

Simon

comment:2 Changed 4 years ago by thomie

Keywords: newcomer added

comment:3 Changed 4 years ago by dfordivam

Owner: set to dfordivam

comment:4 Changed 4 years ago by dfordivam

comment:5 Changed 4 years ago by dfordivam

comment:6 Changed 4 years ago by dfordivam

Owner: dfordivam deleted

comment:7 Changed 4 years ago by thomie

Type of failure: None/UnknownIncorrect warning at compile-time

comment:8 Changed 4 years ago by thomie

Keywords: newcomer removed

comment:9 Changed 19 months ago by simonpj

Keywords: Instances added
Note: See TracTickets for help on using tickets.