Skip to content

Commit 49fd5ac

Browse files
jrose-applejrose-apple
authored
Merge pull request #27682 from ChristopherRogers/master
[ClangImporter] Fix edge cases where custom name matches native name
2 parents d0ad5de + a51bbdf commit 49fd5ac

File tree

7 files changed

+227
-20
lines changed

7 files changed

+227
-20
lines changed

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1354,7 +1354,7 @@ bool ClangImporter::importHeader(StringRef header, ModuleDecl *adapter,
13541354

13551355
// If we've made it to here, this is some header other than the bridging
13561356
// header, which means we can no longer rely on one file's modification time
1357-
// to invalid code completion caches. :-(
1357+
// to invalidate code completion caches. :-(
13581358
Impl.setSinglePCHImport(None);
13591359

13601360
if (!cachedContents.empty() && cachedContents.back() == '\0')
@@ -3702,8 +3702,16 @@ EffectiveClangContext ClangImporter::Implementation::getEffectiveClangContext(
37023702
(nominal->getAttrs().hasAttribute<ObjCAttr>() ||
37033703
(!nominal->getParentSourceFile() && nominal->isObjC()))) {
37043704
// Map the name. If we can't represent the Swift name in Clang.
3705-
// FIXME: We should be using the Objective-C name here!
3706-
auto clangName = exportName(nominal->getName());
3705+
Identifier name = nominal->getName();
3706+
if (auto objcAttr = nominal->getAttrs().getAttribute<ObjCAttr>()) {
3707+
if (auto objcName = objcAttr->getName()) {
3708+
if (objcName->getNumArgs() == 0) {
3709+
// This is an error if not 0, but it should be caught later.
3710+
name = objcName->getSimpleName();
3711+
}
3712+
}
3713+
}
3714+
auto clangName = exportName(name);
37073715
if (!clangName)
37083716
return EffectiveClangContext();
37093717

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 59 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4535,24 +4535,64 @@ 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,
4539+
bool hasKnownSwiftName, ModuleDecl *overlay) {
45394540
const auto &languageVersion =
45404541
Impl.SwiftContext.LangOpts.EffectiveLanguageVersion;
45414542

4543+
auto isMatch = [&](const T *singleResult, bool baseNameMatches) -> bool {
4544+
const DeclAttributes &attrs = singleResult->getAttrs();
4545+
4546+
// Skip versioned variants.
4547+
if (attrs.isUnavailableInSwiftVersion(languageVersion))
4548+
return false;
4549+
4550+
// Skip if type not exposed to Objective-C.
4551+
// If the base name doesn't match, then a matching
4552+
// custom name in an @objc attribute is required.
4553+
if (baseNameMatches && !singleResult->isObjC())
4554+
return false;
4555+
4556+
// If Clang decl has a custom Swift name, then we know that
4557+
// `name` is the base name we're looking for.
4558+
if (hasKnownSwiftName)
4559+
return baseNameMatches;
4560+
4561+
// Skip if a different name is used for Objective-C.
4562+
if (auto objcAttr = attrs.getAttribute<ObjCAttr>())
4563+
if (auto objcName = objcAttr->getName())
4564+
return objcName->getSimpleName() == name;
4565+
4566+
return baseNameMatches;
4567+
};
4568+
4569+
// First look at Swift types with the same name.
45424570
SmallVector<ValueDecl *, 4> results;
45434571
overlay->lookupValue(name, NLKind::QualifiedLookup, results);
45444572
T *found = nullptr;
45454573
for (auto result : results) {
45464574
if (auto singleResult = dyn_cast<T>(result)) {
4547-
// Skip versioned variants.
4548-
const DeclAttributes &attrs = singleResult->getAttrs();
4549-
if (attrs.isUnavailableInSwiftVersion(languageVersion))
4550-
continue;
4551-
4552-
if (found)
4553-
return nullptr;
4575+
if (isMatch(singleResult, /*baseNameMatches=*/true)) {
4576+
if (found)
4577+
return nullptr;
4578+
found = singleResult;
4579+
}
4580+
}
4581+
}
45544582

4555-
found = singleResult;
4583+
if (!found && !hasKnownSwiftName) {
4584+
// Try harder to find a match looking at just custom Objective-C names.
4585+
SmallVector<Decl *, 64> results;
4586+
overlay->getTopLevelDecls(results);
4587+
for (auto result : results) {
4588+
if (auto singleResult = dyn_cast<T>(result)) {
4589+
// The base name _could_ match but it's irrelevant here.
4590+
if (isMatch(singleResult, /*baseNameMatches=*/false)) {
4591+
if (found)
4592+
return nullptr;
4593+
found = singleResult;
4594+
}
4595+
}
45564596
}
45574597
}
45584598

@@ -4565,15 +4605,16 @@ namespace {
45654605

45664606
template <typename T, typename U>
45674607
T *resolveSwiftDecl(const U *decl, Identifier name,
4568-
ClangModuleUnit *clangModule) {
4608+
bool hasKnownSwiftName, ClangModuleUnit *clangModule) {
45694609
if (auto overlay = clangModule->getOverlayModule())
4570-
return resolveSwiftDeclImpl<T>(decl, name, overlay);
4610+
return resolveSwiftDeclImpl<T>(decl, name, hasKnownSwiftName, overlay);
45714611
if (clangModule == Impl.ImportedHeaderUnit) {
45724612
// Use an index-based loop because new owners can come in as we're
45734613
// iterating.
45744614
for (size_t i = 0; i < Impl.ImportedHeaderOwners.size(); ++i) {
45754615
ModuleDecl *owner = Impl.ImportedHeaderOwners[i];
4576-
if (T *result = resolveSwiftDeclImpl<T>(decl, name, owner))
4616+
if (T *result = resolveSwiftDeclImpl<T>(decl, name,
4617+
hasKnownSwiftName, owner))
45774618
return result;
45784619
}
45794620
}
@@ -4586,7 +4627,8 @@ namespace {
45864627
if (!importer::hasNativeSwiftDecl(decl))
45874628
return false;
45884629
auto wrapperUnit = cast<ClangModuleUnit>(dc->getModuleScopeContext());
4589-
swiftDecl = resolveSwiftDecl<T>(decl, name, wrapperUnit);
4630+
swiftDecl = resolveSwiftDecl<T>(decl, name, /*hasCustomSwiftName=*/true,
4631+
wrapperUnit);
45904632
return true;
45914633
}
45924634

@@ -4615,13 +4657,15 @@ namespace {
46154657
*correctSwiftName);
46164658

46174659
Identifier name = importedName.getDeclName().getBaseIdentifier();
4660+
bool hasKnownSwiftName = importedName.hasCustomName();
46184661

46194662
// FIXME: Figure out how to deal with incomplete protocols, since that
46204663
// notion doesn't exist in Swift.
46214664
if (!decl->hasDefinition()) {
46224665
// Check if this protocol is implemented in its overlay.
46234666
if (auto clangModule = Impl.getClangModuleForDecl(decl, true))
46244667
if (auto native = resolveSwiftDecl<ProtocolDecl>(decl, name,
4668+
hasKnownSwiftName,
46254669
clangModule))
46264670
return native;
46274671

@@ -4733,11 +4777,13 @@ namespace {
47334777
*correctSwiftName);
47344778

47354779
auto name = importedName.getDeclName().getBaseIdentifier();
4780+
bool hasKnownSwiftName = importedName.hasCustomName();
47364781

47374782
if (!decl->hasDefinition()) {
47384783
// Check if this class is implemented in its overlay.
47394784
if (auto clangModule = Impl.getClangModuleForDecl(decl, true)) {
47404785
if (auto native = resolveSwiftDecl<ClassDecl>(decl, name,
4786+
hasKnownSwiftName,
47414787
clangModule)) {
47424788
return native;
47434789
}

‎lib/ClangImporter/ImportName.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1781,9 +1781,11 @@ ImportedName NameImporter::importName(const clang::NamedDecl *decl,
17811781
ImportNameVersion version,
17821782
clang::DeclarationName givenName) {
17831783
CacheKeyType key(decl, version);
1784-
if (importNameCache.count(key) && !givenName) {
1785-
++ImportNameNumCacheHits;
1786-
return importNameCache[key];
1784+
if (!givenName) {
1785+
if (auto cachedRes = importNameCache[key]) {
1786+
++ImportNameNumCacheHits;
1787+
return cachedRes;
1788+
}
17871789
}
17881790
++ImportNameNumCacheMisses;
17891791
auto res = importNameImpl(decl, version, givenName);

‎lib/Sema/TypeCheckDeclObjC.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1393,7 +1393,7 @@ IsObjCRequest::evaluate(Evaluator &evaluator, ValueDecl *VD) const {
13931393
// Classes can be @objc.
13941394

13951395
// Protocols and enums can also be @objc, but this is covered by the
1396-
// isObjC() check a the beginning.;
1396+
// isObjC() check at the beginning.
13971397
isObjC = shouldMarkAsObjC(VD, /*allowImplicit=*/false);
13981398
} else if (auto enumDecl = dyn_cast<EnumDecl>(VD)) {
13991399
// Enums can be @objc so long as they have a raw type that is representable

‎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)