Switch to iterative algorithm to prevent stack overflows.

[CyrusNajmabadi]

Fixes #40041

[CyrusNajmabadi]

Tagging @sharwell

[CyrusNajmabadi]

Note: i'd recommend reviewing this a commit at a time.

[CyrusNajmabadi]

i preserved this logic (just in O(1) time instead of O(n)). But, overall, i think preserving this logic is pretty unnecessary. i don't see why we actually need it.

[CyrusNajmabadi]

i preserved this logic (just in O(1) time instead of O(n)). But, overall, i think preserving this logic is pretty unnecessary. i don't see why we actually need it.

[AdamSpeight2008]

It highlights the entire async code section rather than the highlighting the nested inside keywords Async and Await.
Personally i prefer the latter.

[CyrusNajmabadi]

i preserved this logic. But i dont' actually think it's necessary. this basically ensures that we only highlight await in await using var whatever and not in await var whatever. However, the latter is broken code, so it would be fine IMO to highlight there.

[CyrusNajmabadi]

i preserved this logic. But i dont' actually think it's necessary. this basically ensures that we only highlight await in await using var whatever and not in await var whatever. However, the latter is broken code, so it would be fine IMO to highlight there.

[AdamSpeight2008]

The would also need applying to the vb side as well.

[CyrusNajmabadi]

@AdamSpeight2008 my goal is not to harden the entirely of the IDE codebase to stack overflows.

[CyrusNajmabadi]

@sharwell LMK what you need here to proceed.

[CyrusNajmabadi]

@sharwell @jinujoseph LMK what you need here to proceed.

[sharwell]

@CyrusNajmabadi What is the observable impact of e55e787? There are disadvantages to both the before and after code and understanding the impact will help with the decision:

• Before code: disadvantage of changing the behavior of the algorithm. A few more nodes will be visited than before.
• After code: disadvantage of pushing the either tree of nodes into a stack, which is likely to frequently be very large (possibly prohibitively so)

[CyrusNajmabadi]

@CyrusNajmabadi What is the observable impact of e55e787? There are disadvantages to both the before and after code and understanding the impact will help with the decision:

It was actually incorrect. i.e. it didn't highlight things properly. this was because it dove into nodes it shoudln't (i.e. an async lambda inside an async lambda).

After code: disadvantage of pushing the either tree of nodes into a stack, which is likely to frequently be very large (possibly prohibitively so)

Note: thsi was the same in both cases. calling DescendantNodesAndSelf works internally using an explicit (pooled) stack. You always pay this cost.

[sharwell]

Note: thsi was the same in both cases. calling DescendantNodesAndSelf works internally using an explicit (pooled) stack. You always pay this cost.

The part I missed was the use of yield return between pushing things to the stack. I was looking at it as pushing all nodes in the tree to the stack before starting to visit the children. Thanks for the clarification.

[CyrusNajmabadi]

Thanks for the clarification.

Absolutely!

[CyrusNajmabadi]

@sharwell good to merge this in?

[CyrusNajmabadi]

@sharwell everything has passed. merge when you can, thanks!

[CyrusNajmabadi]

Thanks!