Tracking issue for stack overflow exception in devenv and OOP

[Cosifne]

Using sample code:
LongClass.txt

Many places using recursion would fail in this case.

• AddExpressionsAndOperators
• SyntaxWalker
• OperationWalker
  Updates: checked in two tests try to cover the scenario, but it is not stable enough in the CI environment

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[genlu]

We probably need to make sure we use the same pattern to guard against this kind of issues in IDE visitors/walkers

[genlu]

And we need to add an integration test with the sample code here.

[CyrusNajmabadi]

Generally speaking, we've found that there is always a limit to pathological cases and some part of the system will always fall over. So, we try to ensure we don't regress on existing code that works, but we don't use pathologically generated code (like what looks like that test file) as it's a never ending chase.

[Cosifne]

Generally speaking, we've found that there is always a limit to pathological cases and some part of the system will always fall over. So, we try to ensure we don't regress on existing code that works, but we don't use pathologically generated code (like what looks like that test file) as it's a never ending chase.

I feel if the recursion is easy enough to rewrite, we should do this like https://sourceroslyn.io/#Microsoft.CodeAnalysis.Features/Wrapping/BinaryExpression/AbstractBinaryExpressionWrapper.cs,f461f8abcebe1530,references

Other things like SyntaxWalker or SyntaxVisitor, yes it can't works for everything, but the question is, should caller of it try to handle the error (like the link @genlu posted, return an empty array) more gracefully?

[genlu]

@CyrusNajmabadi you are right. But I think @Cosifne's approach makes sense. For IDE features based on compiler visitors/walkers, we need to at least ensure they don't crash on deep recursions (probably by using EnsureSufficientExecutionStack pattern). But for simple recursions (like the one being fixed in #63338) it might be better to simply convert it to iteration.