Add SynExprSequentialTrivia (#16981)

* Add SynExprSequentialTrivia * Add release note * Format * Apply feedback from code review * PR is fine --------- Co-authored-by: Vlad Zarytovskii <vzaritovsky@hotmail.com>
dotnet · Apr 5, 2024 · 050271d · 050271d
1 parent 3d95e4e
commit 050271d
Show file tree

Hide file tree

Showing 30 changed files with 221 additions and 79 deletions.
diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md
@@ -45,6 +45,7 @@
 * Parser: more 'as' pattern recovery ([PR #16837](https://github.com/dotnet/fsharp/pull/16837))
 * Add extended data for `DefinitionsInSigAndImplNotCompatibleAbbreviationsDiffer` (FS0318). ([PR #16811](https://github.com/dotnet/fsharp/pull/16811)))
 * Checker/patterns: recover on unresolved long identifiers ([PR #16842](https://github.com/dotnet/fsharp/pull/16842))
+* SynExprSequentialTrivia ([Issue #16914](https://github.com/dotnet/fsharp/issues/16914), [PR #16981](https://github.com/dotnet/fsharp/pull/16981))
 
 ### Changed
 

diff --git a/src/Compiler/Checking/CheckComputationExpressions.fs b/src/Compiler/Checking/CheckComputationExpressions.fs
@@ -205,7 +205,7 @@ let (|SimpleSemicolonSequence|_|) cenv acceptDeprecated cexpr =
 
     let rec TryGetSimpleSemicolonSequenceOfComprehension expr acc =
         match expr with
-        | SynExpr.Sequential(_, true, e1, e2, _) ->
+        | SynExpr.Sequential(isTrueSeq = true; expr1 = e1; expr2 = e2) ->
             if IsSimpleSemicolonSequenceElement e1 then
                 TryGetSimpleSemicolonSequenceOfComprehension e2 (e1 :: acc)
             else
@@ -752,8 +752,14 @@ let TcComputationExpression (cenv: cenv) env (overallTy: OverallTy) tpenv (mWhol
 
     let (|ForEachThen|_|) synExpr =
         match synExpr with
-        | SynExpr.ForEach(_spFor, _spIn, SeqExprOnly false, isFromSource, pat1, expr1, SynExpr.Sequential(_, true, clause, rest, _), _) ->
-            Some(isFromSource, pat1, expr1, clause, rest)
+        | SynExpr.ForEach(_spFor,
+                          _spIn,
+                          SeqExprOnly false,
+                          isFromSource,
+                          pat1,
+                          expr1,
+                          SynExpr.Sequential(isTrueSeq = true; expr1 = clause; expr2 = rest),
+                          _) -> Some(isFromSource, pat1, expr1, clause, rest)
         | _ -> None
 
     let (|CustomOpId|_|) predicate synExpr =
@@ -998,7 +1004,7 @@ let TcComputationExpression (cenv: cenv) env (overallTy: OverallTy) tpenv (mWhol
 
     let (|OptionalSequential|) e =
         match e with
-        | SynExpr.Sequential(_sp, true, dataComp1, dataComp2, _) -> (dataComp1, Some dataComp2)
+        | SynExpr.Sequential(debugPoint = _sp; isTrueSeq = true; expr1 = dataComp1; expr2 = dataComp2) -> (dataComp1, Some dataComp2)
         | _ -> (e, None)
 
     // "cexpr; cexpr" is treated as builder.Combine(cexpr1, cexpr1)
@@ -1233,7 +1239,14 @@ let TcComputationExpression (cenv: cenv) env (overallTy: OverallTy) tpenv (mWhol
                     // 2. incompatible types: int and string
                     // with SynExpr.ArbitraryAfterError we have only first one
                     let wrapInArbErrSequence l caption =
-                        SynExpr.Sequential(DebugPointAtSequential.SuppressNeither, true, l, (arbExpr (caption, l.Range.EndRange)), l.Range)
+                        SynExpr.Sequential(
+                            DebugPointAtSequential.SuppressNeither,
+                            true,
+                            l,
+                            (arbExpr (caption, l.Range.EndRange)),
+                            l.Range,
+                            SynExprSequentialTrivia.Zero
+                        )
 
                     let mkOverallExprGivenVarSpaceExpr, varSpaceInner =
 
@@ -1529,7 +1542,14 @@ let TcComputationExpression (cenv: cenv) env (overallTy: OverallTy) tpenv (mWhol
                             SynExpr.While(
                                 DebugPointAtWhile.No,
                                 SynExpr.Ident idCond,
-                                SynExpr.Sequential(DebugPointAtSequential.SuppressBoth, true, innerComp, bindCondExpr, mWhile),
+                                SynExpr.Sequential(
+                                    DebugPointAtSequential.SuppressBoth,
+                                    true,
+                                    innerComp,
+                                    bindCondExpr,
+                                    mWhile,
+                                    SynExprSequentialTrivia.Zero
+                                ),
                                 mOrig
                             )
 
@@ -1658,7 +1678,7 @@ let TcComputationExpression (cenv: cenv) env (overallTy: OverallTy) tpenv (mWhol
                 // Now run the consumeCustomOpClauses
                 Some(consumeCustomOpClauses q varSpace dataCompPriorToOp comp false mClause)
 
-            | SynExpr.Sequential(sp, true, innerComp1, innerComp2, m) ->
+            | SynExpr.Sequential(sp, true, innerComp1, innerComp2, m, _) ->
 
                 // Check for 'where x > y' and other mis-applications of infix operators. If detected, give a good error message, and just ignore innerComp1
                 if isQuery && checkForBinaryApp innerComp1 then
@@ -1761,7 +1781,7 @@ let TcComputationExpression (cenv: cenv) env (overallTy: OverallTy) tpenv (mWhol
 
                                             SynExpr.SequentialOrImplicitYield(sp, innerComp1, holeFill, combineExpr, m)
                                         else
-                                            SynExpr.Sequential(sp, true, innerComp1, holeFill, m)
+                                            SynExpr.Sequential(sp, true, innerComp1, holeFill, m, SynExprSequentialTrivia.Zero)
 
                                     translatedCtxt fillExpr)
                             )
@@ -2643,7 +2663,14 @@ let TcComputationExpression (cenv: cenv) env (overallTy: OverallTy) tpenv (mWhol
                                     comp.Range
                                 )
                             else
-                                SynExpr.Sequential(DebugPointAtSequential.SuppressExpr, true, comp, holeFill, comp.Range)
+                                SynExpr.Sequential(
+                                    DebugPointAtSequential.SuppressExpr,
+                                    true,
+                                    comp,
+                                    holeFill,
+                                    comp.Range,
+                                    SynExprSequentialTrivia.Zero
+                                )
 
                         translatedCtxt fillExpr)
 
@@ -2772,14 +2799,14 @@ let TcComputationExpression (cenv: cenv) env (overallTy: OverallTy) tpenv (mWhol
 
             Some(varSpaceExpr, Some(innerComp, mClause))
 
-        | SynExpr.Sequential(sp, true, innerComp1, innerComp2, m) ->
+        | SynExpr.Sequential(sp, true, innerComp1, innerComp2, m, trivia) ->
 
             // Check the first part isn't a computation expression construct
             if isSimpleExpr innerComp1 then
                 // Check the second part is a simple return
                 match convertSimpleReturnToExpr varSpace innerComp2 with
                 | None -> None
-                | Some(innerExpr2, optionalCont) -> Some(SynExpr.Sequential(sp, true, innerComp1, innerExpr2, m), optionalCont)
+                | Some(innerExpr2, optionalCont) -> Some(SynExpr.Sequential(sp, true, innerComp1, innerExpr2, m, trivia), optionalCont)
             else
                 None
 
@@ -2798,7 +2825,7 @@ let TcComputationExpression (cenv: cenv) env (overallTy: OverallTy) tpenv (mWhol
         | SynExpr.ImplicitZero _ -> false
         | OptionalSequential(JoinOrGroupJoinOrZipClause _, _) -> false
         | OptionalSequential(CustomOperationClause _, _) -> false
-        | SynExpr.Sequential(_, _, innerComp1, innerComp2, _) -> isSimpleExpr innerComp1 && isSimpleExpr innerComp2
+        | SynExpr.Sequential(expr1 = innerComp1; expr2 = innerComp2) -> isSimpleExpr innerComp1 && isSimpleExpr innerComp2
         | SynExpr.IfThenElse(thenExpr = thenComp; elseExpr = elseCompOpt) ->
             isSimpleExpr thenComp
             && (match elseCompOpt with
@@ -3122,7 +3149,7 @@ let TcSequenceExpression (cenv: cenv) env tpenv comp (overallTy: OverallTy) m =
 
         | SynExpr.DoBang(_rhsExpr, m) -> error (Error(FSComp.SR.tcDoBangIllegalInSequenceExpression (), m))
 
-        | SynExpr.Sequential(sp, true, innerComp1, innerComp2, m) ->
+        | SynExpr.Sequential(sp, true, innerComp1, innerComp2, m, _) ->
             let env1 =
                 { env with
                     eIsControlFlow =

diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs
@@ -5795,7 +5795,7 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE
         let _, tpenv = suppressErrorReporting (fun () -> TcExpr cenv overallTy env tpenv expr1)
         mkDefault(m, overallTy.Commit), tpenv
 
-    | SynExpr.Sequential (sp, dir, synExpr1, synExpr2, m) ->
+    | SynExpr.Sequential (sp, dir, synExpr1, synExpr2, m, _) ->
         TcExprSequential cenv overallTy env tpenv (synExpr, sp, dir, synExpr1, synExpr2, m)
 
     // Used to implement the type-directed 'implicit yield' rule for computation expressions
@@ -10337,7 +10337,7 @@ and TcLinearExprs bodyChecker cenv env overallTy tpenv isCompExpr synExpr cont =
     let g = cenv.g
 
     match synExpr with
-    | SynExpr.Sequential (sp, true, expr1, expr2, m) when not isCompExpr ->
+    | SynExpr.Sequential (sp, true, expr1, expr2, m, _) when not isCompExpr ->
         let expr1R, _ =
             let env1 = { env with eIsControlFlow = (match sp with | DebugPointAtSequential.SuppressNeither | DebugPointAtSequential.SuppressExpr -> true | _ -> false) }
             TcStmtThatCantBeCtorBody cenv env1 tpenv expr1

diff --git a/src/Compiler/Interactive/fsi.fs b/src/Compiler/Interactive/fsi.fs
@@ -4258,7 +4258,14 @@ type FsiInteractionProcessor
             let m = expr.Range
             // Make this into "(); expr" to suppress generalization and compilation-as-function
             let exprWithSeq =
-                SynExpr.Sequential(DebugPointAtSequential.SuppressExpr, true, SynExpr.Const(SynConst.Unit, m.StartRange), expr, m)
+                SynExpr.Sequential(
+                    DebugPointAtSequential.SuppressExpr,
+                    true,
+                    SynExpr.Const(SynConst.Unit, m.StartRange),
+                    expr,
+                    m,
+                    SynExprSequentialTrivia.Zero
+                )
 
             ExecuteParsedExpressionOnMainThread(ctok, diagnosticsLogger, exprWithSeq, istate))
         |> commitResult

diff --git a/src/Compiler/Service/FSharpParseFileResults.fs b/src/Compiler/Service/FSharpParseFileResults.fs
@@ -96,7 +96,7 @@ type FSharpParseFileResults(diagnostics: FSharpDiagnostic[], input: ParsedInput,
             match expr with
 
             // This lets us dive into subexpressions that may contain the binding we're after
-            | SynExpr.Sequential(_, _, expr1, expr2, _) ->
+            | SynExpr.Sequential(expr1 = expr1; expr2 = expr2) ->
                 if rangeContainsPos expr1.Range pos then
                     walkBinding expr1 workingRange
                 else
@@ -714,7 +714,7 @@ type FSharpParseFileResults(diagnostics: FSharpDiagnostic[], input: ParsedInput,
                             yield! walkFinallySeqPt spFinally
 
                         | SynExpr.SequentialOrImplicitYield(spSeq, e1, e2, _, _)
-                        | SynExpr.Sequential(spSeq, _, e1, e2, _) ->
+                        | SynExpr.Sequential(debugPoint = spSeq; expr1 = e1; expr2 = e2) ->
                             let implicit1 =
                                 match spSeq with
                                 | DebugPointAtSequential.SuppressExpr

diff --git a/src/Compiler/Service/ServiceParsedInputOps.fs b/src/Compiler/Service/ServiceParsedInputOps.fs
@@ -263,8 +263,8 @@ module ParsedInput =
 
         let rec collect expr acc =
             match expr with
-            | SynExpr.Sequential(_, _, e1, (SynExpr.Sequential _ as e2), _) -> collect e2 (e1 :: acc)
-            | SynExpr.Sequential(_, _, e1, e2, _) -> e2 :: e1 :: acc
+            | SynExpr.Sequential(expr1 = e1; expr2 = (SynExpr.Sequential _ as e2)) -> collect e2 (e1 :: acc)
+            | SynExpr.Sequential(expr1 = e1; expr2 = e2) -> e2 :: e1 :: acc
             | _ -> acc
 
         match collect expr [] with

diff --git a/src/Compiler/Service/ServiceStructure.fs b/src/Compiler/Service/ServiceStructure.fs
@@ -337,7 +337,7 @@ module Structure =
                 parseExpr argExpr
                 parseExpr funcExpr
 
-            | SynExpr.Sequential(_, _, e1, e2, _) ->
+            | SynExpr.Sequential(expr1 = e1; expr2 = e2) ->
                 parseExpr e1
                 parseExpr e2
 

diff --git a/src/Compiler/SyntaxTree/SyntaxTree.fs b/src/Compiler/SyntaxTree/SyntaxTree.fs
@@ -630,7 +630,13 @@ type SynExpr =
 
     | Lazy of expr: SynExpr * range: range
 
-    | Sequential of debugPoint: DebugPointAtSequential * isTrueSeq: bool * expr1: SynExpr * expr2: SynExpr * range: range
+    | Sequential of
+        debugPoint: DebugPointAtSequential *
+        isTrueSeq: bool *
+        expr1: SynExpr *
+        expr2: SynExpr *
+        range: range *
+        trivia: SynExprSequentialTrivia
 
     | IfThenElse of
         ifExpr: SynExpr *
@@ -835,9 +841,9 @@ type SynExpr =
         match e with
         // these are better than just .Range, and also commonly applicable inside queries
         | SynExpr.Paren(_, m, _, _) -> m
-        | SynExpr.Sequential(_, _, e1, _, _)
-        | SynExpr.SequentialOrImplicitYield(_, e1, _, _, _)
-        | SynExpr.App(_, _, e1, _, _) -> e1.RangeOfFirstPortion
+        | SynExpr.Sequential(expr1 = e1)
+        | SynExpr.SequentialOrImplicitYield(expr1 = e1)
+        | SynExpr.App(funcExpr = e1) -> e1.RangeOfFirstPortion
         | SynExpr.ForEach(pat = pat; range = r) ->
             let e = (pat.Range: range).Start
             withEnd e r

diff --git a/src/Compiler/SyntaxTree/SyntaxTree.fsi b/src/Compiler/SyntaxTree/SyntaxTree.fsi
@@ -754,7 +754,8 @@ type SynExpr =
         isTrueSeq: bool *
         expr1: SynExpr *
         expr2: SynExpr *
-        range: range
+        range: range *
+        trivia: SynExprSequentialTrivia
 
     /// F# syntax: if expr then expr
     /// F# syntax: if expr then expr else expr

diff --git a/src/Compiler/SyntaxTree/SyntaxTreeOps.fs b/src/Compiler/SyntaxTree/SyntaxTreeOps.fs
@@ -942,7 +942,7 @@ let rec synExprContainsError inpExpr =
 
         | SynExpr.TryFinally(tryExpr = e1; finallyExpr = e2) -> walkExpr e1 || walkExpr e2
 
-        | SynExpr.Sequential(_, _, e1, e2, _) -> walkExpr e1 || walkExpr e2
+        | SynExpr.Sequential(expr1 = e1; expr2 = e2) -> walkExpr e1 || walkExpr e2
 
         | SynExpr.SequentialOrImplicitYield(_, e1, e2, _, _) -> walkExpr e1 || walkExpr e2
 

diff --git a/src/Compiler/SyntaxTree/SyntaxTrivia.fs b/src/Compiler/SyntaxTree/SyntaxTrivia.fs
@@ -115,6 +115,14 @@ type SynExprMatchBangTrivia =
 [<NoEquality; NoComparison>]
 type SynExprAnonRecdTrivia = { OpeningBraceRange: range }
 
+[<NoEquality; NoComparison>]
+type SynExprSequentialTrivia =
+    {
+        SeparatorRange: range option
+    }
+
+    static member val Zero = { SeparatorRange = None }
+
 [<NoEquality; NoComparison>]
 type SynMatchClauseTrivia =
     {

diff --git a/src/Compiler/SyntaxTree/SyntaxTrivia.fsi b/src/Compiler/SyntaxTree/SyntaxTrivia.fsi
@@ -175,6 +175,17 @@ type SynExprAnonRecdTrivia =
         OpeningBraceRange: range
     }
 
+/// Represents additional information for SynExpr.Sequential
+[<NoEquality; NoComparison>]
+type SynExprSequentialTrivia =
+    {
+        /// The syntax range of the `;` token.
+        /// Could also be the `then` keyword.
+        SeparatorRange: range option
+    }
+
+    static member Zero: SynExprSequentialTrivia
+
 /// Represents additional information for SynMatchClause
 [<NoEquality; NoComparison>]
 type SynMatchClauseTrivia =

diff --git a/src/Compiler/pars.fsy b/src/Compiler/pars.fsy
@@ -3895,7 +3895,8 @@ typedSequentialExprEOF:
 
 sequentialExpr:
   | declExpr seps sequentialExpr
-      { SynExpr.Sequential(DebugPointAtSequential.SuppressNeither, true, $1, $3, unionRanges $1.Range $3.Range) }
+      { let trivia = { SeparatorRange = $2 }
+        SynExpr.Sequential(DebugPointAtSequential.SuppressNeither, true, $1, $3, unionRanges $1.Range $3.Range, trivia) }
 
   | declExpr seps
       { $1 }
@@ -3904,10 +3905,12 @@ sequentialExpr:
       { $1 }
 
   | declExpr THEN sequentialExpr %prec prec_then_before
-      { SynExpr.Sequential(DebugPointAtSequential.SuppressNeither, false, $1, $3, unionRanges $1.Range $3.Range) }
+      { let trivia = { SeparatorRange = Some (rhs parseState 2) }
+        SynExpr.Sequential(DebugPointAtSequential.SuppressNeither, false, $1, $3, unionRanges $1.Range $3.Range, trivia) }
 
   | declExpr OTHEN OBLOCKBEGIN typedSequentialExpr oblockend %prec prec_then_before
-      { SynExpr.Sequential(DebugPointAtSequential.SuppressNeither, false, $1, $4, unionRanges $1.Range $4.Range) }
+      { let trivia = { SeparatorRange = Some (rhs parseState 2) }
+        SynExpr.Sequential(DebugPointAtSequential.SuppressNeither, false, $1, $4, unionRanges $1.Range $4.Range, trivia) }
 
   | hardwhiteLetBindings %prec prec_args_error
      { let hwlb, m, mIn = $1
@@ -6658,10 +6661,10 @@ opt_topSeparators:
 
 /* Seprators in either #light or non-#light */
 seps:
-  | OBLOCKSEP { }
-  | SEMICOLON { }
-  | OBLOCKSEP SEMICOLON { }
-  | SEMICOLON OBLOCKSEP { }
+  | OBLOCKSEP { None }
+  | SEMICOLON { Some (rhs parseState 1) }
+  | OBLOCKSEP SEMICOLON { Some (rhs parseState 2) }
+  | SEMICOLON OBLOCKSEP { Some (rhs parseState 1) }
 
 /* An 'end' that's optional only in #light, where an ODECLEND gets inserted, and explicit 'end's get converted to OEND */
 declEnd: