[Git][ghc/ghc][wip/expand-do] - More notes rewriting

Apoorv Ingle (@ani) gitlab at gitlab.haskell.org
Fri Oct 13 00:18:15 UTC 2023



Apoorv Ingle pushed to branch wip/expand-do at Glasgow Haskell Compiler / GHC


Commits:
da7079d1 by Apoorv Ingle at 2023-10-12T19:17:26-05:00
- More notes rewriting

- - - - -


3 changed files:

- compiler/GHC/Rename/Expr.hs
- compiler/GHC/Tc/Gen/Do.hs
- compiler/GHC/Tc/Gen/Expr.hs


Changes:

=====================================
compiler/GHC/Rename/Expr.hs
=====================================
@@ -210,14 +210,14 @@ typecheck the renamed-syntax call (fromLabel @"foo").
 
 Note [Doing HsExpansion in the Renamer vs Typechecker]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-We expand some `HsExpr GhcRn` code at various places, usually on the fly,
+We expand some `HsExpr GhcRn` code at various places, usually, on the fly,
 depending on when it is more convenient. It may be beneficial to have a
 separate `HsExpr GhcRn -> HsExpr GhcRn` pass that does this expansion uniformly
 in the future when we have enough cases to cater for. For the time being,
 this note documents which language feature is expanded at which phase,
 and the reasons for doing so.
 
-  Part 0. OpApp Expansions
+  Part 0. `OpApp` Expansions
   ------------------------
   The typechecker turns `OpApp` into a use of `HsExpansion`
   on the fly, in `GHC.Tc.Gen.Head.splitHsApps`.
@@ -227,16 +227,17 @@ and the reasons for doing so.
   form, because the renamer does precedence rearrangement after name
   resolution. So the renamer leaves an `OpApp` as an `OpApp`.
 
-  Part 1. Record Update Syntax
-  ----------------------------
+  Part 1. Record Update Syntax `RecordUpd` Expansions
+  ---------------------------------------------------
   This is done in the typechecker on the fly (`GHC.Tc.Expr.tcExpr`), and not the renamer, for two reasons:
 
     - (Until we implement GHC proposal #366)
       We need to know the type of the record to disambiguate its fields.
 
-    - We use the type signature of the data constructor to provide IdSigs
-      to the let-bound variables (x', y' in the example above). This is
-      needed to accept programs such as
+    - We use the type signature of the data constructor to provide `IdSigs`
+      to the let-bound variables (x', y' in the example of
+      Note [Handling overloaded and rebindable constructs] above).
+      This is needed to accept programs such as
 
           data R b = MkR { f :: (forall a. a -> a) -> (Int,b), c :: Int }
           foo r = r { f = \ k -> (k 3, k 'x') }
@@ -244,10 +245,10 @@ and the reasons for doing so.
       in which an updated field has a higher-rank type.
       See Wrinkle [Using IdSig] in Note [Record Updates] in GHC.Tc.Gen.Expr.
 
-  Part 2. HsDo Statements
-  -----------------------
-  The expansion for do block statements is done on the fly right before typechecking in
-  `GHC.Tc.Gen.Do.expandDoStmts`. There are 2 reasons:
+  Part 2. `HsDo` Statement Expansions
+  -----------------------------------
+  The expansion for do block statements is done on the fly right before typechecking in `GHC.Tc.Gen.Expr`
+  using `GHC.Tc.Gen.Do.expandDoStmts`. There are 2 main reasons:
 
   -  During the renaming phase, we may not have all the constructor details `HsConDetails` populated in the
      data structure. This would result in an inaccurate irrefutability analysis causing
@@ -255,9 +256,10 @@ and the reasons for doing so.
      See Part 1. of Note [Expanding HsDo with HsExpansion] (test pattern-fails.hs)
 
   -  If the expansion is done on the fly during renaming, expressions that
-     have explicit type applications using (-XTypeApplciations) will not work as the name freshening
-     happens from the root of the AST to the leaves, but the expansion happens in the opposite
-     direction (from leaves to the root), causing the renamer to miss the scoped type variables.
+     have explicit type applications using (-XTypeApplciations) will not work (cf. Let statements expansion)
+     as the name freshening happens from the root of the AST to the leaves,
+     but the expansion happens in the opposite direction (from leaves to the root),
+     causing the renamer to miss the scoped type variables.
 -}
 
 {-


=====================================
compiler/GHC/Tc/Gen/Do.hs
=====================================
@@ -49,7 +49,7 @@ import Data.List ((\\))
 ************************************************************************
 -}
 
