Skip to content

[CodeCompletion] Stop using temporary Lexer in the second pass #28433

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
Dec 4, 2019

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Nov 22, 2019

  • Since we only call one parsing function (i.e. parseBraceItemList, parseDecl, or parseExprOrStmt), the parser stops at the end of the node anyways. It's not necessary to limit the Lexer to set an ArtificialEOF. As a ground work, modify the Parser to not parse the body if the completion happens in the signature
  • Don't restore the parser position after the second pass because it's just not necessary. The parser here lives only for the single second pass.
  • Use getResultInterfaceType() to get the result type in TypeCheckFunctionBodyUntilRequest. So we can type check the body without type checking the signature of the func decl. (This change is actually not needed but I think this is an improvement)

In `TypeCheckFunctionBodyUntilRequest`. So we can type check the body
without typechecking the signature.
Since we only call one parsing function (i.e. parseAbstructFunctionBody,
parseDecl, or parseStmtOrExpr), the parser stops at the end of the node.
It's not necessary to limit the Lexer to set an ArtificialEOF.

To minimize the parsing range, modify the Parser to *not* parse the body
if the completion happens in the signature.
This is just not necessary. This parser lives only for the single second
pass.
func->getResultInterfaceType()->isVoid()) {
// The function returns void. We don't need an explicit return, no matter
// what the type of the expression is. Take the inserted return back out.
body->setFirstElement(func->getSingleExpressionBody());
Copy link
Member Author

Choose a reason for hiding this comment

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

@DougGregor Do you have any concern about this change?

Copy link
Member

Choose a reason for hiding this comment

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

No, this seems like a clear improvement.

@@ -695,9 +695,6 @@ struct InitializerWithNameAndParam {
struct InitializerWithLabels {
init c d: Int {}
// expected-error @-1 {{expected '(' for initializer parameters}}
// expected-error @-2 {{expected declaration}}
// expected-error @-3 {{consecutive declarations on a line must be separated by ';'}}
// expected-note @-5 {{in declaration of 'InitializerWithLabels'}}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is because parseDeclInit now returns an error state. So the parser property skips to the next decl or '}'

@rintaro
Copy link
Member Author

rintaro commented Nov 22, 2019

@swift-ci Please smoke test

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

Nice cleanup!

func->getResultInterfaceType()->isVoid()) {
// The function returns void. We don't need an explicit return, no matter
// what the type of the expression is. Take the inserted return back out.
body->setFirstElement(func->getSingleExpressionBody());
Copy link
Member

Choose a reason for hiding this comment

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

No, this seems like a clear improvement.

@rintaro rintaro merged commit 83c1e7b into swiftlang:master Dec 4, 2019
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