[cxx-interop] Fix spurious ambiguous member lookup for eagerly imported members

[j-hui]

Follow-up from #78132, which did not fix issues related to eagerly imported members like subscripts.

This patch restructures recursive ClangRecordMemberLookup requests to importBaseMemberDecl() in the recursive calls, rather than propagating base member decls up to the initial lookup request and doing the import. Doing so seems to fix lingering resolution issues (which I've added to the regression tests).

rdar://141069984

[j-hui]

@swift-ci Please smoke test

[Xazax-hun]

Some minor questions inline but looks good to me overall.

[Xazax-hun]

I wonder if this dyn_cast could ever fail. Do you know a DeclContext that is not a declaration itself?

[Xazax-hun]

In what cases will this check be triggered?

[j-hui]

That's a great question... and I'm not actually sure. This logic is copied from the original implementation.

I'll try to get to the bottom of this before merging.

[j-hui]

Ok so I think this check is redundant, because ClangDirectLookupRequest is already checking this:

swift/lib/ClangImporter/ClangImporter.cpp

Lines 5028 to 5043 in d50faa8

	llvm::copy_if(foundDecls, std::back_inserter(filteredDecls),
	[clangDecl](SwiftLookupTable::SingleEntry decl) {
	auto foundClangDecl = decl.dyn_cast<clang::NamedDecl *>();
	if (!foundClangDecl)
	return false;
	auto first = foundClangDecl->getDeclContext();
	auto second = cast<clang::DeclContext>(clangDecl);
	if (auto firstDecl = dyn_cast<clang::Decl>(first)) {
	if (auto secondDecl = dyn_cast<clang::Decl>(second))
	return isDirectLookupMemberContext(foundClangDecl,
	firstDecl, secondDecl);
	else
	return false;
	}
	return first == second;
	});

where isDirectLookupMemberContext() is:

swift/lib/ClangImporter/ClangImporter.cpp

Lines 4982 to 5003 in d50faa8

	static bool isDirectLookupMemberContext(const clang::Decl *foundClangDecl,
	const clang::Decl *memberContext,
	const clang::Decl *parent) {
	if (memberContext->getCanonicalDecl() == parent->getCanonicalDecl())
	return true;
	if (auto namespaceDecl = dyn_cast<clang::NamespaceDecl>(memberContext)) {
	if (namespaceDecl->isInline()) {
	if (auto memberCtxParent =
	dyn_cast<clang::Decl>(namespaceDecl->getParent()))
	return isDirectLookupMemberContext(foundClangDecl, memberCtxParent,
	parent);
	}
	}
	// Enum constant decl can be found in the parent context of the enum decl.
	if (auto *ED = dyn_cast<clang::EnumDecl>(memberContext)) {
	if (isa<clang::EnumConstantDecl>(foundClangDecl)) {
	if (auto *firstDecl = dyn_cast<clang::Decl>(ED->getDeclContext()))
	return firstDecl->getCanonicalDecl() == parent->getCanonicalDecl();
	}
	}
	return false;
	}

If anything, ClangDirectLookupRequest seems to be somewhat more thorough and accounting for things like canonical decls and namespaces (though they shouldn't be relevant here), so the check here is more restrictive ClangDirectLookupRequest for reasons that aren't obvious to me (and there don't seem to be clues in the commit history either).

I'm going to remove this check and be ready to revert it in case there's some edge case this check was supposed to account for.

[j-hui]

Whelp, that check was doing something after all. It seemingly prevents a crash from happening in this test: https://github.com/swiftlang/swift/blob/439f7d9b672ab2072ad458900752c39c171fd7f7/test/ClangImporter/enum-anon.swift

I'll revert that change.

[egorzhdan]

I haven't looked into the implementation bits super closely, but this LGTM in principle, thanks!

[j-hui]

@swift-ci Please test

[j-hui]

@swift-ci please test

[j-hui]

Not sure why that failed. Relevant output that I could find:

/Users/ec2-user/jenkins/workspace/swift-PR-macos/branch-main/swift-docc/Sources/SwiftDocCUtilities/PreviewServer/PreviewHTTPHandler.swift:114:34: error: 'assumeIsolated' is inaccessible due to 'internal' protection level
112 |         // If we don't need to keep the connection alive, close `context` after flushing the response
113 |         if !self.keepAlive {
114 |             promise.futureResult.assumeIsolated().whenComplete { _ in context.close(promise: nil) }
    |                                  `- error: 'assumeIsolated' is inaccessible due to 'internal' protection level
115 |         }
116 | 

FAIL: Building swift-docc failed

Gonna try this one more time

[j-hui]

@swift-ci Please test

[j-hui]

swiftlang/swift-docc#1145

@swift-ci please smoke test

[j-hui]

@swift-ci please smoke test

[j-hui]

@swift-ci please smoke test linux