[CodeCompletion] Ensure synthesized members are available before lookup #41735
Conversation
|
@swift-ci Please smoke test |
|
@ahoppen Maybe this is not needed after we migrate |
lib/IDE/ExprContextAnalysis.cpp
Outdated
| auto *baseNominal = baseInstanceTy->getAnyNominal(); | ||
| if (baseNominal) { | ||
| // Force all implicit members synthesized before lookup. | ||
| (void)baseNominal->getAllMembers(); | ||
| } |
There was a problem hiding this comment.
I found it a bit odd this wasn't already handled in the lookup. Is there a bug there? Ie. it does call NominalTypeDecl::prepareLookupTable. Is it intentional that calls getMembers rather than getAlllMembers? getMembers calls loadAllMembers but that doesn't get synthesized implicit members... How confusing
There was a problem hiding this comment.
I agree this is odd. I believe what is happening is that when LookupVisibleDecls does a walk over the members of a nominal it uses getMembers() and not getAllMembers(). Perhaps it should use getAllMembers() if we wish to see all possible synthesized declarations, and not just those that have been synthesized up until now.
Rintaro, do you mind investigating and pushing this down into the appropriate part of LookupVisibleDecls?
There was a problem hiding this comment.
@slavapestov FWIW, this PR is not about LookupVisibleDecls, but DeclContext::lookupQualified().
But before I opened this PR, I tested getAllMembers() in LookupVisibleDecls and eliminating our own logic force triggering synthesizing implicit members. In general, that works, but it caused circular dependency errors.
struct S {
var value = undefined
}
When resolving undefined , the lookup fails and tries to perform typo correction (in test cases which we still do typo correction). When typo correction lookups visible decls, StructDecl("S")->getAllMembers() tries to
- Synthesize member wise initializer which requires the type of
value - Check automatic
@Sendableconformance which requires the type ofvalue - etc.
Do you have a good idea to resolve this circular dependency?
There was a problem hiding this comment.
@bnbarham We talked about this offline, but qualified lookup (prepareLookupTable) uses getMembers() because it doesn't want synthesize all implicit members. It wants only members with the specified name. TypeChecker::lookupMember has the trick installSemanticMembersIfNeeded() which trigger the synthesis of those named members. Probably we should do similar things here instead of calling getAllMembers() which does unnecessary works.
Or, maybe we should pursue sinking this logic inside DeclContext::lookupQualified()
There was a problem hiding this comment.
Yeah, re-using installSemanticMembersIfNeeded() sounds like the correct approach. Sinking the logic down into DeclContext::lookupQualified() will probably take a lot of work to fix all the cycles.
Yes, I hope that we can remove ExprContextAnalysis altogether in the near future. |
In ExprContextAnalyzer, when looking up members, some implicit members weren't populated. Ensure all implicit members available by force synthesizing them. rdar://89773376
rintaro
force-pushed
the
ide-completion-rdar89773376
branch
from
9ada69c to
4e62140
Compare
| @@ -1271,6 +1271,7 @@ func testSkippedCallArgInInvalidResultBuilderBody() { | |||
| Other(action: 2) { | |||
| MyImage(systemName: "", #^INVALID_RESULTBUILDER_ARG^# | |||
| struct Invalid | |||
| } | |||
There was a problem hiding this comment.
This missing } caused a test failure because extension is not allowed in a local context.
|
@swift-ci Please smoke test |
In
ExprContextAnalyzer, when looking up members, some implicit members weren't populated. Ensure all implicit members available by calling 'gelAllMembers()' on the nominal type.rdar://89773376