Skip to content

Commit dff5eba

Browse files
Don't blow the stack when traversing deeply nested sequential expressions (#16882)
* Don't blow stack when traversing deep sequentials * Sequential expressions are more likely than most other expression kinds to be deeply nested, e.g., in very large list or array expressions. Since `traverseSynExpr` is not tail-recursive, we must treat them specially to avoid blowing the stack. * Update release notes * Only when actually nested * Update comments * Only alloc seq when needed * Add very big array test for AST traversal --------- Co-authored-by: Vlad Zarytovskii <[email protected]>
1 parent e4e709f commit dff5eba

File tree

5 files changed

+5061
-2
lines changed

5 files changed

+5061
-2
lines changed

docs/release-notes/.FSharp.Compiler.Service/8.0.300.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
### Fixed
22

3+
* Don't blow the stack when traversing deeply nested sequential expressions. ([PR #16882](https://github.com/dotnet/fsharp/pull/16882))
34
* Fix wrong range start of INTERP_STRING_END. ([PR #16774](https://github.com/dotnet/fsharp/pull/16774), [PR #16785](https://github.com/dotnet/fsharp/pull/16785))
45
* Fix missing warning for recursive calls in list comprehensions. ([PR #16652](https://github.com/dotnet/fsharp/pull/16652))
56
* Code generated files with > 64K methods and generated symbols crash when loaded. Use infered sequence points for debugging. ([Issue #16399](https://github.com/dotnet/fsharp/issues/16399), [#PR 16514](https://github.com/dotnet/fsharp/pull/16514))

src/Compiler/Service/ServiceParseTreeWalk.fs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,31 @@ module SyntaxTraversal =
379379
and traverseSynExpr origPath (expr: SynExpr) =
380380
let pick = pick expr.Range
381381

382+
/// Sequential expressions are more likely than
383+
/// most other expression kinds to be deeply nested,
384+
/// e.g., in very large list or array expressions.
385+
/// We treat them specially to avoid blowing the stack,
386+
/// since traverseSynExpr itself is not tail-recursive.
387+
let rec traverseSequentials path expr =
388+
seq {
389+
match expr with
390+
| SynExpr.Sequential(expr1 = expr1; expr2 = SynExpr.Sequential _ as expr2) ->
391+
// It's a nested sequential expression.
392+
// Visit it, but make defaultTraverse do nothing,
393+
// since we're going to traverse its descendants ourselves.
394+
yield dive expr expr.Range (fun expr -> visitor.VisitExpr(path, traverseSynExpr path, (fun _ -> None), expr))
395+
396+
// Now traverse its descendants.
397+
let path = SyntaxNode.SynExpr expr :: path
398+
yield dive expr1 expr1.Range (traverseSynExpr path)
399+
yield! traverseSequentials path expr2
400+
401+
| _ ->
402+
// It's not a nested sequential expression.
403+
// Traverse it normally.
404+
yield dive expr expr.Range (traverseSynExpr path)
405+
}
406+
382407
let defaultTraverse e =
383408
let path = SyntaxNode.SynExpr e :: origPath
384409
let traverseSynExpr = traverseSynExpr path
@@ -680,11 +705,19 @@ module SyntaxTraversal =
680705
]
681706
|> pick expr
682707

708+
// Nested sequentials.
709+
| SynExpr.Sequential(expr1 = synExpr1; expr2 = synExpr2 & SynExpr.Sequential _) ->
710+
[
711+
dive synExpr1 synExpr1.Range traverseSynExpr
712+
yield! traverseSequentials path synExpr2
713+
]
714+
|> pick expr
715+
716+
| SynExpr.Sequential(expr1 = synExpr1; expr2 = synExpr2)
683717
| SynExpr.Set(targetExpr = synExpr1; rhsExpr = synExpr2)
684718
| SynExpr.DotSet(targetExpr = synExpr1; rhsExpr = synExpr2)
685719
| SynExpr.TryFinally(tryExpr = synExpr1; finallyExpr = synExpr2)
686720
| SynExpr.SequentialOrImplicitYield(expr1 = synExpr1; expr2 = synExpr2)
687-
| SynExpr.Sequential(expr1 = synExpr1; expr2 = synExpr2)
688721
| SynExpr.While(whileExpr = synExpr1; doExpr = synExpr2)
689722
| SynExpr.WhileBang(whileExpr = synExpr1; doExpr = synExpr2)
690723
| SynExpr.DotIndexedGet(objectExpr = synExpr1; indexArgs = synExpr2)

tests/FSharp.Compiler.UnitTests/FSharp.Compiler.UnitTests.fsproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@
7575
</Compile>
7676
<Compile Include="TreeVisitorTests.fs" />
7777
<Compile Include="ParsedInputModuleTests.fs" />
78+
<Compile Include="ParsedInputModuleTests.VeryBigArrayExprTest.fs" />
7879
</ItemGroup>
7980

8081
<ItemGroup>

0 commit comments

Comments
 (0)