Why TcLclEnv and DsGblEnv need to store the same IORef for errors?

Wed Mar 31 14:36:43 UTC 2021

I might still be tempted to do:

data DsMessage =
     ...
   | DsLiftedTcRnMessage !TcRnMessage
   -- ^ A diagnostic coming straight from the Typecheck-renamer.

data TcRnMessage =
     ...
   | TcRnLiftedDsMessage !DsMessage
   -- ^ A diagnostic coming straight from the Desugarer.

tying them together with hs-boot. Yes, that means one can do some silly 
`TcRnLiftedDsMessage . DsLiftedTcRnMessage . TcRnLiftedDsMessage ...`, 
but that could even show up in a render as "while desugaring a splice 
during type checking, while typechecking during desguaring, ..." so 
arguably the information the wrapping isn't purely superfluous.

I think this would pose no practical problem today, while still "soft 
enforcing" the abstraction boundaries we want.

On 3/31/21 3:45 AM, Alfredo Di Napoli wrote:
> Follow up:
>
> Argh! I have just seen that I have a bunch of test failures related to 
> my MR (which, needless to say, it's still WIP).
>
> For example:
>
> run/T9140.run.stdout.normalised 2021-03-31 09:35:48.000000000 +0200
> @@ -1,12 +1,4 @@
> -<interactive>:2:5:
> -    You can't mix polymorphic and unlifted bindings: a = (# 1 #)
> -    Probable fix: add a type signature
> -
> -<interactive>:3:5:
> -    You can't mix polymorphic and unlifted bindings: a = (# 1, 3 #)
> -    Probable fix: add a type signature
> -
>
> So it looks like some diagnostic is now not being reported and, 
> surprise surprise, this was emitted from the DsM monad.
>
> I have the suspect that indeed Richard was right (like he always is :) 
> ) -- when we go from a DsM to a TcM monad (See `initDsTc`) for 
> example, I think we also need to carry into the new monad all the 
> diagnostics we collected so far.
>
> This implies indeed a mutual dependency (as Simon pointed out, heh).
>
>
> So I think my cunning plan of embedding is crumbling -- I suspect we 
> would end up with a type `TcRnDsMessage` which captures the dependency.
>
> Sorry for not seeing it sooner!
>
>
>
>
>
>
>
>
> On Wed, 31 Mar 2021 at 08:05, Alfredo Di Napoli 
> <alfredo.dinapoli at gmail.com <mailto:alfredo.dinapoli at gmail.com>> wrote:
>
>     Morning all,
>
>     *Richard*: sorry! Unfortunately MR !4798 is the cornerstone of
>     this refactoring work but it's also gargantuan. Let's discuss a
>     plan to attack it, but fundamentally there is a critical mass of
>     changes that needs to happen atomically or it wouldn't make much
>     sense, and alas this doesn't play in our favour when it comes to
>     MR size and ease of review. However, to quickly reply to your
>     remak: currently (for the sake of the "minimum-viable-product") I
>     am trying to stabilise the external interfaces, by which I mean
>     giving functions their final type signature while I do what's
>     easiest to make things typecheck. In this phase what I think is
>     the easiest is to wrap the majority of diagnostics into the
>     `xxUnknownxx` constructor, and change them gradually later. A fair
>     warning, though: you say "I would think that a DsMessage would
>     later be wrapped in an envelope." This might be true for Ds
>     messages (didn't actually invest any brain cycles to check that)
>     but in general we have to turn a message into an envelope as soon
>     as we have a chance to do so, because we need to grab the
>     `SrcSpan` and the `DynFlags` *at the point of creation* of the
>     diagnostics. Carrying around a message and make it bubble up at
>     some random point won't be a good plan (even for Ds messages).
>     Having said that, I clearly have very little knowledge about this
>     area of GHC, so feel free to disagree :)
>
>     *John*: Although it's a bit hard to predict how well this is going
>     to evolve, my current embedding, to refresh everyone's memory, is
>     the following:
>
>     data DsMessage =
>
>       DsUnknownMessage !DiagnosticMessage
>
>     -- ^ Stop-gap constructor to ease the migration.
>
>     | DsLiftedTcRnMessage !TcRnMessage
>
>     -- ^ A diagnostic coming straight from the Typecheck-renamer.
>
>     -- More messages added in the future, of course
>
>
>     At first I thought this was the wrong way around, due to Simon's
>     comment, but this actually creates pleasant external interfaces.
>     To give you a bunch of examples from MR !4798:
>
>
>     deSugar :: HscEnv -> ModLocation -> TcGblEnv -> IO (Messages
>     DsMessage, Maybe ModGuts)
>
>     deSugarExpr :: HscEnv -> LHsExpr GhcTc -> IO (Messages DsMessage,
>     Maybe CoreExpr)
>
>     Note something interesting: the second function actually calls
>     `runTcInteractive` inside the body, but thanks to the
>     `DsLiftedTcRnMessage` we can still expose to the consumer an
>     opaque `DsMessage` , which is what I would expect to see from a
>     function called "deSugarExpr". Conversely, I would be puzzled to
>     find those functions returning a `TcRnDsMessage`.
>
>
>     Having said all of that, I am not advocating this design is "the
>     best". I am sure we will iterate on it. I am just reporting that
>     even this baseline seems to be decent from an API perspective :)
>
>
>     On Wed, 31 Mar 2021 at 05:45, John Ericson
>     <john.ericson at obsidian.systems> wrote:
>
>         Alfredo also replied to this pointing his embedding plan. I
>         also prefer that, because I really wish TH didn't smear
>         together the phases so much. Moreover, I hope with
>
>          - GHC proposals
>         https://github.com/ghc-proposals/ghc-proposals/pull/412
>         <https://github.com/ghc-proposals/ghc-proposals/pull/412> /
>         https://github.com/ghc-proposals/ghc-proposals/pull/243
>         <https://github.com/ghc-proposals/ghc-proposals/pull/243>
>
>          - The parallelism work currently be planned in
>         https://gitlab.haskell.org/ghc/ghc/-/wikis/Plan-for-increased-parallelism-and-more-detailed-intermediate-output
>         <https://gitlab.haskell.org/ghc/ghc/-/wikis/Plan-for-increased-parallelism-and-more-detailed-intermediate-output>
>
>
>         we might actually have an opportunity/extra motivation to do
>         that. Splices and quotes will still induce intricate
>         inter-phase dependencies, but I hope that could be mediated by
>         the driver rather than just baked into each phase.
>
>         (One final step would be the "stuck macros" technique of
>         https://www.youtube.com/watch?v=nUvKoG_V_U0
>         <https://www.youtube.com/watch?v=nUvKoG_V_U0> /
>         https://github.com/gelisam/klister
>         <https://github.com/gelisam/klister>, where TH splices would
>         be able to making "blocking queries" of the the compiler in
>         ways that induce more of these fine-grained dependencies.)
>
>         Anyways, while we could also do a "RnTsDsError" and split
>         later, I hope Alfredo's alternative of embedding won't be too
>         much harder and prepare us for these exciting areas of
>         exploration.
>
>         John
>
>         On 3/30/21 10:14 AM, Richard Eisenberg wrote:
>>
>>
>>>         On Mar 30, 2021, at 4:57 AM, Alfredo Di Napoli
>>>         <alfredo.dinapoli at gmail.com
>>>         <mailto:alfredo.dinapoli at gmail.com>> wrote:
>>>
>>>         I'll explore the idea of adding a second IORef.
>>
>>         Renaming/type-checking is already mutually recursive. (The
>>         renamer must call the type-checker in order to rename -- that
>>         is, evaluate -- untyped splices. I actually can't recall why
>>         the type-checker needs to call the renamer.) So we will have
>>         a TcRnError. Now we see that the desugarer ends up mixed in,
>>         too. We could proceed how Alfredo suggests, by adding a
>>         second IORef. Or we could just make TcRnDsError (maybe
>>         renaming that).
>>
>>         What's the disadvantage? Clients will have to potentially
>>         know about all the different error forms with either approach
>>         (that is, using my combined type or using multiple IORefs).
>>         The big advantage to separating is maybe module dependencies?
>>         But my guess is that the dependencies won't be an issue here,
>>         due to the fact that these components are already leaning on
>>         each other. Maybe the advantage is just in having smaller
>>         types? Maybe.
>>
>>         I don't have a great sense as to what to do here, but I would
>>         want a clear reason that e.g. the TcRn monad would have two
>>         IORefs, while other monads will work with GhcMessage (instead
>>         of a whole bunch of IORefs).
>>
>>         Richard
>>
>>         _______________________________________________
>>         ghc-devs mailing list
>>         ghc-devs at haskell.org  <mailto:ghc-devs at haskell.org>
>>         http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs  <http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs>
>         _______________________________________________
>         ghc-devs mailing list
>         ghc-devs at haskell.org <mailto:ghc-devs at haskell.org>
>         http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
>         <http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-devs/attachments/20210331/2dbc49c9/attachment.html>