Skip to content

[CodeCompletion] Enable fast-completion at the top of implicit getter #29789

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

Merged
merged 3 commits into from
Feb 13, 2020

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Feb 12, 2020

If a CC token is right after the '{' we still don't know it's an implicit getter or a start of a accessor block. Previously, the parser used to parse it as an accessor block, but it prevents fast-completion kicks in.

Instead handle it as a part of function body parsing so the fast-completion works.

rdar://problem/58851121

Groundwork:

  • Consolidate AbstructFunctionDecl body parsing functions
  • Add a convenient AccessorDecl::isImplictGetter() function

If a CC token is right after the '{' we still don't know it's an implicit
getter or a start of a accessor block. Previously, the parser used to
parse it as an accessor block, but it prevents fast-completion kicks in.

Instead handle it as a part of function body parsing so the
fast-completion works.

rdar://problem/58851121
@rintaro
Copy link
Member Author

rintaro commented Feb 12, 2020

@swift-ci Please smoke test

@rintaro rintaro requested a review from benlangmuir February 12, 2020 19:09
@rintaro rintaro changed the title [CodeCompletion] Enable fast-completion at the top of accessor decls [CodeCompletion] Enable fast-completion at the top of accessor blocks Feb 12, 2020
@rintaro rintaro changed the title [CodeCompletion] Enable fast-completion at the top of accessor blocks [CodeCompletion] Enable fast-completion at the top of implicit getter Feb 12, 2020
addParametersToScope(AFD->getParameters());

// Establish the new context.
ParseFunctionBody CC(*this, AFD);
Copy link
Contributor

Choose a reason for hiding this comment

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

This changes the lifetime of the body context. Is that correct? Is there any risk this will cause issues if someone modifies this in the future? The body is not "finished" here, since we have the transform below.

Copy link
Member Author

@rintaro rintaro Feb 13, 2020

Choose a reason for hiding this comment

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

The transformation below doesn't use CurDeclContext nor CurLocalContext. So this lifetime change should not affect it. Am I missing something?

Is there any risk this will cause issues if someone modifies this in the future?

In general, a parser developer should know what the current CurDeclContext. What is your concern exactly?

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, just checking.

@rintaro rintaro merged commit 63772d0 into swiftlang:master Feb 13, 2020
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