Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

Sebastian Graf
Hey Sylvain,

In https://gitlab.haskell.org/ghc/ghc/-/merge_requests/3971 I had to fight once more with the transitive dependency set of the parser, the minimality of which is crucial for ghc-lib-parser 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 and https://gitlab.haskell.org/ghc/ghc/-/issues/11301 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

_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

RE: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

GHC - devs mailing list

And for sure the *parser* should not depend on the *desugarer* and *typechecker*.   (Which it does, as described below.)

 

S

 

From: ghc-devs <[hidden email]> On Behalf Of Sebastian Graf
Sent: 10 September 2020 14:12
To: Sylvain Henry <[hidden email]>
Cc: ghc-devs <[hidden email]>
Subject: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

 

Hey Sylvain,

 

In https://gitlab.haskell.org/ghc/ghc/-/merge_requests/3971 I had to fight once more with the transitive dependency set of the parser, the minimality of which is crucial for ghc-lib-parser 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 and https://gitlab.haskell.org/ghc/ghc/-/issues/11301 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


_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

Sylvain Henry-2
In reply to this post by Sebastian Graf

Hi Sebastian,

Last month I tried to make a DynFlags-free parser. The branch is here: https://gitlab.haskell.org/hsyl20/ghc/-/commits/hsyl20/dynflags/parser (doesn't build iirc)

1) The input of the parser is almost DynFlags-free thanks to Alec's patch [1]. On that front, we just have to move `mkParserFlags` out of GHC.Parser. I would put it alongside other functions generating config datatypes from DynFlags in GHC.Driver.Config (added yesterday). It's done in my branch and it only required a bit of plumbing to fix `lexTokenStream` iirc.

2) The output of the parser is the issue, as you point out. The main issue is that it uses SDoc/ErrMsg which are dependent on DynFlags.

In the branch I've tried to avoid the use of SDoc by using ADTs to return errors and warnings so that the client of the parser would be responsible for converting them into SDoc if needed. This is the approach that we would like to generalize [2]. The ADT would look like [3] and the pretty-printing module like [4]. The idea was that ghc-lib-parser wouldn't integrate the pretty-printing module to avoid the dependencies.

I think it's the best interface (for IDEs and other tools) so we just have to complete the work :). The branch stalled because I've tried to avoid SDoc even in the pretty-printing module and used Doc instead of SDoc but it wasn't a good idea... I'll try to resume the work soon.

In the meantime I've been working on making Outputable/SDoc independent of DynFlags. If we merge [5] in some form then the last place where we use `sdocWithDynFlags` will be in CLabel's Outputable instance (to fix this I think we could just depend on the PprStyle (Asm or C) instead of querying the backend in the DynFlags). This could be another approach to make the parser almost as it is today independent of DynFlags. A side-effect of this work is that ghc-lib-parser could include the pretty-printing module too.

So to answer your question:

> Would you say it's reasonable to abstract the definition of `PState` over the `DynFlags` type?

We're close to remove the dependence on DynFlags so I would prefer this instead of trying to abstract over it.

The roadmap:

1. Make Outputable/SDoc independent of DynFlags
1.1 Remove sdocWithDynFlags used to query the platform (!3972)
1.2 Remove sdocWithDynFlags used to query the backend in CLabel's Outputable instance
1.3 Remove sdocWithDynFlags
2. Move mkParserFlags from GHC.Parser to GHC.Driver.Config
3. (Make the parser use ADTs to return errors/warnings)

Cheers,
Sylvain

[1] https://gitlab.haskell.org/ghc/ghc/-/commit/469fe6133646df5568c9486de2202124cb734242
[2] https://gitlab.haskell.org/ghc/ghc/-/wikis/Errors-as-(structured)-values
[3] https://gitlab.haskell.org/hsyl20/ghc/-/blob/hsyl20/dynflags/parser/compiler/GHC/Parser/Errors.hs
[4] https://gitlab.haskell.org/hsyl20/ghc/-/blob/hsyl20/dynflags/parser/compiler/GHC/Parser/Errors/Ppr.hs
[5] https://gitlab.haskell.org/ghc/ghc/-/merge_requests/3972


On 10/09/2020 15:12, Sebastian Graf wrote:
Hey Sylvain,

