Skip to content

Commit c9124d9

Browse files
committed
[ClangImporter] Import Swift 3 versions of top-level decls in Swift 4.
...and Swift 4 versions in Swift 3, and Swift 2 and "raw" versions in both. This allows the compiler to produce sensible errors and fix-its when someone uses the "wrong" name for an API. The diagnostics certainly have room to improve, but at least the essentials are there. Note that this commit only addresses /top-level/ decls, i.e. those found by lookup into a module. We're still limited to producing all members of a nominal type up front, so that'll require a slightly different approach. Part of rdar://problem/29170671
1 parent 84ca8ec commit c9124d9

File tree

11 files changed

+214
-58
lines changed

11 files changed

+214
-58
lines changed

lib/ClangImporter/ClangImporter.cpp

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2738,17 +2738,51 @@ void ClangImporter::Implementation::lookupValue(
27382738
}
27392739
}
27402740

2741-
// If we have a declaration and nothing matched so far, try the Swift 2
2742-
// name.
2741+
// If we have a declaration and nothing matched so far, try the names used
2742+
// in other versions of Swift.
27432743
if (!anyMatching) {
27442744
if (auto clangDecl = entry.dyn_cast<clang::NamedDecl *>()) {
2745-
if (auto swift2Decl = cast_or_null<ValueDecl>(importDeclReal(
2746-
clangDecl->getMostRecentDecl(), ImportNameVersion::Swift2))) {
2747-
if (swift2Decl->getFullName().matchesRef(name) &&
2748-
swift2Decl->getDeclContext()->isModuleScopeContext()) {
2749-
consumer.foundDecl(swift2Decl,
2745+
const clang::NamedDecl *recentClangDecl =
2746+
clangDecl->getMostRecentDecl();
2747+
auto tryImport = [&](ImportNameVersion nameVersion) -> bool {
2748+
// Check to see if the name and context match what we expect.
2749+
ImportedName newName = importFullName(recentClangDecl, nameVersion);
2750+
if (!newName.getDeclName().matchesRef(name))
2751+
return false;
2752+
2753+
const clang::DeclContext *clangDC =
2754+
newName.getEffectiveContext().getAsDeclContext();
2755+
if (!clangDC || !clangDC->isFileContext())
2756+
return false;
2757+
2758+
// Then try to import the decl under the alternate name.
2759+
auto alternateNamedDecl =
2760+
cast_or_null<ValueDecl>(importDeclReal(recentClangDecl,
2761+
nameVersion));
2762+
if (!alternateNamedDecl)
2763+
return false;
2764+
assert(alternateNamedDecl->getFullName().matchesRef(name) &&
2765+
"importFullName behaved differently from importDecl");
2766+
if (alternateNamedDecl->getDeclContext()->isModuleScopeContext()) {
2767+
consumer.foundDecl(alternateNamedDecl,
27502768
DeclVisibilityKind::VisibleAtTopLevel);
2769+
return true;
27512770
}
2771+
return false;
2772+
};
2773+
2774+
// Try importing previous versions of the decl first...
2775+
ImportNameVersion nameVersion = CurrentVersion;
2776+
while (!anyMatching && nameVersion != ImportNameVersion::Raw) {
2777+
--nameVersion;
2778+
anyMatching = tryImport(nameVersion);
2779+
}
2780+
// ...then move on to newer versions if none of the old versions
2781+
// matched.
2782+
nameVersion = CurrentVersion;
2783+
while (!anyMatching && nameVersion != ImportNameVersion::LAST_VERSION) {
2784+
++nameVersion;
2785+
anyMatching = tryImport(nameVersion);
27522786
}
27532787
}
27542788
}

lib/ClangImporter/ImportDecl.cpp

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2009,10 +2009,34 @@ namespace {
20092009
/*fullyQualified=*/correctSwiftName.importAsMember(), os);
20102010
}
20112011

2012-
auto attr = AvailableAttr::createPlatformAgnostic(
2013-
ctx, StringRef(), ctx.AllocateCopy(renamed.str()),
2014-
PlatformAgnosticAvailabilityKind::SwiftVersionSpecific,
2015-
clang::VersionTuple(3));
2012+
unsigned majorVersion = majorVersionNumberForNameVersion(getVersion());
2013+
DeclAttribute *attr;
2014+
if (isActiveSwiftVersion() || getVersion() == ImportNameVersion::Raw) {
2015+
// "Raw" is the Objective-C name, which was never available in Swift.
2016+
// Variants within the active version are usually declarations that
2017+
// have been superseded, like the accessors of a property.
2018+
attr = AvailableAttr::createPlatformAgnostic(
2019+
ctx, /*Message*/StringRef(), ctx.AllocateCopy(renamed.str()),
2020+
PlatformAgnosticAvailabilityKind::UnavailableInSwift);
2021+
} else if (getVersion() < getActiveSwiftVersion()) {
2022+
// A Swift 2 name, for example, was obsoleted in Swift 3.
2023+
attr = AvailableAttr::createPlatformAgnostic(
2024+
ctx, /*Message*/StringRef(), ctx.AllocateCopy(renamed.str()),
2025+
PlatformAgnosticAvailabilityKind::SwiftVersionSpecific,
2026+
clang::VersionTuple(majorVersion + 1));
2027+
} else {
2028+
// Future names are introduced in their future version.
2029+
assert(getVersion() > getActiveSwiftVersion());
2030+
attr = new (ctx) AvailableAttr(
2031+
SourceLoc(), SourceRange(), PlatformKind::none,
2032+
/*Message*/StringRef(), ctx.AllocateCopy(renamed.str()),
2033+
/*Introduced*/clang::VersionTuple(majorVersion), SourceRange(),
2034+
/*Deprecated*/clang::VersionTuple(), SourceRange(),
2035+
/*Obsoleted*/clang::VersionTuple(), SourceRange(),
2036+
PlatformAgnosticAvailabilityKind::SwiftVersionSpecific,
2037+
/*Implicit*/true);
2038+
}
2039+
20162040
decl->getAttrs().add(attr);
20172041
decl->setImplicit();
20182042
}
@@ -3485,31 +3509,12 @@ namespace {
34853509
{decl->param_begin(), decl->param_size()},
34863510
decl->isVariadic(), redundant);
34873511

3488-
// Directly ask the NameImporter for the non-init variant of the Swift 2
3489-
// name.
3490-
auto rawName = Impl.importFullName(decl, ImportNameVersion::Raw);
3491-
if (!rawName)
3492-
return result;
3493-
3494-
auto rawDecl = importNonInitObjCMethodDecl(decl, dc, rawName, selector,
3495-
forceClassMethod);
3496-
if (!rawDecl)
3497-
return result;
3498-
3499-
// Mark the raw imported class method "unavailable", with a useful error
3500-
// message.
3501-
llvm::SmallString<64> message;
3502-
llvm::raw_svector_ostream os(message);
3503-
os << "use object construction '" << decl->getClassInterface()->getName()
3504-
<< "(";
3505-
for (auto arg : importedName.getDeclName().getArgumentNames()) {
3506-
os << arg << ":";
3512+
if (auto rawDecl = Impl.importDecl(decl, ImportNameVersion::Raw)) {
3513+
// We expect the raw decl to always be a method.
3514+
assert(isa<FuncDecl>(rawDecl));
3515+
Impl.addAlternateDecl(result, cast<ValueDecl>(rawDecl));
35073516
}
3508-
os << ")'";
3509-
rawDecl->getAttrs().add(AvailableAttr::createPlatformAgnostic(
3510-
Impl.SwiftContext, Impl.SwiftContext.AllocateCopy(os.str())));
3511-
markAsVariant(rawDecl, importedName);
3512-
Impl.addAlternateDecl(result, cast<ValueDecl>(rawDecl));
3517+
35133518
return result;
35143519
}
35153520

lib/ClangImporter/ImportName.cpp

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,20 @@ importer::nameVersionFromOptions(const LangOptions &langOpts) {
6868
}
6969
}
7070

