Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...
Adam Gundry
adam at well-typed.com
Fri Sep 18 14:17:02 UTC 2020
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:sylvain at 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 <adam at well-typed.com
> <mailto:adam at 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 <adam at well-typed.com
> <mailto:adam at well-typed.com>
> > <mailto:adam at well-typed.com <mailto:adam at 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 <sgraf1337 at gmail.com
> <mailto:sgraf1337 at gmail.com>
> > <mailto:sgraf1337 at gmail.com <mailto:sgraf1337 at gmail.com>>>;
> Sylvain Henry
> > > <sylvain at haskus.fr <mailto:sylvain at haskus.fr>
> <mailto:sylvain at haskus.fr <mailto:sylvain at haskus.fr>>>
> > > *Cc:* ghc-devs <ghc-devs at haskell.org
> <mailto:ghc-devs at haskell.org> <mailto:ghc-devs at haskell.org
> <mailto:ghc-devs at 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-bounces at haskell.org
> <mailto:ghc-devs-bounces at haskell.org>
> > <mailto:ghc-devs-bounces at haskell.org
> <mailto:ghc-devs-bounces at haskell.org>>
> > > <mailto:ghc-devs-bounces at haskell.org
> <mailto:ghc-devs-bounces at haskell.org>
> > <mailto:ghc-devs-bounces at haskell.org
> <mailto:ghc-devs-bounces at haskell.org>>>> *On Behalf Of *Sebastian Graf
> > > *Sent:* 10 September 2020 14:12
> > > *To:* Sylvain Henry <sylvain at haskus.fr
> <mailto:sylvain at haskus.fr> <mailto:sylvain at haskus.fr
> <mailto:sylvain at haskus.fr>>
> > <mailto:sylvain at haskus.fr <mailto:sylvain at haskus.fr>
> <mailto:sylvain at haskus.fr <mailto:sylvain at haskus.fr>>>>
> > > *Cc:* ghc-devs <ghc-devs at haskell.org
> <mailto:ghc-devs at haskell.org> <mailto:ghc-devs at haskell.org
> <mailto:ghc-devs at haskell.org>>
> > <mailto:ghc-devs at haskell.org <mailto:ghc-devs at haskell.org>
> <mailto:ghc-devs at haskell.org <mailto:ghc-devs at 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
More information about the ghc-devs
mailing list