In https://gitlab.haskell.org/ghc/ghc/-/merge_requests/3971 I had to fight once more with the transitive dependency set of the parser, the minimality of which is crucial for ghc-lib-parser 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 and https://gitlab.haskell.org/ghc/ghc/-/issues/11301 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

_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

Sebastian Graf
Hi Sylvain,

Thanks, that answers all my questions! Keep up the great work.

Cheers
Sebastian

Am Do., 10. Sept. 2020 um 17:24 Uhr schrieb Sylvain Henry <[hidden email]>:

Hi Sebastian,

Last month I tried to make a DynFlags-free parser. The branch is here: https://gitlab.haskell.org/hsyl20/ghc/-/commits/hsyl20/dynflags/parser (doesn't build iirc)

1) The input of the parser is almost DynFlags-free thanks to Alec's patch [1]. On that front, we just have to move `mkParserFlags` out of GHC.Parser. I would put it alongside other functions generating config datatypes from DynFlags in GHC.Driver.Config (added yesterday). It's done in my branch and it only required a bit of plumbing to fix `lexTokenStream` iirc.

2) The output of the parser is the issue, as you point out. The main issue is that it uses SDoc/ErrMsg which are dependent on DynFlags.

In the branch I've tried to avoid the use of SDoc by using ADTs to return errors and warnings so that the client of the parser would be responsible for converting them into SDoc if needed. This is the approach that we would like to generalize [2]. The ADT would look like [3] and the pretty-printing module like [4]. The idea was that ghc-lib-parser wouldn't integrate the pretty-printing module to avoid the dependencies.

I think it's the best interface (for IDEs and other tools) so we just have to complete the work :). The branch stalled because I've tried to avoid SDoc even in the pretty-printing module and used Doc instead of SDoc but it wasn't a good idea... I'll try to resume the work soon.

In the meantime I've been working on making Outputable/SDoc independent of DynFlags. If we merge [5] in some form then the last place where we use `sdocWithDynFlags` will be in CLabel's Outputable instance (to fix this I think we could just depend on the PprStyle (Asm or C) instead of querying the backend in the DynFlags). This could be another approach to make the parser almost as it is today independent of DynFlags. A side-effect of this work is that ghc-lib-parser could include the pretty-printing module too.

So to answer your question:

> Would you say it's reasonable to abstract the definition of `PState` over the `DynFlags` type?

We're close to remove the dependence on DynFlags so I would prefer this instead of trying to abstract over it.

The roadmap:

1. Make Outputable/SDoc independent of DynFlags
1.1 Remove sdocWithDynFlags used to query the platform (!3972)
1.2 Remove sdocWithDynFlags used to query the backend in CLabel's Outputable instance
1.3 Remove sdocWithDynFlags
2. Move mkParserFlags from GHC.Parser to GHC.Driver.Config
3. (Make the parser use ADTs to return errors/warnings)

Cheers,
Sylvain

[1] https://gitlab.haskell.org/ghc/ghc/-/commit/469fe6133646df5568c9486de2202124cb734242
[2] https://gitlab.haskell.org/ghc/ghc/-/wikis/Errors-as-(structured)-values
[3] https://gitlab.haskell.org/hsyl20/ghc/-/blob/hsyl20/dynflags/parser/compiler/GHC/Parser/Errors.hs
[4] https://gitlab.haskell.org/hsyl20/ghc/-/blob/hsyl20/dynflags/parser/compiler/GHC/Parser/Errors/Ppr.hs
[5] https://gitlab.haskell.org/ghc/ghc/-/merge_requests/3972


On 10/09/2020 15:12, Sebastian Graf wrote:
Hey Sylvain,

In https://gitlab.haskell.org/ghc/ghc/-/merge_requests/3971 I had to fight once more with the transitive dependency set of the parser, the minimality of which is crucial for ghc-lib-parser 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 and https://gitlab.haskell.org/ghc/ghc/-/issues/11301 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

_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

RE: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

GHC - devs mailing list
In reply to this post by GHC - devs mailing list

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 <[hidden email]>; Sylvain Henry <[hidden email]>
Cc: ghc-devs <[hidden email]>
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.)

 

S

 

