Skip to content

Commit 5776fa5

Browse files
DougGregorDougGregor
committed
Clang importer: migrate "Protocol" suffix computation into importFullName.
Refactoring that lets the Swift lookup tables get the names right for Objective-C protocols that would conflict with another entity in the same module (or within the bridging header). It's an NFC cleanup everywhere else.
1 parent 2244cb4 commit 5776fa5

File tree

3 files changed

+65
-54
lines changed

3 files changed

+65
-54
lines changed

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1679,6 +1679,67 @@ DeclName ClangImporter::Implementation::importFullName(
16791679
baseName = baseName.substr(removePrefix.size());
16801680
}
16811681

1682+
// Objective-C protocols may have the suffix "Protocol" appended if
1683+
// the non-suffixed name would conflict with another entity in the
1684+
// same top-level module.
1685+
SmallString<16> baseNameWithProtocolSuffix;
1686+
if (auto objcProto = dyn_cast<clang::ObjCProtocolDecl>(D)) {
1687+
if (objcProto->hasDefinition()) {
1688+
// Test to see if there is a value with the same name as the protocol
1689+
// in the same module.
1690+
// FIXME: This will miss macros.
1691+
auto clangModule = getClangSubmoduleForDecl(objcProto);
1692+
if (clangModule.hasValue() && clangModule.getValue())
1693+
clangModule = clangModule.getValue()->getTopLevelModule();
1694+
1695+
auto isInSameModule = [&](const clang::Decl *D) -> bool {
1696+
auto declModule = getClangSubmoduleForDecl(D);
1697+
if (!declModule.hasValue())
1698+
return false;
1699+
// Handle the bridging header case. This is pretty nasty since things
1700+
// can get added to it *later*, but there's not much we can do.
1701+
if (!declModule.getValue())
1702+
return *clangModule == nullptr;
1703+
return *clangModule == declModule.getValue()->getTopLevelModule();
1704+
};
1705+
1706+
// Allow this lookup to find hidden names. We don't want the
1707+
// decision about whether to rename the protocol to depend on
1708+
// what exactly the user has imported. Indeed, if we're being
1709+
// asked to resolve a serialization cross-reference, the user
1710+
// may not have imported this module at all, which means a
1711+
// normal lookup wouldn't even find the protocol!
1712+
//
1713+
// Meanwhile, we don't need to worry about finding unwanted
1714+
// hidden declarations from different modules because we do a
1715+
// module check before deciding that there's a conflict.
1716+
bool hasConflict = false;
1717+
clang::LookupResult lookupResult(getClangSema(), D->getDeclName(),
1718+
clang::SourceLocation(),
1719+
clang::Sema::LookupOrdinaryName);
1720+
lookupResult.setAllowHidden(true);
1721+
lookupResult.suppressDiagnostics();
1722+
1723+
if (getClangSema().LookupName(lookupResult, /*scope=*/nullptr)) {
1724+
hasConflict = std::any_of(lookupResult.begin(), lookupResult.end(),
1725+
isInSameModule);
1726+
}
1727+
if (!hasConflict) {
1728+
lookupResult.clear(clang::Sema::LookupTagName);
1729+
if (getClangSema().LookupName(lookupResult, /*scope=*/nullptr)) {
1730+
hasConflict = std::any_of(lookupResult.begin(), lookupResult.end(),
1731+
isInSameModule);
1732+
}
1733+
}
1734+
1735+
if (hasConflict) {
1736+
baseNameWithProtocolSuffix = baseName;
1737+
baseNameWithProtocolSuffix += SWIFT_PROTOCOL_SUFFIX;
1738+
baseName = baseNameWithProtocolSuffix;
1739+
}
1740+
}
1741+
}
1742+
16821743
// Local function to determine whether the given declaration is subject to
16831744
// a swift_private attribute.
16841745
auto hasSwiftPrivate = [this](const clang::NamedDecl *D) {

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -4825,59 +4825,6 @@ namespace {
48254825

48264826
decl = decl->getDefinition();
48274827

4828-
// Test to see if there is a value with the same name as the protocol
4829-
// in the same module.
4830-
// FIXME: This will miss macros.
4831-
auto clangModule = Impl.getClangSubmoduleForDecl(decl);
4832-
if (clangModule.hasValue() && clangModule.getValue())
4833-
clangModule = clangModule.getValue()->getTopLevelModule();
4834-
4835-
auto isInSameModule = [&](const clang::Decl *D) -> bool {
4836-
auto declModule = Impl.getClangSubmoduleForDecl(D);
4837-
if (!declModule.hasValue())
4838-
return false;
4839-
// Handle the bridging header case. This is pretty nasty since things
4840-
// can get added to it *later*, but there's not much we can do.
4841-
if (!declModule.getValue())
4842-
return *clangModule == nullptr;
4843-
return *clangModule == declModule.getValue()->getTopLevelModule();
4844-
};
4845-
4846-
// Allow this lookup to find hidden names. We don't want the
4847-
// decision about whether to rename the protocol to depend on
4848-
// what exactly the user has imported. Indeed, if we're being
4849-
// asked to resolve a serialization cross-reference, the user
4850-
// may not have imported this module at all, which means a
4851-
// normal lookup wouldn't even find the protocol!
4852-
//
4853-
// Meanwhile, we don't need to worry about finding unwanted
4854-
// hidden declarations from different modules because we do a
4855-
// module check before deciding that there's a conflict.
4856-
bool hasConflict = false;
4857-
clang::LookupResult lookupResult(Impl.getClangSema(), decl->getDeclName(),
4858-
clang::SourceLocation(),
4859-
clang::Sema::LookupOrdinaryName);
4860-
lookupResult.setAllowHidden(true);
4861-
lookupResult.suppressDiagnostics();
4862-
4863-
if (Impl.getClangSema().LookupName(lookupResult, /*scope=*/nullptr)) {
4864-
hasConflict = std::any_of(lookupResult.begin(), lookupResult.end(),
4865-
isInSameModule);
4866-
}
4867-
if (!hasConflict) {
4868-
lookupResult.clear(clang::Sema::LookupTagName);
4869-
if (Impl.getClangSema().LookupName(lookupResult, /*scope=*/nullptr)) {
4870-
hasConflict = std::any_of(lookupResult.begin(), lookupResult.end(),
4871-
isInSameModule);
4872-
}
4873-
}
4874-
4875-
if (hasConflict) {
4876-
SmallString<64> nameBuf{name.str()};
4877-
nameBuf += SWIFT_PROTOCOL_SUFFIX;
4878-
name = Impl.SwiftContext.getIdentifier(nameBuf.str());
4879-
}
4880-
48814828
auto dc = Impl.importDeclContextOf(decl);
48824829
if (!dc)
48834830
return nullptr;

‎test/IDE/dump_swift_lookup_tables_objc.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
// CHECK: Base -> full name mappings:
77
// CHECK-NEXT: SNCollision --> SNCollision
8+
// CHECK-NEXT: SNCollisionProtocol --> SNCollisionProtocol
89
// CHECK-NEXT: SomeClass --> SomeClass
910
// CHECK-NEXT: SomeProtocol --> SomeProtocol
1011
// CHECK-NEXT: categoryMethodWithX --> categoryMethodWithX(_:y:), categoryMethodWithX(_:y:z:)
@@ -19,7 +20,9 @@
1920

2021
// CHECK: Full name -> entry mappings:
2122
// CHECK-NEXT: SNCollision:
22-
// CHECK-NEXT: TU: SNCollision, SNCollision
23+
// CHECK-NEXT: TU: SNCollision{{$}}
24+
// CHECK-NEXT: SNCollisionProtocol:
25+
// CHECK-NEXT: TU: SNCollision{{$}}
2326
// CHECK-NEXT: SomeClass:
2427
// CHECK-NEXT: TU: SNSomeClass
2528
// CHECK-NEXT: SomeProtocol:

0 commit comments

Comments
 (0)