Right, we don't want plugins to continue to have access to the *current* DynFlags, because I agree that needs to be cut down. I certainly wouldn't retain it as-is just for plugins.
Instead, I'm merely arguing that we should continue to let plugins modify (a) the command-line-flags-as-AST (which is what I presume DynFlags should become), and (b) the Hooks (perhaps with one or two additions, e.g. log_action). Apologies if I caused confusion earlier by referring to (a) as "DynFlags". Cheers, Adam On 18/09/2020 14:56, Moritz Angermann wrote: > I'm not certain anything in HEAD actually breaks any plugin today. But > the whole idea of plugins having full access to what currently is > "DynFlags" is not something I believe we can sustain. @Sylvain Henry > <mailto:sylv...@haskus.fr> is currently cleaning up a lot of unnecessary > DynFlags usage. I'm not against keeping the necessary infrastructure for > hooks and other interfaces with plugins, but I'd like to advocate > towards not expecting DynFlags to keep existing in eternity. If we > assume a subset of what used to be in DynFlags to be relevant to > Plugins, let's collect that in say PluginHooks, but let's keep that > interface minimal. And maybe that can be specified to stay stable. > > DynFlags is our state kitchensink in GHC, and it is everywhere. The > state is threaded through everything and the module is gargantuous. So > far there seemed to be broad support in removing this wart. > > Cheers, > Moritz > > On Fri, Sep 18, 2020 at 5:52 PM Adam Gundry <a...@well-typed.com > <mailto:a...@well-typed.com>> wrote: > > On 14/09/2020 13:02, Moritz Angermann wrote: > > I believe this to already be broken in HEAD. DynFlags already got > quite > > an overhaul/break. I'd rather we drop supporting DynFlagPlugins. And > > offer alternative stable interfaces. Though to be honest, I > believe our > > Plugin story is rather poor so far. > > > > Do you happen to know of DynFlagPlugins, Adam? > > A few have been mentioned in the thread now. What specifically do you > believe is broken in HEAD regarding DynFlags plugins, and is there an > issue for it? AFAICS the hooks-plugin test which corresponds to the > user's guide text is still there. > > I think it is important to retain the ability for plugins to manipulate > both DynFlags and Hooks, whether the latter are separated out of the > former or not. Both have legitimate use cases, and plugins necessarily > involve using unstable interfaces (at least until someone designs a > stable interface). I agree that the current state of plugins/hooks is > somewhat ad-hoc and could do with more effort put into the design (like > much else in the GHC API!) but that doesn't mean we should remove things > that work already. > > Slightly tangential note: discussing this with Alp I learned about the > log_action/dump_action/trace_action fields of DynFlags, which also seem > to violate Simon's "We should think of DynFlags as an abstract syntax > tree." And indeed it would be useful for plugins to be able to override > log_action, especially combined with #18516, as then we would have a > nice story for plugins overriding error message generation to allow for > domain-specific error messages. > > Cheers, > > Adam > > > > On Mon, Sep 14, 2020 at 7:09 PM Adam Gundry <a...@well-typed.com > <mailto:a...@well-typed.com> > > <mailto:a...@well-typed.com <mailto:a...@well-typed.com>>> wrote: > > > > 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://downloads.haskell.org/~ghc/latest/docs/html/users_guide/extending_ghc.html#dynflags-plugins). > > 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 > <mailto:sgraf1...@gmail.com> > > <mailto:sgraf1...@gmail.com <mailto:sgraf1...@gmail.com>>>; > Sylvain Henry > > > <sylv...@haskus.fr <mailto:sylv...@haskus.fr> > <mailto:sylv...@haskus.fr <mailto:sylv...@haskus.fr>>> > > > *Cc:* ghc-devs <ghc-devs@haskell.org > <mailto:ghc-devs@haskell.org> <mailto:ghc-devs@haskell.org > <mailto: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://downloads.haskell.org/~ghc/latest/docs/html/users_guide/extending_ghc.html#dynflags-plugins > > > S > > > > > > > > > > > > *From:*ghc-devs <ghc-devs-boun...@haskell.org > <mailto:ghc-devs-boun...@haskell.org> > > <mailto:ghc-devs-boun...@haskell.org > <mailto:ghc-devs-boun...@haskell.org>> > > > <mailto:ghc-devs-boun...@haskell.org > <mailto:ghc-devs-boun...@haskell.org> > > <mailto: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> <mailto:sylv...@haskus.fr > <mailto:sylv...@haskus.fr>> > > <mailto:sylv...@haskus.fr <mailto:sylv...@haskus.fr> > <mailto:sylv...@haskus.fr <mailto:sylv...@haskus.fr>>>> > > > *Cc:* ghc-devs <ghc-devs@haskell.org > <mailto:ghc-devs@haskell.org> <mailto:ghc-devs@haskell.org > <mailto:ghc-devs@haskell.org>> > > <mailto:ghc-devs@haskell.org <mailto:ghc-devs@haskell.org> > <mailto: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://gitlab.haskell.org/ghc/ghc/-/merge_requests/3971 > > > > > > > <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.haskell.org%2Fghc%2Fghc%2F-%2Fmerge_requests%2F3971&data=02%7C01%7Csimonpj%40microsoft.com%7C0c3760e72fad4200d39408d8558b3871%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637353404753453548&sdata=fVpIzJgaqFfWaJ5ppCE5daHwdETTQF03o1h0uNtDxGA%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%2Fhackage.haskell.org%2Fpackage%2Fghc-lib-parser&data=02%7C01%7Csimonpj%40microsoft.com%7C0c3760e72fad4200d39408d8558b3871%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637353404753463506&sdata=HZMaqK6t7PLifc26wf%2BqcUef4Ko%2BQcaPRx4o7XLcVq8%3D&reserved=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://gitlab.haskell.org/ghc/ghc/-/issues/10961 > > > > > > > <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.haskell.org%2Fghc%2Fghc%2F-%2Fissues%2F10961&data=02%7C01%7Csimonpj%40microsoft.com%7C0c3760e72fad4200d39408d8558b3871%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637353404753463506&sdata=sn9zv1MO8p%2FSbwsm1NDaSiUaumE%2FvTo4NkGreYOjITA%3D&reserved=0> > > > and https://gitlab.haskell.org/ghc/ghc/-/issues/11301 > > > > > > > <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.haskell.org%2Fghc%2Fghc%2F-%2Fissues%2F11301&data=02%7C01%7Csimonpj%40microsoft.com%7C0c3760e72fad4200d39408d8558b3871%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637353404753463506&sdata=vFTEuEzIQLJTtpu7%2BuwFnOEWMPv8eY%2B%2FvgbrrV18uss%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
