On 14/09/2020 12:45, Simon Peyton Jones via ghc-devs wrote: > | I guess this can be worked around, perhaps by passing hooks separately > | to DynFlags and providing a separate plugin interface to modify hooks. > | But doing so will necessarily break existing plugins. > > I'm not sure it will break anything. It's hard to find just what the API > that plugin authors use.. where is it documented?
The link to the GHC user's guide [1] I gave includes an example of a DynFlags plugin overriding a hook. This was the key reason for introducing DynFlags plugins originally [2] and is definitely used in the wild [3]. TBH I'm not sure DynFlags plugins are yet used for anything *other* than overriding hooks. > All I'm arguing is that Hooks (which contains the hook functions themselves) > should not be part of DynFlags. So for example: > > dsForeigns :: [LForeignDecl GhcTc] > -> DsM (ForeignStubs, OrdList Binding) > dsForeigns fos = getHooked dsForeignsHook dsForeigns' >>= ($ fos) > > Here > getHooked :: (Functor f, HasDynFlags f) => (Hooks -> Maybe a) -> a -> f a > > Why HasDynFlags? Yes, DsM might need a Hooks field, but that's easy. No need > to stuff it in DynFlags! Right, I agree completely. I don't know about the initial implementation of hooks, but I suspect DynFlags was picked as a convenient way to pass them throughout the compiler, at the cost of introducing more cycles. I just suggest that either the Plugin type gets extended with a new hooksPlugin field of type [CommandLineOption] -> Hooks -> IO Hooks or the type of the existing dynflagsPlugin field is changed to something like [CommandLineOption] -> (DynFlags, Hooks) -> IO (DynFlags, Hooks) so that plugins can still set up hooks. Though perhaps there is an even better story lurking here about combining Hooks and Plugins rather than having a somewhat artificial separation between them... Adam [1] https://downloads.haskell.org/~ghc/latest/docs/html/users_guide/extending_ghc.html#dynflags-plugins [2] https://gitlab.haskell.org/ghc/ghc/-/merge_requests/1580 [3] https://gitlab.haskell.org/alp/ghc-external-splices-plugin > Simon > > | -----Original Message----- > | From: ghc-devs <ghc-devs-boun...@haskell.org> On Behalf Of Adam Gundry > | Sent: 14 September 2020 12:08 > | To: ghc-devs@haskell.org > | Subject: Re: Parser depends on DynFlags, depends on Hooks, depends on > | TcM, DsM, ... > | > | I'm supportive of the goal, but a complication with removing hooks from > | DynFlags is that GHC currently supports "DynFlags plugins" that allow > | plugins to install custom hooks > | (https://nam06.safelinks.protection.outlook.com/?url=https:%2F%2Fdownlo > | ads.haskell.org%2F~ghc%2Flatest%2Fdocs%2Fhtml%2Fusers_guide%2Fextending > | _ghc.html%23dynflags- > | plugins&data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f5 > | 4808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63735678554 > | 4069988&sdata=d8wJidQddoONBhJXnv6iI1%2FD4gVDq%2FvC7cgSxJoBOFQ%3D&am > | p;reserved=0). > | I guess this can be worked around, perhaps by passing hooks separately > | to DynFlags and providing a separate plugin interface to modify hooks. > | But doing so will necessarily break existing plugins. > | > | Adam > | > | > | On 14/09/2020 11:25, Simon Peyton Jones via ghc-devs wrote: > | > I thought I’d sent a message about this DynFlags thing, but I can’t > | > trace it now. So here’s a resend. > | > > | > > | > > | > Currently > | > > | > * The DynFlags record includes Hooks > | > * Hooks in contains functions, that mention TcM, DsM etc > | > > | > > | > > | > This is bad. We should think of DynFlags as an *abstract syntax > | tree*. > | > That is, the result of parsing the flag strings, yes, but not much > | > more. So for hooks we should have an algebraic data type > | representing > | > the hook /specification/, but it should not be the hook functions > | > themselves. HsSyn, for example, after parsing, is just a tree with > | > strings in it. No TyCons, Ids, etc. That comes much later. > | > > | > > | > > | > So DynFlags should be a collection of algebraic data types, but > | should > | > not depend on anything else. > | > > | > > | > > | > I think that may cut a bunch of awkward loops. > | > > | > > | > > | > Simon > | > > | > > | > > | > *From:*Simon Peyton Jones > | > *Sent:* 10 September 2020 14:17 > | > *To:* Sebastian Graf <sgraf1...@gmail.com>; Sylvain Henry > | > <sylv...@haskus.fr> > | > *Cc:* ghc-devs <ghc-devs@haskell.org> > | > *Subject:* RE: Parser depends on DynFlags, depends on Hooks, depends > | on > | > TcM, DsM, ... > | > > | > > | > > | > And for sure the **parser** should not depend on the **desugarer** > | and > | > **typechecker**. (Which it does, as described below.) > | > > | > > | > > | https://nam06.safelinks.protection.outlook.com/?url=https:%2F%2Fdownloa > | ds.haskell.org%2F~ghc%2Flatest%2Fdocs%2Fhtml%2Fusers_guide%2Fextending_ > | ghc.html%23dynflags- > | plugins&data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f5 > | 4808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63735678554 > | 4069988&sdata=d8wJidQddoONBhJXnv6iI1%2FD4gVDq%2FvC7cgSxJoBOFQ%3D&am > | p;reserved=0 > | > S > | > > | > > | > > | > *From:*ghc-devs <ghc-devs-boun...@haskell.org > | > <mailto:ghc-devs-boun...@haskell.org>> *On Behalf Of *Sebastian Graf > | > *Sent:* 10 September 2020 14:12 > | > *To:* Sylvain Henry <sylv...@haskus.fr <mailto:sylv...@haskus.fr>> > | > *Cc:* ghc-devs <ghc-devs@haskell.org <mailto:ghc-devs@haskell.org>> > | > *Subject:* Parser depends on DynFlags, depends on Hooks, depends on > | TcM, > | > DsM, ... > | > > | > > | > > | > Hey Sylvain, > | > > | > > | > > | > In > | https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla > | b.haskell.org%2Fghc%2Fghc%2F- > | %2Fmerge_requests%2F3971&data=02%7C01%7Csimonpj%40microsoft.com%7C0 > | e358ff421a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1% > | 7C0%7C637356785544069988&sdata=OV1X6btPhnB7UPxOKvCRzUOZVlH7r5ZpR33d > | 6vil3fI%3D&reserved=0 > | > > | <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl > | ab.haskell.org%2Fghc%2Fghc%2F- > | %2Fmerge_requests%2F3971&data=02%7C01%7Csimonpj%40microsoft.com%7C0 > | e358ff421a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1% > | 7C0%7C637356785544069988&sdata=OV1X6btPhnB7UPxOKvCRzUOZVlH7r5ZpR33d > | 6vil3fI%3D&reserved=0> > | > I had to fight once more with the transitive dependency set of the > | > parser, the minimality of which is crucial for ghc-lib-parser > | > > | <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fhack > | age.haskell.org%2Fpackage%2Fghc-lib- > | parser&data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f54 > | 808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637356785544 > | 079987&sdata=bHnY1UZgFauLsB0dISUXZavGholi6UWzA7nSuByMCns%3D&res > | erved=0> > | > and tested by the CountParserDeps test. > | > > | > > | > > | > I discovered that I need to make (parts of) `DsM` abstract, because > | it > | > is transitively imported from the Parser for example through Parser.y > | -> > | > Lexer.x -> DynFlags -> Hooks -> {DsM,TcM}. > | > > | > Since you are our mastermind behind the "Tame DynFlags" initiative, > | I'd > | > like to hear your opinion on where progress can be/is made on that > | front. > | > > | > > | > > | > I see there is > | https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla > | b.haskell.org%2Fghc%2Fghc%2F- > | %2Fissues%2F10961&data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4 > | 21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6 > | 37356785544079987&sdata=pghmuc9gmaHinB8cLZHx2PqBqDwNkBJBSFZsYlCdwL8 > | %3D&reserved=0 > | > > | <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl > | ab.haskell.org%2Fghc%2Fghc%2F- > | %2Fissues%2F10961&data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4 > | 21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6 > | 37356785544079987&sdata=pghmuc9gmaHinB8cLZHx2PqBqDwNkBJBSFZsYlCdwL8 > | %3D&reserved=0> > | > and > | https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla > | b.haskell.org%2Fghc%2Fghc%2F- > | %2Fissues%2F11301&data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4 > | 21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6 > | 37356785544079987&sdata=tiE1uhe%2BganXJKPbtadDGUCnSi%2F0OMT6WopEZ9s > | MJJY%3D&reserved=0 > | > > | <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl > | ab.haskell.org%2Fghc%2Fghc%2F- > | %2Fissues%2F11301&data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4 > | 21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6 > | 37356785544079987&sdata=tiE1uhe%2BganXJKPbtadDGUCnSi%2F0OMT6WopEZ9s > | MJJY%3D&reserved=0> > | > which ask a related, but different question: They want a DynFlags- > | free > | > interface, but I even want a DynFlags-free *module*. > | > > | > > | > > | > Would you say it's reasonable to abstract the definition of `PState` > | > over the `DynFlags` type? I think it's only used for pretty-printing > | > messages, which is one of your specialties (the treatment of DynFlags > | in > | > there, at least). > | > > | > Anyway, can you think of or perhaps point me to an existing road map > | on > | > that issue? > | > > | > > | > > | > Thank you! > | > > | > Sebastian -- Adam Gundry, Haskell Consultant Well-Typed LLP, https://www.well-typed.com/ Registered in England & Wales, OC335890 118 Wymering Mansions, Wymering Road, London W9 2NF, England _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs