Skip to content

Commit eff9ec4

Browse files
ChristopherRogersChristopherRogers
committed
[ClangImporter] Fix edge cases where custom name matches native name
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
1 parent 2d92894 commit eff9ec4

File tree

4 files changed

+212
-9
lines changed

4 files changed

+212
-9
lines changed

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4535,18 +4535,45 @@ namespace {
45354535
}
45364536

45374537
template <typename T, typename U>
4538-
T *resolveSwiftDeclImpl(const U *decl, Identifier name, ModuleDecl *overlay) {
4538+
T *resolveSwiftDeclImpl(const U *decl, Identifier name, bool hasCustomName,
4539+
ModuleDecl *overlay) {
45394540
const auto &languageVersion =
45404541
Impl.SwiftContext.LangOpts.EffectiveLanguageVersion;
45414542

4543+
// None = not enough information
4544+
// true/false = definitive answer
4545+
auto isMatch = [&](const T *singleResult) -> Optional<bool> {
4546+
const DeclAttributes &attrs = singleResult->getAttrs();
4547+
4548+
// Skip versioned variants.
4549+
if (attrs.isUnavailableInSwiftVersion(languageVersion))
4550+
return false;
4551+
4552+
// If Clang decl has a custom Swift name, then `name` is that name.
4553+
if (hasCustomName)
4554+
return None;
4555+
4556+
// Skip if a different name is used for Objective-C.
4557+
if (auto objcAttr = attrs.getAttribute<ObjCAttr>())
4558+
if (auto objcName = objcAttr->getName())
4559+
return objcName->getSimpleName() == name;
4560+
4561+
return None;
4562+
};
4563+
4564+
// First look at Swift types with the same name.
45424565
SmallVector<ValueDecl *, 4> results;
45434566
overlay->lookupValue(name, NLKind::QualifiedLookup, results);
45444567
T *found = nullptr;
45454568
for (auto result : results) {
45464569
if (auto singleResult = dyn_cast<T>(result)) {
4547-
// Skip versioned variants.
4548-
const DeclAttributes &attrs = singleResult->getAttrs();
4549-
if (attrs.isUnavailableInSwiftVersion(languageVersion))
4570+
// Eliminate false positives (Swift name matches but Obj-C name doesn't).
4571+
Optional<bool> matched = isMatch(singleResult);
4572+
if (matched.hasValue() && !*matched)
4573+
continue;
4574+
4575+
// Skip if type not exposed to Objective-C.
4576+
if (!hasCustomName && !singleResult->isObjC())
45504577
continue;
45514578

45524579
if (found)
@@ -4556,6 +4583,26 @@ namespace {
45564583
}
45574584
}
45584585

4586+
if (!found && !hasCustomName) {
4587+
// Try harder to find match with custom Objective-C name.
4588+
// Only positive matches can be used, since we've already eliminated
4589+
// all native Swift types with the same native name.
4590+
SmallVector<Decl *, 64> results;
4591+
overlay->getTopLevelDecls(results);
4592+
for (auto result : results) {
4593+
if (auto singleResult = dyn_cast<T>(result)) {
4594+
Optional<bool> matched = isMatch(singleResult);
4595+
if (!(matched.hasValue() && *matched))
4596+
continue;
4597+
4598+
if (found)
4599+
return nullptr;
4600+
4601+
found = singleResult;
4602+
}
4603+
}
4604+
}
4605+
45594606
if (found)
45604607
Impl.ImportedDecls[{decl->getCanonicalDecl(),
45614608
getActiveSwiftVersion()}] = found;
@@ -4564,16 +4611,17 @@ namespace {
45644611
}
45654612

45664613
template <typename T, typename U>
4567-
T *resolveSwiftDecl(const U *decl, Identifier name,
4614+
T *resolveSwiftDecl(const U *decl, Identifier name, bool hasCustomName,
45684615
ClangModuleUnit *clangModule) {
45694616
if (auto overlay = clangModule->getOverlayModule())
4570-
return resolveSwiftDeclImpl<T>(decl, name, overlay);
4617+
return resolveSwiftDeclImpl<T>(decl, name, hasCustomName, overlay);
45714618
if (clangModule == Impl.ImportedHeaderUnit) {
45724619
// Use an index-based loop because new owners can come in as we're
45734620
// iterating.
45744621
for (size_t i = 0; i < Impl.ImportedHeaderOwners.size(); ++i) {
45754622
ModuleDecl *owner = Impl.ImportedHeaderOwners[i];
4576-
if (T *result = resolveSwiftDeclImpl<T>(decl, name, owner))
4623+
if (T *result = resolveSwiftDeclImpl<T>(decl, name, hasCustomName,
4624+
owner))
45774625
return result;
45784626
}
45794627
}
@@ -4586,7 +4634,8 @@ namespace {
45864634
if (!importer::hasNativeSwiftDecl(decl))
45874635
return false;
45884636
auto wrapperUnit = cast<ClangModuleUnit>(dc->getModuleScopeContext());
4589-
swiftDecl = resolveSwiftDecl<T>(decl, name, wrapperUnit);
4637+
swiftDecl = resolveSwiftDecl<T>(decl, name, /*hasCustomName=*/true,
4638+
wrapperUnit);
45904639
return true;
45914640
}
45924641

@@ -4615,13 +4664,14 @@ namespace {
46154664
*correctSwiftName);
46164665

46174666
Identifier name = importedName.getDeclName().getBaseIdentifier();
4667+
bool hasCustomName = importedName.hasCustomName();
46184668

46194669
// FIXME: Figure out how to deal with incomplete protocols, since that
46204670
// notion doesn't exist in Swift.
46214671
if (!decl->hasDefinition()) {
46224672
// Check if this protocol is implemented in its overlay.
46234673
if (auto clangModule = Impl.getClangModuleForDecl(decl, true))
4624-
if (auto native = resolveSwiftDecl<ProtocolDecl>(decl, name,
4674+
if (auto native = resolveSwiftDecl<ProtocolDecl>(decl, name, hasCustomName,
46254675
clangModule))
46264676
return native;
46274677

@@ -4733,11 +4783,13 @@ namespace {
47334783
*correctSwiftName);
47344784

47354785
auto name = importedName.getDeclName().getBaseIdentifier();
4786+
bool hasCustomName = importedName.hasCustomName();
47364787

47374788
if (!decl->hasDefinition()) {
47384789
// Check if this class is implemented in its overlay.
47394790
if (auto clangModule = Impl.getClangModuleForDecl(decl, true)) {
47404791
if (auto native = resolveSwiftDecl<ClassDecl>(decl, name,
4792+
hasCustomName,
47414793
clangModule)) {
47424794
return native;
47434795
}

‎test/ClangImporter/MixedSource/Inputs/mixed-framework/Mixed.framework/Headers/Mixed.h

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,62 @@ SWIFT_PROTOCOL("SwiftProto")
6868
id<SwiftProtoWithCustomName> _Nonnull
6969
convertToProto(SwiftClassWithCustomName *_Nonnull obj);
7070

71+
SWIFT_CLASS("ObjCClass")
72+
__attribute__((objc_root_class))
73+
@interface ObjCClass
74+
@end
75+
76+
SWIFT_CLASS("ImplicitlyObjCClass")
77+
@interface ImplicitlyObjCClass : ObjCClass
78+
@end
79+
void consumeImplicitlyObjCClass(ImplicitlyObjCClass *_Nonnull obj);
80+
81+
SWIFT_CLASS("ExplicitlyObjCClass")
82+
__attribute__((objc_root_class))
83+
@interface ExplicitlyObjCClass
84+
@end
85+
void consumeExplicitlyObjCClass(ExplicitlyObjCClass *_Nonnull obj);
86+
87+
SWIFT_CLASS_NAMED("HasSameCustomNameClass")
88+
__attribute__((objc_root_class))
89+
@interface HasSameCustomNameClass
90+
@end
91+
void consumeHasSameCustomNameClass(HasSameCustomNameClass *_Nonnull obj);
92+
93+
SWIFT_CLASS_NAMED("SwiftNativeTypeHasDifferentCustomNameClass")
94+
__attribute__((objc_root_class))
95+
@interface NativeTypeHasDifferentCustomNameClass
96+
@end
97+
SWIFT_CLASS_NAMED("NativeTypeHasDifferentCustomNameClass")
98+
__attribute__((objc_root_class))
99+
@interface ObjCNativeTypeHasDifferentCustomNameClass
100+
@end
101+
void consumeNativeTypeHasDifferentCustomNameClass(NativeTypeHasDifferentCustomNameClass *_Nonnull obj);
102+
void consumeObjCNativeTypeHasDifferentCustomNameClass(ObjCNativeTypeHasDifferentCustomNameClass *_Nonnull obj);
103+
104+
SWIFT_CLASS_NAMED("SwiftNativeTypeIsNonObjCClass")
105+
__attribute__((objc_root_class))
106+
@interface NativeTypeIsNonObjCClass
107+
@end
108+
void consumeNativeTypeIsNonObjCClass(NativeTypeIsNonObjCClass *_Nonnull obj);
109+
110+
@class ForwardImplicitlyObjCClass;
111+
void consumeForwardImplicitlyObjCClass(ForwardImplicitlyObjCClass *_Nonnull obj);
112+
113+
@class ForwardExplicitlyObjCClass;
114+
void consumeForwardExplicitlyObjCClass(ForwardExplicitlyObjCClass *_Nonnull obj);
115+
116+
@class ForwardHasSameCustomNameClass;
117+
void consumeForwardHasSameCustomNameClass(ForwardHasSameCustomNameClass *_Nonnull obj);
118+
119+
@class ForwardNativeTypeHasDifferentCustomNameClass;
120+
@class ObjCForwardNativeTypeHasDifferentCustomNameClass;
121+
void consumeForwardNativeTypeHasDifferentCustomNameClass(ForwardNativeTypeHasDifferentCustomNameClass *_Nonnull obj);
122+
void consumeObjCForwardNativeTypeHasDifferentCustomNameClass(ObjCForwardNativeTypeHasDifferentCustomNameClass *_Nonnull obj);
123+
124+
@class ForwardNativeTypeIsNonObjCClass;
125+
void consumeForwardNativeTypeIsNonObjCClass(ForwardNativeTypeIsNonObjCClass *_Nonnull obj);
126+
71127
SWIFT_CLASS("BOGUS")
72128
@interface BogusClass
73129
@end

‎test/ClangImporter/MixedSource/Inputs/mixed-framework/Mixed.swift

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,70 @@ public class CustomNameClass : CustomName {
3131
@objc func protoMethod()
3232
@objc var protoProperty: Int { get }
3333
}
34+
35+
@objc
36+
public class ObjCClass {
37+
public init() {}
38+
}
39+
40+
public class ImplicitlyObjCClass : ObjCClass {
41+
public override init() { super.init() }
42+
}
43+
44+
@objc
45+
public class ExplicitlyObjCClass {
46+
public init() {}
47+
}
48+
49+
@objc(HasSameCustomNameClass)
50+
public class HasSameCustomNameClass {
51+
public init() {}
52+
}
53+
54+
@objc(ObjCNativeTypeHasDifferentCustomNameClass)
55+
public class NativeTypeHasDifferentCustomNameClass {
56+
public init() {}
57+
}
58+
@objc(NativeTypeHasDifferentCustomNameClass)
59+
public class SwiftNativeTypeHasDifferentCustomNameClass {
60+
public init() {}
61+
}
62+
63+
public class NativeTypeIsNonObjCClass {
64+
public init() {}
65+
}
66+
@objc(NativeTypeIsNonObjCClass)
67+
public class SwiftNativeTypeIsNonObjCClass {
68+
public init() {}
69+
}
70+
71+
public class ForwardImplicitlyObjCClass : ObjCClass {
72+
public override init() { super.init() }
73+
}
74+
75+
@objc
76+
public class ForwardExplicitlyObjCClass {
77+
public init() {}
78+
}
79+
80+
@objc(ForwardHasSameCustomNameClass)
81+
public class ForwardHasSameCustomNameClass {
82+
public init() {}
83+
}
84+
85+
@objc(ObjCForwardNativeTypeHasDifferentCustomNameClass)
86+
public class ForwardNativeTypeHasDifferentCustomNameClass {
87+
public init() {}
88+
}
89+
@objc(ForwardNativeTypeHasDifferentCustomNameClass)
90+
public class SwiftForwardNativeTypeHasDifferentCustomNameClass {
91+
public init() {}
92+
}
93+
94+
public class ForwardNativeTypeIsNonObjCClass {
95+
public init() {}
96+
}
97+
@objc(ForwardNativeTypeIsNonObjCClass)
98+
public class SwiftForwardNativeTypeIsNonObjCClass {
99+
public init() {}
100+
}

‎test/ClangImporter/MixedSource/import-mixed-framework.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,31 @@ func testAnyObject(_ obj: AnyObject) {
3232
obj.protoMethod()
3333
_ = obj.protoProperty
3434
}
35+
36+
consumeImplicitlyObjCClass(ImplicitlyObjCClass())
37+
38+
consumeExplicitlyObjCClass(ExplicitlyObjCClass())
39+
40+
consumeHasSameCustomNameClass(HasSameCustomNameClass())
41+
42+
consumeNativeTypeHasDifferentCustomNameClass(SwiftNativeTypeHasDifferentCustomNameClass())
43+
consumeObjCNativeTypeHasDifferentCustomNameClass(NativeTypeHasDifferentCustomNameClass())
44+
consumeNativeTypeHasDifferentCustomNameClass(NativeTypeHasDifferentCustomNameClass()) // expected-error {{cannot convert value of type 'NativeTypeHasDifferentCustomNameClass' to expected argument type 'SwiftNativeTypeHasDifferentCustomNameClass'}}
45+
consumeObjCNativeTypeHasDifferentCustomNameClass(SwiftNativeTypeHasDifferentCustomNameClass()) // expected-error {{cannot convert value of type 'SwiftNativeTypeHasDifferentCustomNameClass' to expected argument type 'NativeTypeHasDifferentCustomNameClass'}}
46+
47+
consumeNativeTypeIsNonObjCClass(SwiftNativeTypeIsNonObjCClass())
48+
consumeNativeTypeIsNonObjCClass(NativeTypeIsNonObjCClass()) // expected-error {{cannot convert value of type 'NativeTypeIsNonObjCClass' to expected argument type 'SwiftNativeTypeIsNonObjCClass'}}
49+
50+
consumeForwardImplicitlyObjCClass(ForwardImplicitlyObjCClass())
51+
52+
consumeForwardExplicitlyObjCClass(ForwardExplicitlyObjCClass())
53+
54+
consumeForwardHasSameCustomNameClass(ForwardHasSameCustomNameClass())
55+
56+
consumeForwardNativeTypeHasDifferentCustomNameClass(SwiftForwardNativeTypeHasDifferentCustomNameClass())
57+
consumeObjCForwardNativeTypeHasDifferentCustomNameClass(ForwardNativeTypeHasDifferentCustomNameClass())
58+
consumeForwardNativeTypeHasDifferentCustomNameClass(ForwardNativeTypeHasDifferentCustomNameClass()) // expected-error {{cannot convert value of type 'ForwardNativeTypeHasDifferentCustomNameClass' to expected argument type 'SwiftForwardNativeTypeHasDifferentCustomNameClass'}}
59+
consumeObjCForwardNativeTypeHasDifferentCustomNameClass(SwiftForwardNativeTypeHasDifferentCustomNameClass()) // expected-error {{cannot convert value of type 'SwiftForwardNativeTypeHasDifferentCustomNameClass' to expected argument type 'ForwardNativeTypeHasDifferentCustomNameClass'}}
60+
61+
consumeForwardNativeTypeIsNonObjCClass(SwiftForwardNativeTypeIsNonObjCClass())
62+
consumeForwardNativeTypeIsNonObjCClass(ForwardNativeTypeIsNonObjCClass()) // expected-error {{cannot convert value of type 'ForwardNativeTypeIsNonObjCClass' to expected argument type 'SwiftForwardNativeTypeIsNonObjCClass'}}

0 commit comments

Comments
 (0)