Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't blow the stack when traversing deeply nested sequential expressions #16882

Merged
merged 9 commits into from
Mar 20, 2024
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/8.0.300.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
### Fixed

* Don't blow the stack when traversing deeply nested sequential expressions. ([PR #16882](https://github.com/dotnet/fsharp/pull/16882))
* 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))
* Fix missing warning for recursive calls in list comprehensions. ([PR #16652](https://github.com/dotnet/fsharp/pull/16652))
* 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))
33 changes: 32 additions & 1 deletion src/Compiler/Service/ServiceParseTreeWalk.fs
Original file line number Diff line number Diff line change
@@ -379,6 +379,31 @@ module SyntaxTraversal =
and traverseSynExpr origPath (expr: SynExpr) =
let pick = pick expr.Range

/// Sequential expressions are more likely than
/// most other expression kinds to be deeply nested,
/// e.g., in very large list or array expressions.
/// We treat them specially to avoid blowing the stack,
/// since traverseSynExpr itself is not tail-recursive.
let rec traverseSequentials path expr =
psfinaki marked this conversation as resolved.
Show resolved Hide resolved
brianrourkeboll marked this conversation as resolved.
Show resolved Hide resolved
seq {
brianrourkeboll marked this conversation as resolved.
Show resolved Hide resolved
match expr with
| SynExpr.Sequential(expr1 = expr1; expr2 = SynExpr.Sequential _ as expr2) ->
// It's a nested sequential expression.
// Visit it, but make defaultTraverse do nothing,
// since we're going to traverse its descendants ourselves.
yield dive expr expr.Range (fun expr -> visitor.VisitExpr(path, traverseSynExpr path, (fun _ -> None), expr))

// Now traverse its descendants.
let path = SyntaxNode.SynExpr expr :: path
yield dive expr1 expr1.Range (traverseSynExpr path)
yield! traverseSequentials path expr2

| _ ->
// It's not a nested sequential expression.
// Traverse it normally.
yield dive expr expr.Range (traverseSynExpr path)
}

let defaultTraverse e =
let path = SyntaxNode.SynExpr e :: origPath
let traverseSynExpr = traverseSynExpr path
@@ -680,11 +705,17 @@ module SyntaxTraversal =
]
|> pick expr

| SynExpr.Sequential(expr1 = synExpr1; expr2 = synExpr2) ->
[
dive synExpr1 synExpr1.Range traverseSynExpr
yield! traverseSequentials path synExpr2
]
|> pick expr

| SynExpr.Set(targetExpr = synExpr1; rhsExpr = synExpr2)
| SynExpr.DotSet(targetExpr = synExpr1; rhsExpr = synExpr2)
| SynExpr.TryFinally(tryExpr = synExpr1; finallyExpr = synExpr2)
| SynExpr.SequentialOrImplicitYield(expr1 = synExpr1; expr2 = synExpr2)
| SynExpr.Sequential(expr1 = synExpr1; expr2 = synExpr2)
| SynExpr.While(whileExpr = synExpr1; doExpr = synExpr2)
| SynExpr.WhileBang(whileExpr = synExpr1; doExpr = synExpr2)
| SynExpr.DotIndexedGet(objectExpr = synExpr1; indexArgs = synExpr2)
Loading