Skip to content

Commit e7dc05e

Browse files
egorzhdanegorzhdan
committed
[APINotes] Handle nested namespaces and tags in namespaces correctly
There might be several different namespaces with the same name, e.g. `foo` as both `bar::foo` and `bar::baz::foo`. To disambiguate between them, we need to store the parent context of each context. rdar://113403829
1 parent 61d27bf commit e7dc05e

File tree

9 files changed

+117
-43
lines changed

9 files changed

+117
-43
lines changed

‎clang/include/clang/APINotes/APINotesReader.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,9 @@ class APINotesReader {
213213
/// \param name The name of the class we're looking for.
214214
///
215215
/// \returns The ID, if known.
216-
llvm::Optional<ContextID> lookupNamespaceID(llvm::StringRef name);
216+
llvm::Optional<ContextID>
217+
lookupNamespaceID(llvm::Optional<ContextID> parentNamespaceID,
218+
llvm::StringRef name);
217219
};
218220

219221
} // end namespace api_notes

‎clang/include/clang/APINotes/APINotesWriter.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ class APINotesWriter {
5555
///
5656
/// \returns the ID of the class, protocol, or namespace, which can be used to
5757
/// add properties and methods to the class/protocol/namespace.
58-
ContextID addObjCContext(llvm::StringRef name, ContextKind contextKind,
58+
ContextID addObjCContext(std::optional<ContextID> parentContextID,
59+
llvm::StringRef name, ContextKind contextKind,
5960
const ObjCContextInfo &info,
6061
llvm::VersionTuple swiftVersion);
6162

‎clang/lib/APINotes/APINotesReader.cpp

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,8 @@ namespace {
187187
/// Used to deserialize the on-disk Objective-C class table.
188188
class ObjCContextIDTableInfo {
189189
public:
190-
// identifier ID, context kind
191-
using internal_key_type = std::pair<unsigned, char>;
190+
// parent context ID, context kind, identifier ID
191+
using internal_key_type = std::tuple<uint32_t, uint8_t, uint32_t>;
192192
using external_key_type = internal_key_type;
193193
using data_type = unsigned;
194194
using hash_value_type = size_t;
@@ -218,10 +218,11 @@ namespace {
218218
}
219219

220220
static internal_key_type ReadKey(const uint8_t *data, unsigned length) {
221-
auto nameID
222-
= endian::readNext<uint32_t, little, unaligned>(data);
221+
auto parentContextID =
222+
endian::readNext<uint32_t, little, unaligned>(data);
223223
auto contextKind = endian::readNext<uint8_t, little, unaligned>(data);
224-
return { nameID, contextKind };
224+
auto nameID = endian::readNext<uint32_t, little, unaligned>(data);
225+
return {parentContextID, contextKind, nameID};
225226
}
226227

227228
static data_type ReadData(internal_key_type key, const uint8_t *data,
@@ -1829,8 +1830,10 @@ auto APINotesReader::lookupObjCClassID(StringRef name) -> Optional<ContextID> {
18291830
if (!classID)
18301831
return None;
18311832

1832-
auto knownID =
1833-
Impl.ObjCContextIDTable->find({*classID, (uint8_t)ContextKind::ObjCClass});
1833+
// TODO: Can ObjC classes be declared in C++ namespaces? If so, we should pass
1834+
// the parent context and use it instead of -1.
1835+
auto knownID = Impl.ObjCContextIDTable->find(
1836+
{-1, (uint8_t)ContextKind::ObjCClass, *classID});
18341837
if (knownID == Impl.ObjCContextIDTable->end())
18351838
return None;
18361839

@@ -1862,8 +1865,10 @@ auto APINotesReader::lookupObjCProtocolID(StringRef name)
18621865
if (!classID)
18631866
return None;
18641867

1868+
// TODO: Can ObjC classes be declared in C++ namespaces? If so, we should pass
1869+
// the parent context and use it instead of -1.
18651870
auto knownID = Impl.ObjCContextIDTable->find(
1866-
{*classID, (uint8_t)ContextKind::ObjCProtocol});
1871+
{-1, (uint8_t)ContextKind::ObjCProtocol, *classID});
18671872
if (knownID == Impl.ObjCContextIDTable->end())
18681873
return None;
18691874

@@ -2022,7 +2027,8 @@ auto APINotesReader::lookupTypedef(std::optional<Context> context,
20222027
return { Impl.SwiftVersion, *known };
20232028
}
20242029

2025-
auto APINotesReader::lookupNamespaceID(StringRef name)
2030+
auto APINotesReader::lookupNamespaceID(
2031+
Optional<ContextID> parentNamespaceID, StringRef name)
20262032
-> Optional<ContextID> {
20272033
if (!Impl.ObjCContextIDTable)
20282034
return None;
@@ -2031,8 +2037,10 @@ auto APINotesReader::lookupNamespaceID(StringRef name)
20312037
if (!namespaceID)
20322038
return None;
20332039

2040+
uint32_t rawParentNamespaceID =
2041+
parentNamespaceID ? parentNamespaceID->Value : -1;
20342042
auto knownID = Impl.ObjCContextIDTable->find(
2035-
{*namespaceID, (char)ContextKind::Namespace});
2043+
{rawParentNamespaceID, (char)ContextKind::Namespace, *namespaceID});
20362044
if (knownID == Impl.ObjCContextIDTable->end())
20372045
return None;
20382046

‎clang/lib/APINotes/APINotesWriter.cpp

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,21 @@ class APINotesWriter::Implementation {
5656

5757
bool SwiftInferImportAsMember = false;
5858

59-
/// Information about Objective-C contexts (classes or protocols).
59+
/// Information about contexts (Objective-C classes or protocols or C++
60+
/// namespaces).
6061
///
61-
/// Indexed by the identifier ID and a bit indication whether we're looking
62-
/// for a class (0) or protocol (1) and provides both the context ID and
63-
/// information describing the context within that module.
64-
llvm::DenseMap<std::pair<unsigned, char>,
62+
/// Indexed by the parent context ID, context kind and the identifier ID of
63+
/// this context and provides both the context ID and information describing
64+
/// the context within that module.
65+
llvm::DenseMap<std::tuple<uint32_t, uint8_t, uint32_t>,
6566
std::pair<unsigned, VersionedSmallVector<ObjCContextInfo>>>
6667
ObjCContexts;
6768

69+
/// Information about parent contexts for each context.
70+
///
71+
/// Indexed by context ID, provides the parent context ID.
72+
llvm::DenseMap<uint32_t, uint32_t> ParentContexts;
73+
6874
/// Mapping from context IDs to the identifier ID holding the name.
6975
llvm::DenseMap<unsigned, unsigned> ObjCContextNames;
7076

@@ -378,7 +384,7 @@ namespace {
378384
/// Used to serialize the on-disk Objective-C context table.
379385
class ObjCContextIDTableInfo {
380386
public:
381-
using key_type = std::pair<unsigned, char>; // identifier ID, is-protocol
387+
using key_type = std::tuple<uint32_t, uint8_t, uint32_t>; // parent context ID, context kind, identifier ID
382388
using key_type_ref = key_type;
383389
using data_type = unsigned;
384390
using data_type_ref = const data_type &;
@@ -392,7 +398,8 @@ namespace {
392398
std::pair<unsigned, unsigned> EmitKeyDataLength(raw_ostream &out,
393399
key_type_ref key,
394400
data_type_ref data) {
395-
uint32_t keyLength = sizeof(uint32_t) + 1;
401+
uint32_t keyLength =
402+
sizeof(uint32_t) + sizeof(uint8_t) + sizeof(uint32_t);
396403
uint32_t dataLength = sizeof(uint32_t);
397404
endian::Writer writer(out, little);
398405
writer.write<uint16_t>(keyLength);
@@ -402,8 +409,9 @@ namespace {
402409

403410
void EmitKey(raw_ostream &out, key_type_ref key, unsigned len) {
404411
endian::Writer writer(out, little);
405-
writer.write<uint32_t>(key.first);
406-
writer.write<uint8_t>(key.second);
412+
writer.write<uint32_t>(std::get<0>(key));
413+
writer.write<uint8_t>(std::get<1>(key));
414+
writer.write<uint32_t>(std::get<2>(key));
407415
}
408416

409417
void EmitData(raw_ostream &out, key_type_ref key, data_type_ref data,
@@ -1226,13 +1234,16 @@ void APINotesWriter::writeToStream(raw_ostream &os) {
12261234
Impl.writeToStream(os);
12271235
}
12281236

1229-
ContextID APINotesWriter::addObjCContext(StringRef name,
1230-
ContextKind contextKind,
1231-
const ObjCContextInfo &info,
1232-
VersionTuple swiftVersion) {
1237+
ContextID
1238+
APINotesWriter::addObjCContext(std::optional<ContextID> parentContextID,
1239+
StringRef name, ContextKind contextKind,
1240+
const ObjCContextInfo &info,
1241+
VersionTuple swiftVersion) {
12331242
IdentifierID nameID = Impl.getIdentifier(name);
12341243

1235-
std::pair<unsigned, char> key(nameID, (uint8_t)contextKind);
1244+
uint32_t rawParentContextID = parentContextID ? parentContextID->Value : -1;
1245+
std::tuple<uint32_t, uint8_t, uint32_t> key = {rawParentContextID,
1246+
(uint8_t)contextKind, nameID};
12361247
auto known = Impl.ObjCContexts.find(key);
12371248
if (known == Impl.ObjCContexts.end()) {
12381249
unsigned nextID = Impl.ObjCContexts.size() + 1;
@@ -1243,6 +1254,7 @@ ContextID APINotesWriter::addObjCContext(StringRef name,
12431254
.first;
12441255

12451256
Impl.ObjCContextNames[nextID] = nameID;
1257+
Impl.ParentContexts[nextID] = rawParentContextID;
12461258
}
12471259

12481260
// Add this version information.
@@ -1284,12 +1296,13 @@ void APINotesWriter::addObjCMethod(ContextID contextID,
12841296
// If this method is a designated initializer, update the class to note that
12851297
// it has designated initializers.
12861298
if (info.DesignatedInit) {
1287-
assert(Impl.ObjCContexts.count({Impl.ObjCContextNames[contextID.Value],
1288-
(char)ContextKind::ObjCClass}));
1289-
auto &versionedVec =
1290-
Impl.ObjCContexts[{Impl.ObjCContextNames[contextID.Value],
1291-
(char)ContextKind::ObjCClass}]
1292-
.second;
1299+
assert(Impl.ParentContexts.count(contextID.Value));
1300+
uint32_t parentContextID = Impl.ParentContexts[contextID.Value];
1301+
std::tuple<uint32_t, uint8_t, uint32_t> ctxKey = {
1302+
parentContextID, (char)ContextKind::ObjCClass,
1303+
Impl.ObjCContextNames[contextID.Value]};
1304+
assert(Impl.ObjCContexts.count(ctxKey));
1305+
auto &versionedVec = Impl.ObjCContexts[ctxKey].second;
12931306
bool found = false;
12941307
for (auto &versioned : versionedVec) {
12951308
if (versioned.first == swiftVersion) {

‎clang/lib/APINotes/APINotesYAMLCompiler.cpp

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -920,7 +920,8 @@ namespace {
920920
mInfo, swiftVersion);
921921
}
922922

923-
void convertContext(const Class &cl, ContextKind contextKind,
923+
void convertContext(std::optional<ContextID> parentContextID,
924+
const Class &cl, ContextKind contextKind,
924925
VersionTuple swiftVersion) {
925926
// Write the class.
926927
ObjCContextInfo cInfo;
@@ -935,8 +936,8 @@ namespace {
935936
if (cl.SwiftObjCMembers)
936937
cInfo.setSwiftObjCMembers(*cl.SwiftObjCMembers);
937938

938-
ContextID clID = Writer->addObjCContext(cl.Name, contextKind, cInfo,
939-
swiftVersion);
939+
ContextID clID = Writer->addObjCContext(parentContextID, cl.Name,
940+
contextKind, cInfo, swiftVersion);
940941

941942
// Write all methods.
942943
llvm::StringMap<std::pair<bool, bool>> knownMethods;
@@ -997,16 +998,18 @@ namespace {
997998
}
998999
}
9991000

1000-
void convertNamespaceContext(const Namespace &ns,
1001+
void convertNamespaceContext(std::optional<ContextID> parentContextID,
1002+
const Namespace &ns,
10011003
VersionTuple swiftVersion) {
10021004
// Write the namespace.
10031005
ObjCContextInfo cInfo;
10041006

10051007
if (convertCommon(ns, cInfo, ns.Name))
10061008
return;
10071009

1008-
ContextID clID = Writer->addObjCContext(ns.Name, ContextKind::Namespace,
1009-
cInfo, swiftVersion);
1010+
ContextID clID =
1011+
Writer->addObjCContext(parentContextID, ns.Name,
1012+
ContextKind::Namespace, cInfo, swiftVersion);
10101013

10111014
convertTopLevelItems(Context(clID, ContextKind::Namespace), ns.Items,
10121015
swiftVersion);
@@ -1015,6 +1018,9 @@ namespace {
10151018
void convertTopLevelItems(std::optional<Context> context,
10161019
const TopLevelItems &items,
10171020
VersionTuple swiftVersion) {
1021+
std::optional<ContextID> contextID =
1022+
context ? std::optional(context->id) : std::nullopt;
1023+
10181024
// Write all classes.
10191025
llvm::StringSet<> knownClasses;
10201026
for (const auto &cl : items.Classes) {
@@ -1024,7 +1030,7 @@ namespace {
10241030
continue;
10251031
}
10261032

1027-
convertContext(cl, ContextKind::ObjCClass, swiftVersion);
1033+
convertContext(contextID, cl, ContextKind::ObjCClass, swiftVersion);
10281034
}
10291035

10301036
// Write all protocols.
@@ -1036,7 +1042,7 @@ namespace {
10361042
continue;
10371043
}
10381044

1039-
convertContext(pr, ContextKind::ObjCProtocol, swiftVersion);
1045+
convertContext(contextID, pr, ContextKind::ObjCProtocol, swiftVersion);
10401046
}
10411047

10421048
// Write all namespaces.
@@ -1048,7 +1054,7 @@ namespace {
10481054
continue;
10491055
}
10501056

1051-
convertNamespaceContext(ns, swiftVersion);
1057+
convertNamespaceContext(contextID, ns, swiftVersion);
10521058
}
10531059

10541060
// Write all global variables.

‎clang/lib/Sema/SemaAPINotes.cpp

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -819,9 +819,28 @@ void Sema::ProcessAPINotes(Decl *D) {
819819
if (auto NamespaceContext = dyn_cast<NamespaceDecl>(D->getDeclContext())) {
820820
for (auto Reader :
821821
APINotes.findAPINotes(NamespaceContext->getLocation())) {
822-
if (auto id = Reader->lookupNamespaceID(NamespaceContext->getName())) {
823-
APINotesContext = {*id, api_notes::ContextKind::Namespace};
822+
// Retrieve the context ID for the parent namespace of the decl.
823+
std::stack<NamespaceDecl *> NamespaceStack;
824+
{
825+
auto CurrentNamespace = NamespaceContext;
826+
while (CurrentNamespace) {
827+
NamespaceStack.push(CurrentNamespace);
828+
CurrentNamespace =
829+
dyn_cast<NamespaceDecl>(CurrentNamespace->getParent());
830+
}
831+
}
832+
Optional<api_notes::ContextID> NamespaceID;
833+
while (!NamespaceStack.empty()) {
834+
auto CurrentNamespace = NamespaceStack.top();
835+
NamespaceStack.pop();
836+
NamespaceID = Reader->lookupNamespaceID(NamespaceID,
837+
CurrentNamespace->getName());
838+
if (!NamespaceID)
839+
break;
824840
}
841+
if (NamespaceID)
842+
APINotesContext = api_notes::Context(
843+
*NamespaceID, api_notes::ContextKind::Namespace);
825844
}
826845
}
827846

‎clang/test/APINotes/Inputs/Frameworks/CXXInteropKit.framework/Headers/CXXInteropKit.apinotes

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,8 @@ Namespaces:
2525
Tags:
2626
- Name: char_box
2727
SwiftName: NestedCharBox
28+
Namespaces:
29+
- Name: Namespace1
30+
Tags:
31+
- Name: char_box
32+
SwiftName: DeepNestedCharBox

‎clang/test/APINotes/Inputs/Frameworks/CXXInteropKit.framework/Headers/CXXInteropKit.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,9 @@ struct char_box { char c; };
2727

2828
namespace Nested1 {
2929
struct char_box { char c; };
30+
31+
namespace Namespace1 {
32+
struct char_box { char c; };
33+
} // namespace Namespace1
3034
} // namespace Nested1
3135
} // namespace Namespace1

‎clang/test/APINotes/objcxx-swift-name.m

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22
// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/CxxInterop -fdisable-module-hash -fapinotes-modules -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -x objective-c++
33
// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/CxxInterop -fdisable-module-hash -fapinotes-modules -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter SomeClass -x objective-c++ | FileCheck %s
44
// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/CxxInterop -fdisable-module-hash -fapinotes-modules -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter "(anonymous)" -x objective-c++ | FileCheck -check-prefix=CHECK-ANONYMOUS-ENUM %s
5+
// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/CxxInterop -fdisable-module-hash -fapinotes-modules -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter Namespace1::funcInNamespace -x objective-c++ | FileCheck -check-prefix=CHECK-FUNC-IN-NAMESPACE %s
56
// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/CxxInterop -fdisable-module-hash -fapinotes-modules -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter Namespace1::char_box -x objective-c++ | FileCheck -check-prefix=CHECK-STRUCT-IN-NAMESPACE %s
67
// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/CxxInterop -fdisable-module-hash -fapinotes-modules -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter Namespace1::Nested1::char_box -x objective-c++ | FileCheck -check-prefix=CHECK-STRUCT-IN-NESTED-NAMESPACE %s
8+
// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/CxxInterop -fdisable-module-hash -fapinotes-modules -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter Namespace1::Nested1::funcInNestedNamespace -x objective-c++ | FileCheck -check-prefix=CHECK-FUNC-IN-NESTED-NAMESPACE %s
9+
// RUN: %clang_cc1 -fmodules -fblocks -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache/CxxInterop -fdisable-module-hash -fapinotes-modules -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -ast-dump -ast-dump-filter Namespace1::Nested1::Namespace1::char_box -x objective-c++ | FileCheck -check-prefix=CHECK-STRUCT-IN-DEEP-NESTED-NAMESPACE %s
710

811
#import <CXXInteropKit/CXXInteropKit.h>
912

@@ -24,10 +27,23 @@
2427
// CHECK-ANONYMOUS-ENUM-NEXT: EnumDecl {{.+}} imported in CXXInteropKit <undeserialized declarations> 'NSSomeEnumOptions':'unsigned long'
2528
// CHECK-ANONYMOUS-ENUM-NEXT: SwiftNameAttr {{.+}} <<invalid sloc>> "SomeEnum.Options"
2629

30+
// CHECK-FUNC-IN-NAMESPACE: Dumping Namespace1::funcInNamespace:
31+
// CHECK-FUNC-IN-NAMESPACE-NEXT: FunctionDecl {{.+}} imported in CXXInteropKit funcInNamespace 'void ()'
32+
// CHECK-FUNC-IN-NAMESPACE-NEXT: SwiftNameAttr {{.+}} <<invalid sloc>> "swiftFuncInNamespace()"
33+
2734
// CHECK-STRUCT-IN-NAMESPACE: Dumping Namespace1::char_box:
2835
// CHECK-STRUCT-IN-NAMESPACE-NEXT: CXXRecordDecl {{.+}} imported in CXXInteropKit <undeserialized declarations> struct char_box
2936
// CHECK-STRUCT-IN-NAMESPACE: SwiftNameAttr {{.+}} <<invalid sloc>> "CharBox"
3037

38+
// CHECK-FUNC-IN-NESTED-NAMESPACE: Dumping Namespace1::Nested1::funcInNestedNamespace:
39+
// CHECK-FUNC-IN-NESTED-NAMESPACE-NEXT: FunctionDecl {{.+}} imported in CXXInteropKit funcInNestedNamespace 'void (int)'
40+
// CHECK-FUNC-IN-NESTED-NAMESPACE-NEXT: ParmVarDecl {{.+}} i 'int'
41+
// CHECK-FUNC-IN-NESTED-NAMESPACE-NEXT: SwiftNameAttr {{.+}} <<invalid sloc>> "swiftFuncInNestedNamespace(_:)"
42+
3143
// CHECK-STRUCT-IN-NESTED-NAMESPACE: Dumping Namespace1::Nested1::char_box:
3244
// CHECK-STRUCT-IN-NESTED-NAMESPACE-NEXT: CXXRecordDecl {{.+}} imported in CXXInteropKit <undeserialized declarations> struct char_box
3345
// CHECK-STRUCT-IN-NESTED-NAMESPACE: SwiftNameAttr {{.+}} <<invalid sloc>> "NestedCharBox"
46+
47+
// CHECK-STRUCT-IN-DEEP-NESTED-NAMESPACE: Dumping Namespace1::Nested1::Namespace1::char_box:
48+
// CHECK-STRUCT-IN-DEEP-NESTED-NAMESPACE-NEXT: CXXRecordDecl {{.+}} imported in CXXInteropKit <undeserialized declarations> struct char_box
49+
// CHECK-STRUCT-IN-DEEP-NESTED-NAMESPACE: SwiftNameAttr {{.+}} <<invalid sloc>> "DeepNestedCharBox"

0 commit comments

Comments
 (0)