From 1daedd9fb698fbf0907bc3c8c91bd7e33ba0e2e4 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Mon, 21 Aug 2023 17:40:15 +0200 Subject: [PATCH] Revert "Swift: use C++20 constraints and concepts to simplify code" --- misc/codegen/templates/trap_tags_h.mustache | 2 +- swift/extractor/infra/SwiftDispatcher.h | 69 ++++++--- .../infra/SwiftLocationExtractor.cpp | 67 ++++++-- .../extractor/infra/SwiftLocationExtractor.h | 143 +++++++++--------- swift/extractor/print_unextracted/main.cpp | 6 +- swift/extractor/translators/TranslatorBase.h | 52 +++++-- swift/extractor/trap/TrapLabel.h | 8 +- 7 files changed, 226 insertions(+), 121 deletions(-) diff --git a/misc/codegen/templates/trap_tags_h.mustache b/misc/codegen/templates/trap_tags_h.mustache index 6b1f4b9ac3a..c444d28afcb 100644 --- a/misc/codegen/templates/trap_tags_h.mustache +++ b/misc/codegen/templates/trap_tags_h.mustache @@ -6,7 +6,7 @@ namespace codeql { {{#tags}} // {{id}} -struct {{name}}Tag {{#has_bases}}: {{#bases}}{{^first}}, {{/first}}virtual {{base}}Tag{{/bases}} {{/has_bases}}{ +struct {{name}}Tag {{#has_bases}}: {{#bases}}{{^first}}, {{/first}}{{base}}Tag{{/bases}} {{/has_bases}}{ static constexpr const char* prefix = "{{name}}"; }; {{/tags}} diff --git a/swift/extractor/infra/SwiftDispatcher.h b/swift/extractor/infra/SwiftDispatcher.h index c76ba31fad5..7a1548c4f53 100644 --- a/swift/extractor/infra/SwiftDispatcher.h +++ b/swift/extractor/infra/SwiftDispatcher.h @@ -38,6 +38,19 @@ class SwiftDispatcher { const swift::PoundAvailableInfo*, const swift::AvailabilitySpec*>; + template + static constexpr bool IsFetchable = std::is_constructible_v; + + template + static constexpr bool IsLocatable = + std::is_base_of_v> && !std::is_base_of_v>; + + template + static constexpr bool IsDeclPointer = std::is_convertible_v; + + template + static constexpr bool IsTypePointer = std::is_convertible_v; + public: // all references and pointers passed as parameters to this constructor are supposed to outlive // the SwiftDispatcher @@ -63,7 +76,7 @@ class SwiftDispatcher { using Label = std::remove_reference_t; if (!label.valid()) { const char* action; - if constexpr (std::derived_from) { + if constexpr (std::is_base_of_v) { action = "replacing with unspecified element"; label = emitUnspecified(idOf(entry), field, index); } else { @@ -119,7 +132,7 @@ class SwiftDispatcher { template std::optional> idOf(const E& entry) { - if constexpr (requires { entry.id; }) { + if constexpr (HasId::value) { return entry.id; } else { return std::nullopt; @@ -129,14 +142,13 @@ class SwiftDispatcher { // This method gives a TRAP label for already emitted AST node. // If the AST node was not emitted yet, then the emission is dispatched to a corresponding // visitor (see `visit(T *)` methods below). - // clang-format off - template - requires std::constructible_from - TrapLabelOf fetchLabel(const E* e, swift::Type type = {}) { - // clang-format on - if (!e) { - // this will be treated on emission - return undefined_label; + template >* = nullptr> + TrapLabelOf fetchLabel(const E& e, swift::Type type = {}) { + if constexpr (std::is_constructible_v) { + if (!e) { + // this will be treated on emission + return undefined_label; + } } auto& stored = store[e]; if (!stored.valid()) { @@ -162,11 +174,8 @@ class SwiftDispatcher { return ret; } - // clang-format off - template - requires std::constructible_from + template >* = nullptr> TrapLabelOf fetchLabel(const E& e) { - // clang-format on return fetchLabel(&e); } @@ -175,8 +184,7 @@ class SwiftDispatcher { auto createEntry(const E& e) { auto found = store.find(&e); CODEQL_ASSERT(found != store.end(), "createEntry called on non-fetched label"); - using Tag = ConcreteTrapTagOf; - auto label = TrapLabel::unsafeCreateFromUntyped(found->second); + auto label = TrapLabel>::unsafeCreateFromUntyped(found->second); if constexpr (IsLocatable) { locationExtractor.attachLocation(sourceManager, e, label); } @@ -187,8 +195,7 @@ class SwiftDispatcher { // an example is swift::Argument, that are created on the fly and thus have no stable pointer template auto createUncachedEntry(const E& e) { - using Tag = TrapTagOf; - auto label = trap.createTypedLabel(); + auto label = trap.createTypedLabel>(); locationExtractor.attachLocation(sourceManager, &e, label); return TrapClassOf{label}; } @@ -211,7 +218,7 @@ class SwiftDispatcher { auto fetchRepeatedLabels(Iterable&& arg) { using Label = decltype(fetchLabel(*arg.begin())); TrapLabelVectorWrapper ret; - if constexpr (requires { arg.size(); }) { + if constexpr (HasSize::value) { ret.data.reserve(arg.size()); } for (auto&& e : arg) { @@ -244,7 +251,7 @@ class SwiftDispatcher { private: template UntypedTrapLabel createLabel(const E& e, swift::Type type) { - if constexpr (requires { name(e); }) { + if constexpr (IsDeclPointer || IsTypePointer) { if (auto mangledName = name(e)) { if (shouldVisit(e)) { toBeVisited.emplace_back(e, type); @@ -259,7 +266,7 @@ class SwiftDispatcher { template bool shouldVisit(const E& e) { - if constexpr (std::convertible_to) { + if constexpr (IsDeclPointer) { encounteredModules.insert(e->getModuleContext()); if (bodyEmissionStrategy.shouldEmitDeclBody(*e)) { extractedDeclaration(e); @@ -288,6 +295,18 @@ class SwiftDispatcher { module->isNonSwiftModule(); } + template + struct HasSize : std::false_type {}; + + template + struct HasSize().size(), void())> : std::true_type {}; + + template + struct HasId : std::false_type {}; + + template + struct HasId().id, void())> : std::true_type {}; + template TrapLabel fetchLabelFromUnion(const llvm::PointerUnion u) { TrapLabel ret{}; @@ -305,7 +324,7 @@ class SwiftDispatcher { // on `BraceStmt`/`IfConfigDecl` elements), we cannot encounter a standalone `TypeRepr` there, // so we skip this case; extracting `TypeRepr`s here would be problematic as we would not be // able to provide the corresponding type - if constexpr (!std::same_as) { + if constexpr (!std::is_same_v) { if (auto e = u.template dyn_cast()) { output = fetchLabel(e); return true; @@ -329,8 +348,10 @@ class SwiftDispatcher { virtual void visit(const swift::TypeBase* type) = 0; virtual void visit(const swift::CapturedValue* capture) = 0; - template - requires(!std::derived_from) void visit(const T* e, swift::Type) { visit(e); } + template >* = nullptr> + void visit(const T* e, swift::Type) { + visit(e); + } const swift::SourceManager& sourceManager; SwiftExtractorState& state; diff --git a/swift/extractor/infra/SwiftLocationExtractor.cpp b/swift/extractor/infra/SwiftLocationExtractor.cpp index 577b73aa983..fd9ae9487b9 100644 --- a/swift/extractor/infra/SwiftLocationExtractor.cpp +++ b/swift/extractor/infra/SwiftLocationExtractor.cpp @@ -12,24 +12,19 @@ using namespace codeql; -swift::SourceRange detail::getSourceRange(const swift::Token& token) { - const auto charRange = token.getRange(); - return {charRange.getStart(), charRange.getEnd()}; -} - void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager, - const swift::SourceRange& range, + swift::SourceLoc start, + swift::SourceLoc end, TrapLabel locatableLabel) { - if (!range) { + if (!start.isValid() || !end.isValid()) { // invalid locations seem to come from entities synthesized by the compiler return; } - auto file = resolvePath(sourceManager.getDisplayNameForLoc(range.Start)); + auto file = resolvePath(sourceManager.getDisplayNameForLoc(start)); DbLocation entry{{}}; entry.file = fetchFileLabel(file); - std::tie(entry.start_line, entry.start_column) = - sourceManager.getLineAndColumnInBuffer(range.Start); - std::tie(entry.end_line, entry.end_column) = sourceManager.getLineAndColumnInBuffer(range.End); + std::tie(entry.start_line, entry.start_column) = sourceManager.getLineAndColumnInBuffer(start); + std::tie(entry.end_line, entry.end_column) = sourceManager.getLineAndColumnInBuffer(end); SwiftMangledName locName{"loc", entry.file, ':', entry.start_line, ':', entry.start_column, ':', entry.end_line, ':', entry.end_column}; entry.id = trap.createTypedLabel(locName); @@ -48,6 +43,56 @@ TrapLabel SwiftLocationExtractor::emitFile(const std::filesystem::path& return fetchFileLabel(resolvePath(file)); } +void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager, + const swift::SourceRange& range, + TrapLabel locatableLabel) { + attachLocationImpl(sourceManager, range.Start, range.End, locatableLabel); +} + +void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager, + const swift::CapturedValue* capture, + TrapLabel locatableLabel) { + attachLocationImpl(sourceManager, capture->getLoc(), locatableLabel); +} + +void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager, + const swift::IfConfigClause* clause, + TrapLabel locatableLabel) { + attachLocationImpl(sourceManager, clause->Loc, locatableLabel); +} + +void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager, + const swift::AvailabilitySpec* spec, + TrapLabel locatableLabel) { + attachLocationImpl(sourceManager, spec->getSourceRange(), locatableLabel); +} + +void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager, + const swift::KeyPathExpr::Component* component, + TrapLabel locatableLabel) { + attachLocationImpl(sourceManager, component->getSourceRange().Start, + component->getSourceRange().End, locatableLabel); +} + +void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager, + const swift::Token* token, + TrapLabel locatableLabel) { + attachLocationImpl(sourceManager, token->getRange().getStart(), token->getRange().getEnd(), + locatableLabel); +} + +void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager, + swift::SourceLoc loc, + TrapLabel locatableLabel) { + attachLocationImpl(sourceManager, loc, loc, locatableLabel); +} + +void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sourceManager, + const swift::DiagnosticInfo* diagInfo, + TrapLabel locatableLabel) { + attachLocationImpl(sourceManager, diagInfo->Loc, locatableLabel); +} + TrapLabel SwiftLocationExtractor::fetchFileLabel(const std::filesystem::path& file) { if (store.count(file)) { return store[file]; diff --git a/swift/extractor/infra/SwiftLocationExtractor.h b/swift/extractor/infra/SwiftLocationExtractor.h index 65ef79c0018..40b0ade944c 100644 --- a/swift/extractor/infra/SwiftLocationExtractor.h +++ b/swift/extractor/infra/SwiftLocationExtractor.h @@ -15,99 +15,104 @@ namespace codeql { class TrapDomain; -namespace detail { -template -concept HasSourceRange = requires(T e) { - e.getSourceRange(); -}; - -template -concept HasStartAndEndLoc = requires(T e) { - e.getStartLoc(); - e.getEndLoc(); -} -&&!(HasSourceRange); - -template -concept HasOneLoc = requires(T e) { - e.getLoc(); -} -&&!(HasSourceRange)&&(!HasStartAndEndLoc); - -template -concept HasOneLocField = requires(T e) { - e.Loc; -}; - -swift::SourceRange getSourceRange(const HasSourceRange auto& locatable) { - return locatable.getSourceRange(); -} - -swift::SourceRange getSourceRange(const HasStartAndEndLoc auto& locatable) { - if (locatable.getStartLoc() && locatable.getEndLoc()) { - return {locatable.getStartLoc(), locatable.getEndLoc()}; - } - return {locatable.getStartLoc()}; -} - -swift::SourceRange getSourceRange(const HasOneLoc auto& locatable) { - return {locatable.getLoc()}; -} - -swift::SourceRange getSourceRange(const HasOneLocField auto& locatable) { - return {locatable.Loc}; -} - -swift::SourceRange getSourceRange(const swift::Token& token); - -template -swift::SourceRange getSourceRange(const llvm::MutableArrayRef& locatables) { - if (locatables.empty()) { - return {}; - } - auto startRange = getSourceRange(locatables.front()); - auto endRange = getSourceRange(locatables.back()); - if (startRange.Start && endRange.End) { - return {startRange.Start, endRange.End}; - } - return {startRange.Start}; -} -} // namespace detail - -template -concept IsLocatable = requires(E e) { - detail::getSourceRange(e); -}; - class SwiftLocationExtractor { + template + struct HasSpecializedImplementation : std::false_type {}; + public: explicit SwiftLocationExtractor(TrapDomain& trap) : trap(trap) {} TrapLabel emitFile(swift::SourceFile* file); TrapLabel emitFile(const std::filesystem::path& file); - // Emits a Location TRAP entry and attaches it to a `Locatable` trap label + template void attachLocation(const swift::SourceManager& sourceManager, - const IsLocatable auto& locatable, + const Locatable& locatable, TrapLabel locatableLabel) { - attachLocationImpl(sourceManager, detail::getSourceRange(locatable), locatableLabel); + attachLocation(sourceManager, &locatable, locatableLabel); } + // Emits a Location TRAP entry and attaches it to a `Locatable` trap label + template ::value>* = nullptr> void attachLocation(const swift::SourceManager& sourceManager, - const IsLocatable auto* locatable, + const Locatable* locatable, TrapLabel locatableLabel) { - attachLocation(sourceManager, *locatable, locatableLabel); + attachLocationImpl(sourceManager, locatable->getStartLoc(), locatable->getEndLoc(), + locatableLabel); + } + + template ::value>* = nullptr> + void attachLocation(const swift::SourceManager& sourceManager, + const Locatable* locatable, + TrapLabel locatableLabel) { + attachLocationImpl(sourceManager, locatable, locatableLabel); } private: + // Emits a Location TRAP entry for a list of swift entities and attaches it to a `Locatable` trap + // label + template + void attachLocationImpl(const swift::SourceManager& sourceManager, + const llvm::MutableArrayRef* locatables, + TrapLabel locatableLabel) { + if (locatables->empty()) { + return; + } + attachLocationImpl(sourceManager, locatables->front().getStartLoc(), + locatables->back().getEndLoc(), locatableLabel); + } + + void attachLocationImpl(const swift::SourceManager& sourceManager, + swift::SourceLoc start, + swift::SourceLoc end, + TrapLabel locatableLabel); + + void attachLocationImpl(const swift::SourceManager& sourceManager, + swift::SourceLoc loc, + TrapLabel locatableLabel); + void attachLocationImpl(const swift::SourceManager& sourceManager, const swift::SourceRange& range, TrapLabel locatableLabel); + void attachLocationImpl(const swift::SourceManager& sourceManager, + const swift::CapturedValue* capture, + TrapLabel locatableLabel); + + void attachLocationImpl(const swift::SourceManager& sourceManager, + const swift::IfConfigClause* clause, + TrapLabel locatableLabel); + + void attachLocationImpl(const swift::SourceManager& sourceManager, + const swift::AvailabilitySpec* spec, + TrapLabel locatableLabel); + + void attachLocationImpl(const swift::SourceManager& sourceManager, + const swift::Token* token, + TrapLabel locatableLabel); + + void attachLocationImpl(const swift::SourceManager& sourceManager, + const swift::DiagnosticInfo* token, + TrapLabel locatableLabel); + + void attachLocationImpl(const swift::SourceManager& sourceManager, + const swift::KeyPathExpr::Component* component, + TrapLabel locatableLabel); + private: TrapLabel fetchFileLabel(const std::filesystem::path& file); TrapDomain& trap; std::unordered_map, codeql::PathHash> store; }; +template +struct SwiftLocationExtractor::HasSpecializedImplementation< + Locatable, + decltype(std::declval().attachLocationImpl( + std::declval(), + std::declval(), + std::declval>()))> : std::true_type {}; + } // namespace codeql diff --git a/swift/extractor/print_unextracted/main.cpp b/swift/extractor/print_unextracted/main.cpp index 22af3304677..09832ce992e 100644 --- a/swift/extractor/print_unextracted/main.cpp +++ b/swift/extractor/print_unextracted/main.cpp @@ -13,9 +13,9 @@ using namespace codeql; int main() { std::map> unextracted; -#define CHECK_CLASS(KIND, CLASS, PARENT) \ - if constexpr (KIND##Translator::getPolicyFor##CLASS##KIND() == TranslatorPolicy::emitUnknown) { \ - unextracted[#KIND].push_back(#CLASS #KIND); \ +#define CHECK_CLASS(KIND, CLASS, PARENT) \ + if (KIND##Translator::getPolicyFor##CLASS##KIND() == TranslatorPolicy::emitUnknown) { \ + unextracted[#KIND].push_back(#CLASS #KIND); \ } #define DECL(CLASS, PARENT) CHECK_CLASS(Decl, CLASS, PARENT) diff --git a/swift/extractor/translators/TranslatorBase.h b/swift/extractor/translators/TranslatorBase.h index 76ddad10dc6..f3438fac39c 100644 --- a/swift/extractor/translators/TranslatorBase.h +++ b/swift/extractor/translators/TranslatorBase.h @@ -1,7 +1,5 @@ #pragma once -#include - #include #include @@ -20,6 +18,45 @@ class TranslatorBase { : dispatcher{dispatcher}, logger{"translator/" + std::string(name)} {} }; +// define by macro metaprogramming member checkers +// see https://fekir.info/post/detect-member-variables/ for technical details +#define DEFINE_TRANSLATE_CHECKER(KIND, CLASS, PARENT) \ + template \ + struct HasTranslate##CLASS##KIND : std::false_type {}; \ + \ + template \ + struct HasTranslate##CLASS##KIND().translate##CLASS##KIND( \ + std::declval()), \ + void())> : std::true_type {}; + +DEFINE_TRANSLATE_CHECKER(Decl, , ) +#define DECL(CLASS, PARENT) DEFINE_TRANSLATE_CHECKER(Decl, CLASS, PARENT) +#define ABSTRACT_DECL(CLASS, PARENT) DEFINE_TRANSLATE_CHECKER(Decl, CLASS, PARENT) +#include "swift/AST/DeclNodes.def" + +DEFINE_TRANSLATE_CHECKER(Stmt, , ) +#define STMT(CLASS, PARENT) DEFINE_TRANSLATE_CHECKER(Stmt, CLASS, PARENT) +#define ABSTRACT_STMT(CLASS, PARENT) DEFINE_TRANSLATE_CHECKER(Stmt, CLASS, PARENT) +#include "swift/AST/StmtNodes.def" + +DEFINE_TRANSLATE_CHECKER(Expr, , ) +#define EXPR(CLASS, PARENT) DEFINE_TRANSLATE_CHECKER(Expr, CLASS, PARENT) +#define ABSTRACT_EXPR(CLASS, PARENT) DEFINE_TRANSLATE_CHECKER(Expr, CLASS, PARENT) +#include "swift/AST/ExprNodes.def" + +DEFINE_TRANSLATE_CHECKER(Pattern, , ) +#define PATTERN(CLASS, PARENT) DEFINE_TRANSLATE_CHECKER(Pattern, CLASS, PARENT) +#include "swift/AST/PatternNodes.def" + +DEFINE_TRANSLATE_CHECKER(Type, , ) +#define TYPE(CLASS, PARENT) DEFINE_TRANSLATE_CHECKER(Type, CLASS, PARENT) +#define ABSTRACT_TYPE(CLASS, PARENT) DEFINE_TRANSLATE_CHECKER(Type, CLASS, PARENT) +#include "swift/AST/TypeNodes.def" + +DEFINE_TRANSLATE_CHECKER(TypeRepr, , ) +#define TYPEREPR(CLASS, PARENT) DEFINE_TRANSLATE_CHECKER(TypeRepr, CLASS, PARENT) +#define ABSTRACT_TYPEREPR(CLASS, PARENT) DEFINE_TRANSLATE_CHECKER(TypeRepr, CLASS, PARENT) +#include "swift/AST/TypeReprNodes.def" } // namespace detail enum class TranslatorPolicy { @@ -39,15 +76,11 @@ enum class TranslatorPolicy { #define DEFINE_VISIT(KIND, CLASS, PARENT) \ public: \ static constexpr TranslatorPolicy getPolicyFor##CLASS##KIND() { \ - if constexpr (std::same_as, void>) { \ + if constexpr (std::is_same_v, void>) { \ return TranslatorPolicy::ignore; \ - } else if constexpr (requires(CrtpSubclass x, swift::CLASS##KIND e) { \ - x.translate##CLASS##KIND(e); \ - }) { \ + } else if constexpr (detail::HasTranslate##CLASS##KIND::value) { \ return TranslatorPolicy::translate; \ - } else if constexpr (requires(CrtpSubclass x, swift::CLASS##KIND e) { \ - x.translate##PARENT(e); \ - }) { \ + } else if constexpr (detail::HasTranslate##PARENT::value) { \ return TranslatorPolicy::translateParent; \ } else { \ return TranslatorPolicy::emitUnknown; \ @@ -59,6 +92,7 @@ enum class TranslatorPolicy { constexpr auto policy = getPolicyFor##CLASS##KIND(); \ if constexpr (policy == TranslatorPolicy::ignore) { \ LOG_ERROR("Unexpected " #CLASS #KIND); \ + return; \ } else if constexpr (policy == TranslatorPolicy::translate) { \ dispatcher.emit(static_cast(this)->translate##CLASS##KIND(*e)); \ } else if constexpr (policy == TranslatorPolicy::translateParent) { \ diff --git a/swift/extractor/trap/TrapLabel.h b/swift/extractor/trap/TrapLabel.h index e0091a6d536..629aa3e0033 100644 --- a/swift/extractor/trap/TrapLabel.h +++ b/swift/extractor/trap/TrapLabel.h @@ -9,7 +9,6 @@ #include #include #include -#include namespace codeql { @@ -83,8 +82,9 @@ class TrapLabel : public UntypedTrapLabel { static TrapLabel unsafeCreateFromUntyped(UntypedTrapLabel label) { return TrapLabel{label.id_}; } template - requires std::derived_from TrapLabel(const TrapLabel& other) - : UntypedTrapLabel(other) {} + TrapLabel(const TrapLabel& other) : UntypedTrapLabel(other) { + static_assert(std::is_base_of_v, "wrong label assignment!"); + } }; // wrapper class to allow directly assigning a vector of TrapLabel to a vector of @@ -96,8 +96,8 @@ struct TrapLabelVectorWrapper { std::vector> data; template - requires std::derived_from operator std::vector>() && { + static_assert(std::is_base_of_v, "wrong label assignment!"); // reinterpret_cast is safe because TrapLabel instances differ only on the type, not the // underlying data return std::move(reinterpret_cast>&>(data));