Skip to content

Commit bde58ed

Browse files
beccadaxbeccadax
committed
Flatten access note format
Rather than nesting notes for members inside a note for the type, access note files now contain a flat list of notes with parent names embedded in the `Name` property. Fixes rdar://71872830. fixup spacing in access notes
1 parent f99bfba commit bde58ed

File tree

3 files changed

+149
-91
lines changed

3 files changed

+149
-91
lines changed

Diff for: ‎include/swift/AST/AccessNotes.h

+16-2
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,24 @@ namespace swift {
2929
class ASTContext;
3030
class ValueDecl;
3131

32-
class AccessNote {
32+
class AccessNoteDeclName {
3333
public:
34+
std::vector<Identifier> parentNames;
3435
DeclName name;
35-
std::vector<AccessNote> members; // can't be SmallVector due to recursion
36+
37+
AccessNoteDeclName(ASTContext &ctx, StringRef str);
38+
AccessNoteDeclName();
39+
40+
bool matches(ValueDecl *VD) const;
41+
bool empty() const;
42+
43+
void print(llvm::raw_ostream &os) const;
44+
SWIFT_DEBUG_DUMP;
45+
};
46+
47+
class AccessNote {
48+
public:
49+
AccessNoteDeclName name;
3650

3751
Optional<bool> ObjC;
3852
Optional<bool> Dynamic;

Diff for: ‎lib/AST/AccessNotes.cpp

+85-46
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "swift/AST/AccessNotes.h"
1919
#include "swift/AST/Attr.h"
2020
#include "swift/AST/Decl.h"
21+
#include "swift/AST/Module.h" // DeclContext::isModuleScopeContext()
2122
#include "swift/Parse/Parser.h"
2223
#include "llvm/ADT/STLExtras.h"
2324
#include "llvm/ADT/StringRef.h"
@@ -61,39 +62,87 @@ parseObjCSelector(swift::ASTContext &ctx, llvm::StringRef string) {
6162

6263
namespace swift {
6364

64-
NullablePtr<const AccessNote> AccessNotes::lookup(ValueDecl *VD) const {
65-
assert(VD != nullptr);
65+
AccessNoteDeclName::AccessNoteDeclName() : parentNames(), name() { }
66+
67+
AccessNoteDeclName::AccessNoteDeclName(ASTContext &ctx, StringRef str) {
68+
auto parsedName = parseDeclName(str);
69+
70+
StringRef first, rest = parsedName.ContextName;
71+
while (!rest.empty()) {
72+
std::tie(first, rest) = rest.split('.');
73+
parentNames.push_back(ctx.getIdentifier(first));
74+
}
75+
76+
name = parsedName.formDeclName(ctx);
77+
78+
// FIXME: parseDeclName() doesn't handle the special `subscript` name.
79+
// Fixing this without affecting existing uses in import-as-member will need
80+
// a bit of work. Hack around the problem for this specific caller instead.
81+
if (name.getBaseName() == ctx.getIdentifier("subscript"))
82+
name = DeclName(ctx, DeclBaseName::createSubscript(),
83+
name.getArgumentNames());
84+
}
6685

86+
bool AccessNoteDeclName::matches(ValueDecl *VD) const {
6787
auto lookupVD = VD;
6888
if (auto accessor = dyn_cast<AccessorDecl>(VD))
6989
VD = accessor->getStorage();
7090

71-
auto lookupName = lookupVD->getName();
91+
if (!lookupVD->getName().matchesRef(name))
92+
return false;
7293

73-
// FIXME: parseDeclName() doesn't handle the special `subscript` name.
74-
// Fixing this without affecting existing uses in import-as-member will need
75-
// a bit of work. Hack around this by changing to a normal identifier.
76-
if (isa<SubscriptDecl>(lookupVD) &&
77-
lookupName.getBaseName() == DeclBaseName::createSubscript()) {
78-
ASTContext &ctx = lookupVD->getASTContext();
79-
lookupName = DeclName(ctx, ctx.getIdentifier("subscript"),
80-
lookupName.getArgumentNames());
81-
}
94+
// The rest of this checks `parentNames` against the parents of `lookupVD`.
95+
96+
ArrayRef<Identifier> remainingContextNames = parentNames;
97+
DeclContext *nextContext = lookupVD->getDeclContext();
98+
99+
while (!nextContext->isModuleScopeContext()) {
100+
// If we've run out of names without reaching module scope, we've failed.
101+
if (remainingContextNames.empty())
102+
return false;
82103

83-
const std::vector<AccessNote> *notesToSearch = &notes;
104+
Identifier contextName = remainingContextNames.back();
84105

85-
// If nested, look at the parent context's notes.
86-
if (auto parent = lookupVD->getDeclContext()->getSelfNominalTypeDecl()) {
87-
if (auto parentNote = lookup(parent))
88-
notesToSearch = &(parentNote.get()->members);
89-
else
90-
return nullptr;
106+
// If the context is not a type (or extension), we can't name VD in an
107+
// access note and the match fails; if the name doesn't match, the match
108+
// fails too.
109+
auto contextType = nextContext->getSelfNominalTypeDecl();
110+
if (!contextType || contextType->getName() != contextName)
111+
return false;
112+
113+
// Still checking. Move to the parent.
114+
remainingContextNames = remainingContextNames.drop_back();
115+
nextContext = contextType->getParent();
91116
}
92117

93-
auto iter = llvm::find_if(*notesToSearch, [&](const AccessNote &note) -> bool {
94-
return lookupName.matchesRef(note.name);
118+
// If the context is module-scoped, we've succeeded if we're out of names, or
119+
// failed if we still have some names to go.
120+
return remainingContextNames.empty();
121+
}
122+
123+
bool AccessNoteDeclName::empty() const {
124+
return !name;
125+
}
126+
127+
void AccessNoteDeclName::print(llvm::raw_ostream &os) const {
128+
for (auto parentName : parentNames)
129+
os << parentName << '.';
130+
name.print(os, /*skipEmptyArgumentNames=*/false);
131+
}
132+
133+
void AccessNoteDeclName::dump() const {
134+
print(llvm::errs());
135+
llvm::errs() << '\n';
136+
}
137+
138+
NullablePtr<const AccessNote> AccessNotes::lookup(ValueDecl *VD) const {
139+
assert(VD != nullptr);
140+
141+
auto iter = llvm::find_if(notes, [&](const AccessNote &note) -> bool {
142+
return note.name.matches(VD);
95143
});
96-
return NullablePtr<const AccessNote>(iter == notesToSearch->end() ? nullptr : &*iter);
144+
145+
return NullablePtr<const AccessNote>(iter == notes.end() ? nullptr : &*iter);
97146
}
98147

99148
void AccessNotes::dump() const {
@@ -114,8 +163,11 @@ void AccessNotes::dump(llvm::raw_ostream &os) const {
114163
}
115164

116165
void AccessNote::dump(llvm::raw_ostream &os, int indent) const {
117-
os.indent(indent) << "(note name='" << name << "'";
118-
if (name.getBaseName().isSpecial())
166+
os.indent(indent) << "(note name='";
167+
name.print(os);
168+
os << "'";
169+
170+
if (name.name.getBaseName().isSpecial())
119171
os << " is_special_name";
120172

121173
if (ObjC)
@@ -125,22 +177,12 @@ void AccessNote::dump(llvm::raw_ostream &os, int indent) const {
125177
if (Dynamic)
126178
os << " dynamic=" << *Dynamic;
127179

128-
if (!members.empty()) {
129-
os << "\n";
130-
os.indent(indent + 2) << "(members";
131-
for (const auto &member : members) {
132-
os << "\n";
133-
member.dump(os, indent + 4);
134-
}
135-
os << ")";
136-
}
137-
138180
os << ")";
139181
}
140182

141183
}
142184

143-
LLVM_YAML_DECLARE_SCALAR_TRAITS(swift::DeclName, QuotingType::Single)
185+
LLVM_YAML_DECLARE_SCALAR_TRAITS(swift::AccessNoteDeclName, QuotingType::Single);
144186
LLVM_YAML_DECLARE_SCALAR_TRAITS(swift::ObjCSelector, QuotingType::Single);
145187
LLVM_YAML_IS_SEQUENCE_VECTOR(swift::AccessNote)
146188
LLVM_YAML_DECLARE_MAPPING_TRAITS(swift::AccessNotes)
@@ -173,19 +215,19 @@ namespace yaml {
173215
using AccessNote = swift::AccessNote;
174216
using AccessNotes = swift::AccessNotes;
175217
using ASTContext = swift::ASTContext;
176-
using DeclName = swift::DeclName;
218+
using AccessNoteDeclName = swift::AccessNoteDeclName;
177219
using ObjCSelector = swift::ObjCSelector;
178220

179-
void ScalarTraits<DeclName>::output(const DeclName &name, void *ctxPtr,
180-
raw_ostream &os) {
181-
name.print(os, /*skipEmptyArgumentNames=*/false);
221+
void ScalarTraits<AccessNoteDeclName>::
222+
output(const AccessNoteDeclName &name, void *ctxPtr, raw_ostream &os) {
223+
name.print(os);
182224
}
183225

184-
StringRef ScalarTraits<DeclName>::input(StringRef str, void *ctxPtr,
185-
DeclName &name) {
226+
StringRef ScalarTraits<AccessNoteDeclName>::
227+
input(StringRef str, void *ctxPtr, AccessNoteDeclName &name) {
186228
ASTContext &ctx = *static_cast<ASTContext *>(ctxPtr);
187-
name = parseDeclName(ctx, str);
188-
return name ? "" : "invalid declaration name";
229+
name = AccessNoteDeclName(ctx, str);
230+
return name.empty() ? "invalid declaration name" : "";
189231
}
190232

191233
void ScalarTraits<ObjCSelector>::output(const ObjCSelector &selector,
@@ -207,12 +249,9 @@ StringRef ScalarTraits<ObjCSelector>::input(StringRef str, void *ctxPtr,
207249

208250
void MappingTraits<AccessNote>::mapping(IO &io, AccessNote &note) {
209251
io.mapRequired("Name", note.name);
210-
211252
io.mapOptional("ObjC", note.ObjC);
212253
io.mapOptional("Dynamic", note.Dynamic);
213254
io.mapOptional("ObjCName", note.ObjCName);
214-
215-
io.mapOptional("Members", note.members);
216255
}
217256

218257
StringRef MappingTraits<AccessNote>::validate(IO &io, AccessNote &note) {

Diff for: ‎test/SILGen/Inputs/objc_access_notes.accessnotes

+48-43
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,52 @@
11
---
22
Reason: fancy test suite
33
Notes:
4-
- Name: Hoozit
5-
Members:
6-
- Name: 'typical(_:y:)'
7-
ObjC: true
8-
- Name: 'copyFoo()'
9-
ObjC: true
10-
- Name: 'mutableCopyFoo()'
11-
ObjC: true
12-
- Name: 'copy8()'
13-
ObjC: true
14-
- Name: 'makeDuplicate()'
15-
ObjC: true
16-
ObjCName: 'copyDuplicate'
17-
- Name: 'mutableCopyFoo()'
18-
ObjC: true
19-
- Name: 'initFoo()'
20-
ObjC: true
21-
- Name: 'typicalProperty'
22-
ObjC: true
23-
- Name: 'copyProperty'
24-
ObjC: true
25-
- Name: 'propComputed'
26-
ObjC: true
27-
# FIXME: Add a way to specify getter and setter names in an access note
28-
- Name: 'roProperty'
29-
ObjC: true
30-
- Name: 'rwProperty'
31-
ObjC: true
32-
- Name: 'copyRWProperty'
33-
ObjC: true
34-
- Name: 'initProperty'
35-
ObjC: true
36-
- Name: 'subscript(_:)'
37-
ObjC: true
38-
- Name: 'init(int:)'
39-
ObjC: true
40-
Dynamic: true
41-
- Name: 'foof()'
42-
ObjC: true
43-
- Name: Wotsit
44-
Members:
45-
- Name: 'plain()'
46-
ObjC: true
474

5+
# These two should not match anything, but could accidentally match Hoozit.typical(_:y:)
6+
- Name: 'typical(_:y:)'
7+
ObjC: true
8+
ObjCName: 'pickedNoteWithNameThatWasTooShallow_typical:y:'
9+
- Name: 'Hoozit.Hoozit.typical(_:y:)'
10+
ObjC: true
11+
ObjCName: 'pickedNoteWithNameThatWasTooDeep_typical:y:'
12+
13+
# These entries should actually match things.
14+
- Name: 'Hoozit.typical(_:y:)'
15+
ObjC: true
16+
- Name: 'Hoozit.copyFoo()'
17+
ObjC: true
18+
- Name: 'Hoozit.mutableCopyFoo()'
19+
ObjC: true
20+
- Name: 'Hoozit.copy8()'
21+
ObjC: true
22+
- Name: 'Hoozit.makeDuplicate()'
23+
ObjC: true
24+
ObjCName: 'copyDuplicate'
25+
- Name: 'Hoozit.mutableCopyFoo()'
26+
ObjC: true
27+
- Name: 'Hoozit.initFoo()'
28+
ObjC: true
29+
- Name: 'Hoozit.typicalProperty'
30+
ObjC: true
31+
- Name: 'Hoozit.copyProperty'
32+
ObjC: true
33+
- Name: 'Hoozit.propComputed'
34+
ObjC: true
35+
# FIXME: Add a way to specify getter and setter names in an access note
36+
- Name: 'Hoozit.roProperty'
37+
ObjC: true
38+
- Name: 'Hoozit.rwProperty'
39+
ObjC: true
40+
- Name: 'Hoozit.copyRWProperty'
41+
ObjC: true
42+
- Name: 'Hoozit.initProperty'
43+
ObjC: true
44+
- Name: 'Hoozit.subscript(_:)'
45+
ObjC: true
46+
- Name: 'Hoozit.init(int:)'
47+
ObjC: true
48+
Dynamic: true
49+
- Name: 'Hoozit.foof()'
50+
ObjC: true
51+
- Name: 'Wotsit.plain()'
52+
ObjC: true

0 commit comments

Comments
 (0)