Opened 3 years ago

Last modified 5 months ago

#4900 new feature request

DEPENDS pragma

Reported by: cdsmith Owned by:
Priority: normal Milestone: 7.8.3
Component: Compiler Version:
Keywords: Cc: michael@…, patrick@…, sol@…, ryant5000@…, idhameed@…, adam@…, simonmar
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Difficulty: Unknown
Test Case: TH_Depends Blocked By:
Blocking: Related Tickets:

Description

Since a seemingly-growing amount of Haskell code is using Template Haskell to read and include data from external sources, it would be nice if GHC recognized a pragma for specifying additional build dependencies for a source file. Such a pragma could be required to appear before the module headers, and look like:

{-# DEPENDS "file.dat" #-}

ghc --make would add it to other dependencies when deciding what modules to rebuild. The file would NOT be one that's generated by some other build step, but rather play the role of an additional source file, so it would have no effect on the order of compiling modules. My gut says to just make it an error if the file does not exist.

One example of a place this is called for is Michael Snoyman's Yesod and external hamlet files (e.g., hamletFile in the hamlet package). These are plastered all over the place with warnings that if you use them, GHC will no longer automatically rebuild everything it needs to.

Attachments (6)

Change History (74)

comment:1 Changed 3 years ago by simonmar

I like this idea. We ought to fix the problem of not checking timestamps on #include files while we're at it too.

comment:2 Changed 3 years ago by igloo

  • Milestone set to 7.2.1

comment:3 Changed 3 years ago by GregWeber

Hi GHC,

I just did a blog post that will end up getting me involved with making GHC a little bit better. There is a discussion on that site and on Reddit that is mostly about records right now.

From the above blog post:

Template Haskell (TH) that performs IO does not automatically recompile itself when it should. In Yesod we have proved the value of having compile-time templates in external files. This puts Haskell on par with dynamic languages in terms of ease of using templates. It actually puts Haskell in a much better position because the templates are evaluated for errors at compile time. But when an external hamlet template changes, it won't automatically get recompiled by ghc. We are able to work around this in our development environment by watching for changes to external templates. However, this is a frail solution, and we believe that GHC should provide this capability for us. We want to have a function loadQQ, which takes a quasi-quoter and a file path. loadQQ will automatically recompile the hs file when the quasi-quoted file has changed.

Note that I am not expecting GHC to necessarily implement a loadQQ function, but just some primitive that would allow us to write a loadQQ function. cdsmith says:

Sure, the alternative (to the proposal in this ticket) of course would be that TH's Q monad would get something like "addDependentFile" that would write info into the result (presumably the hi file?) indicating that this relationship exists

comment:4 Changed 3 years ago by GregWeber

Sorry about the bad formatting. I don't see how an edit link to fix it :(

comment:5 Changed 3 years ago by snoyberg

  • Cc michael@… added

I really like Chris's addDependentFile proposal, that would work very well for the Hamlet use case.

comment:6 Changed 3 years ago by simonpj

I liked your blog post. Haskell does many things well, but there are definitely aspects of the ecosystem (which often get less attention than the language itself) that should better.

I also appreciate that you are willing to roll up your sleeves and help. That the only way things are going to happen, in the end. GHC is too big for one or two people to do everything. So, thank you!

I'm not at all clear about either the problem, or the solution you propose. A good mechanism is to outline both on a wiki page (a ticket isn't such a good place) and discuss on ghc-users or cvs-ghc depending on the level of detail. There are lots of examples of such pages on http://hackage.haskell.org/trac/ghc/wiki/Commentary under "Contributed documentation".

Simon

comment:7 Changed 3 years ago by snoyberg

I've put up a short explanation of the problem: http://hackage.haskell.org/trac/ghc/wiki/DependencyTracking

comment:8 Changed 3 years ago by simonmar

