[GHC] #13766: Confusing "reudundant pattern match" in 8.0, no warning at all in 8.2
#13766: Confusing "reudundant pattern match" in 8.0, no warning at all in 8.2 -------------------------------------+------------------------------------- Reporter: edsko | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.1 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 couldn't find an existing ticket about this, so I figured I'd file this even though it's probably an instance of a more general problem. Consider {{{#!hs {-# LANGUAGE TypeFamilies #-} {-# OPTIONS_GHC -Wall -Woverlapping-patterns #-} module T where class C a where c :: a -> a instance Int ~ Bool => C Int where c = id }}} yields {{{ T.hs:10:3: warning: [-Woverlapping-patterns] Pattern match is redundant In an equation for ‘c’: c = ... }}} which I suppose makes _some_ amount of sense but is highly confusing nonetheless :) Now in 8.2 I don't get this warning, but in fact I don't get any warning at all, which I'm not entirely sure is better. In the real code obviously the superclass constraint was far more complicated and it was nice of ghc to warn me (albeit in a roundabout way) that it was unsatisfiable. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13766 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13766: Confusing "reudundant pattern match" in 8.0, no warning at all in 8.2 -------------------------------------+------------------------------------- Reporter: edsko | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.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: | -------------------------------------+------------------------------------- Changes (by Iceland_jack): * cc: Iceland_jack (added) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13766#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13766: Confusing "redundant pattern match" in 8.0, no warning at all in 8.2 -------------------------------------+------------------------------------- Reporter: edsko | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.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: | -------------------------------------+------------------------------------- -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13766#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13766: Confusing "redundant pattern match" in 8.0, no warning at all in 8.2 -------------------------------------+------------------------------------- Reporter: edsko | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.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: | -------------------------------------+------------------------------------- Changes (by RyanGlScott): * cc: mpickering (added) Comment: Hoo boy, that's pretty scary. To make it worse, because of this bug you can write blatantly nonsensical things like this: {{{#!hs {-# LANGUAGE FlexibleInstances #-} {-# LANGUAGE TypeFamilies #-} {-# OPTIONS_GHC -Wall -Woverlapping-patterns #-} module T where class C a where c :: a instance (a ~ Bool, a ~ Int) => C a where c = 42 + True }}} And it'll compile on GHC 8.2.1-rc2 with no warnings. (I haven't found a way to actually //use// that bogus `c` definition, but still.) The commit that caused this behavior is adb565aa74582969bbcc3b411d6d518b1c76c3cf (Don't return empty initial uncovered set for an unsat context). Matthew, do you know what is going on here? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13766#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13766: Confusing "redundant pattern match" in 8.0, no warning at all in 8.2 -------------------------------------+------------------------------------- Reporter: edsko | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.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 ezyang): This looks related to some changes to inaccessibility which happened for GHC 8.2. There's more context at #12466, #11066 and #12694, and possibly more (do a search for "inaccessible") -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13766#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13766: Confusing "redundant pattern match" in 8.0, no warning at all in 8.2 -------------------------------------+------------------------------------- Reporter: edsko | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.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 mpickering): This is the expected behaviour, the commit message explains the reasoning. {{{ Previously when the checker encountered an unsatisfiable term of type context it would return an empty initial uncovered set. This caused all pattern matches in the context to be reported as redudant. This is arguably correct behaviour as they will never be reached but it is better to recover and provide accurate warnings for these cases to avoid error cascades. It would perhaps be better to report an error to the user about an inacessible branch but this is certainly better than many confusing redundant match warnings. }}} Warning that the RHS is inaccessible would be valuable but I don't think it's the domain of the pattern match checker to warn indirectly of this by warning of "redundant" clauses. So it looks like the tickets that Edward linked are more relevant. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13766#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13766: Confusing "redundant pattern match" in 8.0, no warning at all in 8.2 -------------------------------------+------------------------------------- Reporter: edsko | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.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 RyanGlScott): I talked with mpickering about this privately, but I'll recap the conversation here as well. This may be expected behavior the sense of comment:5, but I still find it unsettling. The code that adb565aa74582969bbcc3b411d6d518b1c76c3cf targeted was of the form: {{{#!hs f = case [] of (_:_) -> case () of a -> undefined }}} Here, it'll warn that the `(_:_)` match is redundant: {{{ Pattern match is redundant In a case alternative: (_ : _) -> ... }}} And thanks to the aforementioned commit, it will //not// warn about the inaccessible `a -> undefined` case. This is definitely a Good Thing. A similar scenario arises in the code in this example: {{{#!hs instance Int ~ Bool => C Int where c = id }}} Because `Int ~ Bool` is insoluble (and thus all the code inside the instance is inaccessible), it won't bother printing out a warning that `c` is inaccessible. But this is a tad skeevy. In the former example, there was a separate warning that hinted that you were doing something questionable. In the latter example, however, there's no warning at all to point out the dubious nature of your code! Therefore, I think a satisfactory conclusion to this bug would be to come up with a suitable warning about the `Int ~ Bool` constraint. Whether that's the purview of #12694 or some other ticket, I'm not sure. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13766#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#13766: Confusing "redundant pattern match" in 8.0, no warning at all in 8.2 -------------------------------------+------------------------------------- Reporter: edsko | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.1 Resolution: | Keywords: | PatternMatchWarnings Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #12466, #11066, | Differential Rev(s): #12694 | Wiki Page: | -------------------------------------+------------------------------------- Changes (by RyanGlScott): * keywords: => PatternMatchWarnings * related: => #12466, #11066, #12694 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13766#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC