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

Simon Peyton Jones simonpj at microsoft.com
Mon Sep 14 14:31:23 UTC 2020


| 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 <adam at well-typed.com>
|  Sent: 14 September 2020 13:12
|  To: Simon Peyton Jones <simonpj at microsoft.com>
|  Cc: ghc-devs at haskell.org
|  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&data=02%7C01%7Csimonpj%40microsoft.com%7Cbf260389ea3f4e4259
|  f608d858a765a3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63735682328
|  4144388&sdata=oiE3yFoPgiMbQvxH2sftNdv3Gak7QKK2KQkZFS6D8Jo%3D&re
|  served=0
|  [2]
|  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
|  b.haskell.org%2Fghc%2Fghc%2F-
|  %2Fmerge_requests%2F1580&data=02%7C01%7Csimonpj%40microsoft.com%7Cb
|  f260389ea3f4e4259f608d858a765a3%7C72f988bf86f141af91ab2d7cd011db47%7C1%
|  7C0%7C637356823284144388&sdata=6R6KkZe69PQdYRpJHGBYa6vt%2Ff0z8C142f
|  gWX7iqZdA%3D&reserved=0
|  [3]
|  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
|  b.haskell.org%2Falp%2Fghc-external-splices-
|  plugin&data=02%7C01%7Csimonpj%40microsoft.com%7Cbf260389ea3f4e4259f
|  608d858a765a3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637356823284
|  144388&sdata=gamHplH2iLxtQRYPyT3POO%2F0jAPPHRMSIka81ZMCbuQ%3D&r
|  eserved=0
|  
|  
|  > Simon
|  >
|  > |  -----Original Message-----
|  > |  From: ghc-devs <ghc-devs-bounces at haskell.org> On Behalf Of Adam
|  Gundry
|  > |  Sent: 14 September 2020 12:08
|  > |  To: ghc-devs at haskell.org
|  > |  Subject: Re: Parser depends on DynFlags, depends on Hooks, depends
|  on
|  > |  TcM, DsM, ...
|  > |
|  > |  I'm supportive of the goal, but a complication with removing hooks
|  from
|  > |  DynFlags is that GHC currently supports "DynFlags plugins" that
|  allow
|  > |  plugins to install custom hooks
|  > |
|  (https://nam06.safelinks.protection.outlook.com/?url=https:%2F%2Fdownlo
|  > |
|  ads.haskell.org%2F~ghc%2Flatest%2Fdocs%2Fhtml%2Fusers_guide%2Fextending
|  > |  _ghc.html%23dynflags-
|  > |
|  plugins&data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f5
|  > |
|  4808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63735678554
|  > |
|  4069988&sdata=d8wJidQddoONBhJXnv6iI1%2FD4gVDq%2FvC7cgSxJoBOFQ%3D&am
|  > |  p;reserved=0).
|  > |  I guess this can be worked around, perhaps by passing hooks
|  separately
|  > |  to DynFlags and providing a separate plugin interface to modify
|  hooks.
|  > |  But doing so will necessarily break existing plugins.
|  > |
|  > |  Adam
|  > |
|  > |
|  > |  On 14/09/2020 11:25, Simon Peyton Jones via ghc-devs wrote:
|  > |  > I thought I’d sent a message about this DynFlags thing, but I
|  can’t
|  > |  > trace it now.   So here’s a resend.
|  > |  >
|  > |  >
|  > |  >
|  > |  > Currently
|  > |  >
|  > |  >   * The DynFlags record includes Hooks
|  > |  >   * Hooks in contains functions, that mention TcM, DsM etc
|  > |  >
|  > |  >
|  > |  >
|  > |  > This is bad.  We should think of DynFlags as an *abstract syntax
|  > |  tree*.
|  > |  > That is, the result of parsing the flag strings, yes, but not
|  much
|  > |  > more.  So for hooks we should have an algebraic data type
|  > |  representing
|  > |  > the hook /specification/, but it should not be the hook
|  functions
|  > |  > themselves.  HsSyn, for example, after parsing, is just a tree
|  with
|  > |  > strings in it.  No TyCons, Ids, etc. That comes much later.
|  > |  >
|  > |  >
|  > |  >
|  > |  > So DynFlags should be a collection of algebraic data types, but
|  > |  should
|  > |  > not depend on anything else.
|  > |  >
|  > |  >
|  > |  >
|  > |  > I think that may cut a bunch of awkward loops.
|  > |  >
|  > |  >
|  > |  >
|  > |  > Simon
|  > |  >
|  > |  >
|  > |  >
|  > |  > *From:*Simon Peyton Jones
|  > |  > *Sent:* 10 September 2020 14:17
|  > |  > *To:* Sebastian Graf <sgraf1337 at gmail.com>; Sylvain Henry
|  > |  > <sylvain at haskus.fr>
|  > |  > *Cc:* ghc-devs <ghc-devs at haskell.org>
|  > |  > *Subject:* RE: Parser depends on DynFlags, depends on Hooks,
|  depends
|  > |  on
|  > |  > TcM, DsM, ...
|  > |  >
|  > |  >
|  > |  >
|  > |  > And for sure the **parser** should not depend on the
|  **desugarer**
|  > |  and
|  > |  > **typechecker**.   (Which it does, as described below.)
|  > |  >
|  > |  >
|  > |  >
|  > |
|  https://nam06.safelinks.protection.outlook.com/?url=https:%2F%2Fdownloa
|  > |
|  ds.haskell.org%2F~ghc%2Flatest%2Fdocs%2Fhtml%2Fusers_guide%2Fextending_
|  > |  ghc.html%23dynflags-
|  > |
|  plugins&data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f5
|  > |
|  4808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63735678554
|  > |
|  4069988&sdata=d8wJidQddoONBhJXnv6iI1%2FD4gVDq%2FvC7cgSxJoBOFQ%3D&am
|  > |  p;reserved=0
|  > |  > S
|  > |  >
|  > |  >
|  > |  >
|  > |  > *From:*ghc-devs <ghc-devs-bounces at haskell.org
|  > |  > <mailto:ghc-devs-bounces at haskell.org>> *On Behalf Of *Sebastian
|  Graf
|  > |  > *Sent:* 10 September 2020 14:12
|  > |  > *To:* Sylvain Henry <sylvain at haskus.fr
|  <mailto:sylvain at haskus.fr>>
|  > |  > *Cc:* ghc-devs <ghc-devs at haskell.org <mailto:ghc-
|  devs at haskell.org>>
|  > |  > *Subject:* Parser depends on DynFlags, depends on Hooks, depends
|  on
|  > |  TcM,
|  > |  > DsM, ...
|  > |  >
|  > |  >
|  > |  >
|  > |  > Hey Sylvain,
|  > |  >
|  > |  >
|  > |  >
|  > |  > In
|  > |
|  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
|  > |  b.haskell.org%2Fghc%2Fghc%2F-
|  > |
|  %2Fmerge_requests%2F3971&data=02%7C01%7Csimonpj%40microsoft.com%7C0
|  > |
|  e358ff421a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%
|  > |
|  7C0%7C637356785544069988&sdata=OV1X6btPhnB7UPxOKvCRzUOZVlH7r5ZpR33d
|  > |  6vil3fI%3D&reserved=0
|  > |  >
|  > |
|  <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
|  > |  ab.haskell.org%2Fghc%2Fghc%2F-
|  > |
|  %2Fmerge_requests%2F3971&data=02%7C01%7Csimonpj%40microsoft.com%7C0
|  > |
|  e358ff421a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%
|  > |
|  7C0%7C637356785544069988&sdata=OV1X6btPhnB7UPxOKvCRzUOZVlH7r5ZpR33d
|  > |  6vil3fI%3D&reserved=0>
|  > |  > I had to fight once more with the transitive dependency set of
|  the
|  > |  > parser, the minimality of which is crucial for ghc-lib-parser
|  > |  >
|  > |
|  <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fhack
|  > |  age.haskell.org%2Fpackage%2Fghc-lib-
|  > |
|  parser&data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff421a64763f54
|  > |
|  808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637356785544
|  > |
|  079987&sdata=bHnY1UZgFauLsB0dISUXZavGholi6UWzA7nSuByMCns%3D&res
|  > |  erved=0>
|  > |  > and tested by the CountParserDeps test.
|  > |  >
|  > |  >
|  > |  >
|  > |  > I discovered that I need to make (parts of) `DsM` abstract,
|  because
|  > |  it
|  > |  > is transitively imported from the Parser for example through
|  Parser.y
|  > |  ->
|  > |  > Lexer.x -> DynFlags -> Hooks -> {DsM,TcM}.
|  > |  >
|  > |  > Since you are our mastermind behind the "Tame DynFlags"
|  initiative,
|  > |  I'd
|  > |  > like to hear your opinion on where progress can be/is made on
|  that
|  > |  front.
|  > |  >
|  > |  >
|  > |  >
|  > |  > I see there is
|  > |
|  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
|  > |  b.haskell.org%2Fghc%2Fghc%2F-
|  > |
|  %2Fissues%2F10961&data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
|  > |
|  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
|  > |
|  37356785544079987&sdata=pghmuc9gmaHinB8cLZHx2PqBqDwNkBJBSFZsYlCdwL8
|  > |  %3D&reserved=0
|  > |  >
|  > |
|  <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
|  > |  ab.haskell.org%2Fghc%2Fghc%2F-
|  > |
|  %2Fissues%2F10961&data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
|  > |
|  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
|  > |
|  37356785544079987&sdata=pghmuc9gmaHinB8cLZHx2PqBqDwNkBJBSFZsYlCdwL8
|  > |  %3D&reserved=0>
|  > |  > and
|  > |
|  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
|  > |  b.haskell.org%2Fghc%2Fghc%2F-
|  > |
|  %2Fissues%2F11301&data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
|  > |
|  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
|  > |
|  37356785544079987&sdata=tiE1uhe%2BganXJKPbtadDGUCnSi%2F0OMT6WopEZ9s
|  > |  MJJY%3D&reserved=0
|  > |  >
|  > |
|  <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
|  > |  ab.haskell.org%2Fghc%2Fghc%2F-
|  > |
|  %2Fissues%2F11301&data=02%7C01%7Csimonpj%40microsoft.com%7C0e358ff4
|  > |
|  21a64763f54808d8589e9b50%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
|  > |
|  37356785544079987&sdata=tiE1uhe%2BganXJKPbtadDGUCnSi%2F0OMT6WopEZ9s
|  > |  MJJY%3D&reserved=0>
|  > |  > which ask a related, but different question: They want a
|  DynFlags-
|  > |  free
|  > |  > interface, but I even want a DynFlags-free *module*.
|  > |  >
|  > |  >
|  > |  >
|  > |  > Would you say it's reasonable to abstract the definition of
|  `PState`
|  > |  > over the `DynFlags` type? I think it's only used for pretty-
|  printing
|  > |  > messages, which is one of your specialties (the treatment of
|  DynFlags
|  > |  in
|  > |  > there, at least).
|  > |  >
|  > |  > Anyway, can you think of or perhaps point me to an existing road
|  map
|  > |  on
|  > |  > that issue?
|  > |  >
|  > |  >
|  > |  >
|  > |  > Thank you!
|  > |  >
|  > |  > Sebastian
|  
|  
|  --
|  Adam Gundry, Haskell Consultant
|  Well-Typed LLP,
|  https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.w
|  ell-
|  typed.com%2F&data=02%7C01%7Csimonpj%40microsoft.com%7Cbf260389ea3f4
|  e4259f608d858a765a3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637356
|  823284154386&sdata=NG2gXzd3i%2Beu2PABXS0QOIinkAHLynZqJurwOvc9O6A%3D
|  &reserved=0
|  
|  Registered in England & Wales, OC335890
|  118 Wymering Mansions, Wymering Road, London W9 2NF, England


More information about the ghc-devs mailing list