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

[SourceKit] Expand editor placeholder to trailing closures inside a TryExpr #65376

Merged

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Apr 22, 2023

rdar://108391690

@ahoppen
Copy link
Member Author

ahoppen commented Apr 22, 2023

@swift-ci Please smoke test

@@ -1658,7 +1658,14 @@ class PlaceholderExpansionScanner {
auto SR = E->getSourceRange();
if (SR.isValid() && SM.rangeContainsTokenLoc(SR, TargetLoc) &&
!checkCallExpr(E) && !EnclosingCallAndArg.first) {
OuterExpr = E;
if (!isa<TryExpr>(E) && !isa<AwaitExpr>(E) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

getSemanticsProvidingExpr covers all IdentityExpr. FWIW you probably want to reset OuterExpr in these cases as well, ie. even with this change something like the following would fail:

bar(try foo(x: <#T##() -> Void#>))

And I haven't thought too hard about other cases, but TBH it doesn't really matter, we'll replace this with the swift-syntax one soon :).

Copy link
Member Author

@ahoppen ahoppen Apr 22, 2023

Choose a reason for hiding this comment

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

I did not want to use getSemanticsProvidingExpr because IIRC that includes e.g. .self. And IIUC we don’t want to expand

foo(<#T##() -> Void#>).self

to

foo { <#code#> }.self

Or at least that’s what I understood the purpose of OuterExpr is.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be fine with .self. I'm not really sure I agree with what we're trying to prevent here anyway. Ie. the comment says that outer(inner(<#closure#>)) shouldn't expand to trailing closure for inner. But that also seems fine to me 🤷. Certainly the swift-syntax replacement doesn't currently care about that. So something we can talk about later.

Like I said, I'm perfectly happy with what you have now.

@ahoppen ahoppen merged commit 0987156 into swiftlang:main Apr 22, 2023
@ahoppen ahoppen deleted the ahoppen/expand-to-trailing-closure-in-try branch April 22, 2023 04:16
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.

2 participants