[GHC] #14546: -Woverlapping-patterns warns on wrong patterns for Int
#14546: -Woverlapping-patterns warns on wrong patterns for Int -------------------------------------+------------------------------------- Reporter: Lemming | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.2 (Type checker) | Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: Incorrect Unknown/Multiple | error/warning at compile-time Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- GHC-8.0 introduced a new warning on redundant pattern matches in `case` clauses. But there is something strange. {{{ $ cat RedundantCase.hs main :: IO () main = do case 0::Int of 0 -> putStrLn "A" 1 -> putStrLn "B" _ -> putStrLn "C" case 0::Int of 0 -> putStrLn "A" 1 -> putStrLn "B" 2 -> putStrLn "C" case 0::Integer of 0 -> putStrLn "A" 1 -> putStrLn "B" _ -> putStrLn "C" case 0::Integer of 0 -> putStrLn "A" 1 -> putStrLn "B" 2 -> putStrLn "C" case 0::Integer of 1 -> putStrLn "B" 2 -> putStrLn "C" let dummy :: Integer dummy = 0 case dummy of 0 -> putStrLn "A" 1 -> putStrLn "B" _ -> putStrLn "C" case "foo" of "foo" -> putStrLn "A" "bar" -> putStrLn "B" "baz" -> putStrLn "C" if True then putStrLn "X" else putStrLn "Y" }}} {{{ $ ghc-8.2.2 -Wall RedundantCase.hs [1 of 1] Compiling Main ( RedundantCase.hs, RedundantCase.o ) RedundantCase.hs:4:7: warning: [-Woverlapping-patterns] Pattern match is redundant In a case alternative: 0 -> ... | 4 | 0 -> putStrLn "A" | ^^^^^^^^^^^^^^^^^ RedundantCase.hs:5:7: warning: [-Woverlapping-patterns] Pattern match is redundant In a case alternative: 1 -> ... | 5 | 1 -> putStrLn "B" | ^^^^^^^^^^^^^^^^^ RedundantCase.hs:8:4: warning: [-Wincomplete-patterns] Pattern match(es) are non-exhaustive In a case alternative: Patterns not matched: 0 | 8 | case 0::Int of | ^^^^^^^^^^^^^^... RedundantCase.hs:9:7: warning: [-Woverlapping-patterns] Pattern match is redundant In a case alternative: 0 -> ... | 9 | 0 -> putStrLn "A" | ^^^^^^^^^^^^^^^^^ RedundantCase.hs:10:7: warning: [-Woverlapping-patterns] Pattern match is redundant In a case alternative: 1 -> ... | 10 | 1 -> putStrLn "B" | ^^^^^^^^^^^^^^^^^ RedundantCase.hs:11:7: warning: [-Woverlapping-patterns] Pattern match is redundant In a case alternative: 2 -> ... | 11 | 2 -> putStrLn "C" | ^^^^^^^^^^^^^^^^^ RedundantCase.hs:15:7: warning: [-Woverlapping-patterns] Pattern match is redundant In a case alternative: 1 -> ... | 15 | 1 -> putStrLn "B" | ^^^^^^^^^^^^^^^^^ RedundantCase.hs:16:7: warning: [-Woverlapping-patterns] Pattern match is redundant In a case alternative: _ -> ... | 16 | _ -> putStrLn "C" | ^^^^^^^^^^^^^^^^^ RedundantCase.hs:20:7: warning: [-Woverlapping-patterns] Pattern match is redundant In a case alternative: 1 -> ... | 20 | 1 -> putStrLn "B" | ^^^^^^^^^^^^^^^^^ RedundantCase.hs:21:7: warning: [-Woverlapping-patterns] Pattern match is redundant In a case alternative: 2 -> ... | 21 | 2 -> putStrLn "C" | ^^^^^^^^^^^^^^^^^ RedundantCase.hs:23:4: warning: [-Wincomplete-patterns] Pattern match(es) are non-exhaustive In a case alternative: Patterns not matched: 0 | 23 | case 0::Integer of | ^^^^^^^^^^^^^^^^^^... RedundantCase.hs:24:7: warning: [-Woverlapping-patterns] Pattern match is redundant In a case alternative: 1 -> ... | 24 | 1 -> putStrLn "B" | ^^^^^^^^^^^^^^^^^ RedundantCase.hs:25:7: warning: [-Woverlapping-patterns] Pattern match is redundant In a case alternative: 2 -> ... | 25 | 2 -> putStrLn "C" | ^^^^^^^^^^^^^^^^^ RedundantCase.hs:34:4: warning: [-Wincomplete-patterns] Pattern match(es) are non-exhaustive In a case alternative: Patterns not matched: [] (p:_) where p is not one of {'b', 'f'} ['b'] ('b':p:_) where p is not one of {'a'} ... | 34 | case "foo" of | ^^^^^^^^^^^^^... Linking RedundantCase ... }}} In the first case it warns about the wrong patterns, in the second case it warns about all patterns and a non-exhaustive pattern list. In the following three cases using `Integer` the warnings seem to be correct. The next case shows that I can suppress the warning if I hide the constant nature of the case selector a bit. Then a case on a `String` - GHC does not seem to try to detect redundant pattern matches but falls back to a general test on exhaustive pattern lists. Finally, in a constant `case` on `Bool` I can avoid the warning using a plain `if-then-else`. Now I wonder what the purpose of the warning is at all. First I thought that it wants to warn me, that the patterns overlap, i.e. pattern `_` contains the already handled case `0`. This does not seem to be the purpose of the warning - and such overlapping patterns are pretty common, aren't they? Second thought: GHC wants to warn me about redundant patterns in the presence of a constant case selector. But then, it does so with varying degrees of success: For `Integer` it works, but not for `Int` or `String`. Is it really only about constant case selectors? Then, not only some pattern matches are redundant but the whole `case` clause is redundant. Then again, can't we assume that the programmer wrote such a `case` clause by intend? At least, this is how I use a `case` clause with constant selector: as a type-safe way to out-comment alternative implementations. Thus my question: Where is this warning useful? -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14546> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14546: -Woverlapping-patterns warns on wrong patterns for Int -------------------------------------+------------------------------------- Reporter: Lemming | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler (Type | Version: 8.2.2 checker) | Keywords: Resolution: | PatternMatchWarnings Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect | Unknown/Multiple error/warning at compile-time | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by simonpj): * keywords: => PatternMatchWarnings Comment: Thanks. There are quite a few [wiki:PatternMatchCheck tickets related to the pattern-match warning checker]. It'd be great if someone wanted to take leadership on that front. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14546#comment:1> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14546: -Woverlapping-patterns warns on wrong patterns for Int -------------------------------------+------------------------------------- Reporter: Lemming | Owner: sighingnow Type: bug | Status: new Priority: normal | Milestone: Component: Compiler (Type | Version: 8.2.2 checker) | Keywords: Resolution: | PatternMatchWarnings Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect | Unknown/Multiple error/warning at compile-time | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by sighingnow): * owner: (none) => sighingnow -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14546#comment:2> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14546: -Woverlapping-patterns warns on wrong patterns for Int -------------------------------------+------------------------------------- Reporter: Lemming | Owner: sighingnow Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler (Type | Version: 8.2.2 checker) | Keywords: Resolution: | PatternMatchWarnings Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect | Unknown/Multiple error/warning at compile-time | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:4571 Wiki Page: | -------------------------------------+------------------------------------- Changes (by sighingnow): * status: new => patch * differential: => Phab:4571 -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14546#comment:3> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14546: -Woverlapping-patterns warns on wrong patterns for Int -------------------------------------+------------------------------------- Reporter: Lemming | Owner: sighingnow Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler (Type | Version: 8.2.2 checker) | Keywords: Resolution: | PatternMatchWarnings Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect | Unknown/Multiple error/warning at compile-time | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D4571 Wiki Page: | -------------------------------------+------------------------------------- Changes (by RyanGlScott): * differential: Phab:4571 => Phab:D4571 -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14546#comment:4> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14546: -Woverlapping-patterns warns on wrong patterns for Int -------------------------------------+------------------------------------- Reporter: Lemming | Owner: sighingnow Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler (Type | Version: 8.2.2 checker) | Keywords: Resolution: | PatternMatchWarnings Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect | Unknown/Multiple error/warning at compile-time | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D4571 Wiki Page: | -------------------------------------+------------------------------------- Comment (by Ben Gamari <ben@…>): In [changeset:"1f88f541aad1e36d01f22f9e71dfbc247e6558e2/ghc" 1f88f54/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="1f88f541aad1e36d01f22f9e71dfbc247e6558e2" Improve exhaustiveness checking for literal values and patterns, fix #14546 Currently, we parse both the **integral literal** value and the patterns as `OverLit HsIntegral`. For example: ``` case 0::Int of 0 -> putStrLn "A" 1 -> putStrLn "B" _ -> putStrLn "C" ``` When checking the exhaustiveness of pattern matching, we translate the `0` in value position as `PmOLit`, but translate the `0` and `1` in pattern position as `PmSLit`. The inconsistency leads to the failure of `eqPmLit` to detect the equality and report warning of "Pattern match is redundant" on pattern `0`, as reported in #14546. In this patch we remove the specialization of `OverLit` patterns, and keep the overloaded number literal in pattern as it is to maintain the consistency. Now we can capture the exhaustiveness of pattern `0` and the redundancy of pattern `1` and `_`. For **string literals**, we parse the string literals as `HsString`. When `OverloadedStrings` is enabled, it further be turned as `HsOverLit HsIsString`, whether it's type is `String` or not. For example: ``` case "foo" of "foo" -> putStrLn "A" "bar" -> putStrLn "B" "baz" -> putStrLn "C" ``` Previously, the overloaded string values are translated to `PmOLit` and the non-overloaded string values are translated to `PmSLit`. However the string patterns, both overloaded and non-overloaded, are translated to list of characters. The inconsistency leads to wrong warnings about redundant and non-exhaustive pattern matching warnings, as reported in #14546. In order to catch the redundant pattern in following case: ``` case "foo" of ('f':_) -> putStrLn "A" "bar" -> putStrLn "B" ``` In this patch, we translate non-overloaded string literals, both in value position and pattern position, as list of characters. For overloaded string literals, we only translate it to list of characters only when it's type is `stringTy`, since we know nothing about the `toString` methods. But we know that if two overloaded strings are syntax equal, then they are equal. Then if it's type is not `stringTy`, we just translate it to `PmOLit`. We can still capture the exhaustiveness of pattern `"foo"` and the redundancy of pattern `"bar"` and `"baz"` in the following code: ``` {-# LANGUAGE OverloadedStrings #-} main = do case "foo" of "foo" -> putStrLn "A" "bar" -> putStrLn "B" "baz" -> putStrLn "C" ``` Test Plan: make test TEST="T14546" Reviewers: bgamari, simonpj Reviewed By: bgamari, simonpj Subscribers: simonpj, thomie, carter GHC Trac Issues: #14546 Differential Revision: https://phabricator.haskell.org/D4571 }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14546#comment:5> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14546: -Woverlapping-patterns warns on wrong patterns for Int -------------------------------------+------------------------------------- Reporter: Lemming | Owner: sighingnow Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler (Type | Version: 8.2.2 checker) | Keywords: Resolution: fixed | PatternMatchWarnings Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Incorrect | Test Case: error/warning at compile-time | deSugar/should_compile/T14546a, | deSugar/should_compile/T14546b, | deSugar/should_compile/T14546c Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D4571 Wiki Page: | -------------------------------------+------------------------------------- Changes (by RyanGlScott): * status: patch => closed * testcase: => deSugar/should_compile/T14546a, deSugar/should_compile/T14546b, deSugar/should_compile/T14546c * resolution: => fixed -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14546#comment:6> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14546: -Woverlapping-patterns warns on wrong patterns for Int -------------------------------------+------------------------------------- Reporter: Lemming | Owner: sighingnow Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler (Type | Version: 8.2.2 checker) | Keywords: Resolution: fixed | PatternMatchWarnings Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Incorrect | Test Case: error/warning at compile-time | deSugar/should_compile/T14546a, | deSugar/should_compile/T14546b, | deSugar/should_compile/T14546c Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D4571 Wiki Page: | -------------------------------------+------------------------------------- Comment (by jrp): It is probably this patch that breaks test T14547 {{{ cd "./T14547.run" && "/home/jrp/Projects/ghc/inplace/test spaces/ghc- stage2" -c T14547.hs -dcore-lint -dcmm-lint -no-user-package-db -rtsopts -fno-warn-missed-specialisations -fshow-warning-groups -fdiagnostics- color=never -fno-diagnostics-show-caret -dno-debug-output -Wincomplete- patterns Actual stderr output differs from expected: diff -uw "/dev/null" "./T14547.run/T14547.comp.stderr.normalised" --- /dev/null 2018-06-02 19:10:04.172693400 +0100 +++ ./T14547.run/T14547.comp.stderr.normalised 2018-06-03 13:14:15.645102495 +0100 @@ -0,0 +1,4 @@ + +T14547.hs:14:9: warning: [-Wincomplete-patterns (in -Wextra)] + Pattern match(es) are non-exhaustive + In an equation for ‘foo’: Patterns not matched: [] *** unexpected failure for T14547(normal) Unexpected results from: TEST="T14547" }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14546#comment:7> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#14546: -Woverlapping-patterns warns on wrong patterns for Int -------------------------------------+------------------------------------- Reporter: Lemming | Owner: sighingnow Type: bug | Status: closed Priority: normal | Milestone: Component: Compiler (Type | Version: 8.2.2 checker) | Keywords: Resolution: fixed | PatternMatchWarnings Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: Incorrect | Test Case: error/warning at compile-time | deSugar/should_compile/T14546a, | deSugar/should_compile/T14546b, | deSugar/should_compile/T14546c Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D4571 Wiki Page: | -------------------------------------+------------------------------------- Comment (by sighingnow): I'm building ghc-head now. I will look at this problem. Sorry for the break. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14546#comment:8> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
participants (1)
-
GHC