From: ghc-devs <[hidden email]> On Behalf Of Sebastian Graf
Sent: 10 September 2020 14:12
To: Sylvain Henry <[hidden email]>
Cc: ghc-devs <[hidden email]>
Subject: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

 

Hey Sylvain,

 

In https://gitlab.haskell.org/ghc/ghc/-/merge_requests/3971 I had to fight once more with the transitive dependency set of the parser, the minimality of which is crucial for ghc-lib-parser 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 and https://gitlab.haskell.org/ghc/ghc/-/issues/11301 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


_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

Adam Gundry-2
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 <[hidden email]>; Sylvain Henry
> <[hidden email]>
> *Cc:* ghc-devs <[hidden email]>
> *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 <[hidden email]
> <mailto:[hidden email]>> *On Behalf Of *Sebastian Graf
> *Sent:* 10 September 2020 14:12
> *To:* Sylvain Henry <[hidden email] <mailto:[hidden email]>>
> *Cc:* ghc-devs <[hidden email] <mailto:[hidden email]>>
> *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
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

RE: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

GHC - devs mailing list
|  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?

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!

Simon

|  -----Original Message-----
|  From: ghc-devs <[hidden email]> On Behalf Of Adam Gundry
|  Sent: 14 September 2020 12:08
|  To: [hidden email]
|  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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f5
|  4808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63735678554
|  4069988&amp;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 <[hidden email]>; Sylvain Henry
|  > <[hidden email]>
|  > *Cc:* ghc-devs <[hidden email]>
|  > *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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f5
|  4808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63735678554
|  4069988&amp;sdata=d8wJidQddoONBhJXnv6iI1%2FD4gVDq%2FvC7cgSxJoBOFQ%3D&am
|  p;reserved=0
|  > S
|  >
|  >
|  >
|  > *From:*ghc-devs <[hidden email]
|  > <mailto:[hidden email]>> *On Behalf Of *Sebastian Graf
|  > *Sent:* 10 September 2020 14:12
|  > *To:* Sylvain Henry <[hidden email] <mailto:[hidden email]>>
|  > *Cc:* ghc-devs <[hidden email] <mailto:[hidden email]>>
|  > *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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0
|  e358ff421a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%
|  7C0%7C637356785544069988&amp;sdata=OV1X6btPhnB7UPxOKvCRzUOZVlH7r5ZpR33d
|  6vil3fI%3D&amp;reserved=0
|  >
|  <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
|  ab.haskell.org%2Fghc%2Fghc%2F-
|  %2Fmerge_requests%2F3971&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0
|  e358ff421a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%
|  7C0%7C637356785544069988&amp;sdata=OV1X6btPhnB7UPxOKvCRzUOZVlH7r5ZpR33d
|  6vil3fI%3D&amp;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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f54
|  808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637356785544
|  079987&amp;sdata=bHnY1UZgFauLsB0dISUXZavGholi6UWzA7nSuByMCns%3D&amp;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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
|  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
|  37356785544079987&amp;sdata=pghmuc9gmaHinB8cLZHx2PqBqDwNkBJBSFZsYlCdwL8
|  %3D&amp;reserved=0
|  >
|  <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
|  ab.haskell.org%2Fghc%2Fghc%2F-
|  %2Fissues%2F10961&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
|  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
|  37356785544079987&amp;sdata=pghmuc9gmaHinB8cLZHx2PqBqDwNkBJBSFZsYlCdwL8
|  %3D&amp;reserved=0>
|  > and
|  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
|  b.haskell.org%2Fghc%2Fghc%2F-
|  %2Fissues%2F11301&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
|  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
|  37356785544079987&amp;sdata=tiE1uhe%2BganXJKPbtadDGUCnSi%2F0OMT6WopEZ9s
|  MJJY%3D&amp;reserved=0
|  >
|  <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
|  ab.haskell.org%2Fghc%2Fghc%2F-
|  %2Fissues%2F11301&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
|  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
|  37356785544079987&amp;sdata=tiE1uhe%2BganXJKPbtadDGUCnSi%2F0OMT6WopEZ9s
|  MJJY%3D&amp;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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.w
|  ell-
|  typed.com%2F&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64
|  763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637356
|  785544079987&amp;sdata=D9by4KOFhHjYXnRT6okDDCDH6mLKXkZ%2BL%2BvrC4tLCfQ%
|  3D&amp;reserved=0
|  
|  Registered in England & Wales, OC335890
|  118 Wymering Mansions, Wymering Road, London W9 2NF, England
|  _______________________________________________
|  ghc-devs mailing list
|  [hidden email]
|  https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.h
|  askell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc-
|  devs&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f5480
|  8d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63735678554407
|  9987&amp;sdata=3b%2FfWhaolo%2Fsl2m3SFs3lwM7Nfbihf4v6y6gfZ5Qteo%3D&amp;r
|  eserved=0
_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