I think the proposed solution is fine. (note that a related problem was fixed in GHC 7.2.1: see #481)

It will need some plumbing and some refactoring in GHC. The list of depended-upon files has to be collected in the Q monad and stored in the ModIface, and then we have to check the dates of those files when deciding whether to recompile. The obvious place to do the check is in MkIface.checkOldIface, but it doesn't have information about the modification time of the object file - we would have to plumb that information in, probably by elaborating the SourceModified parameter.

This would also be useful in fixing another bug we have: when using CPP, we don't recompile when a #included file changes.

comment:9 Changed 3 years ago by simonpj

Simon is right. I've elaborated the wiki page (and renamed it to http://hackage.haskell.org/trac/ghc/wiki/DependencyTracking, because Trac doesn't do well for pages with spaces in their names).

Looks like a plan to me.

Simon

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

Note that the DEPENDS pragma idea in this ticket is better than an addDependentFile function in TH, for two reasons:

  • ghc -M could emit these dependencies, because we only need to read the pragma, not compile the module
  • Cabal and other external tools will never be able to track these dependencies. There are plans to do this in Cabal in the future, and addDependentFile would just not work there.

DEPENDS is not nearly as nice as addDependentFile, but if we want to be able to do dependency analysis without compiling code, I think we have to stick with DEPENDS. We could still have a checkDependentFile that would emit a warning if you had forgotten the DEPENDS pragma.

Another design question is whether the filename is relative or absolute - do we resolve it to an absolute name at compile time?

comment:11 Changed 3 years ago by GregWeber

DEPENDS presumably wants *just* a file path. In Yesod, we allow our users to configure where their templates will be located.

module Settings (hamletFile) where

hamletFile :: FilePath -> Q Exp
hamletFile f = "myTemplateDir/" ++ f 
module UseTemplate where
import Settings

$(renderHamlet $(hamletFile "myfile.hamlet"))

Is there a way to do this in a compiler pragma?

comment:12 in reply to: ↑ 10 Changed 3 years ago by cdsmith

Replying to simonmar:

Note that the DEPENDS pragma idea in this ticket is better than an addDependentFile function in TH, for two reasons:

Agreed, there are definitely advantages that way, which is a lot of why I originally proposed it that way.

I recall that in the context of optimizing the fclabels package a few weeks ago, it came up that TH could actually emit INLINE pragmas. Is there a design where it would be possible for TH to emit DEPENDS pragmas? Then we'd have two scenarios:

  1. You could add an explicit DEPENDS pragma, and this would make ghc -M and other external tools work.
  1. You could have your TH emit a DEPENDS pragma, which means it would be opaque to static tools like ghc -M and cabal, but GHC would still use it when deciding whether to rebuild.

In some ways, this seems messy; but the only effect of resolving TH would be to *add* dependencies, so the statically derived list would at least be an approximation to the complete list.

comment:13 Changed 3 years ago by simonpj

Note that addDependentFile is pretty much synonymous with "emitting a DEPENDS pragma".

comment:14 Changed 3 years ago by GregWeber

I understand that it would be much better to have a DEPENDS pragma, however, I am wondering if we can get away without requiring it. There could be a warning that can be turned on and off. The warning could state the exact DEPENDS information missing to make it easy to add DEPENDS with the proper file path. And a library writer distributing their code should heed such a warning, but maybe it is ok for the application writer (our main Yesod use case) to ignore it?

comment:15 Changed 3 years ago by simonpj

