[cxx-interop] Prioritize C++ modules over namespaces in interface files #79180
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci Please test |
|
@swift-ci Please test |
j-hui
requested review from
zoecarver,
hyp,
egorzhdan,
Xazax-hun,
hborla,
slavapestov and
xedin
as code owners
|
@swift-ci Please test |
tshortli
reviewed
Feb 6, 2025
lib/AST/UnqualifiedLookup.cpp
Outdated
| @@ -282,6 +282,10 @@ void UnqualifiedLookupFactory::lookUpTopLevelNamesInModuleScopeContext( | |||
| return; | |||
| } | |||
|
|
|||
| bool inInterface = false; | |||
There was a problem hiding this comment.
Nit: You can use DeclContext::isInSwiftinterface() to simplify this code a bit
tshortli
approved these changes
Feb 6, 2025
|
@swift-ci Please test |
|
Closing this in favor of a different approach |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In Swift, there is currently no way to refer to a module that has been shadowed by a top-level declaration. This is a problem in Swift module interfaces, which fully (*most) qualifies identifiers, e.g.,
modulename.member. If there is another top-level declaration namedmodulename,modulename.memberwill resolve the first component to that declaration rather than to the module; attempting to look upmemberin the declaration will lead to a missing member error (or resolve to the wrong member if the declaration happens to also definemember).*One exception is within the body of
@inlinablefunctions, which are splatted in interface files almost exactly as written by the user (i.e., usually unqualified). This is also why it is potentially dangerous to attempt any fix that assumes all module interface identifiers are fully qualified.This problem is more pronounced with C++ interop, where naming a namespace the same as the module is a fairly common idiom (e.g.,
os, andsimd). Worse, this problem is difficult to detect ahead of time---i.e., when a module interface is being generated---because the module might originally be compiled without C++ interop, where header guards prevent the C++ namespace from being defined. The problem only arises when the module is later recompiled from its interface with C++ interop (in fact, the module may need to be recompiled because its user is using C++ interop; see #77754).This patch fixes this issue specifically for C++ namespace/module name collisions, to avoid potential fall-out that may ensue from a broader fix. With this patch, we only favor a module over a colliding member if (1) we are in an interface file, and (2) the member is a namespace.
It should be possible to drop condition (2) if we can ensure all identifiers are fully-qualified in interface files, but that is not currently the case. That would also fix this issue for Swift modules whose name conflicts with one of its members'.
Related:
namespace os#78634 works around this issue for theosmodule.simdmodule, but that caused other problems; Revert "[cxx-interop] Workaround name lookup issues with namespace simd" #78852 reverted it.rdar://143352205