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