Unfoldings in interface files are sucked in too eagerly
This ticket is a standalone report of a discovery we made in a merge request discussion !6735 (merged); specifically this comment.
There is a long-standing bug in typecking the unfoldings from interface files.
- A call to
hasNoBinding
will force the Id'sUnfolding
- Because of a bug in
tcUnfolding
(see below), that will force the thunk created bytcPragExpr
- And that lint the rhs before returning it
- And Linting will call
hasNoBinding
on Ids mentioned in the rhs - Which will force their unfoldings
- And so on. Never mind loops -- this forces way too much stuff to get sucked in from interface files -- essentially the transitive closure of everything.
Other notes:
-
TODO
tcPragExpr
should be renamed to betcUnfoldingRhs
. -
Note [Linting Unfoldings from Interfaces]
is nothing to do with loops. Consider this decl in an interface file M.hi:f = \x. let y = blah in blah2
When we Lint f's unfolding we will Lint y's unfolding too: see
lintLetBind top_lvl rec_flag binder rhs rhs_ty = do { ... ; ; addLoc (UnfoldingOf binder) $ lintIdUnfolding binder binder_ty (idUnfolding binder)
So there is no point in also Linting y's unfolding during
tcIfaceExpr
on f's RHS. That's all. TODO Could the Note be make clearer? -
I don't see how
idUnfolding
vsrealIdUnfolding
makes the slightest difference. The difference is only that the former consults OccInfo first.
The Real Bug is here:
tcUnfolding toplvl name _ info (IfCoreUnfold stable if_expr)
= do { uf_opts <- unfoldingOpts <$> getDynFlags
; mb_expr <- tcPragExpr False toplvl name if_expr
; ...
; return $ case mb_expr of
Nothing -> NoUnfolding
Just expr -> mkFinalUnfolding uf_opts unf_src strict_sig expr
}
Notice that to choose between NoUnfolding
and CoreUnfolding
(which is returned by mkFinalUnfolding
), we force mb_expr
. ALAS! That is the very thunk we are trying delay forcing!
I think we should always return a CoreUnfolding
thus:
; return $ mkFinalUnfolding uf_opts unf_src strict_sig
(case mb_expr of { Just e -> e; Nothing -> panic })
Or, more directly, use forkM
(not forkM_maybe
) in tcPragExpr
.
This is just what Sam suggests:
What I am trying to do currently is to architect
tcUnfolding
so that it immediately returns an unfolding which we can inspect to obtain the value ofhasNoBinding
Why do we ever use forkM_maybe
? There is a mysterious comment:
forkM_maybe doc thing_inside
= do { unsafeInterleaveM $
do { traceIf (text "Starting fork {" <+> doc)
; mb_res <- tryM thing_inside ;
case mb_res of
Right r -> do { traceIf (text "} ending fork" <+> doc)
; return (Just r) }
Left exn -> do {
-- Bleat about errors in the forked thread, if -ddump-if-trace is on
-- Otherwise we silently discard errors. Errors can legitimately
-- happen when compiling interface signatures (see tcInterfaceSigs)
ifOptM Opt_D_dump_if_trace
(print_errs (hang (text "forkM failed:" <+> doc)
4 (text (show exn))))
I have no idea what these "legitimate" errors are. git-blame tells me that this comment dates back to before this 2003 commit:
commit 98688c6e8fd33f31c51218cf93cbf03fe3a5e73d
Author: simonpj <unknown>
Date: Thu Oct 9 11:59:43 2003 +0000
[project @ 2003-10-09 11:58:39 by simonpj]
-------------------------
GHC heart/lung transplant
-------------------------
This major commit changes the way that GHC deals with importing
types and functions defined in other modules, during renaming and
typechecking. On the way I've changed or cleaned up numerous other
things, including many that I probably fail to mention here.
Moreover this commit removes tcInterfaceSigs
. So I think it's historical.
Proposal (TODO): get rid of forkM_maybe
, and use forkM
all the time.
I think that will solve our problem. But let's leave some careful Notes to explain
this. Notably, it's important that mkFinalUnfolding
and friends all return
a CoreUnfolding
without forcing the unfolding rhs.
All clear?