--- | Expand the `do` statments into expressions right after renaming
+-- | Expand the `do`-statments into expressions right after renaming
 --   so that they can be typechecked.
 --   See Note [Expanding HsDo with HsExpansion] below for `HsDo` specific commentary
 --   and Note [Handling overloaded and rebindable constructs] for high level commentary
@@ -227,19 +227,24 @@ mk_fail_block _ _ _ _ = pprPanic "mk_fail_block: impossible happened" empty
 
 {- Note [Expanding HsDo with HsExpansion]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-We expand `do` blocks before typechecking it rather than after type checking it using the
-`HsExpansions` and `RebindableSyntax` machinery.
-
+We expand `do`-blocks before typechecking it, rather than type checking it and then
+desugaring it by re-using the existing `HsExpansions` and `RebindableSyntax` machinery.
 This is very similar to:
   1. Expansions done in `GHC.Rename.Expr.rnHsIf` for expanding `HsIf`; and
   2. `desugarRecordUpd` in `GHC.Tc.Gen.Expr.tcExpr` for expanding `RecordUpd`
 
+To disabmiguate desugaring (`HsExpr GhcTc -> Core.Expr`) we use the phrase expansion
+(`HsExpr GhcRn -> HsExpr GhcRn`)
+
 See Note [Handling overloaded and rebindable constructs] in GHC.Rename.Expr
 
-In previous versions of GHC, the “do” notation wasn't expanded before typechecking,
+This expansion is done right before typechecking and after renaming
+See Part 2. of Note [Doing HsExpansion in the Renamer vs Typechecker] in `GHC.Rename.Expr`
+
+In previous versions of GHC, the `do`-notation wasn't expanded before typechecking,
 instead the typechecker would operate directly on the original.
 Why? because it ensured that type error messages were explained in terms of
-what the programmer has written. In practice, however, it this didn't work very well:
+what the programmer has written. In practice, however, this didn't work very well:
 
 * Attempts to typecheck the original source code turned out to be buggy, and virtually impossible
   to fix (#14963, #15598, #21206 and others)
@@ -255,57 +260,57 @@ what the programmer has written. In practice, however, it this didn't work very
 
 * With `ImpredicativeTypes` the programmer will expect Quick Look to instantiate
   the quantifiers impredicatively (#18324). Again, that happens automatically if
-  you typecheck the expansion expression.
+  you typecheck the expanded expression.
 
-* Equationally speaking, we have the following schema for expanding `do` statements.
-  They capture the essense of statment expansions as implemented in `expand_do_stmts`
+* Equationally speaking, we have the following schema for expanding `do`-statements.
+  They capture the essence of statement expansions as implemented in `expand_do_stmts`
 
-  DO【 _ 】 takes in a sequence of do statements and converts them into expressions, recursively
+  DO【 _ 】 maps a sequence of do statements and recursively converts them into expressions
 
           (1) DO【 s; ss 】      = ‹ExpansionStmt s›((>>) s (‹PopErrCtxt›DO【 ss 】))
 
-          (2) DO【 s 】          = s
-
-          (3) DO【 p <- s; ss 】 = if p is irrefutable
-                                   then ‹ExpansionStmt (p <- s)›
+          (2) DO【 p <- e; ss 】 = if p is irrefutable
+                                   then ‹ExpansionStmt (p <- e)›
                                           (>>=) s (‹PopExprCtxt›(\ p -> DO【 ss 】))
-                                   else ‹ExpansionStmt (p <- s)›
+                                   else ‹ExpansionStmt (p <- e)›
                                           (>>=) s (‹PopExprCtxt›(\case p -> DO【 ss 】
                                                                        _ -> fail "pattern p failure"))
 
-For a concrete example, consider a `do` block written by the user
+          (3) DO【 let x = e; ss 】
+                                = ‹ExpansionStmt (let x = e)› (let x = e in (‹PopErrCtxt›DO【 ss 】))
+
+          (4) DO【 s 】          = s
+
+For a concrete example, consider a `do`-block written by the user
 
     f = {l0} do {l1} {pl}p <- {l1'} e1
                 {l2} g p
                 {l3} return {l3'} p
 
-The {l1} etc are location/source span information stored in the AST,
-{g1} are compiler generated source spans
-
-The expanded version (performed by `expand_do_stmts`) looks as follows:
+The expanded version (performed by `expand_do_stmts`) looks like:
 
     f = {g1} (>>=) ({l1'} e1) (\ {pl}p ->
                    {g2} (>>) ({l2} g p)
                              ({l3} return p))
 
-The 4 main points to consider are:
-1. Wrap the expression with a `fail` block if the pattern match is not irrefutable.
-   See Part 1. Below
+The {l1} etc are location/source span information stored in the AST by the parser,
+{g1} are compiler generated source spans.
 
-2. Generate appropriate warnings for discarded results in a body statement
-   eg. say `do { .. ; (g p :: m Int) ; ... }`
-   See Part 2. Below
 
-3. Generating appropriate type error messages which blame the correct source spans
-   See Part 3 Below
+The 3 main points to consider are:
 
-4. Why is this expansion done in the type checker pass and not in renamer pass?
-   See Part 2. of Note [Doing HsExpansion in the Renamer vs Typechecker] in `GHC.Rename.Expr`
+ 1. Wrap the expression with a `fail` block if the pattern match is not irrefutable.
+    See Part 1. Below
+ 2. Generate appropriate warnings for discarded results in a body statement
+    eg. say `do { .. ; (g p :: m Int) ; ... }`
+    See Part 2. Below
+ 3. Generating appropriate type error messages which blame the correct source spans
+    See Part 3 Below
 
 Part 1. Wrapping failable patterns with fail blocks
 ---------------------------------------------------
 If `p` is a failable pattern---checked by `GHC.Tc.Gen.Pat.isIrrefutableHsPatRnTcM`---
-we need to wrap it with a `fail` block. For example, the expansion of the `do` block
+we need to wrap it with a `fail`-block. For example, the expansion of the `do`-block
 
         do { Just p <- e1; e2 }
 
@@ -316,7 +321,7 @@ we need to wrap it with a `fail` block. For example, the expansion of the `do` b
                  Just p -> e2
                  _      -> fail "failable pattern p at location")
 
-The `fail` block wrapping is done in `GHC.Tc.Gen.Do.mk_failable_expr`.
+The `fail`-block wrapping is done in `GHC.Tc.Gen.Do.mk_failable_expr`.
 
 * Note the explicit call to `fail`, in the monad of the `do`-block.  Part of the specification
   of do-notation is that if the pattern match fails, we fail in the monad, *not* just crash
@@ -326,31 +331,32 @@ The `fail` block wrapping is done in `GHC.Tc.Gen.Do.mk_failable_expr`.
   pattern is irrefuable, we don't want to generate that `fail` alternative, else we'll generate
   a `MonadFail` constraint that isn't needed.
 
-* Why an anonymous lambda?
+* Why an anonymous continuation lambda?
   We need a lambda for the types to match: this expression is a second
-  argument to bind so it needs to be of type `a -> m b`
-  It is anonymous because we do not want to introduce a new name that will
-  never be seen by the user anyway.
-
-* Wrinkle 1: For pattern synonyms, we always wrap it with a `fail` block
-  as the irrefutable pattern checker returns false but then during desugaring
-  we would then get pattern match redundant warnings. To avoid such
+  argument to `(>>=)` so it needs to be of type `a -> m b`, a function.
+  It is anonymous because:
+     a. the name will be compiler generated and will never be seen by the user, and;
+     b. we are in the post renaming stage fresh naming will require non-trivial amount of plumbing for little gain.
+
+* Wrinkle 1: For pattern synonyms, we always wrap it with a `fail`-block.
+  The irrefutable pattern checker returns false for pattern synonyms, but then after desugaring
+  we would get a pattern match checker's redundant pattern warnings. To avoid such
   spurious warnings we filter out those type patterns that appear in a do expansion generated match
   in `HsToCore.Match.matchWrapper`. (see testcase Typeable1.hs)
 
-* Wrinkle 2: The `fail` call will give rise to a `MonadFail` constraint. What `CtOrigin` do we
+* Wrinkle 2: The call to `fail` will give rise to a `MonadFail` constraint. What `CtOrigin` do we
   attach to that constraint?  It should be a good one, because it'll show up in error
-  messages if the `MonadFail` constraint can't be solved.  Ideally it should identify the
+  messages when the `MonadFail` constraint can't be solved.  Ideally, it should identify the
   pattern `p`.  Hence, we wrap the `fail` alternative expression with a `ExpandedPat`
   that tags the fail expression with the failable pattern. (See testcase MonadFailErrors.hs)
 
 Part 2. Generate warnings for discarded body statement results
 --------------------------------------------------------------
-If the `do` blocks' body statement is an expression that returns a
-value that is not Unit or `()`, then we need to warn the user about discarded
-value when -Wunused-binds flag is turned on (See testcase T3263-2.hs)
+If the `do`-blocks' body statement is an expression that returns a
+value that is not of type `()`, we need to warn the user about discarded
+the value when `-Wunused-binds` flag is turned on. (See testcase T3263-2.hs)
 
-For example the do expression
+For example the `do`-block
 
     do { e1;  e2 }
 
@@ -358,79 +364,81 @@ expands to
 
     (>>) e1 e2
 
-* If `e1` returns a non-() value then we emit a warning. This check is done during desugaring
-  `HsToCore.dsExpr` for the `HsApp` case calls `warnUnusedBindValue`.
+* If `e1` returns a non-() value then we emit a value discarded warning. This check is done during desugaring
+  `HsToCore.dsExpr` in the `HsApp` with a call to `HsToCore.warnUnusedBindValue`.
 
-* The decision to trigger the warning is: if the function is a `(>>)`, it is a compiler generated
-  and its first argument e1 is a non-() value
+* The decision to trigger the warning is: if the function is a compiler generated `(>>)`,
+  and its first argument `e1` has a non-() type
 
-Part 3. Blaming offending source code and Generating Appropriate Error Messages
+Part 3. Blaming Offending Source Code and Generating Appropriate Error Messages
 -------------------------------------------------------------------------------
 To ensure we correctly track source of the offending user written source code,
-in this case do statements, we need to keep track of
+in this case the `do`-statement, we need to keep track of
 which source statement's expansion the typechecker is currently typechecking.
 For this purpose we use the `XXExprGhcRn.ExpansionRn`.
 It stores the original statement (with location) and the expanded expression
 
   A. Expanding Body Statements
   -----------------------------
-  For example, the do expression
+  For example, the `do`-block
 
       do { e1;  e2 }
 
   expands (ignoring the location info) to
 
-      ‹ExpandedExpr do { e1; e2 }›                               -- Original Do Expression
+      ‹ExpandedThingRn do { e1; e2 }›                               -- Original Do Expression
                                                                  -- Expanded Do Expression
-          (‹ExpandedStmt e1›                                     -- Original Statement
+          (‹ExpandedThingRn e1›                                     -- Original Statement
                         ({(>>) e1}                               -- Expanded Expression
-                            ‹PopErrCtxt› (‹ExpandedStmt e2› e2)))
+                            ‹PopErrCtxt› (‹ExpandedThingRn e2› {e2})))
 
-  * Whenever the typechecker steps through an `ExpandedStmt`,
-    we push the original statement in the error context and typecheck the expanded expression.
+  * Whenever the typechecker steps through an `ExpandedThingRn`,
+    we push the original statement in the error context, set the error location to the
+    location of the statement, and then typecheck the expanded expression.
     This is similar to vanilla `HsExpansion` and rebindable syntax
     See Note [Rebindable syntax and HsExpansion] in `GHC.Hs.Expr`.
 
   * Recall, that when a source function arugment fails to typecheck,
     we print an error message like "In the second argument of the function f..".
-    However, `(>>)` is generated thus we don't want to display that to the user, it would be confusing.
+    However, `(>>)` is generated thus, we don't want to display that to the user; it would be confusing.
     But also, we do not want to completely ignore it as we do want to keep the error blame carets
-    as precise as possible and not just blame the complete `do` block.
+    as precise as possible, and not just blame the complete `do`-block.
     Thus, when we typecheck the application `(>>) e1`, we push the "In the stmt of do block e1" with
-    the source location of `e1` in the error context stack as we walk inside an `ExpandedStmt`.
-    See also Note [splitHsApps]
+    the source location of `e1` in the error context stack as we walk inside an `ExpandedThingRn`.
+    See also Note [splitHsApps].
 
-  * After the expanded expression of a `do` statement is typechecked
-    and before moving to the next statement, we need to first pop the top
-    of the error context which contains the error message for
+  * After the expanded expression of a `do`-statement is typechecked
+    and before moving to the next statement of the `do`-block, we need to first pop the top
+    of the error context stack which contains the error message for
     the previous statement: eg. "In the stmt of a do block: e1".
     This is explicitly encoded in the expansion expression using
-    the `XXExprGhcRn.PopErrCtxt`. Whenever `tcExpr` sees a `PopErrCtxt`
-    it calls `popErrCtxt` to pop of the top of error context stack.
-    See ‹PopErrCtxt› in the example above. Without this popping business for error context stack,
-    If there is a type error in `e2`, we would get a spurious and confusing error message
+    the `XXExprGhcRn.PopErrCtxt`. Whenever `GHC.Tc.Gen.Expr.tcExpr` (via `GHC.Tc.Gen.tcXExpr`)
+    sees a `PopErrCtxt` it calls `GHC.Tc.Utils.Monad.popErrCtxt` to pop of the top of error context stack.
+    See ‹PopErrCtxt› in the example above.
+    Without this popping business for error context stack,
+    if there is a type error in `e2`, we would get a spurious and confusing error message
     which mentions "In the stmt of a do block e1" along with the message
     "In the stmt of a do block e2".
 
   B. Expanding Bind Statements
   -----------------------------
-  A `do` block with with a bind statement:
+  A `do`-block with a bind statement:
 
       do { p <- e1; e2 }
 
   expands (ignoring the location information) to
 
-     ‹ExpandedExpr do{ p <- e1; e2 }›                                      -- Original Do Expression
+     ‹ExpandedThingRn do{ p <- e1; e2 }›                                      -- Original Do Expression
                                                                            --
-         (‹ExpandedStmt (p <- e1)›                                         -- Original Statement
+         (‹ExpandedThingRn (p <- e1)›                                         -- Original Statement
                         (((>>=) e1)                                        -- Expanded Expression
-                           ‹PopErrCtxt› ((\ p -> ‹ExpandedStmt (e2)› e2)))
+                           ‹PopErrCtxt› ((\ p -> ‹ExpandedThingRn (e2)› e2)))
          )
 
 
-  However, the expansion lambda `(\p -> e2)` is special as it is generated from a `do` block expansion
+  However, the expansion lambda `(\p -> e2)` is special as it is generated from a `do`-stmt expansion
   and if a type checker error occurs in the pattern `p` (which is source generated), we need to say
-  "in a pattern binding in a do block" and not "in a lambda abstraction" (cf. Typeable1.hs).
+  "in a pattern binding in a do block" and not "in the pattern of a lambda" (cf. Typeable1.hs).
   We hence use a tag `GenReason` in `Ghc.Tc.Origin`. When typechecking a `HsLam` in `Tc.Gen.Expr.tcExpr`
   the `match_ctxt` is set to a `StmtCtxt` if `GenOrigin` is a `DoExpansionOrigin`.
 -}


=====================================
compiler/GHC/Tc/Gen/Expr.hs
=====================================
@@ -270,7 +270,7 @@ tcExpr e@(HsLam x lam_variant matches) res_ty
   where
     match_ctxt
       | Just f <- doExpansionFlavour (mg_ext matches)
-      -- See Part 3. of Note [Expanding HsDo with HsExpansion] in `GHC.Tc.Gen.Do`. Testcase: Typeable1
+      -- See Part 3. B. of Note [Expanding HsDo with HsExpansion] in `GHC.Tc.Gen.Do`. Testcase: Typeable1
       = MC { mc_what = StmtCtxt (HsDoStmt f)
            , mc_body = tcBodyNC -- NB: Do not add any error contexts
                                 -- It has already been done
@@ -660,7 +660,7 @@ tcXExpr (PopErrCtxt (L loc e)) res_ty
 
 tcXExpr xe@(ExpandedThingRn thing) res_ty
   | OrigStmt ls@(L loc s at LetStmt{}) <- original thing
-  , HsLet x tkLet binds tkIn e <- (expanded thing)
+  , HsLet x tkLet binds tkIn e <- expanded thing
   =  do { (binds', e') <-  setSrcSpanA loc $
                             addStmtCtxt s $
                             tcLocalBinds binds $
@@ -679,11 +679,9 @@ tcXExpr xe@(ExpandedThingRn thing) res_ty
   | OrigStmt ls@(L loc _) <- original thing
   = setSrcSpanA loc $
        mkExpandedStmtTc ls <$> tcApp (XExpr xe) res_ty
--- TODO: there is disgusting amount of packing unpacking going on here. Need to fix mkExpandStmtTc API
---       and addErrCtxt API
 
 tcXExpr xe res_ty = tcApp (XExpr xe) res_ty
-                           -- See Wrinkle 2. of Note [Expanding HsDo with HsExpansion] in `GHC.Tc.Gen.Do`
+
 {-
 ************************************************************************
 *                                                                      *



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/da7079d11d428eca52f24e84b584082378fa6f5b

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/da7079d11d428eca52f24e84b584082378fa6f5b
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20231012/556abd2a/attachment-0001.html>


More information about the ghc-commits mailing list