[cxx-interop] [NFC] Migrate lookup-related code to ClangLookup.cpp #86139
+1,361
−1,272
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
Remove the ClangImporter::Implementation::getClangOwningModule() method, and replaces all call sites with the static free-standing function it calls, now exposed in the swift::importer namespace as an internal API. This makes it easier to use in contexts where an Implementation instance is not readily available. Also, clone the documentation to the public ClangImporter API of the same name.
This cast was previously refactored from llvm::PointerUnion::get, which both abort upon an invalid cast rather than returning a nullptr, so there is no point checking this in a condition.
ClangImporter.cpp is so large that it is getting difficult to navigate.
Start to migrate lookup-related code to a separate file, ClangLookup.cpp.
This first patch moves ClangDirectLookupRequest and related helpers to a new
file, ClangLookup.cpp. This request looks for decls imported from Clang
according to their Swift name, from mappings stored in the SwiftLookupTable
that ClangImporter writes into (its copy of ) the Clang modules it imports.
This request also includes separate handling of lookup in class template
specializations, which are not in the SwiftLookupTable.
At this time of refactoring, lookup works like this. If we are looking in some
struct Foo for a member Bar, we look for _everything_ named "Bar" in the module
that owns Foo, and then filter out everything that isn't a member of Foo
(according to DeclContext). This is inefficient, but correctly accounts for
members that have been "injected" into structs via the SIWFT_NAME, and also
accounts for other name mappings done by the NameImporter.
For the inevitable future Git archaeologist, what follows are the commits whose
blame is destroyed by this refactoring:
b8e52a7 [cxx-interop] Lazily import members of Clang namespaces and records via requests.
0ca8dd3 [nfc][cxx-interop] Add three requests `ClangDirectLookupRequest`, `CXXNamespaceMemberLookup`, and `ClangRecordMemberLookup`.
1089959 [interop] clang name lookup should find declarations in inline namespaces
c952fc1 [cxx-interop] Fix lookup of class template instantiations
0eed1a9 [cxx-interop] correctly add and lookup enumerators added to enums that correspond to namespaces
ClangImporter.cpp is so large that it is getting difficult to navigate.
Migrate lookup-related code to a separate file, ClangLookup.cpp.
This patch moves CXXNamespaceMemberLookup and its related helper. This request
looks for decls in a namespace, by performing a ClangDirectLookupRequest in each
redeclaration of that namespace. The results are imported to convert them to
swift::ValueDecls.
For the inevitable future Git archaeologist, what follows are the commits whose
blame is destroyed by this refactoring:
b8e52a7 [cxx-interop] Lazily import members of Clang namespaces and records via requests.
0ca8dd3 [nfc][cxx-interop] Add three requests `ClangDirectLookupRequest`, `CXXNamespaceMemberLookup`, and `ClangRecordMemberLookup`.
0c7b1ce [cxx-interop] serialize x-refs for class template specializations
84a4a8b [Importer] Ensure that we can see macro-expanded declarations in C++ namespaces
ClangImporter.cpp is so large that it is getting difficult to navigate.
Migrate lookup-related code to a separate file, ClangLookup.cpp.
This patch moves the ClangRecordMemberLookup request and the base member cloning
logic to ClangLookup.cpp. This request looks for members of a given (Swift) name
in a (imported) Clang record, and any of its base classes. The results are
imported to convert them to swift::ValueDecls.
Members found in base classes are not directly added to the inherting class,
because Swift does not model inheritance for imported structs. Instead, they are
"cloned" from base members via a synthesized accessor that is constructed
lazily. This indirection delegates the low-level details of the base member
access to Clang.
At this time of refactoring, lookup for automatically synthesized members like
.pointee and .successor relies on the RecordDecl import logic being eager.
Inherited lookup also has known limitations around inherited C-style enums and
static member functions.
The lookup logic for non-public members is gated under ImportNonPublicCxxMembers
feature, to mitigate issues that may arise from exposing ClangImporter to
private decls that it cannot yet robustly handle. For instance, it does not
deduplicate non-public members that seem to appear twice during lookup due to
UsingShadowDecl.
Also, the inherited lookup logic does not leverage the existing facilities from
Clang, because it needs to handle Swift-specific details like including members
injected by SWIFT_NAME and dropping constructors looked up from foreign
reference types. For member functions, it performs some member deduplication
according to parameter count, to avoid creating spurious ambiguity.
For the inevitable future Git archaeologist, what follows are the commits whose
blame is destroyed by this refactoring:
b8e52a7 [cxx-interop] Lazily import members of Clang namespaces and records via requests.
bb00e8d [cxx-interop] Rudimentary support for importing base classes.
0ca8dd3 [nfc][cxx-interop] Add three requests `ClangDirectLookupRequest`, `CXXNamespaceMemberLookup`, and `ClangRecordMemberLookup`.
2e0398f [cxx-interop] Add support for subscripts in base classes.
336cd29 [cxx-interop] Add support for calling members of base classes on UFOs.
e73bb6f [cxx-interop] Import attributes on inherited C++ methods
3805828 [cxx-interop] Add `CxxShim` library; move `__swift_interopStaticCast` into it.
51a1176 [cxx-interop] Clang member lookup should not look into Swift extensions
109d44c [cxx-interop] Emit LValue type in a base subscript assignment expression
8df9fca [cxx-interop] Don't import constructors of foreign reference types.
a103325 [cxx-interop] Do not add base class members that cause lookup ambiguities with a derived class member of the same name
ba5b1ba [cxx-interop] Use a synthesized C++ method when invoking a base method from a derived class synthesized method
4150450 [cxx-interop] Use a synthesized C++ method when accessing a base field or subscript from a derived class synthesized method
f7ce9aa [cxx-interop] Synthesized derived-to-base field getter should copy out a retainable FRT value
f9bf957 [cxx-interop] Do not import inherited methods with rvalue this
bacc58e [cxx-interop] provide correct referential access to non-copyable base fields from a derived value type
d3460cb [cxx-interop] Do not crash when synthesizing a base method call
16a8ae4 [cxx-interop] fix the use of '.pointee' with address accessors for derived-to-base synthesized accessors
0296448 [cxx-interop] fix the use of '.pointee' with getter accessor for derived-to-base synthesized accessor
44d7d06 Don't lie about the intermediate result type when synthesizing a call to a function that returns UnsafeMutablePointer.
6140ba1 [cxx-interop] Do not try to use `UnsafePointer<T>` for non-copyable `T` when adding base member accessors
5ae2f6b CxxInterop: use Unsafe*Pointer for move-only
b509551 [cxx-interop] Clone all of the attributes from base method correctly
796075a [cxx-interop] Fix spurious ambiguous member lookup
1341516 [cxx-interop] Fix spurious ambiguous member lookup for eagerly imported members (swiftlang#78673)
be73254 [cxx-interop] Import private members
66c2e2c [cxx-interop] Import non-public inherited members
edc7420 [cxx-interop] Make experimental flag ImportNonPublicCxxMembers
e3618dd [Clang importer] Report auxiliary decls from C++ member lookup
84a4a8b [Importer] Ensure that we can see macro-expanded declarations in C++ namespaces
bbf92fd [cxx-interop] Import non-public members of SWIFT_PRIVATE_FILEID-annotated classes
e8bcc52 [cxx-interop] Fix access check for nested private C++ enums
1f2107f [cxx-interop] Avoid unchecked recursion when importing C++ classes with circular inheritance
79227e7 [cxx-interop] Fix ambiguous methods in long chains of inheritance
4de9265 [cxx-interop] Fix unqualified name lookup failure
585ca5e [cxx-interop] Adding swift_name attributes to virtual methods overrides
72be0e0 [Clang importer] Import incomplete SWIFT_SHARED_REFERENCE types
563c014 [cxx-interop] Fix inherited nested types
j-hui
requested review from
Xazax-hun,
beccadax,
egorzhdan,
hborla,
ian-twilightcoder,
slavapestov,
susmonteiro and
xedin
as code owners
Contributor
Author
|
@swift-ci please smoke test |
egorzhdan
approved these changes
Dec 19, 2025
Contributor
egorzhdan
left a comment
egorzhdan
left a comment
There was a problem hiding this comment.
This will make clangd so much snappier on ClangImporter. Thank you!
Contributor
|
Thanks! |
Contributor
|
Nice! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
ClangImporter.cpp is so large that it is getting difficult to navigate. This patch set migrates lookup-related code to a separate file, ClangLookup.cpp. In particular,
ClangDirectLookupRequest,CXXNamespaceMemberLookup, andClangRecordMemberLookup, as well as the base member cloning logic, are all moved to ClangLookup.cpp, in addition to any helpers they depend on.This refactor clobbers quite a bit of Git history, but I tried to make the changes incremental. I've also done my best to summarize the current state of the code in the commit messages. Finally, in each of the refactoring commits, I also list the relevant commits that can no longer be blamed as a result of this refactoring.