Opened 6 months ago

Last modified 6 months ago

#13363 new bug

Wildcarn patterns and COMPLETE sets can lead to misleading redundant pattern-match warnings

Reported by: RyanGlScott Owned by:
Priority: normal Milestone: 8.4.1
Component: Compiler Version: 8.1
Keywords: PatternSynonyms, PatternMatchWarnings Cc: mpickering, gkaracha
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect error/warning at compile-time Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

Consider this program:

{-# LANGUAGE PatternSynonyms #-}
{-# LANGUAGE ViewPatterns #-}
{-# OPTIONS_GHC -Wincomplete-patterns #-}
module Bug where

data Boolean = F | T
  deriving Eq

pattern TooGoodToBeTrue :: Boolean
pattern TooGoodToBeTrue <- ((== T) -> True)
  where
    TooGoodToBeTrue = T
{-# COMPLETE F, TooGoodToBeTrue #-}

catchAll :: Boolean -> Int
catchAll F               = 0
catchAll TooGoodToBeTrue = 1

This compiles with no warnings with -Wall. But if you tweak catchAll to add a catch-all case at the end:

{-# LANGUAGE PatternSynonyms #-}
{-# LANGUAGE ViewPatterns #-}
{-# OPTIONS_GHC -Wincomplete-patterns #-}
module Bug where

data Boolean = F | T
  deriving Eq

pattern TooGoodToBeTrue :: Boolean
pattern TooGoodToBeTrue <- ((== T) -> True)
  where
    TooGoodToBeTrue = T
{-# COMPLETE F, TooGoodToBeTrue #-}

catchAll :: Boolean -> Int
catchAll F               = 0
catchAll TooGoodToBeTrue = 1
catchAll _               = error "impossible"

Then if you compile it with -Wall, you'll get a very misleading warning:

$ ~/Software/ghc2/inplace/bin/ghc-stage2 --interactive Bug.hs -Wall
GHCi, version 8.1.20170228: http://www.haskell.org/ghc/  :? for help
Loaded GHCi configuration from /home/rgscott/.ghci
[1 of 1] Compiling Bug              ( Bug.hs, interpreted )

Bug.hs:17:1: warning: [-Woverlapping-patterns]
    Pattern match is redundant
    In an equation for ‘catchAll’: catchAll TooGoodToBeTrue = ...
   |
17 | catchAll TooGoodToBeTrue = 1
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I would have expected the warning to be on the catchAll _ = error "impossible" case!

Change History (7)

comment:1 Changed 6 months ago by simonpj

Cc: gkaracha added

George, this is your patch.

comment:2 in reply to:  1 Changed 6 months ago by gkaracha

Replying to simonpj:

George, this is your patch.

Hello, I think this is Matthew's patch. If I remember mpickering's implementation correctly, both {F, TooGoodToBeTrue} and {F,T} are complete (especially in the module where everything is visible). So both combinations are exhaustive:

F, TooGoodToBeTrue -- Using COMPLETE               => _               redundant
F, _               -- Using the Boolean signature  => TooGoodToBeTrue redundant

Hence, I think this is just non-specified semantics, but not necessarily a bug. Maybe Matthew can shed some more light on this.

Last edited 6 months ago by gkaracha (previous) (diff)

comment:3 Changed 6 months ago by gkaracha

Keywords: PatternMatchWarnings added

comment:4 Changed 6 months ago by mpickering

I think this is a bug because the match is not "redundant" as removing it will change the semantics of the function.

Similarly if you write

f T = 1
f F = 0
f _ = 3

then removing the F case leaves a complete definition but the correct thing to do is to remove the redundant wildcard clause.

comment:5 Changed 6 months ago by gkaracha

Yes, I guess you're right. This seems like the discussion we had on Phab:D2669, but I thought you actually disabled the redundancy check when COMPLETE pragmas are involved so that these discrepancies don't show up, with the commit:

  • Don't warn about redundancy when COMPLETE is involved

comment:6 Changed 6 months ago by mpickering

But that is not the problem.

This warning comes from the built-in constructor set. I think there needs to be some more clever augmentation to the intermediate result in the case that a result of allCompleteMatches does not contain the constructor which was used to do the lookup. For example, if ToGoodToBeTrue is not in T, F.

comment:7 Changed 6 months ago by dfeuer

Milestone: 8.4.1
Note: See TracTickets for help on using tickets.