71+
unsigned importer::majorVersionNumberForNameVersion(ImportNameVersion version) {
72+
switch (version) {
73+
case ImportNameVersion::Raw:
74+
return 0;
75+
case ImportNameVersion::Swift2:
76+
return 2;
77+
case ImportNameVersion::Swift3:
78+
return 3;
79+
case ImportNameVersion::Swift4:
80+
return 4;
81+
}
82+
}
83+
84+
7185
/// Determine whether the given Clang selector matches the given
7286
/// selector pieces.
7387
static bool isNonNullarySelector(clang::Selector selector,
@@ -560,17 +574,48 @@ determineCtorInitializerKind(const clang::ObjCMethodDecl *method) {
560574
return None;
561575
}
562576

577+
template <typename A>
578+
static bool matchesVersion(A *versionedAttr, ImportNameVersion version) {
579+
clang::VersionTuple attrVersion = versionedAttr->getVersion();
580+
if (attrVersion.empty())
581+
return version == ImportNameVersion::LAST_VERSION;
582+
return attrVersion.getMajor() == majorVersionNumberForNameVersion(version);
583+
}
584+
563585
const clang::SwiftNameAttr *
564586
importer::findSwiftNameAttr(const clang::Decl *decl,
565587
ImportNameVersion version) {
566-
// Find the attribute.
588+
if (version == ImportNameVersion::Raw)
589+
return nullptr;
590+
591+
// Handle versioned API notes for Swift 3 and later. This is the common case.
592+
if (version != ImportNameVersion::Swift2) {
593+
for (auto *attr : decl->attrs()) {
594+
if (auto *versionedAttr = dyn_cast<clang::SwiftVersionedAttr>(attr)) {
595+
if (!matchesVersion(versionedAttr, version))
596+
continue;
597+
if (auto *added =
598+
dyn_cast<clang::SwiftNameAttr>(versionedAttr->getAttrToAdd())) {
599+
return added;
600+
}
601+
}
602+
603+
if (auto *removeAttr = dyn_cast<clang::SwiftVersionedRemovalAttr>(attr)) {
604+
if (!matchesVersion(removeAttr, version))
605+
continue;
606+
if (removeAttr->getAttrKindToRemove() == clang::attr::SwiftName)
607+
return nullptr;
608+
}
609+
}
610+
611+
return decl->getAttr<clang::SwiftNameAttr>();
612+
}
613+
614+
// The remainder of this function emulates the limited form of swift_name
615+
// supported in Swift 2.
567616
auto attr = decl->getAttr<clang::SwiftNameAttr>();
568617
if (!attr) return nullptr;
569618

570-
// If we're not emulating the Swift 2 behavior, return what we got.
571-
if (version != ImportNameVersion::Swift2)
572-
return attr;
573-
574619
// API notes produce implicit attributes; ignore them because they weren't
575620
// used for naming in Swift 2.
576621
if (attr->isImplicit()) return nullptr;

lib/ClangImporter/ImportName.h

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,6 @@
2525
#include "swift/AST/ForeignErrorConvention.h"
2626
#include "clang/Sema/Sema.h"
2727

28-
// TODO: remove when we drop import name options
29-
#include "clang/AST/Decl.h"
30-
3128
namespace swift {
3229
namespace importer {
3330
struct PlatformAvailability;
@@ -55,18 +52,36 @@ enum class ImportNameVersion : unsigned {
5552

5653
/// Names as they appeared in Swift 4 family
5754
Swift4,
55+
56+
/// A placeholder for the latest version, to be used in loops and such.
57+
LAST_VERSION = Swift4
5858
};
59-
enum { NumImportNameVersions = 4 };
6059

6160
static inline void
6261
forEachImportNameVersion(llvm::function_ref<void(ImportNameVersion)> action) {
63-
for (unsigned raw = 0; raw < NumImportNameVersions; ++raw)
62+
auto limit = static_cast<unsigned>(ImportNameVersion::LAST_VERSION);
63+
for (unsigned raw = 0; raw <= limit; ++raw)
6464
action(static_cast<ImportNameVersion>(raw));
6565
}
6666

67-
/// Map a language version into an import name version
67+
static inline ImportNameVersion &operator++(ImportNameVersion &value) {
68+
assert(value != ImportNameVersion::LAST_VERSION);
69+
value = static_cast<ImportNameVersion>(static_cast<unsigned>(value) + 1);
70+
return value;
71+
}
72+
73+
static inline ImportNameVersion &operator--(ImportNameVersion &value) {
74+
assert(value != ImportNameVersion::Raw);
75+
value = static_cast<ImportNameVersion>(static_cast<unsigned>(value) - 1);
76+
return value;
77+
}
78+
79+
/// Map a language version into an import name version.
6880
ImportNameVersion nameVersionFromOptions(const LangOptions &langOpts);
6981

82+
/// Map an import name version into a language version.
83+
unsigned majorVersionNumberForNameVersion(ImportNameVersion version);
84+
7085
/// Describes a name that was imported from Clang.
7186
class ImportedName {
7287
friend class NameImporter;

lib/ClangImporter/SwiftLookupTable.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1541,9 +1541,8 @@ void importer::addEntryToLookupTable(SwiftLookupTable &table,
15411541
}
15421542

15431543
// If we have a name to import as, add this entry to the table.
1544-
// FIXME: It doesn't actually matter which version we use here, but it should
1545-
// probably follow the ASTContext anyway.
1546-
ImportNameVersion currentVersion = ImportNameVersion::Swift3;
1544+
ImportNameVersion currentVersion =
1545+
nameVersionFromOptions(nameImporter.getLangOpts());
15471546
if (auto importedName = nameImporter.importName(named, currentVersion)) {
15481547
SmallPtrSet<DeclName, 8> distinctNames;
15491548
distinctNames.insert(importedName.getDeclName());

test/APINotes/versioned.swift

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,28 @@
99

1010
// CHECK-SWIFT-4: func accept(_ ptr: UnsafeMutablePointer<Double>)
1111
// CHECK-SWIFT-3: func acceptPointer(_ ptr: UnsafeMutablePointer<Double>?)
12+
13+
// 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
14+
// 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
15+
16+
import APINotesFrameworkTest
17+
18+
func testRenamedTopLevel() {
19+
var value = 0.0
20+
21+
// CHECK-DIAGS-4-NOT: versioned.swift:[[@LINE+1]]
22+
accept(&value)
23+
// CHECK-DIAGS-3: versioned.swift:[[@LINE-1]]:3: error: 'accept' has been renamed to 'acceptPointer(_:)'
24+
// CHECK-DIAGS-3: note: 'accept' was introduced in Swift 4
25+
26+
// CHECK-DIAGS-4-NOT: versioned.swift:[[@LINE+1]]
27+
acceptPointer(&value)
28+
// CHECK-DIAGS-4: versioned.swift:[[@LINE-1]]:3: error: 'acceptPointer' has been renamed to 'accept(_:)'
29+
// CHECK-DIAGS-4: note: 'acceptPointer' was obsoleted in Swift 4
30+
31+
acceptDoublePointer(&value)
32+
// CHECK-DIAGS: versioned.swift:[[@LINE-1]]:3: error: 'acceptDoublePointer' has been renamed to
33+
// CHECK-DIAGS-4: 'accept(_:)'
34+
// CHECK-DIAGS-3: 'acceptPointer(_:)'
35+
// CHECK-DIAGS: note: 'acceptDoublePointer' was obsoleted in Swift 3
36+
}

test/ClangImporter/Inputs/SwiftPrivateAttr.txt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,14 @@ class Foo : NSObject, __PrivProto {
99
func __oneArg(_ arg: Int32)
1010
func __twoArgs(_ arg: Int32, other arg2: Int32)
1111
class func __withNoArgs() -> Self!
12+
@available(*, unavailable, renamed: "init(__oneArg:)", message: "Not available in Swift")
13+
class func __fooWithOneArg(_ arg: Int32) -> Self!
1214
convenience init!(__oneArg arg: Int32)
15+
@available(*, unavailable, renamed: "init(__twoArgs:other:)", message: "Not available in Swift")
16+
class func __fooWithTwoArgs(_ arg: Int32, other arg2: Int32) -> Self!
1317
convenience init!(__twoArgs arg: Int32, other arg2: Int32)
18+
@available(*, unavailable, renamed: "init(__:)", message: "Not available in Swift")
19+
class func __foo(_ arg: Int32) -> Self!
1420
convenience init!(__ arg: Int32)
1521
func objectForKeyedSubscript(_ index: Any!) -> Any!
1622
func __setObject(_ object: Any!, forKeyedSubscript index: Any!)

test/ClangImporter/attr-swift_private.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,8 @@ _ = 1 as __PrivInt
135135

136136
#if !IRGEN
137137
func testRawNames() {
138-
let _ = Foo.__fooWithOneArg(0) // expected-error {{'__fooWithOneArg' is unavailable: use object construction 'Foo(__oneArg:)'}}
139-
let _ = Foo.__foo // expected-error{{'__foo' is unavailable: use object construction 'Foo(__:)'}}
138+
let _ = Foo.__fooWithOneArg(0) // expected-error {{'__fooWithOneArg' has been replaced by 'init(__oneArg:)'}}
139+
let _ = Foo.__foo // expected-error{{'__foo' has been replaced by 'init(__:)'}}
140140
}
141141
#endif
142142

test/ClangImporter/objc_factory_method.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func testNonInstanceTypeFactoryMethod(_ s: String) {
8484
}
8585

8686
func testUseOfFactoryMethod(_ queen: Bee) {
87-
_ = Hive.hiveWithQueen(queen) // expected-error{{'hiveWithQueen' is unavailable: use object construction 'Hive(queen:)'}}
87+
_ = Hive.hiveWithQueen(queen) // expected-error{{'hiveWithQueen' has been replaced by 'init(queen:)'}} {{11-25=}} {{26-26=queen: }}
8888
}
8989

9090
func testNonsplittableFactoryMethod() {
@@ -97,18 +97,18 @@ func testFactoryMethodBlacklist() {
9797

9898
func test17261609() {
9999
_ = NSDecimalNumber(mantissa:1, exponent:1, isNegative:true)
100-
_ = NSDecimalNumber.decimalNumberWithMantissa(1, exponent:1, isNegative:true) // expected-error{{'decimalNumberWithMantissa(_:exponent:isNegative:)' is unavailable: use object construction 'NSDecimalNumber(mantissa:exponent:isNegative:)'}}
100+
_ = NSDecimalNumber.decimalNumberWithMantissa(1, exponent:1, isNegative:true) // expected-error{{'decimalNumberWithMantissa(_:exponent:isNegative:)' has been replaced by 'init(mantissa:exponent:isNegative:)'}} {{22-48=}} {{49-49=mantissa: }}
101101
}
102102

103103
func testURL() {
104104
let url = NSURL(string: "http://www.llvm.org")!
105-
_ = NSURL.URLWithString("http://www.llvm.org") // expected-error{{'URLWithString' is unavailable: use object construction 'NSURL(string:)'}}
105+
_ = NSURL.URLWithString("http://www.llvm.org") // expected-error{{'URLWithString' has been replaced by 'init(string:)'}} {{12-26=}} {{27-27=string: }}
106106

107107
NSURLRequest(string: "http://www.llvm.org") // expected-warning{{unused}}
108108
NSURLRequest(url: url as URL) // expected-warning{{unused}}
109109

110-
_ = NSURLRequest.requestWithString("http://www.llvm.org") // expected-error{{'requestWithString' is unavailable: use object construction 'NSURLRequest(string:)'}}
111-
_ = NSURLRequest.URLRequestWithURL(url as URL) // expected-error{{'URLRequestWithURL' is unavailable: use object construction 'NSURLRequest(url:)'}}
110+
_ = NSURLRequest.requestWithString("http://www.llvm.org") // expected-error{{'requestWithString' has been replaced by 'init(string:)'}}
111+
_ = NSURLRequest.URLRequestWithURL(url as URL) // expected-error{{'URLRequestWithURL' has been replaced by 'init(url:)'}}
112112
}
113113

114114
// FIXME: Remove -verify-ignore-unknown.

test/ClangImporter/objc_implicit_with.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func testNonInstanceTypeFactoryMethod(_ s: String) {
5050
}
5151

5252
func testUseOfFactoryMethod(_ queen: Bee) {
53-
_ = Hive.hiveWithQueen(queen) // expected-error{{'hiveWithQueen' is unavailable: use object construction 'Hive(queen:)'}}
53+
_ = Hive.hiveWithQueen(queen) // expected-error{{'hiveWithQueen' has been replaced by 'init(queen:)'}} {{11-25=}} {{26-26=queen: }}
5454
}
5555

5656
func testNonsplittableFactoryMethod() {

0 commit comments

Comments
 (0)