Opened 9 years ago

Closed 3 years ago

Last modified 6 months ago

#481 closed bug (fixed)

Recompilation check fails for TH

Reported by: simonpj Owned by: simonmar
Priority: normal Milestone: 7.2.1
Component: Template Haskell Version: 6.4.1
Keywords: Cc: jonas.duregard@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect result at runtime Difficulty: Unknown
Test Case: TH_recompile Blocked By:
Blocking: Related Tickets:

Description (last modified by igloo)

The recompilation check only recompiles a module when
the interface of a module it imports changes. But
with Template Haskell, it may need to be recompiled
when the implementation changes.

Concrete example below. It's quite awkward to fix.

  • Perhaps a module that contains any splices should be recompiled always.
  • Perhaps a module that exports any TH stuff (how would we tell?) should be flagged as changed if anything about it changes.

Simon

The following scenario reproduces this error
(thanks to Bulat Ziganshin bulatz@…):

1) create Main.hs containing code

module Main where
import Sub
main = print $x

and Sub.hs containing code

module Sub where
x = [| 1 |]

2) compile them with --make:

C:\!\Haskell\!>ghc --make -fth Main.hs
Chasing modules from: Main.hs
Compiling Sub              ( ./Sub.hs, ./Sub.o )
Compiling Main             ( Main.hs, Main.o )
Loading package base-1.0 ... linking ... done.
Loading package haskell98-1.0 ... linking ... done.
Loading package template-haskell-1.0 ... linking ... 
done.
Linking ...

C:\!\Haskell\!>main.exe
1

3) now change Sub.hs to the following code:

module Sub where
x = [| 2 |]

4) and recompile program:

C:\!\Haskell\!>ghc --make -fth Main.hs
Chasing modules from: Main.hs
Compiling Sub              ( ./Sub.hs, ./Sub.o )
Skipping  Main             ( Main.hs, Main.o )
Linking ...

C:\!\Haskell\!>main.exe
1

As you see, Main.hs is not recompiled despite the fact
that definition
of x is changed and now program must print "2"

Change History (14)

comment:1 Changed 8 years ago by simonmar

  • Architecture set to Unknown
  • Description modified (diff)
  • Difficulty set to Unknown
  • Operating System set to Unknown
  • Version changed from None to 6.4.1

comment:2 Changed 8 years ago by igloo

  • Description modified (diff)
  • Milestone set to _|_
  • Test Case set to TH_recompile

comment:3 Changed 6 years ago by simonmar

  • Architecture changed from Unknown to Unknown/Multiple

comment:4 Changed 6 years ago by simonmar

  • Operating System changed from Unknown to Unknown/Multiple

comment:5 Changed 5 years ago by simonmar

  • Type of failure set to Incorrect result at runtime

comment:6 Changed 4 years ago by igloo

  • Description modified (diff)

comment:7 Changed 3 years ago by simonpj

  • Cc jonas.duregard@… added

See also http://www.haskell.org/pipermail/glasgow-haskell-users/2011-July/020552.html

Jonas says "This has caused headaches for me on several occasions lately".
I wonder how many other people have experienced such headaches.

Simon

comment:8 follow-up: Changed 3 years ago by simonmar

I think we should just disable the recompilation check for modules that use TH or annotations. The compilation manager will still do the basic check based on modification times of object files, so if nothing has changed below the TH module we won't recompile it, but if anything has changed we will.

comment:9 in reply to: ↑ 8 Changed 3 years ago by JonasDuregard

Replying to simonmar:

I think we should just disable the recompilation check for modules that use TH or annotations.

I agree. The problem now is that the recompilation checker gives a false negative, a few false positives is not a big issue in comparison.

Here is an alternative solution, if I have understood the mechanisms involved correctly:

  • We flag modules as one of three three change levels: unchanged, implementation-changed and interface-changed. This should be simple.
  • We mark imports as compile time and/or run time depending on if imported functions are used in compile time code. I guess this might be a bit more tricky.
  • We recompile if any import is interface-changed or if a compile time import is implementation-changed.

As a bonus, marking imports is a step towards an optimization which I'm not sure is currently implemented: if an import is marked TH only, then we can drop it after evaluating compile time code (this only works if the generated code does not use functions from the imported module).

I have intended to get hacking on GHC for some time, so maybe looking into this alternative is a good start.

Jonas

comment:10 Changed 3 years ago by simonpj

  • Milestone changed from _|_ to 7.2.1
  • Owner changed from simonpj to simonmar
  • Priority changed from low to normal

Simon's going to implement the simplest correct thing.

comment:11 Changed 3 years ago by marlowsd@…

commit 48bc81ad466edfc80237015dbe5d78ba70eb5095

Author: Simon Marlow <marlowsd@gmail.com>
Date:   Wed Jul 20 09:37:54 2011 +0100

    Fix #481: use a safe recompilation check when Template Haskell is
    being used.
    
    We now track whether a module used any TH splices in the ModIface (and
    at compile time in the TcGblEnv and ModGuts).  If a module used TH
    splices last time it was compiled, then we ignore the results of the
    normal recompilation check and recompile anyway, *unless* the module
    is "stable" - that is, none of its dependencies (direct or indirect)
    have changed.  The stability test is pretty important - otherwise ghc
    --make would always recompile TH modules even if nothing at all had
    changed, but it does require some extra plumbing to get this
    information from GhcMake into HscMain.
    
    test in driver/recomp009

 compiler/deSugar/Desugar.lhs      |   10 +++-
 compiler/iface/BinIface.hs        |   14 ++++--
 compiler/iface/LoadIface.lhs      |    1 +
 compiler/iface/MkIface.lhs        |   66 +++++++++++++++------------
 compiler/main/DriverPipeline.hs   |   24 ++++++----
 compiler/main/GHC.hs              |    5 ++
 compiler/main/GhcMake.hs          |   23 +++++----
 compiler/main/HscMain.lhs         |   92 +++++++++++++++++++++++++------------
 compiler/main/HscTypes.lhs        |   33 ++++++++++++-
 compiler/typecheck/TcRnDriver.lhs |    5 +-
 compiler/typecheck/TcRnMonad.lhs  |    9 +++-
 compiler/typecheck/TcRnTypes.lhs  |    5 ++
 compiler/typecheck/TcSplice.lhs   |    2 +
 13 files changed, 196 insertions(+), 93 deletions(-)

comment:12 Changed 3 years ago by simonmar

  • Status changed from new to merge

comment:13 Changed 3 years ago by igloo

  • Resolution changed from None to fixed
  • Status changed from merge to closed

comment:14 Changed 6 months ago by Ian Lynagh <igloo@…>

In 27d0701d92b71c29f5260998d7579e60611ef87d/ghc:

Add test TH_recompile for trac #481 (Recompilation check fails for TH)
Note: See TracTickets for help on using tickets.