Skip to content

Commit 7c764f3

Browse files
committed
Swift: use C++20 constraints and concepts to simplify code
This simplifies several instances of metaprogramming by leveraging [constraints and concepts from C++20][1]. This: * gets rid of `std::enable_if` by usage of `requires`, making it more readable and yield better compiler messages. * uses `requires` instead of `static_assert` to enforce `TrapLabel` typing * simplifies all compile-time tests for validity of a given expression * uses some standard library concepts where possible * generalizes and simplifies `SwiftLocationExtractor` Notice that in order to use the `std::derived_from` concept, `virtual` inheritance had to be added to the label tags, because diamond inheritance is a problem otherwise. That's because `std::derived_from<T, U>` requires that `T*` be convertible to `U*`, which is false if there are multiple non-virtual inheritance paths from `U` to `T`. As tags never get actually instantiated, there is no runtime performance penalty in using `virtual` inheritance. [1]: https://en.cppreference.com/w/cpp/language/constraints
1 parent 75cc1d8 commit 7c764f3

File tree

6 files changed

+111
-250
lines changed

6 files changed

+111
-250
lines changed

misc/codegen/templates/trap_tags_h.mustache

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace codeql {
66
{{#tags}}
77

88
// {{id}}
9-
struct {{name}}Tag {{#has_bases}}: {{#bases}}{{^first}}, {{/first}}{{base}}Tag{{/bases}} {{/has_bases}}{
9+
struct {{name}}Tag {{#has_bases}}: {{#bases}}{{^first}}, {{/first}}virtual {{base}}Tag{{/bases}} {{/has_bases}}{
1010
static constexpr const char* prefix = "{{name}}";
1111
};
1212
{{/tags}}

swift/extractor/infra/SwiftDispatcher.h

Lines changed: 19 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,6 @@ class SwiftDispatcher {
3838
const swift::PoundAvailableInfo*,
3939
const swift::AvailabilitySpec*>;
4040

41-
template <typename E>
42-
static constexpr bool IsFetchable = std::is_constructible_v<Handle, const E&>;
43-
44-
template <typename E>
45-
static constexpr bool IsLocatable =
46-
std::is_base_of_v<LocatableTag, TrapTagOf<E>> && !std::is_base_of_v<TypeTag, TrapTagOf<E>>;
47-
48-
template <typename E>
49-
static constexpr bool IsDeclPointer = std::is_convertible_v<E, const swift::Decl*>;
50-
51-
template <typename E>
52-
static constexpr bool IsTypePointer = std::is_convertible_v<E, const swift::TypeBase*>;
53-
5441
public:
5542
// all references and pointers passed as parameters to this constructor are supposed to outlive
5643
// the SwiftDispatcher
@@ -76,7 +63,7 @@ class SwiftDispatcher {
7663
using Label = std::remove_reference_t<decltype(label)>;
7764
if (!label.valid()) {
7865
const char* action;
79-
if constexpr (std::is_base_of_v<typename Label::Tag, UnspecifiedElementTag>) {
66+
if constexpr (std::derived_from<UnspecifiedElementTag, typename Label::Tag>) {
8067
action = "replacing with unspecified element";
8168
label = emitUnspecified(idOf(entry), field, index);
8269
} else {
@@ -132,7 +119,7 @@ class SwiftDispatcher {
132119

133120
template <typename E>
134121
std::optional<TrapLabel<ElementTag>> idOf(const E& entry) {
135-
if constexpr (HasId<E>::value) {
122+
if constexpr (requires { entry.id; }) {
136123
return entry.id;
137124
} else {
138125
return std::nullopt;
@@ -142,9 +129,10 @@ class SwiftDispatcher {
142129
// This method gives a TRAP label for already emitted AST node.
143130
// If the AST node was not emitted yet, then the emission is dispatched to a corresponding
144131
// visitor (see `visit(T *)` methods below).
145-
template <typename E, std::enable_if_t<IsFetchable<E>>* = nullptr>
146-
TrapLabelOf<E> fetchLabel(const E& e, swift::Type type = {}) {
147-
if constexpr (std::is_constructible_v<bool, const E&>) {
132+
template <typename E>
133+
requires std::constructible_from<Handle, E> TrapLabelOf<E> fetchLabel(const E& e,
134+
swift::Type type = {}) {
135+
if constexpr (std::constructible_from<bool, const E&>) {
148136
if (!e) {
149137
// this will be treated on emission
150138
return undefined_label;
@@ -174,8 +162,8 @@ class SwiftDispatcher {
174162
return ret;
175163
}
176164

177-
template <typename E, std::enable_if_t<IsFetchable<E*>>* = nullptr>
178-
TrapLabelOf<E> fetchLabel(const E& e) {
165+
template <typename E>
166+
requires std::constructible_from<Handle, E*> TrapLabelOf<E> fetchLabel(const E& e) {
179167
return fetchLabel(&e);
180168
}
181169

@@ -184,8 +172,9 @@ class SwiftDispatcher {
184172
auto createEntry(const E& e) {
185173
auto found = store.find(&e);
186174
CODEQL_ASSERT(found != store.end(), "createEntry called on non-fetched label");
187-
auto label = TrapLabel<ConcreteTrapTagOf<E>>::unsafeCreateFromUntyped(found->second);
188-
if constexpr (IsLocatable<E>) {
175+
using Tag = ConcreteTrapTagOf<E>;
176+
auto label = TrapLabel<Tag>::unsafeCreateFromUntyped(found->second);
177+
if constexpr (requires { locationExtractor.attachLocation(sourceManager, e, label); }) {
189178
locationExtractor.attachLocation(sourceManager, e, label);
190179
}
191180
return TrapClassOf<E>{label};
@@ -195,7 +184,8 @@ class SwiftDispatcher {
195184
// an example is swift::Argument, that are created on the fly and thus have no stable pointer
196185
template <typename E>
197186
auto createUncachedEntry(const E& e) {
198-
auto label = trap.createTypedLabel<TrapTagOf<E>>();
187+
using Tag = TrapTagOf<E>;
188+
auto label = trap.createTypedLabel<Tag>();
199189
locationExtractor.attachLocation(sourceManager, &e, label);
200190
return TrapClassOf<E>{label};
201191
}
@@ -218,7 +208,7 @@ class SwiftDispatcher {
218208
auto fetchRepeatedLabels(Iterable&& arg) {
219209
using Label = decltype(fetchLabel(*arg.begin()));
220210
TrapLabelVectorWrapper<typename Label::Tag> ret;
221-
if constexpr (HasSize<Iterable>::value) {
211+
if constexpr (std::ranges::sized_range<Iterable>) {
222212
ret.data.reserve(arg.size());
223213
}
224214
for (auto&& e : arg) {
@@ -251,7 +241,7 @@ class SwiftDispatcher {
251241
private:
252242
template <typename E>
253243
UntypedTrapLabel createLabel(const E& e, swift::Type type) {
254-
if constexpr (IsDeclPointer<E> || IsTypePointer<E>) {
244+
if constexpr (requires { name(e); }) {
255245
if (auto mangledName = name(e)) {
256246
if (shouldVisit(e)) {
257247
toBeVisited.emplace_back(e, type);
@@ -266,7 +256,7 @@ class SwiftDispatcher {
266256

267257
template <typename E>
268258
bool shouldVisit(const E& e) {
269-
if constexpr (IsDeclPointer<E>) {
259+
if constexpr (std::convertible_to<E, const swift::Decl*>) {
270260
encounteredModules.insert(e->getModuleContext());
271261
if (bodyEmissionStrategy.shouldEmitDeclBody(*e)) {
272262
extractedDeclaration(e);
@@ -295,18 +285,6 @@ class SwiftDispatcher {
295285
module->isNonSwiftModule();
296286
}
297287

298-
template <typename T, typename = void>
299-
struct HasSize : std::false_type {};
300-
301-
template <typename T>
302-
struct HasSize<T, decltype(std::declval<T>().size(), void())> : std::true_type {};
303-
304-
template <typename T, typename = void>
305-
struct HasId : std::false_type {};
306-
307-
template <typename T>
308-
struct HasId<T, decltype(std::declval<T>().id, void())> : std::true_type {};
309-
310288
template <typename Tag, typename... Ts>
311289
TrapLabel<Tag> fetchLabelFromUnion(const llvm::PointerUnion<Ts...> u) {
312290
TrapLabel<Tag> ret{};
@@ -324,7 +302,7 @@ class SwiftDispatcher {
324302
// on `BraceStmt`/`IfConfigDecl` elements), we cannot encounter a standalone `TypeRepr` there,
325303
// so we skip this case; extracting `TypeRepr`s here would be problematic as we would not be
326304
// able to provide the corresponding type
327-
if constexpr (!std::is_same_v<T, swift::TypeRepr*>) {
305+
if constexpr (!std::same_as<T, swift::TypeRepr*>) {
328306
if (auto e = u.template dyn_cast<T>()) {
329307
output = fetchLabel(e);
330308
return true;
@@ -348,10 +326,8 @@ class SwiftDispatcher {
348326
virtual void visit(const swift::TypeBase* type) = 0;
349327
virtual void visit(const swift::CapturedValue* capture) = 0;
350328

351-
template <typename T, std::enable_if<!std::is_base_of_v<swift::TypeRepr, T>>* = nullptr>
352-
void visit(const T* e, swift::Type) {
353-
visit(e);
354-
}
329+
template <typename T>
330+
requires(!std::derived_from<T, swift::TypeRepr>) void visit(const T* e, swift::Type) { visit(e); }
355331

356332
const swift::SourceManager& sourceManager;
357333
SwiftExtractorState& state;

swift/extractor/infra/SwiftLocationExtractor.cpp

Lines changed: 11 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,24 @@
1212

1313
using namespace codeql;
1414

15+
swift::SourceRange detail::getSourceRange(const swift::Token& token) {
16+
const auto charRange = token.getRange();
17+
return {charRange.getStart(), charRange.getEnd()};
18+
}
19+
1520
void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager,
16-
swift::SourceLoc start,
17-
swift::SourceLoc end,
21+
const swift::SourceRange& range,
1822
TrapLabel<LocatableTag> locatableLabel) {
19-
if (!start.isValid() || !end.isValid()) {
23+
if (!range) {
2024
// invalid locations seem to come from entities synthesized by the compiler
2125
return;
2226
}
23-
auto file = resolvePath(sourceManager.getDisplayNameForLoc(start));
27+
auto file = resolvePath(sourceManager.getDisplayNameForLoc(range.Start));
2428
DbLocation entry{{}};
2529
entry.file = fetchFileLabel(file);
26-
std::tie(entry.start_line, entry.start_column) = sourceManager.getLineAndColumnInBuffer(start);
27-
std::tie(entry.end_line, entry.end_column) = sourceManager.getLineAndColumnInBuffer(end);
30+
std::tie(entry.start_line, entry.start_column) =
31+
sourceManager.getLineAndColumnInBuffer(range.Start);
32+
std::tie(entry.end_line, entry.end_column) = sourceManager.getLineAndColumnInBuffer(range.End);
2833
SwiftMangledName locName{"loc", entry.file, ':', entry.start_line, ':', entry.start_column,
2934
':', entry.end_line, ':', entry.end_column};
3035
entry.id = trap.createTypedLabel<DbLocationTag>(locName);
@@ -43,56 +48,6 @@ TrapLabel<FileTag> SwiftLocationExtractor::emitFile(const std::filesystem::path&
4348
return fetchFileLabel(resolvePath(file));
4449
}
4550

46-
void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager,
47-
const swift::SourceRange& range,
48-
TrapLabel<LocatableTag> locatableLabel) {
49-
attachLocationImpl(sourceManager, range.Start, range.End, locatableLabel);
50-
}
51-
52-
void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager,
53-
const swift::CapturedValue* capture,
54-
TrapLabel<LocatableTag> locatableLabel) {
55-
attachLocationImpl(sourceManager, capture->getLoc(), locatableLabel);
56-
}
57-
58-
void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager,
59-
const swift::IfConfigClause* clause,
60-
TrapLabel<LocatableTag> locatableLabel) {
61-
attachLocationImpl(sourceManager, clause->Loc, locatableLabel);
62-
}
63-
64-
void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager,
65-
const swift::AvailabilitySpec* spec,
66-
TrapLabel<LocatableTag> locatableLabel) {
67-
attachLocationImpl(sourceManager, spec->getSourceRange(), locatableLabel);
68-
}
69-
70-
void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager,
71-
const swift::KeyPathExpr::Component* component,
72-
TrapLabel<LocatableTag> locatableLabel) {
73-
attachLocationImpl(sourceManager, component->getSourceRange().Start,
74-
component->getSourceRange().End, locatableLabel);
75-
}
76-
77-
void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager,
78-
const swift::Token* token,
79-
TrapLabel<LocatableTag> locatableLabel) {
80-
attachLocationImpl(sourceManager, token->getRange().getStart(), token->getRange().getEnd(),
81-
locatableLabel);
82-
}
83-
84-
void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager,
85-
swift::SourceLoc loc,
86-
TrapLabel<LocatableTag> locatableLabel) {
87-
attachLocationImpl(sourceManager, loc, loc, locatableLabel);
88-
}
89-
90-
void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager,
91-
const swift::DiagnosticInfo* diagInfo,
92-
TrapLabel<LocatableTag> locatableLabel) {
93-
attachLocationImpl(sourceManager, diagInfo->Loc, locatableLabel);
94-
}
95-
9651
TrapLabel<FileTag> SwiftLocationExtractor::fetchFileLabel(const std::filesystem::path& file) {
9752
if (store.count(file)) {
9853
return store[file];

0 commit comments

Comments
 (0)