Skip to content

Commit 2183d38

Browse files
redsun82redsun82
committed
Swift: implement review suggestions
1 parent 1504736 commit 2183d38

File tree

5 files changed

+40
-80
lines changed

5 files changed

+40
-80
lines changed

‎swift/extractor/infra/SwiftLocationExtractor.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sour
2525
entry.file = fetchFileLabel(file);
2626
std::tie(entry.start_line, entry.start_column) = sourceManager.getLineAndColumnInBuffer(start);
2727
std::tie(entry.end_line, entry.end_column) = sourceManager.getLineAndColumnInBuffer(end);
28-
SwiftMangledName locName{{"loc", entry.file, ":", entry.start_line, ":", entry.start_column, ":",
29-
entry.end_line, ":", entry.end_column}};
28+
SwiftMangledName locName{"loc", entry.file, ':', entry.start_line, ':', entry.start_column,
29+
':', entry.end_line, ':', entry.end_column};
3030
entry.id = trap.createTypedLabel<DbLocationTag>(locName);
3131
trap.emit(entry);
3232
trap.emit(LocatableLocationsTrap{locatableLabel, entry.id});
@@ -99,7 +99,7 @@ TrapLabel<FileTag> SwiftLocationExtractor::fetchFileLabel(const std::filesystem:
9999
}
100100

101101
DbFile entry({});
102-
entry.id = trap.createTypedLabel<DbFileTag>({{"file_", file.string()}});
102+
entry.id = trap.createTypedLabel<DbFileTag>({"file_", file.string()});
103103
entry.name = file.string();
104104
trap.emit(entry);
105105
store[file] = entry.id;

‎swift/extractor/infra/SwiftMangledName.cpp‎

Lines changed: 2 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -3,52 +3,13 @@
33