Moritz Angermann-2
In reply to this post by Adam Gundry-2
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?

Cheers,
 Moritz

On Mon, Sep 14, 2020 at 7:09 PM Adam Gundry <[hidden email]> 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 <[hidden email]>; Sylvain Henry
> <[hidden email]>
> *Cc:* ghc-devs <[hidden email]>
> *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 <[hidden email]
> <mailto:[hidden email]>> *On Behalf Of *Sebastian Graf
> *Sent:* 10 September 2020 14:12
> *To:* Sylvain Henry <[hidden email] <mailto:[hidden email]>>
> *Cc:* ghc-devs <[hidden email] <mailto:[hidden email]>>
> *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
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

Adam Gundry-2
In reply to this post by GHC - devs mailing list
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 <[hidden email]> On Behalf Of Adam Gundry
> |  Sent: 14 September 2020 12:08
> |  To: [hidden email]
> |  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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f5
> |  4808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63735678554
> |  4069988&amp;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 <[hidden email]>; Sylvain Henry
> |  > <[hidden email]>
> |  > *Cc:* ghc-devs <[hidden email]>
> |  > *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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f5
> |  4808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63735678554
> |  4069988&amp;sdata=d8wJidQddoONBhJXnv6iI1%2FD4gVDq%2FvC7cgSxJoBOFQ%3D&am
> |  p;reserved=0
> |  > S
> |  >
> |  >
> |  >
> |  > *From:*ghc-devs <[hidden email]
> |  > <mailto:[hidden email]>> *On Behalf Of *Sebastian Graf
> |  > *Sent:* 10 September 2020 14:12
> |  > *To:* Sylvain Henry <[hidden email] <mailto:[hidden email]>>
> |  > *Cc:* ghc-devs <[hidden email] <mailto:[hidden email]>>
> |  > *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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0
> |  e358ff421a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%
> |  7C0%7C637356785544069988&amp;sdata=OV1X6btPhnB7UPxOKvCRzUOZVlH7r5ZpR33d
> |  6vil3fI%3D&amp;reserved=0
> |  >
> |  <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> |  ab.haskell.org%2Fghc%2Fghc%2F-
> |  %2Fmerge_requests%2F3971&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0
> |  e358ff421a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%
> |  7C0%7C637356785544069988&amp;sdata=OV1X6btPhnB7UPxOKvCRzUOZVlH7r5ZpR33d
> |  6vil3fI%3D&amp;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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f54
> |  808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637356785544
> |  079987&amp;sdata=bHnY1UZgFauLsB0dISUXZavGholi6UWzA7nSuByMCns%3D&amp;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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
> |  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
> |  37356785544079987&amp;sdata=pghmuc9gmaHinB8cLZHx2PqBqDwNkBJBSFZsYlCdwL8
> |  %3D&amp;reserved=0
> |  >
> |  <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> |  ab.haskell.org%2Fghc%2Fghc%2F-
> |  %2Fissues%2F10961&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
> |  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
> |  37356785544079987&amp;sdata=pghmuc9gmaHinB8cLZHx2PqBqDwNkBJBSFZsYlCdwL8
> |  %3D&amp;reserved=0>
> |  > and
> |  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
> |  b.haskell.org%2Fghc%2Fghc%2F-
> |  %2Fissues%2F11301&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
> |  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
> |  37356785544079987&amp;sdata=tiE1uhe%2BganXJKPbtadDGUCnSi%2F0OMT6WopEZ9s
> |  MJJY%3D&amp;reserved=0
> |  >
> |  <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> |  ab.haskell.org%2Fghc%2Fghc%2F-
> |  %2Fissues%2F11301&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
> |  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
> |  37356785544079987&amp;sdata=tiE1uhe%2BganXJKPbtadDGUCnSi%2F0OMT6WopEZ9s
> |  MJJY%3D&amp;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
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

