Opened 3 years ago

Closed 16 months ago

#12567 closed bug (fixed)

`ghc --make` recompiles unchanged files when using `-fplugin` OPTIONS

Reported by: heisenbug Owned by:
Priority: normal Milestone: 8.6.1
Component: Compiler Version: 7.10.3
Keywords: plugin, RecompilationCheck Cc: osa1
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Compile-time performance bug Test Case:
Blocked By: Blocking:
Related Tickets: #7414 Differential Rev(s):
Wiki Page:

Description

Consider this bare-bones module

{-# OPTIONS -fplugin GHC.TypeLits.Normalise #-}

module A where

compiling it will always say

$ ghc --make A.hs
[1 of 1] Compiling A                ( A.hs, A.o ) [GHC.TypeLits.Normalise changed]

even when the module was compiled before.

$ ghc A.hs -c -ddump-if-trace -ddump-hi-diffs

will give the reason:

imported module ‘GHC.TypeLits.Normalise’ is from package ‘ghc-typelits-natnormalise-0.5’, which is not among previous dependencies

Which is the probable cause: when writing the .hi file, the used plugins' package dependencies should be also written out. AFAICS they are extracted fron the DynFlags but for this comparison in

checkDependencies :: HscEnv -> ModSummary -> ModIface -> IfG RecompileRequired

but not found in the ModSummary that was originally written to disk.

This is pretty nasty for clash compilations and the suspected bug is documented here: https://github.com/clash-lang/ghc-typelits-natnormalise/issues/2

Change History (13)

comment:1 Changed 3 years ago by heisenbug

Following patch seems to fix the problem:

  • compiler/iface/MkIface.hs

    diff --git a/compiler/iface/MkIface.hs b/compiler/iface/MkIface.hs
    index 8115583..f6856ae 100644
    a b checkDependencies hsc_env summary iface 
    11121112                         return (RecompBecause reason)
    11131113                 else
    11141114                         return UpToDate
     1115          | moduleName mod `elem` pluginModNames (hsc_dflags hsc_env)
     1116                  ->  do traceHiDiffs $
     1117                           text "imported module " <> quotes (ppr mod) <>
     1118                           text " is from package " <> quotes (ppr pkg) <>
     1119                           text ", and is a plugin"
     1120                         return UpToDate
    11151121          | otherwise
    11161122           -> if pkg `notElem` (map fst prev_dep_pkgs)
    11171123                 then do traceHiDiffs $

Of course I am playing fast and loose with other checks. I should e.g. make sure that it is not regularly found first.

Anyway, can somebody review this, so that I know I am going in the right general direction?

For example as suggested above, the plugin modules' packages could be saved into the .hi files. But I did not find a way to do that, as the DynFlags don't track the packages of plugins, only their ModuleNames.

comment:2 Changed 3 years ago by heisenbug

Type of failure: None/UnknownCompile-time performance bug

comment:3 Changed 3 years ago by carter

If a fix for this this could be considered for 8.0.2 it would seriously improve the user experience for compiler plugins greatly!

comment:4 Changed 3 years ago by Gabor Greif <ggreif@…>

In f8b139f/ghc:

test #12567: add new testcase with expected plugin behaviour

comment:5 Changed 3 years ago by heisenbug

When this issue is fixed 18057549ffebea244d9170377889d096ca9fdbcd can be simply reverted.

comment:6 Changed 3 years ago by ezyang

So, there's actually a deeper set of bugs here. See also #7414 and #7277. It would be nice if we could get our hands on an implementation hash; then we can just record it in the interface file directly as a plugin hash and fix things more properly.

comment:7 Changed 3 years ago by _deepfire

As a person who suffers a lot from this -- what can be done? What is the "next" step? How much competence is required to debug this? How can a relatively clueless person (like me) help?

EDIT: spelling

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

comment:8 Changed 2 years ago by bgamari

Keywords: plugin RecompilationCheck added

comment:9 Changed 23 months ago by augustss

Any chance we can get a fix for this? It really, really hurts. At a minimum, give us a flag to ignore plugin dependencies.

comment:10 Changed 20 months ago by osa1

Cc: osa1 added

comment:11 Changed 20 months ago by mpickering

I wrote a proposal which would resolve this issue.

https://github.com/ghc-proposals/ghc-proposals/pull/108

comment:12 Changed 16 months ago by Ben Gamari <ben@…>

In 1d1e2b7/ghc:

Implement "An API for deciding whether plugins should cause recompilation"

This patch implements the API proposed as pull request #108 for plugin
authors to influence the recompilation checker.

It adds a new field to a plugin which computes a `FingerPrint`. This is
recorded in interface files and if it changes then we recompile the
module. There are also helper functions such as `purePlugin` and
`impurePlugin` for constructing plugins which have simple recompilation
semantics but in general, an author can compute a hash as they wish.

Fixes #12567 and #7414

https://github.com/ghc-proposals/ghc-proposals/blob/master/proposals/002
2-plugin-recompilation.rst

Reviewers: bgamari, ggreif

Reviewed By: bgamari

Subscribers: rwbarton, thomie, carter

GHC Trac Issues: #7414, #12567

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

comment:13 Changed 16 months ago by bgamari

Milestone: 8.6.1
Resolution: fixed
Status: newclosed

Yay, this is fixed!

Note: See TracTickets for help on using tickets.