Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#12035 closed bug (fixed)

hs-boot knot tying insufficient for ghc --make

Reported by: ezyang Owned by:
Priority: normal Milestone:
Component: Compiler (Type checker) Version: 8.1
Keywords: hs-boot backpack Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D2213
Wiki Page:

Description

GHC in one-shot mode goes through great pains to make sure that if we are typechecking A.hs with an A.hs-boot, any references to A in the interfaces we load in should point to the TyThings from our LOCAL tcg_type_env (_var), not the declarations from the hs-boot. Indeed, it is never right to load the hi-boot file into the EPS (#10182, #10336). Because in one-shot compilation we are loading things into the EPS anew, there is not much risk of having stale TyThings lying around which still refer to the old hs-boot ModDetails.

ghc --make does not do a sufficient job as knot tying. Specifically, any Ids/TyThings from the HPT which were built against the hs-boot ModDetails will have out-of-date information. We do manage to tie the knot if we use tcLookupGlobal directly, which does look in the local type environment (e.g., #12034 affects both -c and --make), but if we get our hands on a TyThing indirectly, we're in trouble. The case where this is most obvious is unfoldings; see #10083 for an example of GHC behaving differently between -c and --make (although, ironically, --makes bad behavior masks the bug).

It's not entirely clear how to fix this problem. In analogy to what happens in -c, we should clear the stale TyThings from our HPT and retypecheck them after we have setup the type environment for our hs file. But presently in --make we don't lazily load types from our home package. Maybe we should!

Change History (11)

comment:1 Changed 3 years ago by simonpj

I don't understand. In --make mode, doesn't GhcMake.typecheckLoop re-typecheck all the modules that depended on the hs-boot file? Isn't that precisely what typecheckLoop is for?

comment:2 Changed 3 years ago by ezyang

This PDF gives the full story, http://web.mit.edu/~ezyang/Public/backpack-symbol-tables.pdf but the short answer is that there are two ways we loop tie hs-boot. The first, which is correctly implemented for --make, is retypechecking all the modules that depended on the hs-boot file. The second is tying the loop at the same time we are typechecking the hs module which implements the hs-boot file; i.e., it's what tcg_type_env_var is for. If we have:

-- A.hs-boot
module A where
x :: Bool

-- B.hs
module B where
import {-# SOURCE #-} A
y = not x

-- A.hs
module A where
import B
x = True
z = not y

When we typecheck A.hs, does the unfolding for y have an up-to-date unfolding for x? In one-shot mode the answer is yes (and thus we see the behavior of #10083), but in make mode the answer is no. The retypecheck loop has nothing to do with it, since it happens AFTER we finish building the hs file.

comment:3 Changed 3 years ago by ezyang

Differential Rev(s): Phab:D2213
Status: newpatch

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

In 8fd1848/ghc:

Retypecheck both before and after finishing hs-boot loops in --make.

Summary:
This makes ghc --make's retypecheck behavior more in line
with ghc -c, which is able to tie the knot as we are typechecking.

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

Test Plan: validate

Reviewers: simonpj, austin, bgamari

Subscribers: thomie

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

GHC Trac Issues: #12035

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

In e528061/ghc:

We also need to retypecheck before when we do parallel make.

Summary:
Kept this seperate from the previous patch for clarity.
Comes with a test.

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

Test Plan: validate

Reviewers: simonpj, austin, bgamari

Subscribers: thomie

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

GHC Trac Issues: #12035

comment:6 Changed 3 years ago by ezyang

Resolution: fixed
Status: patchclosed

comment:7 Changed 3 years ago by simonpj

Edward, both these changes are small but subtle. But they both lack any commentary. Would it be worth adding some?

Simon

comment:9 Changed 3 years ago by simonpj

Very good -- but that wiki page is not referred to by the crucial code changes in the above patches. And it might be worth connecting the code changes to the tickets... an example (in the ticket) is often so illuminating.

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

In 1f75440/ghc:

Extra comments, as per SPJ in #12035.

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

comment:11 Changed 3 years ago by ezyang

Done!

Note: See TracTickets for help on using tickets.