44
namespace codeql {
55

6-
namespace {
7-
void appendPart(std::string& out, const std::string& prefix) {
8-
out += prefix;
9-
}
10-
11-
void appendPart(std::string& out, UntypedTrapLabel label) {
12-
absl::StrAppend(&out, "{", label.str(), "}");
13-
}
14-
15-
void appendPart(std::string& out, unsigned index) {
16-
absl::StrAppend(&out, index);
17-
}
18-
19-
} // namespace
20-
21-
std::string SwiftMangledName::str() const {
22-
std::string out;
23-
for (const auto& part : parts) {
24-
std::visit([&](const auto& contents) { appendPart(out, contents); }, part);
25-
}
26-
return out;
27-
}
28-
296
SwiftMangledName& SwiftMangledName::operator<<(UntypedTrapLabel label) & {
30-
assert(label && "using undefined label in mangled name");
31-
parts.emplace_back(label);
7+
absl::StrAppend(&value, "{", label.str(), "}");
328
return *this;
339
}
3410

3511
SwiftMangledName& SwiftMangledName::operator<<(unsigned int i) & {
36-
parts.emplace_back(i);
37-
return *this;
38-
}
39-
40-
SwiftMangledName& SwiftMangledName::operator<<(SwiftMangledName&& other) & {
41-
parts.reserve(parts.size() + other.parts.size());
42-
for (auto& p : other.parts) {
43-
parts.emplace_back(std::move(p));
44-
}
45-
other.parts.clear();
46-
return *this;
47-
}
48-
49-
SwiftMangledName& SwiftMangledName::operator<<(const SwiftMangledName& other) & {
50-
parts.reserve(parts.size() + other.parts.size());
51-
parts.insert(parts.end(), other.parts.begin(), other.parts.end());
12+
absl::StrAppend(&value, i);
5213
return *this;
5314
}
5415

‎swift/extractor/infra/SwiftMangledName.h‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,13 @@
99

1010
namespace codeql {
1111

12-
struct SwiftMangledName {
12+
class SwiftMangledName {
13+
public:
1314
using Part = std::variant<UntypedTrapLabel, std::string, unsigned>;
1415

15-
std::vector<Part> parts;
16+
explicit operator bool() const { return !value.empty(); }
1617

17-
explicit operator bool() const { return !parts.empty(); }
18-
19-
std::string str() const;
18+
const std::string& str() const { return value; }
2019

2120
// let's avoid copying as long as we don't need it
2221
SwiftMangledName() = default;
@@ -25,28 +24,40 @@ struct SwiftMangledName {
2524
SwiftMangledName(SwiftMangledName&&) = default;
2625
SwiftMangledName& operator=(SwiftMangledName&&) = default;
2726

27+
template <typename... Args>
28+
SwiftMangledName(Args&&... args) {
29+
(operator<<(std::forward<Args>(args)), ...);
30+
}
31+
2832
// streaming labels or ints into a SwiftMangledName just appends them
2933
SwiftMangledName& operator<<(UntypedTrapLabel label) &;
3034
SwiftMangledName& operator<<(unsigned i) &;
3135

36+
template <typename Tag>
37+
SwiftMangledName& operator<<(TrapLabel<Tag> label) & {
38+
return operator<<(static_cast<UntypedTrapLabel>(label));
39+
}
40+
3241
// streaming string-like stuff will add a new part it only if strictly required, otherwise it will
3342
// append to the last part if it is a string
3443
template <typename T>
3544
SwiftMangledName& operator<<(T&& arg) & {
36-
if (parts.empty() || !std::holds_alternative<std::string>(parts.back())) {
37-
parts.emplace_back("");
38-
}
39-
std::get<std::string>(parts.back()) += std::forward<T>(arg);
45+
value += arg;
4046
return *this;
4147
}
4248

43-
SwiftMangledName& operator<<(SwiftMangledName&& other) &;
44-
SwiftMangledName& operator<<(const SwiftMangledName& other) &;
49+
SwiftMangledName& operator<<(const SwiftMangledName& other) {
50+
value += other.value;
51+
return *this;
52+
}
4553

4654
template <typename T>
4755
SwiftMangledName&& operator<<(T&& arg) && {
4856
return std::move(operator<<(std::forward<T>(arg)));
4957
}
58+
59+
private:
60+
std::string value;
5061
};
5162

5263
} // namespace codeql

‎swift/extractor/mangler/SwiftMangler.cpp‎

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,22 @@ std::string_view getTypeKindStr(const swift::TypeBase* type) {
4040

4141
template <typename E>
4242
UntypedTrapLabel SwiftMangler::fetch(E&& e) {
43-
return dispatcher.fetchLabel(std::forward<E>(e));
43+
auto ret = dispatcher.fetchLabel(std::forward<E>(e));
44+
// TODO use a generic logging handle for Swift entities here, once it's available
45+
CODEQL_ASSERT(ret.valid(), "using an undefined label in mangling");
46+
return ret;
4447
}
4548

4649
SwiftMangledName SwiftMangler::initMangled(const swift::TypeBase* type) {
47-
return SwiftMangledName() << getTypeKindStr(type) << '_';
50+
return {getTypeKindStr(type), '_'};
4851
}
4952

5053
SwiftMangledName SwiftMangler::initMangled(const swift::Decl* decl) {
51-
return SwiftMangledName() << swift::Decl::getKindName(decl->getKind()) << "Decl_"
52-
<< fetch(getParent(decl));
54+
return {swift::Decl::getKindName(decl->getKind()), "Decl_", fetch(getParent(decl))};
5355
}
5456

5557
SwiftMangledName SwiftMangler::mangleModuleName(std::string_view name) {
56-
return SwiftMangledName() << "ModuleDecl_" << name;
58+
return {"ModuleDecl_", name};
5759
}
5860

5961
SwiftMangledName SwiftMangler::visitModuleDecl(const swift::ModuleDecl* decl) {
@@ -124,10 +126,12 @@ unsigned SwiftMangler::getExtensionIndex(const swift::ExtensionDecl* decl,
124126
} else if (auto iterableParent = llvm::dyn_cast<swift::IterableDeclContext>(parent)) {
125127
indexExtensions(iterableParent->getAllMembers());
126128
} else {
127-
assert(false && "non-local context must be module or iterable decl context");
129+
// TODO use a generic logging handle for Swift entities here, once it's available
130+
CODEQL_ASSERT(false, "non-local context must be module or iterable decl context");
128131
}
129132
auto found = preloadedExtensionIndexes.extract(decl);
130-
assert(found && "extension not found within parent");
133+
// TODO use a generic logging handle for Swift entities here, once it's available
134+
CODEQL_ASSERT(found, "extension not found within parent");
131135
return found.mapped();
132136
}
133137

@@ -141,32 +145,17 @@ void SwiftMangler::indexExtensions(llvm::ArrayRef<swift::Decl*> siblings) {
141145
}
142146
}
143147

144-
SwiftMangledName SwiftMangler::visitGenericTypeDecl(const swift::GenericTypeDecl* decl) {
145-
auto ret = visitValueDecl(decl);
146-
if (!ret) {
147-
return {};
148-
}
149-
if (auto genericParams = decl->getParsedGenericParams()) {
150-
ret << '<' << genericParams->size() << '>';
151-
}
152-
return ret;
153-
}
154-
155148
SwiftMangledName SwiftMangler::visitAbstractTypeParamDecl(
156149
const swift::AbstractTypeParamDecl* decl) {
157150
return visitValueDecl(decl, /* force */ true);
158151
}
159152

160153
SwiftMangledName SwiftMangler::visitGenericTypeParamDecl(const swift::GenericTypeParamDecl* decl) {
161-
auto ret = visitAbstractTypeParamDecl(decl);
162-
ret << '_' << decl->getDepth() << '_' << decl->getIndex();
163-
return ret;
154+
return visitAbstractTypeParamDecl(decl) << '_' << decl->getDepth() << '_' << decl->getIndex();
164155
}
165156

166157
SwiftMangledName SwiftMangler::visitModuleType(const swift::ModuleType* type) {
167-
auto ret = initMangled(type);
168-
ret << fetch(type->getModule());
169-
return ret;
158+
return initMangled(type) << fetch(type->getModule());
170159
}
171160

172161
SwiftMangledName SwiftMangler::visitTupleType(const swift::TupleType* type) {

‎swift/extractor/mangler/SwiftMangler.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ class SwiftMangler : private swift::TypeVisitor<SwiftMangler, SwiftMangledName>,
6363
SwiftMangledName visitAbstractFunctionDecl(const swift::AbstractFunctionDecl* decl);
6464
SwiftMangledName visitSubscriptDecl(const swift::SubscriptDecl* decl);
6565
SwiftMangledName visitVarDecl(const swift::VarDecl* decl);
66-
SwiftMangledName visitGenericTypeDecl(const swift::GenericTypeDecl* decl);
6766
SwiftMangledName visitAbstractTypeParamDecl(const swift::AbstractTypeParamDecl* decl);
6867
SwiftMangledName visitGenericTypeParamDecl(const swift::GenericTypeParamDecl* decl);
6968

0 commit comments

Comments
 (0)