Skip to content

[Parse][CodeCompletion] Don't special case code completion when forming single-expression closures/function bodies (NFC) #34253

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

nathawes
Copy link
Contributor

@nathawes nathawes commented Oct 9, 2020

Code completion used to avoid forming single expression closures/function bodies when the single expression contained the code completion expression because a contextual type mismatch could result in types not being applied to the AST, giving no completions.

Completions that have been migrated to the new solver-based completion mechanism don't need this behavior, however. Rather than trying to guess whether the type of completion we're going to end up performing is one of the ones that haven't been migrated to the solver yet when parsing, instead just always form single-expression closures/function bodies (like we do for regular compilation) and undo the transformation if and when we know we're going to perform a completion kind we haven't migrated yet.

Once all completion kinds are migrated, the undo-ing code can be removed.

@nathawes nathawes requested a review from rintaro October 9, 2020 21:11
@nathawes
Copy link
Contributor Author

nathawes commented Oct 9, 2020

@swift-ci please test

Comment on lines 6110 to 6116
SourceRange Range = RS->getSourceRange();
if (Range.isInvalid())
return false;

if (!SM.rangeContainsCodeCompletionLoc(Range))
return false;
Copy link
Member

Choose a reason for hiding this comment

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

It's not clear why we need to compare the range. We know completion is happening inside this DC, right? Are these return false actually reachable? If so, could you give me some examples?

Also, if we really need to check it, RS->getResult()->getSourceRange() is more relevant I think.

Copy link
Contributor Author

@nathawes nathawes Oct 9, 2020

Choose a reason for hiding this comment

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

This was just me attempting to match the Status.hasCompletion() check in the old logic, but you're right - we already know that's true here. I'll remove it.

@nathawes nathawes force-pushed the nfc-always-form-single-expression-return-and-undo-if-needed branch from c99be57 to acdcac4 Compare October 9, 2020 22:24
…ng single-expression closures/function bodies (NFC)

Code completion used to avoid forming single expression closures/function
bodies when the single expression contained the code completion expression
because a contextual type mismatch could result in types not being applied
to the AST, giving no completions.

Completions that have been migrated to the new solver-based completion
mechanism don't need this behavior, however. Rather than trying to guess
whether the type of completion we're going to end up performing is one of
the ones that haven't been migrated to the solver yet when parsing, instead
just always form single-expression closures/function bodies (like we do for
regular compilation) and undo the transformation if and when we know we're
going to perform a completion kind we haven't migrated yet.

Once all completion kinds are migrated, the undo-ing code can be removed.
@nathawes nathawes force-pushed the nfc-always-form-single-expression-return-and-undo-if-needed branch from acdcac4 to a91cede Compare October 9, 2020 23:02
@nathawes
Copy link
Contributor Author

nathawes commented Oct 9, 2020

@swift-ci please test

@nathawes nathawes requested a review from rintaro October 9, 2020 23:03
Copy link
Member

@rintaro rintaro left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@nathawes nathawes merged commit cf34fa5 into swiftlang:main Oct 10, 2020
@nathawes nathawes deleted the nfc-always-form-single-expression-return-and-undo-if-needed branch October 10, 2020 04:56
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