From ec23dcaaac86bb48cbc8cb58c8842bc24915e377 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Sat, 13 May 2017 14:51:36 -0700 Subject: [PATCH] [ClangImporter] Find Swift 3 / 4 names via dynamic lookup too. Finishes rdar://problem/29170671 --- lib/ClangImporter/ClangImporter.cpp | 45 ++++++++----------- lib/ClangImporter/ImportDecl.cpp | 44 +++++++----------- lib/ClangImporter/ImporterImpl.h | 17 +++++++ .../Headers/APINotesFrameworkTest.apinotes | 3 ++ .../Headers/Classes.h | 1 + .../versioned-objc-dynamic-lookup.swift | 35 +++++++++++++++ 6 files changed, 91 insertions(+), 54 deletions(-) create mode 100644 test/APINotes/versioned-objc-dynamic-lookup.swift diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 171e28d31ef..ccaf626d385 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -3041,37 +3041,30 @@ void ClangImporter::Implementation::lookupObjCMembers( // If the entry is not visible, skip it. if (!isVisibleClangEntry(clangCtx, clangDecl)) continue; - // Import the declaration. - auto decl = - cast_or_null(importDeclReal(clangDecl, CurrentVersion)); - if (!decl) - continue; + forEachDistinctName(clangDecl, + [&](ImportedName importedName, + ImportNameVersion nameVersion) { + // Import the declaration. + auto decl = + cast_or_null(importDeclReal(clangDecl, nameVersion)); + if (!decl) + return; - // If the name we found matches, report the declaration. - bool matchedAny = false; - if (decl->getFullName().matchesRef(name)) { - consumer.foundDecl(decl, DeclVisibilityKind::DynamicLookup); - matchedAny = true; - } - - // Check for an alternate declaration; if it's name matches, - // report it. - for (auto alternate : getAlternateDecls(decl)) { - if (alternate->getFullName().matchesRef(name)) { - consumer.foundDecl(alternate, DeclVisibilityKind::DynamicLookup); - matchedAny = true; + // If the name we found matches, report the declaration. + // FIXME: If we didn't need to check alternate decls here, we could avoid + // importing the member at all by checking importedName ahead of time. + if (decl->getFullName().matchesRef(name)) { + consumer.foundDecl(decl, DeclVisibilityKind::DynamicLookup); } - } - // If we didn't find anything, try under the Swift 2 name. - if (!matchedAny) { - if (auto swift2Decl = cast_or_null( - importDeclReal(clangDecl, Version::Swift2))) { - if (swift2Decl->getFullName().matchesRef(name)) { - consumer.foundDecl(swift2Decl, DeclVisibilityKind::DynamicLookup); + // Check for an alternate declaration; if its name matches, + // report it. + for (auto alternate : getAlternateDecls(decl)) { + if (alternate->getFullName().matchesRef(name)) { + consumer.foundDecl(alternate, DeclVisibilityKind::DynamicLookup); } } - } + }); } } diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 929f578cca1..e1750ff8436 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -345,27 +345,15 @@ static bool isNSDictionaryMethod(const clang::ObjCMethodDecl *MD, return true; } -/// Attempts to import the name of \p decl with each possible ImportNameVersion. -/// \p action will be called with each unique name. -/// -/// In this case, "unique" means either the full name is distinct or the -/// effective context is distinct. This method does not attempt to handle -/// "unresolved" contexts in any special way---if one name references a -/// particular Clang declaration and the other has an unresolved context that -/// will eventually reference that declaration, the contexts will still be -/// considered distinct. -/// -/// The names are generated in the same order as -/// forEachImportNameVersionFromCurrent. The current name is always first. -static void forEachDistinctName( - ClangImporter::Implementation &impl, const clang::NamedDecl *decl, +void ClangImporter::Implementation::forEachDistinctName( + const clang::NamedDecl *decl, llvm::function_ref action) { using ImportNameKey = std::pair; SmallVector seenNames; - forEachImportNameVersionFromCurrent(impl.CurrentVersion, + forEachImportNameVersionFromCurrent(CurrentVersion, [&](ImportNameVersion nameVersion) { // Check to see if the name is different. - ImportedName newName = impl.importFullName(decl, nameVersion); + ImportedName newName = importFullName(decl, nameVersion); ImportNameKey key(newName, newName.getEffectiveContext()); bool seen = llvm::any_of(seenNames, [&key](const ImportNameKey &existing) -> bool { @@ -2669,9 +2657,9 @@ namespace { switch (enumKind) { case EnumKind::Constants: case EnumKind::Unknown: - forEachDistinctName(Impl, constant, - [&](ImportedName newName, - ImportNameVersion nameVersion) { + Impl.forEachDistinctName(constant, + [&](ImportedName newName, + ImportNameVersion nameVersion) { Decl *imported = Impl.importDecl(constant, nameVersion); if (!imported) return; @@ -2682,9 +2670,9 @@ namespace { }); break; case EnumKind::Options: - forEachDistinctName(Impl, constant, - [&](ImportedName newName, - ImportNameVersion nameVersion) { + Impl.forEachDistinctName(constant, + [&](ImportedName newName, + ImportNameVersion nameVersion) { if (!contextIsEnum(newName)) return; SwiftDeclConverter converter(Impl, nameVersion); @@ -2741,9 +2729,9 @@ namespace { } } - forEachDistinctName(Impl, constant, - [&](ImportedName newName, - ImportNameVersion nameVersion) { + Impl.forEachDistinctName(constant, + [&](ImportedName newName, + ImportNameVersion nameVersion) { if (nameVersion == getActiveSwiftVersion()) return; if (!contextIsEnum(newName)) @@ -7737,8 +7725,8 @@ ClangImporter::Implementation::loadAllMembers(Decl *D, uint64_t extra) { // Only continue members in the same submodule as this extension. if (decl->getImportedOwningModule() != submodule) continue; - forEachDistinctName(*this, decl, [&](ImportedName newName, - ImportNameVersion nameVersion) { + forEachDistinctName(decl, [&](ImportedName newName, + ImportNameVersion nameVersion) { // Quickly check the context and bail out if it obviously doesn't // belong here. if (auto *importDC = newName.getEffectiveContext().getAsDeclContext()) @@ -7806,7 +7794,7 @@ ClangImporter::Implementation::loadAllMembers(Decl *D, uint64_t extra) { if (!nd || nd != nd->getCanonicalDecl()) continue; - forEachDistinctName(*this, nd, + forEachDistinctName(nd, [&](ImportedName name, ImportNameVersion nameVersion) { auto member = importDecl(nd, nameVersion); if (!member) diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h index b70a7b18c47..32f7c33dbea 100644 --- a/lib/ClangImporter/ImporterImpl.h +++ b/lib/ClangImporter/ImporterImpl.h @@ -1169,6 +1169,23 @@ public: /// Determine the effective Clang context for the given Swift nominal type. EffectiveClangContext getEffectiveClangContext(NominalTypeDecl *nominal); + /// Attempts to import the name of \p decl with each possible + /// ImportNameVersion. \p action will be called with each unique name. + /// + /// In this case, "unique" means either the full name is distinct or the + /// effective context is distinct. This method does not attempt to handle + /// "unresolved" contexts in any special way---if one name references a + /// particular Clang declaration and the other has an unresolved context that + /// will eventually reference that declaration, the contexts will still be + /// considered distinct. + /// + /// The names are generated in the same order as + /// forEachImportNameVersionFromCurrent. The current name is always first. + void forEachDistinctName( + const clang::NamedDecl *decl, + llvm::function_ref action); + /// Dump the Swift-specific name lookup tables we generate. void dumpSwiftLookupTables(); diff --git a/test/APINotes/Inputs/custom-frameworks/APINotesFrameworkTest.framework/Headers/APINotesFrameworkTest.apinotes b/test/APINotes/Inputs/custom-frameworks/APINotesFrameworkTest.framework/Headers/APINotesFrameworkTest.apinotes index 080dd575430..a4c65162a8e 100644 --- a/test/APINotes/Inputs/custom-frameworks/APINotesFrameworkTest.framework/Headers/APINotesFrameworkTest.apinotes +++ b/test/APINotes/Inputs/custom-frameworks/APINotesFrameworkTest.framework/Headers/APINotesFrameworkTest.apinotes @@ -103,6 +103,9 @@ SwiftVersions: - Name: "importantClassProperty" PropertyKind: Class SwiftName: "swift3ClassProperty" + - Name: "importantInstanceProperty" + PropertyKind: Instance + SwiftName: "swift3InstanceProperty" Protocols: - Name: ProtoWithVersionedUnavailableMember Methods: diff --git a/test/APINotes/Inputs/custom-frameworks/APINotesFrameworkTest.framework/Headers/Classes.h b/test/APINotes/Inputs/custom-frameworks/APINotesFrameworkTest.framework/Headers/Classes.h index fb1dccb26a0..73e44fab7c6 100644 --- a/test/APINotes/Inputs/custom-frameworks/APINotesFrameworkTest.framework/Headers/Classes.h +++ b/test/APINotes/Inputs/custom-frameworks/APINotesFrameworkTest.framework/Headers/Classes.h @@ -14,6 +14,7 @@ - (void)doImportantThings __attribute__((swift_name("finalDoImportantThings()"))); @property (class, nullable) id importantClassProperty __attribute__((swift_name("finalClassProperty"))); +@property (nullable) id importantInstanceProperty __attribute__((swift_name("finalInstanceProperty"))); @end #pragma clang assume_nonnull end diff --git a/test/APINotes/versioned-objc-dynamic-lookup.swift b/test/APINotes/versioned-objc-dynamic-lookup.swift new file mode 100644 index 00000000000..d3bf2b6fccb --- /dev/null +++ b/test/APINotes/versioned-objc-dynamic-lookup.swift @@ -0,0 +1,35 @@ +// RUN: rm -rf %t && mkdir -p %t + +// RUN: not %target-swift-frontend -typecheck -F %S/Inputs/custom-frameworks -swift-version 4 %s 2>&1 | %FileCheck -check-prefix=CHECK-DIAGS -check-prefix=CHECK-DIAGS-4 %s +// RUN: not %target-swift-frontend -typecheck -F %S/Inputs/custom-frameworks -swift-version 3 %s 2>&1 | %FileCheck -check-prefix=CHECK-DIAGS -check-prefix=CHECK-DIAGS-3 %s + +// REQUIRES: objc_interop + +import APINotesFrameworkTest + +func testRenamedClassMembers(obj: AnyObject) { + // CHECK-DIAGS-3: [[@LINE+1]]:{{[0-9]+}}: error: 'doImportantThings()' has been renamed to 'swift3DoImportantThings()' + obj.doImportantThings() + // CHECK-DIAGS-4: [[@LINE-1]]:{{[0-9]+}}: error: 'doImportantThings()' has been renamed to 'finalDoImportantThings()' + + // CHECK-DIAGS-3-NOT: :[[@LINE+1]]:{{[0-9]+}}: + obj.swift3DoImportantThings() + // CHECK-DIAGS-4: [[@LINE-1]]:{{[0-9]+}}: error: 'swift3DoImportantThings()' has been renamed to 'finalDoImportantThings()' + + // CHECK-DIAGS-3: [[@LINE+1]]:{{[0-9]+}}: error: 'finalDoImportantThings()' has been renamed to 'swift3DoImportantThings()' + obj.finalDoImportantThings() + // CHECK-DIAGS-4-NOT: :[[@LINE-1]]:{{[0-9]+}}: + + + // CHECK-DIAGS-3: [[@LINE+1]]:{{[0-9]+}}: error: 'importantInstanceProperty' has been renamed to 'swift3InstanceProperty' + _ = obj.importantInstanceProperty + // CHECK-DIAGS-4: [[@LINE-1]]:{{[0-9]+}}: error: 'importantInstanceProperty' has been renamed to 'finalInstanceProperty' + + // CHECK-DIAGS-3-NOT: :[[@LINE+1]]:{{[0-9]+}}: + _ = obj.swift3InstanceProperty + // CHECK-DIAGS-4: [[@LINE-1]]:{{[0-9]+}}: error: 'swift3InstanceProperty' has been renamed to 'finalInstanceProperty' + + // CHECK-DIAGS-3: [[@LINE+1]]:{{[0-9]+}}: error: 'finalInstanceProperty' has been renamed to 'swift3InstanceProperty' + _ = obj.finalInstanceProperty + // CHECK-DIAGS-4-NOT: :[[@LINE-1]]:{{[0-9]+}}: +} \ No newline at end of file