-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[CodeCompletion] Add 'IsSystem' flag to code completion result item #31646
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
[CodeCompletion] Add 'IsSystem' flag to code completion result item #31646
Conversation
|
@swift-ci Please test |
include/swift/IDE/CodeCompletion.h
Outdated
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.
I think 63 is still more than enough.
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.
@benlangmuir WDYT?
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.
Why reduce it? Isn't the bitfield within 32 bits either way?
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.
As per offline discussion, we decided to keep it 127.
|
Build failed |
|
Build failed |
dd65ee2 to
92bde31
Compare
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
92bde31 to
f453c12
Compare
'key.is_system: 1' is added if the associated declaration is from a system module. rdar://problem/62617558
f453c12 to
75a0c9f
Compare
|
@swift-ci Please smoke test |
| Builder.setNumBytesToErase(dist); | ||
| Builder.addOverrideKeyword(); | ||
| Builder.addDeclIntroducer(DeclStr.str().substr(0, NameOffset)); | ||
| } |
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.
@akyrtzi There was no MaxNumBytesToErase check for override completion.
If the dist exceed MaxNumBytesToErase, completion just gives up adding override keyword before the introducer (e.g. func, var).
include/swift/IDE/CodeCompletion.h
Outdated
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.
Why reduce it? Isn't the bitfield within 32 bits either way?
| // CHECK: { | ||
| // CHECK: key.kind: source.lang.swift.decl.function.method.instance, | ||
| // CHECK: key.name: "makeIterator()", | ||
| // CHECK-NEXT: key.sourcetext: "makeIterator()", |
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.
Do we really care about testing all these fields here? Can we just check is_system specifically?
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.
Is there a way to check that the line is in the same item (i.e. before }, line )?
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.
It's not a great solution, but
CHECK: key.name: "blah blah blah"
CHECK-NOT: key.name
CHECK: key.is_system: 1
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.
Cool, that actually works for is_system: 1 checks. I still haven't found a way to check the item does not have is_system: 1 line.
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.
// CHECK: key.name: "makeIterator()"
// CHECK-NOT: },
// CHECK: key.is_system: 1
// CHECK: },
// CHECK: key.name: "startIndex"
// CHECK-NOT: },
// CHECK-NOT: key.is_system: 1
// CHECK: },
This seems to work.
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.
Only danger is that we could add a dictionary in the middle of the response some day, but not sure there is a better solution right now.
Only check lines we really care about.
|
@swift-ci Please smoke test |
|
@swift-ci Please smoke test macOS |
key.is_system: 1is added if the associated declaration is from a system module.rdar://problem/62617558
(This will conflict with #31637, I will rebase it after merging this)