From 5e74df3882b1d7e1f79cb1917f04114ff1608c3e Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Wed, 13 Jul 2022 19:05:05 +0200 Subject: [PATCH] Swift: cache file paths This required a bit of a generalization of `TrapLabelStore` to not work only with pointers. --- swift/extractor/SwiftExtractor.cpp | 22 ++---- swift/extractor/infra/FilePath.h | 24 ++++++ swift/extractor/infra/SwiftDispatcher.h | 99 +++++++++++++++---------- swift/extractor/infra/SwiftTagTraits.h | 17 +++-- swift/extractor/trap/TrapLabelStore.h | 6 +- swift/extractor/trap/TrapTagTraits.h | 3 +- swift/extractor/visitors/SwiftVisitor.h | 2 +- 7 files changed, 105 insertions(+), 68 deletions(-) create mode 100644 swift/extractor/infra/FilePath.h diff --git a/swift/extractor/SwiftExtractor.cpp b/swift/extractor/SwiftExtractor.cpp index 60ac3436fa8..666b7a90044 100644 --- a/swift/extractor/SwiftExtractor.cpp +++ b/swift/extractor/SwiftExtractor.cpp @@ -104,29 +104,19 @@ static void extractDeclarations(const SwiftExtractorConfiguration& config, dumpArgs(*trapTarget, config); TrapDomain trap{*trapTarget}; - // TODO: move default location emission elsewhere, possibly in a separate global trap file + // TODO: remove this and recreate it with IPA when we have that // the following cannot conflict with actual files as those have an absolute path starting with / - auto unknownFileLabel = trap.createLabel("unknown"); - auto unknownLocationLabel = trap.createLabel("unknown"); - trap.emit(FilesTrap{unknownFileLabel}); - trap.emit(LocationsTrap{unknownLocationLabel, unknownFileLabel}); + File unknownFileEntry{trap.createLabel("unknown")}; + Location unknownLocationEntry{trap.createLabel("unknown")}; + unknownLocationEntry.file = unknownFileEntry.id; + trap.emit(unknownFileEntry); + trap.emit(unknownLocationEntry); SwiftVisitor visitor(compiler.getSourceMgr(), trap, module, primaryFile); auto topLevelDecls = getTopLevelDecls(module, primaryFile); for (auto decl : topLevelDecls) { visitor.extract(decl); } - // TODO the following will be moved to the dispatcher when we start caching swift file objects - // for the moment, topLevelDecls always contains the current module, which does not have a file - // associated with it, so we need a special case when there are no top level declarations - if (topLevelDecls.size() == 1) { - // In the case of empty files, the dispatcher is not called, but we still want to 'record' the - // fact that the file was extracted - llvm::SmallString name(filename); - llvm::sys::fs::make_absolute(name); - auto fileLabel = trap.createLabel(name.str().str()); - trap.emit(FilesTrap{fileLabel, name.str().str()}); - } } static std::unordered_set collectInputFilenames(swift::CompilerInstance& compiler) { diff --git a/swift/extractor/infra/FilePath.h b/swift/extractor/infra/FilePath.h new file mode 100644 index 00000000000..48288b928e4 --- /dev/null +++ b/swift/extractor/infra/FilePath.h @@ -0,0 +1,24 @@ +#pragma once + +#include +namespace codeql { + +// wrapper around `std::string` mainly intended to unambiguously go into an `std::variant` +// TODO probably not needed once we can use `std::filesystem::path` +struct FilePath { + FilePath() = default; + FilePath(const std::string& path) : path{path} {} + FilePath(std::string&& path) : path{std::move(path)} {} + + std::string path; + + bool operator==(const FilePath& other) const { return path == other.path; } +}; +} // namespace codeql + +namespace std { +template <> +struct hash { + size_t operator()(const codeql::FilePath& value) { return hash{}(value.path); } +}; +} // namespace std diff --git a/swift/extractor/infra/SwiftDispatcher.h b/swift/extractor/infra/SwiftDispatcher.h index a9ecb6996ff..b0cc951dd31 100644 --- a/swift/extractor/infra/SwiftDispatcher.h +++ b/swift/extractor/infra/SwiftDispatcher.h @@ -8,6 +8,7 @@ #include "swift/extractor/trap/TrapDomain.h" #include "swift/extractor/infra/SwiftTagTraits.h" #include "swift/extractor/trap/generated/TrapClasses.h" +#include "swift/extractor/infra/FilePath.h" namespace codeql { @@ -17,6 +18,24 @@ namespace codeql { // Since SwiftDispatcher sees all the AST nodes, it also attaches a location to every 'locatable' // node (AST nodes that are not types: declarations, statements, expressions, etc.). class SwiftDispatcher { + // types to be supported by assignNewLabel/fetchLabel need to be listed here + using Store = TrapLabelStore; + + template + static constexpr bool IsStorable = std::is_constructible_v; + + template + static constexpr bool IsLocatable = std::is_base_of_v>; + public: // all references and pointers passed as parameters to this constructor are supposed to outlive // the SwiftDispatcher @@ -27,7 +46,12 @@ class SwiftDispatcher { : sourceManager{sourceManager}, trap{trap}, currentModule{currentModule}, - currentPrimarySourceFile{currentPrimarySourceFile} {} + currentPrimarySourceFile{currentPrimarySourceFile} { + if (currentPrimarySourceFile) { + // we make sure the file is in the trap output even if the source is empty + fetchLabel(getFilePath(currentPrimarySourceFile->getFilename())); + } + } template void emit(const Entry& entry) { @@ -61,9 +85,11 @@ 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). - template - TrapLabelOf fetchLabel(E* e) { - assert(e && "trying to fetch a label on nullptr, maybe fetchOptionalLabel is to be used?"); + template >* = nullptr> + TrapLabelOf fetchLabel(const E& e) { + if constexpr (std::is_constructible_v) { + assert(e && "fetching a label on a null entity, maybe fetchOptionalLabel is to be used?"); + } // this is required so we avoid any recursive loop: a `fetchLabel` during the visit of `e` might // end up calling `fetchLabel` on `e` itself, so we want the visit of `e` to call `fetchLabel` // only after having called `assignNewLabel` on `e`. @@ -76,7 +102,7 @@ class SwiftDispatcher { visit(e); // TODO when everything is moved to structured C++ classes, this should be moved to createEntry if (auto l = store.get(e)) { - if constexpr (!std::is_base_of_v) { + if constexpr (IsLocatable) { attachLocation(e, *l); } return *l; @@ -93,15 +119,16 @@ class SwiftDispatcher { return fetchLabelFromUnion(node); } - TrapLabel fetchLabel(const swift::StmtConditionElement& element) { - return fetchLabel(&element); + template >* = nullptr> + TrapLabelOf fetchLabel(const E& e) { + return fetchLabel(&e); } // Due to the lazy emission approach, we must assign a label to a corresponding AST node before // it actually gets emitted to handle recursive cases such as recursive calls, or recursive type // declarations - template - TrapLabelOf assignNewLabel(E* e, Args&&... args) { + template >* = nullptr> + TrapLabelOf assignNewLabel(const E& e, Args&&... args) { assert(waitingForNewLabel == Store::Handle{e} && "assignNewLabel called on wrong entity"); auto label = trap.createLabel>(std::forward(args)...); store.insert(e, label); @@ -109,20 +136,20 @@ class SwiftDispatcher { return label; } - template >* = nullptr> + template >* = nullptr> TrapLabelOf assignNewLabel(const E& e, Args&&... args) { return assignNewLabel(&e, std::forward(args)...); } // convenience methods for structured C++ creation - template >* = nullptr> + template auto createEntry(const E& e, Args&&... args) { - return TrapClassOf{assignNewLabel(&e, std::forward(args)...)}; + return TrapClassOf{assignNewLabel(e, std::forward(args)...)}; } // used to create a new entry for entities that should not be cached // an example is swift::Argument, that are created on the fly and thus have no stable pointer - template >* = nullptr> + template auto createUncachedEntry(const E& e, Args&&... args) { auto label = trap.createLabel>(std::forward(args)...); attachLocation(&e, label); @@ -206,17 +233,6 @@ class SwiftDispatcher { } private: - // types to be supported by assignNewLabel/fetchLabel need to be listed here - using Store = TrapLabelStore; - void attachLocation(swift::SourceLoc start, swift::SourceLoc end, TrapLabel locatableLabel) { @@ -224,16 +240,16 @@ class SwiftDispatcher { // invalid locations seem to come from entities synthesized by the compiler return; } - std::string filepath = getFilepath(start); - auto fileLabel = trap.createLabel(filepath); - // TODO: do not emit duplicate trap entries for Files - trap.emit(FilesTrap{fileLabel, filepath}); - auto [startLine, startColumn] = sourceManager.getLineAndColumnInBuffer(start); - auto [endLine, endColumn] = sourceManager.getLineAndColumnInBuffer(end); - auto locLabel = trap.createLabel('{', fileLabel, "}:", startLine, ':', startColumn, - ':', endLine, ':', endColumn); - trap.emit(LocationsTrap{locLabel, fileLabel, startLine, startColumn, endLine, endColumn}); - trap.emit(LocatableLocationsTrap{locatableLabel, locLabel}); + auto file = getFilePath(sourceManager.getDisplayNameForLoc(start)); + Location entry{{}}; + entry.file = fetchLabel(file); + std::tie(entry.start_line, entry.start_column) = sourceManager.getLineAndColumnInBuffer(start); + std::tie(entry.end_line, entry.end_column) = sourceManager.getLineAndColumnInBuffer(end); + entry.id = trap.createLabel('{', entry.file, "}:", entry.start_line, ':', + entry.start_column, ':', entry.end_line, ':', + entry.end_column); + emit(entry); + emit(LocatableLocationsTrap{locatableLabel, entry.id}); } template @@ -256,21 +272,18 @@ class SwiftDispatcher { return false; } - std::string getFilepath(swift::SourceLoc loc) { + static FilePath getFilePath(llvm::StringRef path) { // TODO: this needs more testing // TODO: check canonicaliztion of names on a case insensitive filesystems // TODO: make symlink resolution conditional on CODEQL_PRESERVE_SYMLINKS=true - auto displayName = sourceManager.getDisplayNameForLoc(loc); llvm::SmallString realPath; - if (std::error_code ec = llvm::sys::fs::real_path(displayName, realPath)) { - std::cerr << "Cannot get real path: '" << displayName.str() << "': " << ec.message() << "\n"; + if (std::error_code ec = llvm::sys::fs::real_path(path, realPath)) { + std::cerr << "Cannot get real path: '" << path.str() << "': " << ec.message() << "\n"; return {}; } return realPath.str().str(); } - // TODO: The following methods are supposed to redirect TRAP emission to correpsonding visitors, - // which are to be introduced in follow-up PRs virtual void visit(swift::Decl* decl) = 0; virtual void visit(swift::Stmt* stmt) = 0; virtual void visit(const swift::StmtCondition* cond) = 0; @@ -281,6 +294,12 @@ class SwiftDispatcher { virtual void visit(swift::TypeRepr* type) = 0; virtual void visit(swift::TypeBase* type) = 0; + void visit(const FilePath& file) { + auto entry = createEntry(file); + entry.name = file.path; + emit(entry); + } + const swift::SourceManager& sourceManager; TrapDomain& trap; Store store; diff --git a/swift/extractor/infra/SwiftTagTraits.h b/swift/extractor/infra/SwiftTagTraits.h index 7447fa8f9b3..a4949b3ec5f 100644 --- a/swift/extractor/infra/SwiftTagTraits.h +++ b/swift/extractor/infra/SwiftTagTraits.h @@ -5,6 +5,7 @@ #include #include "swift/extractor/trap/TrapTagTraits.h" #include "swift/extractor/trap/generated/TrapTags.h" +#include "swift/extractor/infra/FilePath.h" namespace codeql { @@ -16,12 +17,12 @@ using SILFunctionTypeTag = SilFunctionTypeTag; using SILTokenTypeTag = SilTokenTypeTag; using SILBoxTypeReprTag = SilBoxTypeReprTag; -#define MAP_TYPE_TO_TAG(TYPE, TAG) \ - template <> \ - struct detail::ToTagFunctor { \ - using type = TAG; \ +#define MAP_TYPE_TO_TAG(TYPE, TAG) \ + template <> \ + struct detail::ToTagFunctor { \ + using type = TAG; \ } -#define MAP_TAG(TYPE) MAP_TYPE_TO_TAG(TYPE, TYPE##Tag) +#define MAP_TAG(TYPE) MAP_TYPE_TO_TAG(swift::TYPE, TYPE##Tag) #define MAP_SUBTAG(TYPE, PARENT) \ MAP_TAG(TYPE); \ static_assert(std::is_base_of_v, \ @@ -36,7 +37,7 @@ using SILBoxTypeReprTag = SilBoxTypeReprTag; MAP_TAG(Stmt); MAP_TAG(StmtCondition); -MAP_TYPE_TO_TAG(StmtConditionElement, ConditionElementTag); +MAP_TYPE_TO_TAG(swift::StmtConditionElement, ConditionElementTag); MAP_TAG(CaseLabelItem); #define ABSTRACT_STMT(CLASS, PARENT) MAP_SUBTAG(CLASS##Stmt, PARENT) #define STMT(CLASS, PARENT) ABSTRACT_STMT(CLASS, PARENT) @@ -63,7 +64,7 @@ MAP_TAG(TypeRepr); #define TYPEREPR(CLASS, PARENT) ABSTRACT_TYPEREPR(CLASS, PARENT) #include -MAP_TYPE_TO_TAG(TypeBase, TypeTag); +MAP_TYPE_TO_TAG(swift::TypeBase, TypeTag); #define ABSTRACT_TYPE(CLASS, PARENT) MAP_SUBTAG(CLASS##Type, PARENT) #define TYPE(CLASS, PARENT) ABSTRACT_TYPE(CLASS, PARENT) #include @@ -71,6 +72,8 @@ MAP_TYPE_TO_TAG(TypeBase, TypeTag); OVERRIDE_TAG(FuncDecl, ConcreteFuncDeclTag); OVERRIDE_TAG(VarDecl, ConcreteVarDeclTag); +MAP_TYPE_TO_TAG(FilePath, FileTag); + #undef MAP_TAG #undef MAP_SUBTAG #undef MAP_TYPE_TO_TAG diff --git a/swift/extractor/trap/TrapLabelStore.h b/swift/extractor/trap/TrapLabelStore.h index 93ca4212185..f9b0edd1753 100644 --- a/swift/extractor/trap/TrapLabelStore.h +++ b/swift/extractor/trap/TrapLabelStore.h @@ -20,10 +20,10 @@ namespace codeql { template class TrapLabelStore { public: - using Handle = std::variant; + using Handle = std::variant; template - std::optional> get(const T* e) { + std::optional> get(const T& e) { if (auto found = store_.find(e); found != store_.end()) { return TrapLabelOf::unsafeCreateFromUntyped(found->second); } @@ -31,7 +31,7 @@ class TrapLabelStore { } template - void insert(const T* e, TrapLabelOf l) { + void insert(const T& e, TrapLabelOf l) { auto [_, inserted] = store_.emplace(e, l); assert(inserted && "already inserted"); } diff --git a/swift/extractor/trap/TrapTagTraits.h b/swift/extractor/trap/TrapTagTraits.h index 9c29ff22330..f0f8c41cc8d 100644 --- a/swift/extractor/trap/TrapTagTraits.h +++ b/swift/extractor/trap/TrapTagTraits.h @@ -26,7 +26,8 @@ struct ToTrapClassFunctor; } // namespace detail template -using TrapTagOf = typename detail::ToTagOverride>::type; +using TrapTagOf = + typename detail::ToTagOverride>>::type; template using TrapLabelOf = TrapLabel>; diff --git a/swift/extractor/visitors/SwiftVisitor.h b/swift/extractor/visitors/SwiftVisitor.h index a13479246f4..d9d27f2a7ed 100644 --- a/swift/extractor/visitors/SwiftVisitor.h +++ b/swift/extractor/visitors/SwiftVisitor.h @@ -15,7 +15,7 @@ class SwiftVisitor : private SwiftDispatcher { using SwiftDispatcher::SwiftDispatcher; template - void extract(T* entity) { + void extract(const T& entity) { fetchLabel(entity); }