I'm ok with that, thanks. Can you put your comments below into DsMonad.hs-boot so that we don't lose the reasoning? It's devilish hard to work out *why* a hs-boot file must exist, sometimes.
Maybe also update http://ghc.haskell.org/trac/ghc/wiki/Commentary/ModuleStructure which tries to document some these loops too. Simon | -----Original Message----- | From: Edsko de Vries [mailto:[email protected]] | Sent: 11 September 2013 15:33 | To: Simon Peyton-Jones | Cc: Luite Stegeman; ghc-devs; Edsko de Vries | Subject: Re: GHC 7.8 release status | | Hi all, | | So I managed to remove 3 out of 4 of the -boot files. The one that | remains, ironically, is the DsMonad.hs-boot. DsMonad has a | (transitive) dependency on Hooks in at least two ways: once through | Finder, which imports Packages, which imports Hooks; but that's easily | solved, because Finder can import PackageState instead. However, it is | less obvious to me how to resolve the following import cycle | | - DsMonad imports tcIfaceGlobal from TcIface | - TcIface imports (loadWiredInHomeIface, loadInterface, loadDecls, | findAndReadIface) from LoadIface | - LoadIFace imports Hooks | | (There might be still others, this is the most direct one at the | moment.) | | (Just to be clear, Hooks imports DsMonad because it needs the DsM type | for the dsForeignsHook.) | | I'm sure this cycle can be broken somehow, but I'm not familiar enough | with this part of the compiler to see if there is a natural point to | do it. As things stand, we have a DsMonad.hs-boot which just exports | the DsGblEnv, DsLclEnv, and DsM types. I don't know if this is | something we should be worrying about or not? | | Just to summarize: the hooks patch as things stand now introduces the | Hooks enumeration, rather than a separate type per hook so that we | have a central and type checked list of all hooks; in order to do | that, it moves some things around (some times moves to HscTypes), | introduces a new module called PipelineMonad as per SPJ's suggestion, | and introduces a single additional boot file for the DsMonad module. | | Edsko | | On Tue, Sep 10, 2013 at 12:40 PM, Simon Peyton-Jones | <[email protected]> wrote: | > I do like the single record. | > | > | > | > I would really really like a strong clear Note [blah] on the | hooks::Dynamic | > field of DynFlags. It's *so* non-obvious why it's dynamic, and the | reason is | > a really bad one, namely the windows DLL split nonsense. (Not our | fault but | > still needs very clear signposting.) | > | > | > | > I don't understand why we need 4 new hs-boot files. Eg why | DsMonad.hs-boot? | > It should be safely below Hooks. | > | > | > | > Linker.hs-boot is solely because of LibrarySpec. It would be possible | to | > push that into HscTypes. (Again with a comment to explain why.) | > | > | > | > DriverPipeline is aleady 2,100 lines long, and could reasonably be | split | > with CompPipeline in the PipelineMonad module, say. | > | > | > | > | > | > In other words, a bit of refactoring might eliminate the loops *and* | > sometimes arguably improve the code. | > | > | > | > | > | > I don't feel terribly strongly about all this. It does feel a bit ad | hoc... | > in a variety of places (eg deep in Linker.hs) there are calls to | hooks, and | > it's not clear to me why exactly those are the right places. But I | suppose | > they are simply driven by what has been needed. | > | > | > | > Anyway if you two are happy (no one else seems to mind either way) | then go | > ahead. | > | > | > | > Simon | > | > | > | > | > | > From: Luite Stegeman [mailto:[email protected]] | > Sent: 10 September 2013 08:37 | > To: Edsko de Vries | > Cc: Simon Peyton-Jones; ghc-devs; Edsko de Vries | > | > | > Subject: Re: GHC 7.8 release status | > | > | > | > Edsko has done some work of rearranging imports in DynFlags to make | the DLL | > split work, and I've implemented the hooks on top of this, in a | record, as | > discussed: | > | > | > | > - | > https://github.com/ghcjs/ghcjs-build/blob/master/refs/patches/ghc- | hooks-record.patch | > (not final yet, but should be usable for testing) | > | > - demo program: https://gist.github.com/luite/6506064 | > | > | > | > Some disadvantages: | > | > - as long as the DLL split exists, more restructuring will be required | if a | > new hook is added to something in a module on which DynFlags depends | > | > - 4 new hs-boot files required, new hooks will often require | additional | > hs-boot files (when module A has a hook (so A imports Hooks, this | can't be a | > source import), the hook will often have some types defined by A, so | Hooks | > will have to import A) | > | > | > | > Advantages (over type families / Dynamic hooks): | > | > - Hooks neatly defined together in a single record | > | > | > | > I'm not so sure myself, but if everyone agrees that this is better | than the | > older hooks I'll convert GHCJS to the new implementation later today | and | > finalize the patch (comments are a bit out of date, and I'm not 100% | sure | > yet that GHCJS doesn't need another hook for TH support in certain | setups) | > and update the wiki. | > | > | > | > luite | > | > | > | > On Mon, Sep 9, 2013 at 4:55 PM, Edsko de Vries | <[email protected]> | > wrote: | > | > Simon, | > | > I talked to Luite this morning and I think we can come up with a | > design that includes the enumeration we prefer, with a single use of | > Dynamic in DynFlags -- it involves splitting off a PackageState module | > from Packages so that DynFlags doesn't depend on the entirely of | > Packages anymore (which would then, transitively, mean that it depends | > on Hooks and hence on a large part of ghc), but I think that should be | > doable. I'm working on that now. | > | > Edsko | > | > | > On Mon, Sep 9, 2013 at 3:51 PM, Simon Peyton-Jones | > <[email protected]> wrote: | >> Edsko | >> | >> | >> | >> I'm very short of time right now. I think you understand the issues | here. | >> Can you do a round or two with Luite and emerge with a design that | you | >> both | >> think is best? | >> | >> | >> | >> As I said earlier I'm uncomfortable with doing design work so late in | the | >> cycle, and I feel that I don't have time to study the various | alternatives | >> properly in the next four days. But since you tell me it's crucial | for | >> GHCJS, I suppose that a possible compromise is this. We release a | GHC | >> with | >> some design for hooks, but specifically say that the hook design is | >> evolving | >> and may well change with the next version. And then you two, with | Thomas | >> and other interested parties, work together to evolve a design that | >> everyone | >> is happy with. | >> | >> | >> | >> Does that sound ok? | >> | >> | >> | >> Simon | >> | >> | >> | >> From: Luite Stegeman [mailto:[email protected]] | >> Sent: 07 September 2013 22:04 | >> To: Simon Peyton-Jones | >> Cc: Thomas Schilling; Edsko de Vries; ghc-devs | >> | >> | >> Subject: Re: GHC 7.8 release status | >> | >> | >> | >> * Why aren't you using Data.Dynamic for the hook things? You | are | >> precisely doing dynamic typing after all. (Moreover I want to change | >> Data.Dynamic so that it says | >> | >> data Dynamic where | >> Dyn :: Typeable a => a -> Dynamic | >> and you want to take advantage of this. | >> | >> | >> | >> Ah the goal is to avoid the Typeable constraint on the hooked | function, | >> and | >> to identify what things are actually hooks. Wrapping it in a newtype | also | >> achieves the first goal and does make the design a bit simpler. | >> | >> | >> | >> No need for these strange "data DsForeignsHook = DsForeignsHook" | things, | >> or | >> for the Hook type family. Simple! | >> | >> But it means that hooks are no longer recognisable by their type, | they're | >> just some Typeable (the type families approach would at least prevent | >> users | >> from accidentally inserting wrong hooks, even though they would still | be | >> able to make bogus instances on purpose) | >> | >> * The design *must* list all the hooks that GHC uses and | their | >> types. There's no point in adding a hook that GHC doesn't call! | >> | >> It appears to be difficult to define all hooks in one module or have | them | >> in | >> one record because of dependencies and the DLL split on Windows. | >> Re-exporting everything from a single module can be done, but would | offer | >> no | >> guarantees about completeness. | >> | >> | >> | >> With the type families design, everything that's an instance of Hook | is a | >> hook, although the definitions are scattered throughout the GHC | source. | >> The | >> Dynamic design would just have to rely on a consistent naming | convention. | >> Would listing the hooks in comments (in the Hooks module) and on the | wiki | >> be | >> a reasonable way to document them? | >> | >> | >> | >> I've uploaded a new patch, using Dynamic, although I'm not sure if | it's an | >> improvement over the original one: | >> | >> | >> | >> - patch: | >> | >> https://github.com/ghcjs/ghcjs-build/blob/master/refs/patches/ghc- | hooks-dynamic.patch | >> | >> - updated hooksDemo: https://gist.github.com/luite/6478973 | >> | >> | >> | >> It also adds hscParse' and tcRnModule' exports for Edsko's use case | (I | >> think | >> that makes it somewhat more flexible than exporting another version | of | >> hscFileFrontend, since it allows users to write a hook that does | something | >> between parsing and typechecking or one that overrides one of these | >> phases) | >> | >> | >> | >> luite | >> | >> | > | >> _______________________________________________ | >> ghc-devs mailing list | >> [email protected] | >> http://www.haskell.org/mailman/listinfo/ghc-devs | >> | > | > _______________________________________________ ghc-devs mailing list [email protected] http://www.haskell.org/mailman/listinfo/ghc-devs
