panic! TEST=tc198: lookupVers2 GHC.Stack.Types CallStack

changed milestone to %8.0.1

changed weight to 10

Thanks, I'm out of town this week but will investigate when I get back!

A couple initial observations:

The panic happens when ghc builds the ModIface, specifically when fingerprinting GHC.Stack.Types.CallStack.
The panic only occurs with -O. The module is so simple that the Core is unchanged by optimizations, but the optimized Core does include an unfolding that mentions the CallStack type.

So, it looks like GHC is trying to fingerprint CallStack to write out the interface file, but fails because GHC.Stack.Types was never loaded. The question then is why did we not load GHC.Stack.Types? It's clearly imported by GHC.Err, the interface file confirms. But -ddump-if-trace doesn't mention GHC.Stack.Types..

I ran into this problem a few weeks ago. It seems that the place where GHC.Stack.Types normally gets loaded is the dsLookupDataCon call in DsBinds.dsEvCallStack. I found this peculiar at the time (why doesn't it get loaded earlier?) but the original source of the error (for me) was elsewhere. When I fixed that (to produce an EvCallStack), the problem went away.

I don't know why this all is happening, but the knowledge that that particular call of dsLookupDataCon triggers loading in the successful case was a bit hard-won, so I thought I'd share.

That actually makes a lot of sense, thanks! The CallStack tycon is wired-in, so the type-checker wouldn't have to load GHC.Types.Stack. The datacon is not, nor is the newer pushCallStack function we use in the desugarer, so that's why it would be loaded during desugaring.

In this case we're not generating any CallStacks, so dsEvCallStack is not called, so the module is not loaded. Hrm.. What is the usual rule for loading modules that contain wired-in things? Or is it even necessary to fingerprint a wired-in thing?

(By the way, how did you figure out what was triggering the load?)

Look at Note [Loading instances for wired-in things] in LoadIface.

You should call checkWiredInTyCon when (but only when) you are using CallStack but not mentioning it. A classic example is when you write

f :: [a] -> [a]

The explicit list syntax [a] uses listTyCon but does not mention it explicitly. Ditto tuples. Hence the calls to checkWiredInTyCon in TcHsType.

Ah! I see what is happening.

tc198 uses undefined, which is wired-in.
But the type of undefined mentions CallStack.
In tcImportDecl_maybe, we check for wired-in things, and call loadWiredInHomeInterface -- but only if needWiredInHomeInterface is True.
needWiredInHomeInterace is False of Ids; and even if it were True, we'd load undefined's home interface not CallStack's.

The reason for the loadWiredInHomeInterface stuff is really to get instance declarations. And I suppose that if we have a wired-in Id whose type is Bool -> M.T, then we perhaps ought to check that M.T's home interface is loaded in case there are any instances for it which we might need.

So one solution is to adapt the wired-in-Id case of tcImportDecl_maybe to call checkWiredInTyCon on any TyCons in the Id's type.

But in fact the reason we need the home interface is not because of instances but to get its fingerprint. And it does seem bizarre to me that the fingerprint of a wired-in thing appears in the interface file if thing itself does not. Can't we give all wired-in things a fingerprint of zero? After all, they are wired-in. If we change them we change the compiler, and there are many cases in which changing the compiler requires a clean rebuild. In this case the worst that could happen would be some missing recompilation, and that is also an issue when, for example, we change interface file format.

So I say that for wired-in things

make MkIface.mkHashFun return zero for wired-in names
filter out wired-in names when constructing fingerprints

That would solve this ticket. Technically there is still a potential problem with missing instances (see above) but in practice it doesn't occur, and closing that theoretical hole would take more code and slow down every compilation.

Any other observations/thoughts?

Replying to [ticket:11331#comment:112611 gridaphobe]:

(By the way, how did you figure out what was triggering the load?)

Erm... I used this brand new feature called implicit call stacks to trace back who was making the calls to load GHC.Stack.Types in the successful case. It's a bit odd how useful the feature was in debugging its own implementation.

Thanks for the explanation Simon. I agree with your proposal re wired-in things, I'll send a patch.

It looks like things are a bit more complicated than we thought. Judging by

https://github.com/ghc/ghc/blob/master/compiler/iface/MkIface.hs#L219

and

https://github.com/ghc/ghc/blob/master/compiler/iface/MkIface.hs#L425

ghc already filters out wired-in names during fingerprinting.

In fact, the real problem is that the GHC.Stack.Types.CallStack Name that triggers the exception '''isn't''' wired-in, which is completely bogus.

Trac metadata

Trac field	Value
Differential revisions	- → D1739

Ah yes, that patch looks right. Does it cure the problem?

Yep, make slowtest TEST=tc198 passes locally now.

D1739 is currently blocked on goldfire, since we would like to unwire error and undefined to simplify this whole mess.

mentioned in issue #11405 (closed)

mentioned in commit a7b751db

added 1 deleted label

removed 1 deleted label

closed

This was merged to ghc-8.0 in b31aafb7.

Trac metadata

Trac field	Value
Resolution	Unresolved → ResolvedFixed

mentioned in commit 84782844

added Phighest label

Trac field	Value
Version	7.11
Type	Bug
TypeOfFailure	OtherFailure
Priority	highest
Resolution	Unresolved
Component	Compiler
Test case
Differential revisions
BlockedBy
Related
Blocking
CC	gridaphobe
Operating system
Architecture

panic! TEST=tc198: lookupVers2 GHC.Stack.Types CallStack

Child items ...

Activity