bugfix/issue-2485-sequentials-stack-overflow (#2495)

* * Switches Sequentials to an imperative impl for efficiency and to not blow up stack * * Further simplifications * * Switches to tail recursion * Adds unit-test * * Adds note on #2485 to changelog * Add release notes for beta 10. Co-authored-by: Florian Verdonck <[email protected]>
fsprojects · Sep 10, 2022 · e1bd963 · e1bd963
1 parent 1d1cda7
commit e1bd963
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,17 +1,18 @@
 # Changelog
 
-## [Unreleased]
+## [5.0.0-beta-010] - 2022-09-10
 
 ### Fixed
-* Record definition with accessibility modifier error using stroustrup style [#2481](https://github.com/fsprojects/fantomas/issues/2481)
+* Record definition with accessibility modifier error using stroustrup style. [#2481](https://github.com/fsprojects/fantomas/issues/2481)
+* Comments between tuple type are lost. [#2149](https://github.com/fsprojects/fantomas/issues/2149)
+* Stack overflow on macOS with a big source-file. [#2485](https://github.com/fsprojects/fantomas/issues/2485)
 
 ## [5.0.0-beta-009] - 2022-09-02
 
 ### Fixed
 * --check should fail if the number of newlines is different. [#2461](https://github.com/fsprojects/fantomas/issues/2461)
 * Comment after closing brace of computation expression is lost. [#2466](https://github.com/fsprojects/fantomas/issues/2466)
 * Match statements should not be split into separate lines between parentheses. [#2044](https://github.com/fsprojects/fantomas/issues/2044)
-* Comments between tuple type are lost #2149. [#2149](https://github.com/fsprojects/fantomas/issues/2149)
 
 ## [5.0.0-beta-008] - 2022-08-30
 
@@ -44,7 +45,7 @@
 
 ## [5.0.0-beta-005] - 2022-08-03
 
-### Changed 
+### Changed
 * Update FCS to 'SynType with single slash and type', commit e1f735546a907b0a511b818f10829b1d75139021
 
 ### Fixed

diff --git a/src/Fantomas.Core.Tests/ASTTransformerTests.fs b/src/Fantomas.Core.Tests/ASTTransformerTests.fs
@@ -0,0 +1,56 @@
+module Fantomas.Core.Tests.ASTTransformerTests
+
+#if RELEASE
+open NUnit.Framework
+open FsUnit
+open FSharp.Compiler.Text
+open FSharp.Compiler.Xml
+open FSharp.Compiler.Syntax
+open FSharp.Compiler.SyntaxTrivia
+
+[<Test>]
+let ``avoid stack-overflow in long array/list, 2485`` () =
+    let testIdent = Ident("Test", Range.Zero)
+
+    let mkStringExpr () =
+        SynExpr.Const(
+            SynConst.String((System.Guid.NewGuid().ToString("N"), SynStringKind.Regular, Range.Zero)),
+            Range.Zero
+        )
+
+    let longArrayExpr: SynExpr =
+        let rec mkArray count childExpr =
+            if count = 20_000 then
+                childExpr
+            else
+                mkArray
+                    (count + 1)
+                    (SynExpr.Sequential(
+                        DebugPointAtSequential.SuppressNeither,
+                        true,
+                        mkStringExpr (),
+                        childExpr,
+                        Range.Zero
+                    ))
+
+        SynExpr.ArrayOrListComputed(true, mkArray 0 (mkStringExpr ()), Range.Zero)
+
+    let rootNode =
+        Fantomas.Core.AstTransformer.astToNode
+            Range.Zero
+            []
+            [ SynModuleOrNamespace(
+                  [ testIdent ],
+                  false,
+                  SynModuleOrNamespaceKind.AnonModule,
+                  [ SynModuleDecl.Expr(longArrayExpr, Range.Zero) ],
+                  PreXmlDoc.Empty,
+                  [],
+                  None,
+                  Range.Zero,
+                  { ModuleKeyword = None
+                    NamespaceKeyword = None }
+              ) ]
+
+    Assert.Pass()
+#endif
diff --git a/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj b/src/Fantomas.Core.Tests/Fantomas.Core.Tests.fsproj
@@ -114,6 +114,7 @@
     <Compile Include="AstExtensionsTests.fs" />
     <Compile Include="MaxIfThenShortWidthTests.fs" />
     <Compile Include="CodePrinterHelperFunctionsTests.fs" />
+    <Compile Include="ASTTransformerTests.fs" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\Fantomas.Core\Fantomas.Core.fsproj" />

diff --git a/src/Fantomas.Core/SourceParser.fs b/src/Fantomas.Core/SourceParser.fs
@@ -608,10 +608,16 @@ let (|Sequential|_|) =
     | SynExpr.Sequential (_, isSeq, e1, e2, _) -> Some(e1, e2, isSeq)
     | _ -> None
 
-let rec (|Sequentials|_|) =
-    function
-    | Sequential (e, Sequentials es, _) -> Some(e :: es)
-    | Sequential (e1, e2, _) -> Some [ e1; e2 ]
+let (|Sequentials|_|) e =
+    let rec visit (e: SynExpr) (finalContinuation: SynExpr list -> SynExpr list) : SynExpr list =
+        match e with
+        | Sequential (e1, e2, _) -> visit e2 (fun xs -> e1 :: xs |> finalContinuation)
+        | e -> finalContinuation [ e ]
+
+    match e with
+    | Sequential (e1, e2, _) ->
+        let xs = visit e2 id
+        Some(e1 :: xs)
     | _ -> None
 
 let (|SimpleExpr|_|) =