[GHC] #11796: Warn about unwanted instances in a modular way

[GHC]

#11796: Warn about unwanted instances in a modular way
-------------------------------------+-------------------------------------
        Reporter:  Lemming           |                Owner:
            Type:  feature request   |               Status:  new
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.1
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #11219            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------
Changes (by RyanGlScott):

 * related:   => #11219




New description:

 I like to propose the following way to warn about instances that are
 unwanted by some programmers. First step is to mark the instances at their
 definition site like so:
 {{{#!hs
 {-# WARN_INSTANCE tuple #-}
 instance Foldable ((,) a) where ...

 {-# WARN_INSTANCE tuple #-}
 instance Functor ((,) a) where ...

 {-# WARN_INSTANCE tuple #-}
 instance Foldable ((,,) a b) where ...

 {-# WARN_INSTANCE tuple #-}
 instance Functor ((,,) a b) where ...
 }}}

 This way, all the above instances are collected in an instance group
 labelled `tuple`. At the use sites we introduce a GHC warning option like
 `-fwarn-instance=tuple`. This warns about any place where any of the
 `tuple` instances is used. We can either place
 {{{
 GHC-Options: -fwarn-instance=tuple
 }}}
 in a Cabal package description in order to issue warnings in a whole
 package or we can put
 {{{
 {-# OPTIONS_GHC -fwarn-instance=tuple #-}
 }}}
 at the top of a module in order to enable the warning per module.

 Another candidate for an instance group might be `numeric` for numeric
 instances of functions and tuples in the `NumInstances` package.

 What does it mean to use an instance? I would say, if omitting an
 `instance X Y` would lead to a "missing instance" type error at place Z in
 a module, then `instance X Y` is used at place Z.


 There might be an even more restrictive option like `-fforbid-
 instance=tuple`.

 This would not only warn about an instance usage, but it would cause a
 type error. Essentially it should treat all `tuple` instances as if they
 were not defined. (Other instances might depend on `tuple` instances and
 if the `tuple` instances weren't there the compiler would not even reach
 the current module. I do not know, whether this case needs special
 treatment. We might require that any instance depending on `tuple` must be
 added to the `tuple` group as well or it might be added automatically.)

 The advantage of a type error is that we see all problems from `tuple`
 instances also in the presence of other type errors. Warnings would only
 show up after a module is otherwise type correct.

 This solution requires cooperation of the instance implementor. Would that
 work in practice? Otherwise we must think about ways to declare instance
 groups independently from the instance declaration and we get the problem
 of bringing the instance group names into the scope of the importing
 module.

 A separate discussion must be held on whether `-fwarn-instance=tuple`
 should be part of `-Wall`. I think that people should be warned about
 `tuple` instances early because they won't expect that there is a trap
 when using `length` and `maximum` and so on.


 One might also think about generalizations, e.g. whether
 {{{
 {-# WARN_INSTANCE tuple, functor #-}
 }}}
 should be allowed in order to put an instance in several groups or whether
 there should be a way to compose a group from subgroups.

 Another topic would be a form of instance group disambiguation. Instance
 groups might be qualified with module or package names. I think package
 names are more appropriate, like so `-fwarn-instance=base:tuple`.

--

Comment:

 I think I could get on board with this proposal, but I have some
 suggestions:

 * We currently have `WARNING` and `DEPRECATED` pragmas
 ([http://downloads.haskell.org/~ghc/8.0.1/docs/html/users_guide/glasgow_exts.html
 #warning-and-deprecated-pragmas link]) that serve a very similar role to
 what you're proposing. I think the key ingredient in your proposal that
 you're lacking is the ability to toggle their severity with separate
 flags. For example, we currently have:

   {{{#!hs
   {-# WARNING unsafePerformIO "Be careful" #-}
   }}}

   This always emits a warning. But we could envision a flags like
 `-Wpragma-error="unsafePerformIO"` or `-Wpragma-
 suppress="unsafePerformIO"` that would either elevate a `WARNING` pragma
 to an error or suppress its warning altogether, respectively. (We'd also
 need `-Wpragma-warn` for when we want to explicitly request a warning,
 rather than an error). So implementing `-Wpragma-error`, `-Wpragma-warn`,
 and `-Wpragma-suppress` seems like an important step.

 * Then we'd need to extend `WARNING`/`DEPRECATED` to be able to attach
 them to instances. Syntactically, I think this would make the most sense:

   {{{#!hs
   instance Foo Int where
     {-# WARNING instance FooInt "Beware of this instance" #-}
     ...
   }}}

   I think you'd need to put the pragma within the instance like so,
 because otherwise GHC would have a hard time figuring out which `WARNING`
 corresponds to which instance. We'd also need to give a unique string
 (e.g., `FooInt`) for the reason above.

 * We'd need a variation of `WARNING` that allows you to give it a unique
 string, but doesn't emit a warning by default when used (`SILENT` or
 `SILENT_WARNING`, perhaps?)

 * Lastly, we'd need some form of conflict resolution for unique strings. I
 could certainly see a scenario where a package has multiple `FooInt`
 strings floating around. Perhaps we could use a representation of
 `-Wpragma-warn=<package>:<module>:<uniq_string>`, where `<package>` and
 `<module>` are optional.

 Would this accomplish what you want?

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/11796#comment:3>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler