Opened 8 years ago

Closed 6 years ago

Last modified 8 months ago

#5252 closed bug (fixed)

UNPACK without optimisation leads to panic

Reported by: simonpj Owned by:
Priority: normal Milestone: 7.6.2
Component: Compiler Version: 7.6.1
Keywords: Cc: qdunkan@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case: deSugar/should_compile/T5252, T5252Take2
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

Here's a two-module progam

module Foo where
  import Bar
  blah :: S -> T
  blah (MkS x _) = x

module Bar( S(..), T ) where
  data T = MkT Int Int
  data S = MkS {-# UNPACK #-}!T Int

Now with ghc 7.0.3 we get

bash-3.1$ ghc -c Bar.hs
bash-3.1$ ghc -c Foo.hs
ghc.exe: panic! (the 'impossible' happened)
  (GHC version 7.0.3 for i386-unknown-mingw32):
	reboxProduct: not a product main:Foo1.T{tc r2}

The problem is that

  • We are compiling with -O so GHC tries to put as little as possible into the interface file Bar.hi. And it does not put in T's constructors
      data S
          RecFlag NonRecursive
          Generics: no
          = MkS :: Foo1.T -> GHC.Types.Int -> S
            HasWrapper
                Stricts: {-# UNPACK #-} ! _
    43edb8535d0555fb50e9f93a9c3203bf
      data T
          RecFlag NonRecursive
          Generics: no
          {- abstract -}
    
  • However the pattern match in Foo requires that GHC can see the full representation for T, becuase it UNPACK's the argument.
  • A workaround is to export MkT from Bar.

The solution I am implementing is to ignore UNPACK pragmas when OmitInterfacePragmas is on. This flag is the one that causes trimming of the exposed constructors.

Attachments (1)

Change History (11)

comment:1 Changed 8 years ago by simonpj

Resolution: fixed
Status: newclosed

Fixed by

commit 792449f555bb4dfa8e718079f6d42dc9babe938a
Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Sat Jun 11 14:26:34 2011 +0100

    Ignore UNPACK pragmas with OmitInterfacePragmas is on (fixes Trac #5252)
    
    The point here is that if a data type chooses a representation that
    unpacks an argument field, the representation of the argument field
    must be visible to clients.  And it may not be if OmitInterfacePragmas
    is on.

>---------------------------------------------------------------

 compiler/typecheck/TcInstDcls.lhs   |    3 +-
 compiler/typecheck/TcTyClsDecls.lhs |   44 +++++++++++++++++-----------------
 2 files changed, 23 insertions(+), 24 deletions(-)

comment:2 Changed 8 years ago by simonpj

Test Case: deSugar/should_compile/T5252

comment:3 Changed 6 years ago by simonpj

Cc: qdunkan@… added
difficulty: Unknown
Resolution: fixed
Status: closednew
Version: 7.0.37.6.1

Evan Laforge writes (to ghc-users): I have something that looks similar to #5252, namely, given these two modules:

% cat Midi.hs
{-# OPTIONS_GHC -funbox-strict-fields #-}
module Midi (
    WriteMessage(..)
    , WriteDevice
    -- TODO due ghc bug: http://hackage.haskell.org/trac/ghc/ticket/5252
    -- , WriteDevice(WriteDevice)
) where
import qualified Data.ByteString as ByteString

data WriteMessage = WriteMessage !WriteDevice
newtype WriteDevice = WriteDevice ByteString.ByteString

% cat CoreMidi.hs
module CoreMidi where
import qualified Midi

write_message :: Midi.WriteMessage -> IO Bool
write_message (Midi.WriteMessage _) = return True

If I compile thus I get a compiler crash with GHC 7.6.1

% ghc -c Midi.hs
% ghc -c CoreMidi.hs
ghc: panic! (the 'impossible' happened)
  (GHC version 7.6.1 for x86_64-apple-darwin):
	reboxProduct: not a product main:Midi.WriteDevice{tc r2M}

Oddly, if I put {-# UNPACK #-} on the strict WriteDevice and remove -funbox-strict-fields, I don't get a crash anymore. Also, it has to be ByteString inside, I guess it has to do with the optimization ByteString applies.

comment:4 Changed 6 years ago by simonpj

Quite right. My fix for #5252 had a bug. Patch coming.

Simon

comment:5 Changed 6 years ago by simonpj@…

commit 5bae803a18b17bdb158a7780e6b6ac3c520e5b39

Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Sat Sep 15 23:09:25 2012 +0100

    Fix UNPACK with -fomit-interface-pragmas.
    
    We were missing a case, so that you could expose a constructor
    with UNPACKed fields, but the field tpye was trimmed, and hence
    can't be expanded.
    
    Fixes Trac #5252 (revived)

 compiler/typecheck/TcTyClsDecls.lhs |   29 ++++++++++++++++-------------
 1 files changed, 16 insertions(+), 13 deletions(-)

comment:6 Changed 6 years ago by simonpj

Status: newmerge
Test Case: deSugar/should_compile/T5252deSugar/should_compile/T5252, T5252Take2

Needs this patch too:

commit ba8fd081ba9b222dd5f93604d7deeaca372e4511
Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Mon Sep 17 18:22:10 2012 +0100

    Make the call to chooseBoxingStrategy lazy again
    
    I made it strict, as an incidental consequence of this patch:
    
      commit 5bae803a18b17bdb158a7780e6b6ac3c520e5b39
      Author: Simon Peyton Jones <simonpj@microsoft.com>
      Date:   Sat Sep 15 23:09:25 2012 +0100
          Fix UNPACK with -fomit-interface-pragmas.
    
    But it's very important that chooseBoxingStrategy is lazy, else
    (in bigger programs with lots of recursion in types) GHC can
    loop. This showed up in Data.Sequence; and I think it was making
    haddock loop as well.
    
    Anyway this patch makes it lazy again.

 compiler/typecheck/TcTyClsDecls.lhs |   34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

Regression test added.

Merge to 7.6 branch.

comment:7 Changed 6 years ago by igloo

Milestone: 7.6.2

comment:8 Changed 6 years ago by pcapriotti

Resolution: fixed
Status: mergeclosed

comment:9 Changed 17 months ago by Ben Gamari <ben@…>

In 121fee99/ghc:

Remove unnecessary GHC option from SrcLoc

This was an old workaround for #5252. Fixes #13173.

Reviewers: austin, bgamari

Reviewed By: bgamari

Subscribers: rwbarton, thomie

Differential Revision: https://phabricator.haskell.org/D3763

comment:10 Changed 8 months ago by Ben Gamari <ben@…>

In 4a16804/ghc:

Fix markup in the UNPACK pragma section of the user's guide.

The ticket 5252 wasn't linked earlier.
Note: See TracTickets for help on using tickets.