RE: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

GHC - devs mailing list

| This was the key reason for
|  introducing DynFlags plugins originally [2]

But [2] says:

"We do so with a new field in the Plugin data type, which lets plugin authors
supply a Hooks -> Hooks function that then gets applied to the hooks already
present in the DynFlags. If several plugins override the hooks, we compose
all the corresponding Hooks -> Hooks functions."

Note "already present in DynFlags".  If we take it out of DynFlags (my goal in this thread), and put it in (say) ModGuts instead, the API will stay the same, no?

Simon

|  -----Original Message-----
|  From: Adam Gundry <[hidden email]>
|  Sent: 14 September 2020 13:12
|  To: Simon Peyton Jones <[hidden email]>
|  Cc: [hidden email]
|  Subject: Re: Parser depends on DynFlags, depends on Hooks, depends on
|  TcM, DsM, ...
|  
|  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://nam06.safelinks.protection.outlook.com/?url=https:%2F%2Fdownloa
|  ds.haskell.org%2F~ghc%2Flatest%2Fdocs%2Fhtml%2Fusers_guide%2Fextending_
|  ghc.html%23dynflags-
|  plugins&amp;data=02%7C01%7Csimonpj%40microsoft.com%7Cbf260389ea3f4e4259
|  f608d858a765a3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63735682328
|  4144388&amp;sdata=oiE3yFoPgiMbQvxH2sftNdv3Gak7QKK2KQkZFS6D8Jo%3D&amp;re
|  served=0
|  [2]
|  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
|  b.haskell.org%2Fghc%2Fghc%2F-
|  %2Fmerge_requests%2F1580&amp;data=02%7C01%7Csimonpj%40microsoft.com%7Cb
|  f260389ea3f4e4259f608d858a765a3%7C72f988bf86f141af91ab2d7cd011db47%7C1%
|  7C0%7C637356823284144388&amp;sdata=6R6KkZe69PQdYRpJHGBYa6vt%2Ff0z8C142f
|  gWX7iqZdA%3D&amp;reserved=0
|  [3]
|  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
|  b.haskell.org%2Falp%2Fghc-external-splices-
|  plugin&amp;data=02%7C01%7Csimonpj%40microsoft.com%7Cbf260389ea3f4e4259f
|  608d858a765a3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637356823284
|  144388&amp;sdata=gamHplH2iLxtQRYPyT3POO%2F0jAPPHRMSIka81ZMCbuQ%3D&amp;r
|  eserved=0
|  
|  
|  > Simon
|  >
|  > |  -----Original Message-----
|  > |  From: ghc-devs <[hidden email]> On Behalf Of Adam
|  Gundry
|  > |  Sent: 14 September 2020 12:08
|  > |  To: [hidden email]
|  > |  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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f5
|  > |
|  4808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63735678554
|  > |
|  4069988&amp;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 <[hidden email]>; Sylvain Henry
|  > |  > <[hidden email]>
|  > |  > *Cc:* ghc-devs <[hidden email]>
|  > |  > *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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f5
|  > |
|  4808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63735678554
|  > |
|  4069988&amp;sdata=d8wJidQddoONBhJXnv6iI1%2FD4gVDq%2FvC7cgSxJoBOFQ%3D&am
|  > |  p;reserved=0
|  > |  > S
|  > |  >
|  > |  >
|  > |  >
|  > |  > *From:*ghc-devs <[hidden email]
|  > |  > <mailto:[hidden email]>> *On Behalf Of *Sebastian
|  Graf
|  > |  > *Sent:* 10 September 2020 14:12
|  > |  > *To:* Sylvain Henry <[hidden email]
|  <mailto:[hidden email]>>
|  > |  > *Cc:* ghc-devs <[hidden email] <mailto:ghc-
|  [hidden email]>>
|  > |  > *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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0
|  > |
|  e358ff421a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%
|  > |
|  7C0%7C637356785544069988&amp;sdata=OV1X6btPhnB7UPxOKvCRzUOZVlH7r5ZpR33d
|  > |  6vil3fI%3D&amp;reserved=0
|  > |  >
|  > |
|  <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
|  > |  ab.haskell.org%2Fghc%2Fghc%2F-
|  > |
|  %2Fmerge_requests%2F3971&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0
|  > |
|  e358ff421a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%
|  > |
|  7C0%7C637356785544069988&amp;sdata=OV1X6btPhnB7UPxOKvCRzUOZVlH7r5ZpR33d
|  > |  6vil3fI%3D&amp;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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f54
|  > |
|  808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637356785544
|  > |
|  079987&amp;sdata=bHnY1UZgFauLsB0dISUXZavGholi6UWzA7nSuByMCns%3D&amp;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&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
|  > |
|  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
|  > |
|  37356785544079987&amp;sdata=pghmuc9gmaHinB8cLZHx2PqBqDwNkBJBSFZsYlCdwL8
|  > |  %3D&amp;reserved=0
|  > |  >
|  > |
|  <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
|  > |  ab.haskell.org%2Fghc%2Fghc%2F-
|  > |
|  %2Fissues%2F10961&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
|  > |
|  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
|  > |
|  37356785544079987&amp;sdata=pghmuc9gmaHinB8cLZHx2PqBqDwNkBJBSFZsYlCdwL8
|  > |  %3D&amp;reserved=0>
|  > |  > and
|  > |
|  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
|  > |  b.haskell.org%2Fghc%2Fghc%2F-
|  > |
|  %2Fissues%2F11301&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
|  > |
|  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
|  > |
|  37356785544079987&amp;sdata=tiE1uhe%2BganXJKPbtadDGUCnSi%2F0OMT6WopEZ9s
|  > |  MJJY%3D&amp;reserved=0
|  > |  >
|  > |
|  <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
|  > |  ab.haskell.org%2Fghc%2Fghc%2F-
|  > |
|  %2Fissues%2F11301&amp;data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
|  > |
|  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
|  > |
|  37356785544079987&amp;sdata=tiE1uhe%2BganXJKPbtadDGUCnSi%2F0OMT6WopEZ9s
|  > |  MJJY%3D&amp;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://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.w
|  ell-
|  typed.com%2F&amp;data=02%7C01%7Csimonpj%40microsoft.com%7Cbf260389ea3f4
|  e4259f608d858a765a3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637356
|  823284154386&amp;sdata=NG2gXzd3i%2Beu2PABXS0QOIinkAHLynZqJurwOvc9O6A%3D
|  &amp;reserved=0
|  
|  Registered in England & Wales, OC335890
|  118 Wymering Mansions, Wymering Road, London W9 2NF, England
_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

