#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: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): What's the use-case for this? Is there a community of users that want it? What if two different library authors both happened to choose the same string for their "warn-instance" string? Thanks -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11796#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#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.h... #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>:`, 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

A unification with the WARNING pragma looks like a good idea to me. I

The machinery sketched in #11219 seems like a good way to switch between

#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): * cc: hvr (added) Comment: Replying to [comment:4 Lemming]: think I did not consider it, because currently warnings cannot be grouped. But if WARNINGs can be grouped we could re-use this capability for `instance` warnings. `WARNING`s can be grouped, actually: {{{#!hs hello = "hello" world = "world" {-# WARNING hello, world "Beware" #-} }}} But this is much easier for top-level identifiers, since they have clearly defined names. Instances, on the other hand, don't have a convenient shorthand for referring to them, so it's not clear to me how you could group them together at the top-level. I believe that due to the way GHC parses pragmas, it cannot know that a pragma is "next" to an instance, so you have to nest it inside the instance to make it clear that this pragma belongs to that particular instance. An alternative would perhaps be to specify the unique string for an instance with a separate pragma, e.g., {{{#!hs instance Foo Int where {-# INSTANCE_TAG FooInt #-} ... instance Foo Char where {-# INSTANCE_TAG FooChar #-} ... {-# WARNING FooInt, FooChar "Watch out" #-} }}} Then you could group together multiple instances within a single `WARNING` at the top level. the effect of hitting a warning. I agree! The difference here is that we're not elevating/suppressing warnings wholesale in this case, but rather configuring them on a tag-by- tag basis. Nonetheless, we'd need similar machinery to what #11219 needs.

For `instance`s the module name should not be part of the identifier. The definition of the instance can be anywhere in the package in a private module. This module can be renamed without even a minor version bump. But when I think about it, the same is true for function identifiers.

Indeed. In fact, there could be two different private modules in the same package that export two functions with the same name, or two instances with the same tag. I can't think of a better way to tell them apart, can you? Relying on the exact location is a bit dangerous, admittedly, but it's the best form of disambiguation I know of. Ideally, you wouldn't have to use `<package>:<module>` very much in the first place, but it'd be nice to have for the few cases where it matters. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11796#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#11796: Warn about unwanted instances in a modular way -------------------------------------+------------------------------------- Reporter: Lemming | Owner: (none) 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 bergmark): * cc: bergmark (added) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/11796#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler