-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[AST] Don't suggest unusable values for typo correction and code completion #20904
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
[AST] Don't suggest unusable values for typo correction and code completion #20904
Conversation
@swift-ci Please smoke test |
Supersedes #20677. @benlangmuir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than my comment about naming, LGTM.
lib/AST/LookupVisibleDecls.cpp
Outdated
SourceLoc Loc; | ||
VisibleDeclConsumer &Consumer; | ||
|
||
bool checkLocalValue(ValueDecl *VD, DeclVisibilityKind Reason) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename this to something that makes it clear what true
and false
return values mean - e.g. isUsableLocalValue
.
lib/AST/LookupVisibleDecls.cpp
Outdated
|
||
switch (Reason) { | ||
case DeclVisibilityKind::LocalVariable: | ||
// Use 'TypeDecl's before declaration is allowed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: Use of
@DougGregor may also have opinions here from his work on ASTScope. (Specifically, lookupVisibleDecls isn't supposed to produce these invalid values in the first place.) |
d417025
to
1f17088
Compare
…letion Filter out these cases: - use within its own initial value - use before declaration for local values rdar://problem/25068938
1f17088
to
a54d2ff
Compare
@swift-ci Please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good direction. It's what ASTScope
would naturally do, but we don't want to wait for ASTScope
.
Filter out:
rdar://problem/25068938