Ben Gamari-3
In reply to this post by Moritz Angermann-2
Moritz Angermann <[hidden email]> writes:

> 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.
>
To fill in a bit of history here, DynFlags plugins were introduced in
!1827, which arose as an alternative to !1580. The latter proposed a
much more specialised interface specifically allowing plugins to
introduce Hooks. Personally, I far prefer the approach taken in !1580. To
quote my comment on !1580:

> I agree that overriding DynFlags is excessive and, moreover, it
> entrenches the structure of DynFlags as a semi-stable interface. In my
> opinion the current state of DynFlags is a very uneasy compromise and
> really should be refactored (at very least split up into smaller
> records). While it's true that the Hsc capability given to parser
> plugins allows DynFlags to be modified, I would consider this to be
> very much a backdoor and not a supported use.
>
> Hooks, on the other hand, are intended to be extension points for the
> compiler. Consequently it is quite natural for them to be set by
> plugins.
In light of how quickly DynFlags is now changing, I somewhat regret not
pushing back more vigorously against the DynFlags-centric approach. I
tend to agree that we should remove the interface and revert to a more
limited interface that simply deals in Hooks.

Cheers,

- Ben


_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

signature.asc (497 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

Alp Mestanogullari-2

My original motivation for !1580 and !1827 (the latter of which ended up getting merged) would be equally well supported by an interface with more limited scope. My only requirement there was to be able to override the meta hook. I therefore would not mind going back to the approach I initially took, in !1580, which I preferred back then already. As long as we leave a way for plugins to override hooks, my use case will not suffer.

On 14/09/2020 21:20, Ben Gamari wrote:
Moritz Angermann [hidden email] writes:

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.

To fill in a bit of history here, DynFlags plugins were introduced in
!1827, which arose as an alternative to !1580. The latter proposed a
much more specialised interface specifically allowing plugins to
introduce Hooks. Personally, I far prefer the approach taken in !1580. To
quote my comment on !1580:

I agree that overriding DynFlags is excessive and, moreover, it
entrenches the structure of DynFlags as a semi-stable interface. In my
opinion the current state of DynFlags is a very uneasy compromise and
really should be refactored (at very least split up into smaller
records). While it's true that the Hsc capability given to parser
plugins allows DynFlags to be modified, I would consider this to be
very much a backdoor and not a supported use.

Hooks, on the other hand, are intended to be extension points for the
compiler. Consequently it is quite natural for them to be set by
plugins.
In light of how quickly DynFlags is now changing, I somewhat regret not
pushing back more vigorously against the DynFlags-centric approach. I
tend to agree that we should remove the interface and revert to a more
limited interface that simply deals in Hooks.

Cheers,

- Ben


_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

Alfredo Di Napoli

Do you happen to know of DynFlagPlugins, Adam?

Alas the newly release LiquidHaskell plugin relies on the `dynflagsPlugin` action, so it would be nice if this was not removed:


Ignoring the other options, we rely on `Opt_KeepRawTokenStreams` which is *key* for the correct behaviour of the plugin. If we were not intercepting the `DynFlags` at this stage, the lexer would drop any block comments from the parsed sources and we wouldn't have the chance of parsing the LH annotations contained within.

Now, truth to be told, due to the fact we ended up re-parsing each module anyway (for unfortunate reasons) we *could* survive without it by tweaking the `DynFlags` inside the `ModSummary` before we call `parseModule`, but in an ideal world we wouldn't need this, and we would be using directly the parsed source that the `parsedResultAction` would give us. Without the `dynflagsPlugin` I don't think we would be able to do that anymore.




On Mon, 14 Sep 2020 at 23:20, Alp Mestanogullari <[hidden email]> wrote:

My original motivation for !1580 and !1827 (the latter of which ended up getting merged) would be equally well supported by an interface with more limited scope. My only requirement there was to be able to override the meta hook. I therefore would not mind going back to the approach I initially took, in !1580, which I preferred back then already. As long as we leave a way for plugins to override hooks, my use case will not suffer.

On 14/09/2020 21:20, Ben Gamari wrote:
Moritz Angermann [hidden email] writes:

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.

To fill in a bit of history here, DynFlags plugins were introduced in
!1827, which arose as an alternative to !1580. The latter proposed a
much more specialised interface specifically allowing plugins to
introduce Hooks. Personally, I far prefer the approach taken in !1580. To
quote my comment on !1580:

I agree that overriding DynFlags is excessive and, moreover, it
entrenches the structure of DynFlags as a semi-stable interface. In my
opinion the current state of DynFlags is a very uneasy compromise and
really should be refactored (at very least split up into smaller
records). While it's true that the Hsc capability given to parser
plugins allows DynFlags to be modified, I would consider this to be
very much a backdoor and not a supported use.

Hooks, on the other hand, are intended to be extension points for the
compiler. Consequently it is quite natural for them to be set by
plugins.
In light of how quickly DynFlags is now changing, I somewhat regret not
pushing back more vigorously against the DynFlags-centric approach. I
tend to agree that we should remove the interface and revert to a more
limited interface that simply deals in Hooks.

Cheers,

- Ben


_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

Adam Gundry-2
In reply to this post by Moritz Angermann-2
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 <[hidden email]
> <mailto:[hidden email]>> 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 <[hidden email]
>     <mailto:[hidden email]>>; Sylvain Henry
>     > <[hidden email] <mailto:[hidden email]>>
>     > *Cc:* ghc-devs <[hidden email] <mailto:[hidden email]>>
>     > *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 <[hidden email]
>     <mailto:[hidden email]>
>     > <mailto:[hidden email]
>     <mailto:[hidden email]>>> *On Behalf Of *Sebastian Graf
>     > *Sent:* 10 September 2020 14:12
>     > *To:* Sylvain Henry <[hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email] <mailto:[hidden email]>>>
>     > *Cc:* ghc-devs <[hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email] <mailto:[hidden email]>>>
>     > *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
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

Moritz Angermann-2
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. [hidden email] 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 <[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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 <[hidden email]
>     <mailto:[hidden email]>>; Sylvain Henry
>     > <[hidden email] <mailto:[hidden email]>>
>     > *Cc:* ghc-devs <[hidden email] <mailto:[hidden email]>>
>     > *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 <[hidden email]
>     <mailto:[hidden email]>
>     > <mailto:[hidden email]
>     <mailto:[hidden email]>>> *On Behalf Of *Sebastian Graf
>     > *Sent:* 10 September 2020 14:12
>     > *To:* Sylvain Henry <[hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email] <mailto:[hidden email]>>>
>     > *Cc:* ghc-devs <[hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email] <mailto:[hidden email]>>>
>     > *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
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

RE: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

GHC - devs mailing list

But NB: The wart would be much less warty if it was only an abstract syntax tree.   Then it would have practically NO dependencies!  The only wartiness is that if f :: DynFlags -> bha, it’s hard to tell how much of DynFlags f reads.  That’s a software engineering issue, but a much less painful one than having giant module dependency loops.

 

Simon

 

From: ghc-devs <[hidden email]> On Behalf Of Moritz Angermann
Sent: 18 September 2020 14:56
To: Adam Gundry <[hidden email]>; Sylvain Henry <[hidden email]>
Cc: ghc-devs <[hidden email]>
Subject: Re: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

 

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. [hidden email] 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 <[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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 <[hidden email]
>     <mailto:[hidden email]>>; Sylvain Henry
>     > <[hidden email] <mailto:[hidden email]>>
>     > *Cc:* ghc-devs <[hidden email] <mailto:[hidden email]>>
>     > *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 <[hidden email]
>     <mailto:[hidden email]>
>     > <mailto:[hidden email]
>     <mailto:[hidden email]>>> *On Behalf Of *Sebastian Graf
>     > *Sent:* 10 September 2020 14:12
>     > *To:* Sylvain Henry <[hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email] <mailto:[hidden email]>>>
>     > *Cc:* ghc-devs <[hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email] <mailto:[hidden email]>>>
>     > *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
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: Parser depends on DynFlags, depends on Hooks, depends on TcM, DsM, ...

Adam Gundry-2
In reply to this post by Moritz Angermann-2
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:[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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 <[hidden email]
>     <mailto:[hidden email]>
>     > <mailto:[hidden email] <mailto:[hidden email]>>> 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 <[hidden email]
>     <mailto:[hidden email]>
>     >     <mailto:[hidden email] <mailto:[hidden email]>>>;
>     Sylvain Henry
>     >     > <[hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email] <mailto:[hidden email]>>>
>     >     > *Cc:* ghc-devs <[hidden email]
>     <mailto:[hidden email]> <mailto:[hidden email]
>     <mailto:[hidden email]>>>
>     >     > *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 <[hidden email]
>     <mailto:[hidden email]>
>     >     <mailto:[hidden email]
>     <mailto:[hidden email]>>
>     >     > <mailto:[hidden email]
>     <mailto:[hidden email]>
>     >     <mailto:[hidden email]
>     <mailto:[hidden email]>>>> *On Behalf Of *Sebastian Graf
>     >     > *Sent:* 10 September 2020 14:12
>     >     > *To:* Sylvain Henry <[hidden email]
>     <mailto:[hidden email]> <mailto:[hidden email]
>     <mailto:[hidden email]>>
>     >     <mailto:[hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email] <mailto:[hidden email]>>>>
>     >     > *Cc:* ghc-devs <[hidden email]
>     <mailto:[hidden email]> <mailto:[hidden email]
>     <mailto:[hidden email]>>
>     >     <mailto:[hidden email] <mailto:[hidden email]>
>     <mailto:[hidden email] <mailto:[hidden email]>>>>
>     >     > *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
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs