Skip to content

[AST] Always walk folded SequenceExpr if available #80242

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

Conversation

hamishknight
Copy link
Contributor

Expand the special-cased ASTWalker behavior for folded SequenceExprs such that we always walk the folded expression when available. This ensures that we don't attempt to add the same node multiple times when expanding ASTScopes during pre-checking.

rdar://147751795

@hamishknight
Copy link
Contributor Author

hamishknight commented Mar 24, 2025

Expand the special-cased ASTWalker behavior for folded SequenceExprs
such that we always walk the folded expression when available. This
ensures that we don't attempt to add the same node multiple times
when expanding ASTScopes during pre-checking.

rdar://147751795
@hamishknight hamishknight force-pushed the mutable-problems-require-mutable-solutions branch from c8c080b to 738c70e Compare March 24, 2025 20:18
@hamishknight
Copy link
Contributor Author

@swift-ci please test

if (!newExpr)
return nullptr;

E->setFoldedExpr(newExpr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, I suggested to Allan to do it this way in his original PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh interesting, @tshortli any objections to changing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to land for now

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No objections, I just wasn't interested in taking on extra risk by changing the behavior fundamentally for 6.1. If this seems like the right fix in general I'm happy to have it simplified.

@hamishknight
Copy link
Contributor Author

@swift-ci please test source compatibility Debug

@hamishknight hamishknight merged commit b719498 into swiftlang:main Mar 26, 2025
5 of 6 checks passed
@hamishknight hamishknight deleted the mutable-problems-require-mutable-solutions branch March 26, 2025 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants