Code-completion, indexing, cursor-info, etc. for KeyPath dynamic member lookup #24072
Conversation
Currently includes both the implicit references to the subscript(dynamicMember:) and the explicit references to the underlying property/subscript declarations. rdar://49028783
When building the implicit subscript expression, set the "implicit" bit correctly and pass it through in the indexer so that we get implicit refernces to the subscript. This would be useful for e.g. searching for all uses of the dynamic subscript.
Ensure the various entity walkers handle the implicit subscript reference correctly (usually by ignoring it) and fall through to the underlying declarations. rdar://49028895
Looks into the root type of the keypath to find additional members. This does not currently map the type of the completion to the subscript's return type. rdar://49029126
|
@swift-ci please smoke test |
lib/Sema/LookupVisibleDecls.cpp
Outdated
|
|
||
| auto subs = | ||
| baseType->getMemberSubstitutionMap(dc->getParentModule(), subscript); | ||
| if (auto memberType = rootType->subst(subs)) |
There was a problem hiding this comment.
if (memberType->mayHaveMembers()). Just in case.
lib/Sema/LookupVisibleDecls.cpp
Outdated
| @@ -839,6 +886,14 @@ static void lookupVisibleMemberDecls( | |||
| lookupVisibleMemberDeclsImpl(BaseTy, overrideConsumer, CurrDC, LS, Reason, | |||
| TypeResolver, GSB, Visited); | |||
|
|
|||
| llvm::DenseSet<DeclBaseName> knownMembers; | |||
| for (auto &kv : overrideConsumer.FoundDecls) { | |||
| knownMembers.insert(kv.first); | |||
There was a problem hiding this comment.
Could we avoid this copy unless it hasDynamicMemberLookupAttribute()?
There was a problem hiding this comment.
Fixed in my latest changes. Also added the check for mayHaveMembers.
| // Dynamic lookup members are only visible if they are not shadowed by | ||
| // non-dynamic members. | ||
| if (seen.count(VD->getBaseName()) == 0) | ||
| consumer.foundDecl(VD, reason); |
There was a problem hiding this comment.
The reason here is necessarily accurate. Maybe, we should introduce DeclVisibilityKind::DynamicMember?
Also we might want to propagate subscript and memberType to the consumer, somehow 🤔 (so that we can get the accurate type of the member)
There was a problem hiding this comment.
Let's pursue this in follow-up PRs.
There was a problem hiding this comment.
I thought about a new DeclVisibilityKind, but hesitated because it's orthogonal from some of the existing cases:
MemberOfCurrentNominalMemberOfProtocolImplementedByCurrentNominalMemberOfSuper
Ideally we could distinguish dynamic member of current nominal vs. dynamic member of super. It's also not clear to me how to prioritize dynamic members vs regular members - does it affect priority? If so, how?
We could add three new kinds, or possibly add a separate bit, I'm not sure.
I agree with the overall point that we probably want to add this new information in order to handle the type mapping correctly in the future. Do you think it would be sufficient for the lookup code to determine a type using the subscript + memberType and pass that single type back to code-completion, or do we want to bundle up all the pieces so that code-completion can also add the contextual type before solving?
…ocation The implicit subscript is not located at the "." instead of the identifier, so update the index test.
This test baked in an incorrect USR from a bug that has since been fixed. But we don't really need to check this anyway, so remove it.
Per review feedback. Also add some test cases that should fail dynamic member lookup.
|
@swift-ci please test |
|
@swift-ci please test Linux |
| // CHECK: [[WBR_LINE:[0-9]+]]:17 | instance-property/Swift | y | [[PY_USR]] | Ref,Writ,RelCont | rel: 1 | ||
| // CHECK: [[WBR_LINE:[0-9]+]]:17 | instance-method/acc-set/Swift | setter:y | [[PY_SET_USR]] | Ref,Call,Impl,RelCall,RelCont | rel: 1 | ||
|
|
||
| // FIXME: crashes typechecker rdar://problem/49533404 |
There was a problem hiding this comment.
From that radar, it looks like Pavel fixed this already.
There was a problem hiding this comment.
Ah, forgot I still had this FIXME. I'll take care of this in a follow-up.
|
Build failed |
|
Foundation Linux failure is unrelated (again). |
Add support for code-completion looking up keypath dynamic members, and cursor-info/indexing walking the underlying decls.
rdar://49028783
rdar://49028895
rdar://49029126