Skip to content

[Parse/CodeCompletion] Cleanup code completion facilities in Parse #28182

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
Nov 13, 2019

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Nov 11, 2019

  • Remove unused FunctionBodyState from PersistentParserState
  • Remove delayParseFromBeginningToHere(). Use delayDecl() instead
  • Inline PersistentParserState::delayTopLevel()
  • Cleanup completion handling in parseDecl()
  • Rename code completion related names in PersistentParserState so it's clear when you are using
  • Refactor performCodeCompletionSecondPass(): Inline and consolidate parse*Delayed() functions because they used to share some code

rdar://problem/56926367

@rintaro
Copy link
Member Author

rintaro commented Nov 11, 2019

@swift-ci Please smoke test

@rintaro rintaro requested a review from benlangmuir November 12, 2019 08:04
@rintaro
Copy link
Member Author

rintaro commented Nov 12, 2019

This is a PR for #28014 (review)
@benlangmuir I ended up with refactoring the facilities. What do you think?

auto EndLexerState = L->getStateForEndOfTokenLoc(info.BodyEnd);
auto range = SourceRange(info.BodyPos.Loc, info.BodyEnd);
llvm::errs().write_escaped(
L->getCharSourceRangeFromSourceRange(SourceMgr, range).str());
Copy link
Contributor

Choose a reason for hiding this comment

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

Debugging output?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, thanks!

// Expressions can't begin with a closure literal at statement position. This
// prevents potential ambiguities with trailing closure syntax.
if (Tok.is(tok::l_brace)) {
diagnose(Tok, diag::statement_begins_with_closure);
Copy link
Contributor

Choose a reason for hiding this comment

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

I see you removed this code - do we know why it was originally there? It seems odd to emit a diagnostic during code completion like this.

Copy link
Member Author

@rintaro rintaro Nov 12, 2019

Choose a reason for hiding this comment

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

I guess this was added when diag::statement_begins_with_closure was implemented (ad78204) without knowing how this code path was used.

};

enum class DelayedDeclKind {
enum class CodeCompletionSecondPassInfoKind {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about CodeCompletionDelayedDeclKind?

TopLevelCodeDecl,
Decl,
FunctionBody,
};

class DelayedDeclState {
class CodeCompletionSecondPassInfoTy {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about CodeCompletionDelayedDeclState?

FunctionBodyState is no longer used.
- delayParseFromBeginningToHere is useless. Use delayDecl() instead
- Inline PersistentParserState::delayTopLevel()
- Cleanup completion handling in parseDecl()
- Rename code completion related names in 'PersistentParserState' so
  it's clear when you are using.
- Refactor 'performCodeCompletionSecondPass()': Inline and consolidate
  'parse*Delayed()' because they used to share many code.

rdar://problem/56926367
@rintaro rintaro force-pushed the ide-completion-rdar56926367 branch from 20b7d0e to cd8ebe4 Compare November 12, 2019 18:21
@rintaro
Copy link
Member Author

rintaro commented Nov 12, 2019

@swift-ci Please smoke test

@rintaro rintaro requested a review from benlangmuir November 12, 2019 19:10
Copy link
Contributor

@benlangmuir benlangmuir 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!

@rintaro rintaro merged commit 63c1f84 into swiftlang:master Nov 13, 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.

2 participants