[GHC] #16314: Improve confusing error message with MINIMAL pragma

#16314: Improve confusing error message with MINIMAL pragma -------------------------------------+------------------------------------- Reporter: lerkok | Owner: (none) Type: feature | Status: new request | Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: None/Unknown Unknown/Multiple | Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- I got bitten by this recently, and while I can see GHC's reasoning, I wish it told me something different and more useful. {{{#!hs class X a where foo :: a {-# MINIMAL foo #-} foo = undefined instance X Int }}} For this program, 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", since I made `foo` part of `MINIMAL`; but it's very confusing, because I know I added a default definition. I wish GHC instead said something like: {{{ You made `foo` MINIMAL, but also gave an explicit definition for it. }}} I can see the logic behind the current error message, but it was rather confusing. The background is that my class had many other methods and a `MINIMAL` pragma. Much later I realized I could give a default definition of one of those methods but I forgot to remove it from the `MINIMAL` list. So, I had to puzzle a while at the error messages that were coming from far away modules that had nothing to do with the class definition itself. This is not a showstopper by any means, but unless there are good reasons to do otherwise, it would be nice to get a warning right where the class is defined, as opposed to where it is instantiated. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/16314 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#16314: Improve confusing error message with MINIMAL pragma -------------------------------------+------------------------------------- Reporter: lerkok | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 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):
it would be nice to get a warning right where the class is defined, as opposed to where it is instantiated.
I'm not sure that would be a good change. Here's a typical use-case (taken from the user manual) {{{ class Eq a where (==), (/=) :: a -> a -> Bool (==) a b = not (a /= b) (/=) a b = not (a == b) {-# MINIMAL (==) | (/=) #-} }}} Both have default methods, but you must define one or the other, or you get an infinite loop. At an instance we specifically want a warning, even though both have default methods. This was the original motivation for MINIMAL. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/16314#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#16314: Improve confusing error message with MINIMAL pragma -------------------------------------+------------------------------------- Reporter: lerkok | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 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 lerkok): Thanks Simon. But I think your example is quite different. In your case the pragma says `f | g`. So, I do agree the definitions for them should not cause a warning. But if I said `MINIMAL f, g` (i.e., `f` and `g` *must* both be defined) and have also given a definition for one of them, surely GHC can warn about that? In case there's a complex "boolean" expression in `MINIMAL` (such as `f | g, h` etc.) this might be hard to determine, but pretty much all my use cases have been a straight list of functions and defining one should be easy to detect to violate the requirements of `MINIMAL`. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/16314#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#16314: Improve confusing error message with MINIMAL pragma -------------------------------------+------------------------------------- Reporter: lerkok | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 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): OK, fine. This sounds like a feature request! The next thing to do is to write down a specification that says precisely what warnings you'd like to see under what circumstances. Maybe even worth a GHC proposal; but certainly a specification! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/16314#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#16314: Improve confusing error message with MINIMAL pragma -------------------------------------+------------------------------------- Reporter: lerkok | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 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 lerkok): Thanks Simon. Will do! I think the logic is quite simple even with the boolean operators, and hopefully should be trivial to implement. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/16314#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#16314: Improve confusing error message with MINIMAL pragma -------------------------------------+------------------------------------- Reporter: lerkok | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 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 lerkok): Another observation. I've just found out that if you try this: {{{#!hs class X a where foo :: a {-# MINIMAL #-} }}} Then GHC says: {{{ a.hs:1:1: warning: • The MINIMAL pragma does not require: ‘foo’ but there is no default implementation. • In the class declaration for ‘X’ }}} which is absolutely fantastic! So, it does implement the other side of the check. What's missing is if `MINIMAL` requires something, yet you also give a default definition for it, then it keeps quiet about it. So this request seems to fall well within the current practice of warning about these cases. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/16314#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#16314: Improve confusing error message with MINIMAL pragma -------------------------------------+------------------------------------- Reporter: lerkok | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 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 lerkok): Here's a modest proposal to handle this case. The user guide says the syntax for `MINIMAL` is: {{{#!hs mindef ::= name | '(' mindef ')' | mindef '|' mindef | mindef ',' mindef }}} Strictly speaking the above is actually incorrect because it doesn't capture the fact that you can simply write `{-# MINIMAL #-}`, i.e., no methods listed at all, but that obviously would correspond to the empty set. Abusing the notation in the obvious way, define the following function from a `MINIMAL` expression to a set of names: {{{#!hs required empty_decls = Set.empty -- the missing case in the grammar 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: {{{#!hs D = set of all methods with default definitions R = the required set, as defined above E = D `Set.difference` R }}} 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. I don't think we want to put definitions given with the "default signatures" extension into the set `D`; because they can be arguably considered as missing for the general case. But perhaps including them in `D` can be even a stricter check. I'd vote to leave them out to avoid false positives. This sounds simple enough to be worth an explicit GHC proposal, but I'm happy to create one if you deem it's worth it. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/16314#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#16314: Improve confusing error message with MINIMAL pragma -------------------------------------+------------------------------------- Reporter: lerkok | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 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):
This sounds simple enough to be worth an explicit GHC proposal, but I'm happy to create one if you deem it's worth it.
Yes, now I understand the proposal, thanks. I do think it's worth a proposal. I've been struck by how often people chime in with helpful comments that make the idea better. And it won't take long, and it documents the intent. Plus, it's all about what users want, so asking users is a good plan. Why not add the empty case at the same time (only a user-manual change). Thanks! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/16314#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#16314: Improve confusing error message with MINIMAL pragma -------------------------------------+------------------------------------- Reporter: lerkok | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 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 lerkok): Will do! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/16314#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#16314: Improve confusing error message with MINIMAL pragma -------------------------------------+------------------------------------- Reporter: lerkok | Owner: (none) Type: feature request | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 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 lerkok): Proposal submitted at: https://github.com/ghc-proposals/ghc- proposals/pull/210 Feedback most welcome! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/16314#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC