Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#5424 closed bug (fixed)

Validate error under 7.2.1 caused by incomplete pattern warnings in new code gen (GADTs)

Reported by: dterei Owned by: simonpj
Priority: normal Milestone:
Component: Compiler Version: 7.2.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Building GHC failed Difficulty:
Test Case: gadt/T5424 Blocked By:
Blocking: Related Tickets:

Description

the files compiler/cmm/Cmm.hs, compiler/cmm/CmmNode.hs and compiler/cmm/CmmStackLayout.hs all contain the following lines at the top:

{-# OPTIONS_GHC -fno-warn-incomplete-patterns #-}
#if __GLASGOW_HASKELL__ >= 701
-- GHC 7.0.1 improved incomplete pattern warnings with GADTs
{-# OPTIONS_GHC -fwarn-incomplete-patterns #-}
#endif

This causes a validation error when building with 7.2.1 as this version of GHC still seems to incorrectly warn about incorrect patterns despite the above comment. (The above comment is wrong anyway as the cpp if statement only triggers with GHC 7.2.1 and greater, not with GHC 7.0.1).

For now I've dealt with this simply by changing the cpp if statement to be:

#if __GLASGOW_HASKELL__ >= 703
...

Someone more familiar with the code and the state of '-fwarn-incomplete-patterns' should actually fix this up though.

Change History (5)

comment:1 Changed 3 years ago by davidterei@…

commit 26cf7e526c8e88d8c161c2e30865a914fc48ed6c

Author: David Terei <davidterei@gmail.com>
Date:   Fri Aug 19 12:13:43 2011 -0700

    'Fix' a validation problem when bootstrap is 7.2.1
    
    Problem is with GADTs in new code gen and incomplete pattern
    warnings. Just disabled the warning really and created #5424
    to track an actual fix.

 compiler/cmm/Cmm.hs            |    2 +-
 compiler/cmm/CmmNode.hs        |    2 +-
 compiler/cmm/CmmStackLayout.hs |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

This is simply the temporary fix commit.

comment:2 Changed 3 years ago by igloo

  • Owner set to simonpj

hoopl is compiled with -O0 by the bootstrapping compiler, and that causes this problem. I can't see why that should cause a problem, though.

Cutdown example:

Q.hs:

module Q where

data X
data Y

W.hs:

{-# LANGUAGE GADTs #-}

module W where

import Q

data D a where
    C1 :: D X
    C2 :: D Y

f :: D X -> Int
f C1 = 1
$ ghc -Wall -fforce-recomp -O -c Q.hs
$ ghc -Wall -fforce-recomp -O -c W.hs
$ ghc -Wall -fforce-recomp -O0 -c Q.hs
$ ghc -Wall -fforce-recomp -O -c W.hs 

W.hs:13:1:
    Warning: Pattern match(es) are non-exhaustive
             In an equation for `f': Patterns not matched: C2

comment:3 Changed 3 years ago by simonpj@…

commit de9b85fa3fb6d4cd593366e1f2383cd0b492c056

Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Fri Sep 2 17:43:53 2011 +0100

    Export a tiny bit more info with AbstractTyCon (fixes #5424)
    
    When we compile -O0 we put type constructors in the interface file
    without their data constructors -- an AbstractTyCon.  But in a
    client module, to give good pattern-match exhaustiveness warnings,
    we need to know the difference between a data type and a newtype.
    (The latter can be coerced to another type, but a data type can't.)
    See Note [Pruning dead case alternatives] in Unify.
    
    Because we weren't conveying this info, we were getting bogus
    warnings about inexhaustive patterm matches with GADTs, and
    (confusingly) these warnings woudl come and go depending on
    whether you were compiling with -O.
    
    This patch makes AbstractTyCon carry a flag indicating whether
    the type constructor is "distinct"; two distinct TyCons cannot
    be coerced into eachother (except by unsafeCoerce, in which case
    all bets are off).
    
    HEADS UP: interface file format changes slightly, so you need
    to make clean.

 compiler/iface/BinIface.hs          |    4 +-
 compiler/iface/BuildTyCl.lhs        |    7 ++--
 compiler/iface/IfaceSyn.lhs         |   24 +++++++-------
 compiler/iface/MkIface.lhs          |    2 +-
 compiler/iface/TcIface.lhs          |    2 +-
 compiler/typecheck/TcDeriv.lhs      |    2 +-
 compiler/typecheck/TcRnDriver.lhs   |    4 ++-
 compiler/typecheck/TcTyClsDecls.lhs |    4 +-
 compiler/types/TyCon.lhs            |   57 +++++++++++++++++++++++++++-------
 compiler/types/Unify.lhs            |   10 +++---
 10 files changed, 76 insertions(+), 40 deletions(-)

comment:4 Changed 3 years ago by simonpj

  • Resolution set to fixed
  • Status changed from new to closed
  • Test Case set to gadt/T5424

Nice one! Thanks.

comment:5 Changed 3 years ago by simonpj@…

commit 8595c61cfae514bc9582d4447ccca5db5a201133

Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Mon Sep 5 09:38:50 2011 +0100

    Eliminate isHiBootTyCon in favour of isAbstractTyCon
    
    Fixes Trac #5456, which was a buglet arising from
    
      commit de9b85fa3fb6d4cd593366e1f2383cd0b492c056
      Author: Simon Peyton Jones <simonpj@microsoft.com>
      Date:   Fri Sep 2 17:43:53 2011 +0100
    
        Export a tiny bit more info with AbstractTyCon (fixes #5424)
    
    It only shows up when compiling with -O0!

 compiler/stgSyn/CoreToStg.lhs    |    6 +++---
 compiler/typecheck/TcTyDecls.lhs |    2 +-
 compiler/types/TyCon.lhs         |    7 -------
 3 files changed, 4 insertions(+), 11 deletions(-)
Note: See TracTickets for help on using tickets.