MINIMAL should warn about extra definitions

Currently, GHC issues no warnings if a class has a MINIMAL pragma requiring foo, and yet you also give a default definition for foo. This proposal suggests the addition of this warning.

Motivation

I was recently quite confused about an error message from GHC, originating from code like this:

class X a where
  foo :: a

  {-# MINIMAL foo #-}
  foo = undefined

instance X Int

For this, GHC says:

GHCi, version 8.6.3: http://www.haskell.org/ghc/  :? for help
[1 of 1] Compiling Main             ( a.hs, interpreted )

a.hs:7:10: warning: [-Wmissing-methods]
     No explicit implementation for
        foo
     In the instance declaration for X Int
  |
7 | instance X Int
  |

This is arguably correct, but not helpful. Since I made foo part of MINIMAL, GHC rightly complains the instance definition should not skip it. But that’s very confusing: because I know I just added a default definition for it. Of course, the mistake I made was that I forgot to remove it from the MINIMAL pragma; but I was not warned about that.

The important point is that the warning comes at the instantiation site instead of the class definition, which can be miles apart. Even worse: The class might be coming from a library and the library author was never warned about this obvious mistake.

This proposal suggests that GHC should warn about this discrepancy right at the point where the class X is defined, with a message of the form:

You made `foo` MINIMAL, but also gave an explicit definition for it.

Per usual GHC strategy, this can be tied to a flag named -Wredundant-minimal-methods or similar. This flag should be on by default. (Exact wording of the warning text and the flag name can be determined by the implementor.)

I have filed this as a feature request: https://gitlab.haskell.org/ghc/ghc/issues/16314. Simon PJ asked me to create a proposal so it can gather feedback.

Proposed Change Specification

The change is quite mimimal. First, we need a minor change to the user manual, where the grammar of MINIMAL descriptions are given as:

mindef ::= name
        |  '(' mindef ')'
        |  mindef '|' mindef
        |  mindef ',' mindef

This is already not quite correct, because GHC actually allows empty minimal declarations, as in {-# MINIMAL #-}, which isn’t in the language generated by this grammar. We should change it to:

mindef  :: <empty>
         | mindef1

mindef1 ::= name
         |  '(' mindef1 ')'
         |  mindef1 '|' mindef1
         |  mindef1 ',' mindef1

Abusing the notation in the obvious way, define the following function from a MINIMAL expression to a set of names:

required <empty>          = Set.empty
required name             = Set.singleton name
required ('(' expr ')')   = required expr
required (left '|' right) = required left `Set.intersection` required right
required (left ',' right) = required left `Set.union`        required right

For each class declaration with a MINIMAL pragma, compute:

D = set of all methods with default definitions
R = the required set, as defined above
E = D `Set.difference` R

Note that D should not contain definitions that have default signatures, i.e., those with a default-definition only at a more specific type with extra constraints. See below in the “Effects and Interactions” section for the motivation on this.

If E is not empty, then GHC should emit a warning saying the methods in E are required by the MINIMAL pragma but also are given a default definition. If E is empty, no warning is generated.

As per usual GHC strategy, this warning should be tied to a flag, -Wredundant-minimal-methods, though the implementor can choose something more appropriate. The flag should be on by default.

Simon PJ comments: The MINIMAL definition requires that every method m in R is defined in the instance declaration. Giving a default method for m, in the class declaration, is therefore redundant. The warning encourages the programmer either to remove the default definition or to adjust the MINIMAL pragma.

Effect and Interactions

If a method as a definition via the default signatures extension, then that definition should not be added to the set D as defined above. The motivation for this is that the author of the library provided a weaker definition (in the sense of the type) than required by the class for that particular method, and thus the author should be free to mention that method in the MINIMAL pragma without getting a warning.

The presence of a definition with a default signature should be something that’s checked at the instantiation site of this class, not at the definition site, for violations of the MINIMAL requirements.

Costs and Drawbacks

Cost: The compiler probably already has all the necessary bits and pieces to do this in short order. For someone familiar with that part of the code, I doubt it’s more than an afternoon worth of work; including test cases and integration.

Drawbacks: I don’t think there is any!

Alternatives

Do nothing. But in a large refactoring case (which prompted this proposal in the first place) it is much nicer to get warnings close to where the problem is, as opposed to later on. In the particular case of the class being defined in a library and the instance being in user code, this issue gets amplified as there is really nothing the user of the library can do.

Unresolved Questions

None.

Implementation Plan

TBD.