[ClangImporter] Fix edge cases where custom name matches native name #27682
Conversation
lib/ClangImporter/ImportDecl.cpp
Outdated
|
|
||
| // If Clang decl has a custom Swift name, then `name` is that name. | ||
| if (hasCustomName) | ||
| return None; |
There was a problem hiding this comment.
Can you explain more what this special case is about?
There was a problem hiding this comment.
This is in order to bypass comparing the custom Objective-C name with the known custom Swift name.
lib/ClangImporter/ImportDecl.cpp
Outdated
| if (auto objcName = objcAttr->getName()) | ||
| return objcName->getSimpleName() == name; | ||
|
|
||
| return None; |
There was a problem hiding this comment.
…and this one? If there's no explicit ObjC attribute, can't we assume we know enough about the name to make a definitive statement?
There was a problem hiding this comment.
Instead of returning a tristate, how about passing in another argument saying whether the class's basename is already known to match? Or even just checking that up front?
There was a problem hiding this comment.
I’ve gotten rid of the tristate so the code should make a lot more sense now.
There was a problem hiding this comment.
Types need to be skipped over where the name matches but the type isn’t exposed to Objective-C. That was happening from the caller side before.
lib/ClangImporter/ImportDecl.cpp
Outdated
| @@ -4586,7 +4634,8 @@ namespace { | |||
| if (!importer::hasNativeSwiftDecl(decl)) | |||
| return false; | |||
| auto wrapperUnit = cast<ClangModuleUnit>(dc->getModuleScopeContext()); | |||
| swiftDecl = resolveSwiftDecl<T>(decl, name, wrapperUnit); | |||
| swiftDecl = resolveSwiftDecl<T>(decl, name, /*hasCustomName=*/true, | |||
There was a problem hiding this comment.
Maybe this suggests that "hasKnownSwiftName" would be a better name for the parameter?
The code does naive lookup of Swift types using the type name, but sometimes the Swift type we're looking for only has that name in its @objc attribute. This change makes the compiler exclude certain Swift declarations from matching even if the Swift name is the same (namely, not being available in Obj-C or having a mismatched `@objc` name) and continue to find the correct declaration without using lookup by name. Fixes SR-4827
ChristopherRogers
force-pushed
the
master
branch
from
2cb14d6 to
7b49c74
Compare
ChristopherRogers
force-pushed
the
master
branch
from
7b49c74 to
a51bbdf
Compare
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
|
Reverting this for now; we found a case where people were relying on the old behavior ( |
The code does naive lookup of Swift types using the type name, but sometimes the Swift type we're looking for only has that name in its @objc attribute. This change makes the compiler exclude certain Swift declarations from matching even if the Swift name is the same (namely, not being available in Obj-C or having a mismatched
@objcname) and continue to find the correct declaration without using lookup by name.I'm not quite sure how some cases worked before this, so some modifications may be necessary.
The main change is in the first commit. The other commits are some other changes I had sitting around. I don't have any tests for the Obj-C name commit (for the FIXME) and am not quite sure if it helps (tests still pass if I revert those changes), so I can drop that commit.
On a personal note, this also removes the need for a workaround I’ve had in my codebase at work to get Swift to associate a forward declaration to its Swift declaration (which involves creating a Clang module of the same name containing the generated Obj-C interface of the problematic class). 🎉
Resolves SR-4827.