Talking to Simon we understand the following language extensions:

  • A {-# DEPENDS "foo" #-} pragma
  • A TH function addDependentFile

These would support two somewhat separate features:

  1. ghc -M would generate dependencies based on
    • import declarations (as now)
    • {-# DEPENDS "foo" #-} pragmas
    • #line pragamas generated by CPP
  1. The ghc recompilation check (which skips compilation when nothing has changed) would take account of
    • import declarations
    • {-# DEPENDS "foo" #-} pragmas
    • #line pragamas generated by CPP
    • addDependentFile calls produced in the previous compilation

Note that (1) and (2) are not identical... ghc -M can't run TH to see what addDependentFile calls it makes.

So this is not entirely satisfactory, but perhaps useful enough to be worth doing. If anyone wants to volunteer -- it's not too hard.

Simon

comment:16 Changed 3 years ago by GregWeber

I chose the wrong time to want to improve GHC :). I am really busy right now but trying to reserve some time to help with this. Because of that, I made a call for help on the Yesod mail list and blog, but nobody was interested.
Let us know if a deadline comes up for getting into the next GHC release. Thanks!

comment:17 Changed 3 years ago by igloo

For GHC 7.4, it needs to be in the repo by the end of October.

comment:18 Changed 3 years ago by GregWeber

So I started making changes based on the excellent pointers here and on the wiki page. I am not sending a pull request on github, but I though we could view the diff over there:
https://github.com/gregwebs/ghc/compare/ghc:29a97fded4010bd01aa0a17945c84258e285d421...dependent

I am very new to ghc, doing the best I can, but need some help. In particular I am wondering if this needs to be integrated with ModGuts?. I seem to have something which might possibly work for the mkIfaceTc entry point, but not for mkIface entry point.

comment:19 Changed 3 years ago by simonmar

Yes, you'll need to add the list of dependent files to ModGuts too, so that it gets plumbed through from the typechecker to the final iface, which is generated much later.

comment:20 Changed 3 years ago by GregWeber

So I am fiddling with the Quasi Monad now. I would like to call
getGblEnv/setGblEnv from withing a new addDependentFile function, although I know that isn't right. How do I actually go about this?

comment:21 Changed 3 years ago by GregWeber

I am hoping to finish this weekend during the VirtuaHac? so i will be done in time for the next GHC release.

Still need help though, particularly with connecting GHC to the Quasi Monad. Latest diff is here:
https://github.com/gregwebs/ghc/compare/ghc:ceef80b2fbd414c701bb2a346226a357475983ad...dependent2

comment:22 Changed 3 years ago by GregWeber

I figured out how to hook up GHC to TH now. compiles, need to test. TH changes can be seen here: https://github.com/yesodweb/packages-template-haskell/compare/master...dependent2

comment:23 Changed 2 years ago by GregWeber

  • Status changed from new to patch

comment:24 Changed 2 years ago by simonpj

Thanks for the patches. They look right to me. Except that don't you need something in TcSplice, where we make TcM an instance of Quasi to give the appropriate instance for qAddDependentFile?

Ian will commit, but first.... Template Haskell is greviously lacking in documentation. The right place to document the new operation is in the Haddock docs; current snapshot is here:http://www.haskell.org/ghc/dist/current/docs/html/libraries/template-haskell-2.6.0.0/Language-Haskell-TH.html

Could you add documentation for your new operation, and for any others you feel moved to document; report, recover, reifyInstances, isInstance seem prime candidates, but you may have other ideas too.

Many thanks

Simon

comment:25 Changed 2 years ago by GregWeber

  • Status changed from patch to infoneeded

Thanks for looking. Did you missed the instance for TcSplice? at the end of the diff:

https://github.com/gregwebs/ghc/compare/ghc:ceef80b2fbd414c701bb2a346226a357475983ad...dependent2#L7R943

Wanted to make sure that what I was doing made sense. Testing now.

comment:26 Changed 2 years ago by simonpj

Ah yes, that looks right -- good!

comment:27 Changed 2 years ago by GregWeber

  • Status changed from infoneeded to patch

comment:28 Changed 2 years ago by luite

Thanks Greg, great work. I just built and tested it on OS X 10.7.2 with XCode 4.1, and found no problems. I hope it's still in time for 7.4.1

comment:29 Changed 2 years ago by GregWeber

Test case we used is here: https://gist.github.com/1341783

comment:30 Changed 2 years ago by simonpj

  • Owner set to igloo
  • Priority changed from normal to high

Ian, can you commit, please? Thanks

Simon

comment:31 Changed 2 years ago by greg@…

commit b994313a1f7b233ec5da31d004a5db92758b0836

Author: Greg Weber <greg@gregweber.info>
Date:   Fri Oct 7 17:28:15 2011 -0700

    addDependentFile #4900
    
    Let GHC know about an external dependency that Template Haskell uses
    so that GHC can recompile when the dependency changes.
    No support for ghc -M
    
    There is a corresponding addition to the template-haskell library

 compiler/deSugar/Desugar.lhs      |    5 +++-
 compiler/iface/BinIface.hs        |   40 ++++++++++++++++++++++++++++--------
 compiler/iface/LoadIface.lhs      |    2 +
 compiler/iface/MkIface.lhs        |   41 +++++++++++++++++++++++++++----------
 compiler/main/HscMain.hs          |    3 +-
 compiler/main/HscTypes.lhs        |   10 +++++++-
 compiler/typecheck/TcRnDriver.lhs |    6 ++++-
 compiler/typecheck/TcRnMonad.lhs  |    5 +++-
 compiler/typecheck/TcRnTypes.lhs  |    2 +
 compiler/typecheck/TcSplice.lhs   |    5 ++++
 10 files changed, 93 insertions(+), 26 deletions(-)

comment:32 Changed 2 years ago by igloo

  • Resolution set to fixed
  • Status changed from patch to closed
  • Test Case set to TH_Depends

Pulled, and test added.

comment:33 Changed 2 years ago by GregWeber

  • Owner igloo deleted
  • Resolution fixed deleted
  • Status changed from closed to new

Thank you for the swift commit and making a real test case!

This ticket was originally opened for a DEPENDS pragma, and the patch implemented just addDependentFile, which is my personal concern. Also mentioned was #include support. If I can get pointed in the right direction for #include changes I could look at doing that next.

I am re-opening the ticket.

comment:34 Changed 2 years ago by simonmar

I'm looking at doing the changes to track #includes (see also #3589).

comment:35 Changed 2 years ago by marlowsd@…

commit 3f34e0913efcc82dd90be56d04c5e57ec60d3677

Author: Simon Marlow <marlowsd@gmail.com>
Date:   Fri Nov 18 12:46:01 2011 +0000

    Track #included files for recompilation checking (#4900, #3589)
    
    This was pretty straightforward: collect the filenames in the lexer,
    and add them in to the tcg_dependent_files list that the typechecker
    collects.
    
    Note that we still don't get #included files in the ghc -M output.
    Since we don't normally lex the whole file in ghc -M, this same
    mechanism can't be used directly.

 compiler/main/GHC.hs              |   11 ++++++---
 compiler/main/HscMain.hs          |   41 +++++++++++++++++++++++++++++-------
 compiler/main/HscTypes.lhs        |   21 +++++++++++++++++++
 compiler/parser/Lexer.x           |    6 +++++
 compiler/typecheck/TcRnDriver.lhs |   16 ++++++++++---
 compiler/typecheck/TcRnMonad.lhs  |    6 +++++
 compiler/typecheck/TcRnTypes.lhs  |    2 +-
 7 files changed, 86 insertions(+), 17 deletions(-)

comment:36 Changed 2 years ago by simonmar

Still to do:

  • the DEPENDS pragma itself (although I think we've covered the important use cases with qAddDependentFile and the #include support I just added)
  • Support for tracking #include files in ghc -M, covered by #3588

comment:37 Changed 2 years ago by igloo

  • Milestone changed from 7.4.1 to 7.6.1
  • Priority changed from high to normal

Remilestoning for the DEPENDS pragma part.

comment:38 Changed 2 years ago by shelarcy

I found another problem. GHC 7.4.1 RC 1 doesn't export addDependentFile function.

comment:39 Changed 2 years ago by GregWeber

  • Owner set to igloo

Sorry for not exporting! Ian, can you please get the export into the final GHC release?

diff --git a/Language/Haskell/TH/Syntax.hs b/Language/Haskell/TH/Syntax.hs
index 8533266..9c464cf 100644

--- a/Language/Haskell/TH/Syntax.hs
+++ b/Language/Haskell/TH/Syntax.hs
@@ -26,7 +26,7 @@ module Language.Haskell.TH.Syntax(
        Q, runQ, 
        report, recover, reify, 
         lookupTypeName, lookupValueName,
-       location, runIO,
+       location, runIO, addDependentFile,
         isInstance, reifyInstances,
 
        -- * Names

comment:40 Changed 2 years ago by igloo

  • Difficulty set to Unknown
  • Milestone changed from 7.6.1 to 7.4.1
  • Priority changed from normal to highest

OK, although I'm a little confused as to why we export a number of specialised functions, rather than just having people use the generic qAddDependentFile etc.

comment:41 Changed 2 years ago by GregWeber

I don't know exactly, honestly I am just following the pattern, which if we wanted to change we should change for all.

comment:42 Changed 2 years ago by igloo

  • Milestone changed from 7.4.1 to 7.6.1
  • Owner igloo deleted
  • Priority changed from highest to normal

Yep, makes sense. Done in 7.4.1 (73e2749647f8ac21dbb0eea3d6df7c6834494a04) and HEAD (a711f138302b6d7ca043b67e8f0a907a560fcefb).

Remilestoning again for the DEPENDS pragma part.

comment:43 Changed 2 years ago by parcs

  • Cc patrick@… added
  • Status changed from new to patch

Hi,

Currently this feature does not work with GHCi, as that has its own recompilation checking for determining whether to pass a module down to the "standard" compilation pipeline.

I have attached a patch that extends this feature to work GHCi, so that modules will be reloaded when their usage files have changed. Overview of changes:

  1. A field is added to ModSummary? to keep track of the latest modification time of the module's usage files (ms_uf_date)
  2. When summarising a module/file, ms_uf_date is checked for freshness and updated if necessary.
  3. A module is now marked unstable if ms_uf_date is later than its 'linkableTime'.

Is this a reasonable solution?

comment:44 Changed 2 years ago by simonmar

  • Component changed from Build System to Compiler
  • Owner set to simonmar

Ah yes, I had forgotten about the stability test. The patch looks good, but I'll add a comment or two when I merge it.

comment:45 Changed 2 years ago by parcs

Okay, great. I will study your comments and try to better my understanding of what kind of code requires commentary so that I can provide more complete patches in the future.

comment:46 Changed 2 years ago by simonpj

Simon M will apply and add a comment

comment:47 follow-up: Changed 2 years ago by simonmar

While looking at the patch I noticed something odd: in order to look up the modification times of the "usage files" while constructing the ModSummary, we have to look for the ModIface in the HPT. The first time around, it won't be there, so we'll have Nothing for the ms_uf_date, but the second time it will, which will force us to re-summarise all the modules. This is very fishy. I'm worried about (a) correctness when the module is not in the HPT, and (b) performance.

I'm leaving the patch for now until I figure out the right thing to do.

comment:48 Changed 2 years ago by SimonHengel

  • Cc sol@… added

It would be awesome to get support for GHCi into 7.6.1.

comment:49 Changed 22 months ago by simonpj

  • Owner simonmar deleted
  • Status changed from patch to new

Simon H: Simon M is deeply snowed under at the moment. Might you look into the question he raises above? We're a bit stalled.

comment:50 in reply to: ↑ 47 Changed 22 months ago by parcs

  • Status changed from new to patch

Replying to simonmar:

While looking at the patch I noticed something odd: in order to look up the modification times of the "usage files" while constructing the ModSummary, we have to look for the ModIface in the HPT. The first time around, it won't be there, so we'll have Nothing for the ms_uf_date, but the second time it will, which will force us to re-summarise all the modules. This is very fishy. I'm worried about (a) correctness when the module is not in the HPT, and (b) performance.

I'm leaving the patch for now until I figure out the right thing to do.

It turns out that, very conveniently, the mtimes of a module's usage files are already calculated during compilation. With that in mind, I have created a new patch addresses this issue and also makes it a little simpler to reason about.

comment:51 Changed 22 months ago by parcs

Oops, I meant to rewrite the previous patch.

comment:52 Changed 22 months ago by simonpj

  • Owner set to simonmar

comment:53 follow-up: Changed 22 months ago by simonmar

I looked at the new patch, and maybe I'm being stupid here, but it looks like my previous concerns still apply. Looking at the HPT during the summarise stage rings alarm bells - the hsc_env should only be used for its DynFlags and the finder cache. So I'm still not sure how to reason that this patch works correctly.

comment:54 Changed 22 months ago by simonmar

  • Owner simonmar deleted
  • Status changed from patch to new

comment:55 in reply to: ↑ 53 Changed 22 months ago by parcs

Replying to simonmar:

I looked at the new patch, and maybe I'm being stupid here, but it looks like my previous concerns still apply. Looking at the HPT during the summarise stage rings alarm bells - the hsc_env should only be used for its DynFlags and the finder cache. So I'm still not sure how to reason that this patch works correctly.

Regarding your previous concerns, the important part of this new patch is that ms_uf_date is not read at all during the summarize stage; it is only used to propagate the latest usage file date computed during the summarize stage to the stability check. Instead of comparing the previous ms_uf_date to the latest ms_uf_date, as the last patch did, the usg_mtimes are compared and calculated piecewise from the module's HMI. This is better because these mtimes get calculated during the upsweep, meaning that they will be as accurate as they can be for the next load cycle. (Compare the current isOkUsageFileDate with the previous patch's getUsageFileDate).

I _believe_ that reading the HPT in the way that is done in the patch is safe -- it is only read if an old summary is available. Is it guaranteed that if an old summary is available, then a corresponding HMI exists within the HPT? It seems so, but I am not sure.

Also, I should mention that it is necessary to compute and check the usage file dates before or during the summarise stage, because that's when a file is ran through the preprocessor, and a usage file may be a CPP include. Without the HPT, the only real information you have at that point is an old ModSummary. Therefore, if the HPT should not be read during the summarise stage, then it seems that the only alternative would be to stuff the UsageFile data from the HMI to a field of the corresponding ModSummary. An HMI is created during the upsweep stage, so the hsc_mod_graph would have to be updated after the upsweep occurs, which is currently not done and doing so would probably introduce its own subtleties. (Currently, the mod graph is only updated right after the summarise stage). Does this seem more reasonable to you?

Changed 22 months ago by parcs

Alternative implementation

comment:56 Changed 22 months ago by parcs

I have attached a patch that implements this functionality in such a way that the HPT is not touched at all during the summarise stage. Instead, the required information that would have been read from the HPT during the summarise stage is now pulled out of the generated ModIface and into a field in the ModSummary during the upsweep stage. After the upsweep, the module graph is reset with the updated ModSummarys containing the necessary, fresh data for use in the next load cycle. Overall, I think this implementation is nicer and easier to reason about than the previous one.

(As a bonus, the hsc_mod_graph is now kept sorted in bottom..top order and so the "Ok, modules loaded: ..." message will list the modules in a logical order instead of the seemingly-random order they're currently listed. This change makes one of the tests in the ghci testsuite trivially fail, though.)

I think the patch is incomplete, still, at least until the uses of linkableTime are looked over to make sure they don't operate under the assumption that linkableTime is simply the timestamp of the source file anymore.

comment:57 Changed 20 months ago by igloo

  • Milestone changed from 7.6.1 to 7.6.2

comment:58 Changed 19 months ago by ryant5000

  • Cc ryant5000@… added

comment:59 Changed 16 months ago by ihameed

  • Cc idhameed@… added

comment:60 Changed 6 months ago by adamgundry

  • Cc adam@… added
  • Milestone changed from 7.6.2 to 7.8.1
  • Status changed from new to patch

I've updated Patrick's patch to the latest HEAD, which involved switching to use file content hashes rather than modification times (per the fix to #8144) and integrating with the new parallel upsweep (#910).

comment:61 Changed 6 months ago by simonpj

Thanks. I think we need Simon M's ok here, and then perhaps it's good to do. I hope there is good user documentation.

Simon

comment:62 Changed 6 months ago by thoughtpolice

  • Cc simonmar added

This patch looks good to me.

SimonM - if it passes your glance, then I'll merge ASAP.

comment:63 follow-up: Changed 6 months ago by simonmar

I still need to review this patch carefully. At first glance it's not obviously right, since we don't normally modify ModSummary during the upsweep - the ModSummary is something that is supposed to be collected by the downsweep. But I don't have all the details in my head right now, I'll try to come back to this when I have more time.

comment:64 in reply to: ↑ 63 Changed 6 months ago by parcs

Replying to simonmar:

At first glance it's not obviously right, since we don't normally modify ModSummary during the upsweep - the ModSummary is something that is supposed to be collected by the downsweep.

A module's list of usage files can only be determined _after_ compiling the module because the list includes files added by TH's qAddDependencies. So one way or another, usage-file information has to be propagated from a previous upsweep to a future downsweep (and to the stability check that follows).

I proposed two independent solutions:

  1. During the downsweep, look inside the HPT to extract the usage-file information from an existing HMI and stuff it into the ModSummary.
  2. During the upsweep, extract the usage-file information from the new HMI and update the module graph with this information. A future downsweep will use this updated module graph.

But these solutions don't exactly satisfy the existing invariants of the downsweep and upsweep. For (1), we shouldn't read the HPT during the downsweep. For (2), we shouldn't update a ModSummary during the upsweep. But I can't think of another solution. I personally like (1), especially since there are two independent upsweeps now. The patch that Adam attached is an updated version of (2).

I think it would help review if a future patch was split into several logical patches. It's not a lot of code but it's somewhat tricky code.

This is just my understanding of the state of this ticket.

comment:65 follow-up: Changed 6 months ago by simonmar

I don't want to block this fix, since there's a real problem. On the other hand, the fix is not a simple one because both proposed implementations change some key assumption about the flow of data, in what is already a complicated part of the compiler.

I think I'm ok with the idea of adding the usg_file information to the ModSummary? during the downsweep after a previous upsweep. I think this is the first patch, correct? But what we need is some careful documentation about how it works, preferably with a worked example ("the second downsweep adds the following information to the ModSummary?, but it doesn't cause recompilation because..."). I'm concerned about things like re-summarising modules unnecessarily. The other thing we need is some good tests (the recomp tests are under driver), testing for both too much and too little recompilation.

comment:66 Changed 6 months ago by parcs

  • Owner set to parcs

Okay, great. I will try to create an extensive patchset.

comment:67 in reply to: ↑ 65 Changed 5 months ago by parcs

I hit a couple of obstacles regarding the implementation:

  1. There is no way to distinguish between usage files added via TH and other usage files (e.g. #included files). This is necessary to avoid re-summarization when only the former kind of usage file has changed.
  2. The usage files of a boot module get lost after compiling the corresponding source module. This means that modifying a usage file included by a boot module will not trigger recompilation of the boot module in GHCi.

The first obstacle could be solved by adding a new field to the UsageFile constructor containing the kind of usage file (LINE pragma, #include, TH, etc..) and checking only the relevant kinds of usage files during re-summarisation.

I don't know what could be done about 2nd obstacle, though. It looks like we can't depend on reading the HPT during the downsweep since the boot interface gets overwritten by the source interface after the source module gets compiled.

Last edited 5 months ago by parcs (previous) (diff)

comment:68 Changed 5 months ago by simonmar

  • Owner parcs deleted
  • Status changed from patch to new

Moving out of the patch state pending resolution of obstacles...

Changed 4 months ago by edsko

Updated Adam's patch for latest HEAD

Note: See TracTickets for help on using tickets.