Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#10182 closed bug (fixed)

lookupIfaceGlobal crash with SOURCE import

Reported by: simonpj Owned by: ezyang
Priority: normal Milestone: 8.0.1
Component: Compiler Version: 7.8.4
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): https://phabricator.haskell.org/D860
Wiki Page:

Description (last modified by simonpj)

Try this (in testsuite/tests/stranal/should_compile)

bash$ ghc -c T8743.hs-boot
bash$ ghc -c T8743.hs
ghc: panic! (the 'impossible' happened)
  (GHC version 7.8.2 for x86_64-unknown-linux):
	tcIfaceGlobal (local): not found:
    main:T8743.$fxToRowMaybe{v r2J}
    [(r2L, Class ‘main:T8743.ToRow{tc r2L}’),
     (r2M, Data constructor ‘main:T8743.D:ToRow{d r2M}’),
     (r2P, Identifier ‘main:T8743.D:ToRow{v r2P}’),
     (roB, Identifier ‘main:T8743.toRow{v roB}’),
     (rqT, Coercion axiom ‘main:T8743.NTCo:ToRow{tc rqT}’)]

Something clearly wrong here. It think this happens with prettty much any version of GHC. I found this when poking about in #10181.

Change History (15)

comment:1 Changed 3 years ago by simonpj

Description: modified (diff)

comment:2 Changed 3 years ago by Joachim Breitner <mail@…>

In 567db32b074860723e2b7c38f119b1880a803775/ghc:

New lint check: Check idArity invariants (#10181)

The arity of an id should not be larger than what the type allows, and
it should also not contradict the strictness signature. This adds a lint
check for that.

This broke test T8743, uncovering a bug in the SOURCE import machinery,
which is now filed as #10182.

comment:3 Changed 3 years ago by ezyang

Note: it has to be a self boot-import; indirecting it through another module eliminates the error. Additionally, it only fails on one-shot compilation, not make.

Last edited 3 years ago by ezyang (previous) (diff)

comment:4 Changed 3 years ago by ezyang

I can reproduce with an even more minimized test-case, on a stage 2 quick build:

-- T8743.hs-boot
module T8743 where
class ToRow a
instance ToRow ()
-- T8743.hs
module T8743 where
import {-# SOURCE #-} T8743 ()
class ToRow a
instance ToRow () where

With some more tracing you can get a clue: tcIfaceGlobal is being called because GHC is trying to report a duplicate instance error. That certainly should not happen! So in one-shot compilation, GHC is seeing an instance in its environment that is NOT supposed to be there.

Also, with an orphan instance, I can reproduce even if a self-import is not involved:

-- T8743.hs-boot
module T8743 where
instance Show (a -> b)
-- T8743a.hs
module T8743a where
import {-# SOURCE #-} T8743
-- T8743.hs
module T8743 where
import T8743a
instance Show (a -> b) where
  show _ = ""
Last edited 3 years ago by simonpj (previous) (diff)

comment:5 Changed 3 years ago by ezyang

Here is what's going on: when a self-import occurs, the hi-boot for the current module is being loaded into the EPS by loadInterface. This should never happen. However, in the absence of type class instances, this is relatively harmless, as we won't actually look in the EPS to find names for locally defined identifiers. However, there is one case where we always consult the EPS: instance lookup!

An orphan instance also causes an EPS-updating load of the hi-boot, even absent a direct import, as the boot file will be recorded as an orphan and thus we'll try to load it.

One thing that is a bit tiresome is that on a direct {-# SOURCE #-} import, we really do need to read in the boot interface (just not add it to the EPS), since we might be some identifiers in another namespace:

-- A.hs-boot
module A(Bool) where
-- A.hs
module A(Bool) where
import {-# SOURCE #-} qualified A as A2
data X = X A2.Bool

But this has never actually worked with declarations in the hs-boot itself, see #7672.

(I spent some time being confused at the interface trace, which claimed Reading [boot] interface for T8743 even in non buggy cases. This is because we always load the hi-boot interface to compare against the "Real Thing"; however, THIS goes through tcHiBootIface, which DOES NOT update the EPS.)

I suspect simonpj's original test case was not fully reduced due to #10333.

comment:6 Changed 3 years ago by simonpj

Owner: set to ezyang

We agreed to fix this by make a self SOURCE import simply an error. (With a Note in the code to explain why it's awkward to allow it.) Edward will do this.

Simon

comment:7 Changed 3 years ago by ezyang

Differential Rev(s): https://phabricator.haskell.org/D860
Status: newpatch

comment:8 Changed 3 years ago by ezyang

Patch doesn't validate; there are about 11 test-cases that use a self boot import to tickle the hs-boot mechanism. I'm converting them to have an intermediate module (bleh!)

comment:9 Changed 3 years ago by nomeata

With Edwards’s change to the test case to not do a self-SOURCE-import, but rather indirected through another module, we get:

*** Core Lint errors : in result of Simplifier ***
T8743.hs-boot:3:10: warning:
    [RHS of $fxToRowMaybe :: forall a_anp. ToRow (Maybe a_anp)]
    idArity 1 exceeds typeArity 0: $fxToRowMaybe

The error message is misleading: It occurs when compiling T8743.hs proper. The desugared code before optimization is fine, after it occurs. My guess is that still from compiling T8740.hs-boot, GHC has ToRow in its environment as an abstract data type, without the information that it is just a newtype around a -> [()]. This is used by the renamer when parsing T8740.hs, so typeArity, which looks at the DataTypCon, returns 0 instead of 1.

Rough guess: The renamer should use the T8743.hs’s definition of ToRow, even if a (less useful) definition is already known from a boot file.

Edward, does this explanation help you hunt down the problem? I know little about the plumbing of the various environments and such.

comment:10 Changed 3 years ago by nomeata

Looks like SPJ fixed this as I was talking: changeset:9b9fc4c732baab126b057b4031bebcbd67d6e348

comment:11 Changed 3 years ago by simonpj

Current status is confusing:

  • This ticket is about self-SOURCE import, and will be fixed by Phab:D860.
  • Independently, the test for #8473 was marked as failing because of the Lint arity error, even with --make, but Simon's fix fixed that.

Conclusion: just commit Phab:D862 (remove self-SOURCE imports from test suite) and Phab:D860 (fix this ticket) and everything will be fine. Edward will do this.

comment:12 Changed 3 years ago by Edward Z. Yang <ezyang@…>

In 1bd1ceffce7f3027ae632086844f95976ed600c8/ghc:

Don't use self {-# SOURCE #-} import in test-cases.

Summary:
It's kind of buggy, c.f. #10182, and isn't motivated by any
real world programs, so we're going to get rid of it (despite
it being handy for GHC test cases.)

Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>

Test Plan: validate

Reviewers: simonpj, austin

Subscribers: thomie

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

comment:13 Changed 3 years ago by Edward Z. Yang <ezyang@…>

In a2f9fef1d90c073ad9c2a727c5ee617057ca6c1d/ghc:

Fix #10182 by disallowing/avoiding self {-# SOURCE #-} imports

Summary:
hs-boot declarations were leaking into the EPS due to
self {-# SOURCE #-} imports, and interface loading induced by
orphan instances.  For the former, we simply disallow self
{-# SOURCE #-} imports; for the latter, we simply just don't
load an interface if it would be ourself.

Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>

Test Plan: validate

Reviewers: simonpj, austin

Subscribers: thomie

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

GHC Trac Issues: #10182

comment:14 Changed 3 years ago by ezyang

Resolution: fixed
Status: patchclosed

comment:15 Changed 2 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

Note: See TracTickets for help on using tickets.