From ded0e570b02dcd10d06f2e3ed427b85912b127b9 Mon Sep 17 00:00:00 2001 From: Alex Denisov Date: Tue, 28 Feb 2023 14:09:51 +0100 Subject: [PATCH 1/5] Swift: extract lazy declarations --- swift/extractor/SwiftBuiltinSymbols.h | 73 ------------ swift/extractor/SwiftExtractor.cpp | 107 ++++++++++++------ swift/extractor/SwiftExtractor.h | 1 + swift/extractor/config/SwiftExtractorState.h | 3 + .../infra/SwiftBodyEmissionStrategy.cpp | 3 + .../infra/SwiftBodyEmissionStrategy.h | 8 +- swift/extractor/infra/SwiftDispatcher.h | 7 ++ swift/extractor/infra/TargetDomains.cpp | 2 + swift/extractor/infra/TargetDomains.h | 1 + swift/extractor/main.cpp | 1 + swift/extractor/translators/DeclTranslator.h | 2 + 11 files changed, 97 insertions(+), 111 deletions(-) delete mode 100644 swift/extractor/SwiftBuiltinSymbols.h diff --git a/swift/extractor/SwiftBuiltinSymbols.h b/swift/extractor/SwiftBuiltinSymbols.h deleted file mode 100644 index 7b0e12ad6136..000000000000 --- a/swift/extractor/SwiftBuiltinSymbols.h +++ /dev/null @@ -1,73 +0,0 @@ -#pragma once - -#include - -namespace codeql { -constexpr std::array swiftBuiltins = { - "zeroInitializer", - "BridgeObject", - "Word", - "NativeObject", - "RawPointer", - "Executor", - "Job", - "RawUnsafeContinuation", - "addressof", - "initialize", - "reinterpretCast", - "Int1", - "Int8", - "Int16", - "Int32", - "Int64", - "IntLiteral", - "FPIEEE16", - "FPIEEE32", - "FPIEEE64", - "FPIEEE80", - "Vec2xInt8", - "Vec4xInt8", - "Vec8xInt8", - "Vec16xInt8", - "Vec32xInt8", - "Vec64xInt8", - "Vec2xInt16", - "Vec4xInt16", - "Vec8xInt16", - "Vec16xInt16", - "Vec32xInt16", - "Vec64xInt16", - "Vec2xInt32", - "Vec4xInt32", - "Vec8xInt32", - "Vec16xInt32", - "Vec32xInt32", - "Vec64xInt32", - "Vec2xInt64", - "Vec4xInt64", - "Vec8xInt64", - "Vec16xInt64", - "Vec32xInt64", - "Vec64xInt64", - "Vec2xFPIEEE16", - "Vec4xFPIEEE16", - "Vec8xFPIEEE16", - "Vec16xFPIEEE16", - "Vec32xFPIEEE16", - "Vec64xFPIEEE16", - "Vec2xFPIEEE32", - "Vec4xFPIEEE32", - "Vec8xFPIEEE32", - "Vec16xFPIEEE32", - "Vec32xFPIEEE32", - "Vec64xFPIEEE32", - "Vec2xFPIEEE64", - "Vec4xFPIEEE64", - "Vec8xFPIEEE64", - "Vec16xFPIEEE64", - "Vec32xFPIEEE64", - "Vec64xFPIEEE64", - "buildDefaultActorExecutorRef", - "buildMainActorExecutorRef", -}; -} diff --git a/swift/extractor/SwiftExtractor.cpp b/swift/extractor/SwiftExtractor.cpp index 246d45a469f5..6a99a350e03e 100644 --- a/swift/extractor/SwiftExtractor.cpp +++ b/swift/extractor/SwiftExtractor.cpp @@ -10,10 +10,10 @@ #include "swift/extractor/translators/SwiftVisitor.h" #include "swift/extractor/infra/TargetDomains.h" -#include "swift/extractor/SwiftBuiltinSymbols.h" #include "swift/extractor/infra/file/Path.h" #include "swift/extractor/infra/SwiftLocationExtractor.h" #include "swift/extractor/infra/SwiftBodyEmissionStrategy.h" +#include "swift/extractor/mangler/SwiftMangler.h" using namespace codeql; using namespace std::string_literals; @@ -43,10 +43,16 @@ static void archiveFile(const SwiftExtractorConfiguration& config, swift::Source } } -static fs::path getFilename(swift::ModuleDecl& module, swift::SourceFile* primaryFile) { +static fs::path getFilename(swift::ModuleDecl& module, + swift::SourceFile* primaryFile, + const swift::Decl* lazyDeclaration) { if (primaryFile) { return resolvePath(primaryFile->getFilename()); } + if (lazyDeclaration) { + static SwiftMangler mangler; + return mangler.mangledName(*lazyDeclaration); + } // PCM clang module if (module.isNonSwiftModule()) { // Several modules with different names might come from .pcm (clang module) files @@ -72,49 +78,40 @@ static fs::path getFilename(swift::ModuleDecl& module, swift::SourceFile* primar return resolvePath(filename); } -/* The builtin module is special, as it does not publish any top-level declaration - * It creates (and caches) declarations on demand when a lookup is carried out - * (see BuiltinUnit in swift/AST/FileUnit.h for the cache details, and getBuiltinValueDecl in - * swift/AST/Builtins.h for the creation details) - * As we want to create the Builtin trap file once and for all so that it works for other - * extraction runs, rather than collecting what we need we pre-populate the builtin trap with - * what we expect. This list might need thus to be expanded. - * Notice, that while swift/AST/Builtins.def has a list of builtin symbols, it does not contain - * all information required to instantiate builtin variants. - * Other possible approaches: - * * create one trap per builtin declaration when encountered - * * expand the list to all possible builtins (of which there are a lot) - */ -static void getBuiltinDecls(swift::ModuleDecl& builtinModule, - llvm::SmallVector& decls) { - llvm::SmallVector values; - for (auto symbol : swiftBuiltins) { - builtinModule.lookupValue(builtinModule.getASTContext().getIdentifier(symbol), - swift::NLKind::QualifiedLookup, values); - } - decls.insert(decls.end(), values.begin(), values.end()); -} - static llvm::SmallVector getTopLevelDecls(swift::ModuleDecl& module, - swift::SourceFile* primaryFile = nullptr) { + swift::SourceFile* primaryFile, + const swift::Decl* lazyDeclaration) { llvm::SmallVector ret; + if (lazyDeclaration) { + ret.push_back(const_cast(lazyDeclaration)); + return ret; + } ret.push_back(&module); if (primaryFile) { primaryFile->getTopLevelDecls(ret); - } else if (module.isBuiltinModule()) { - getBuiltinDecls(module, ret); } else { module.getTopLevelDecls(ret); } return ret; } +static TrapType getTrapType(swift::SourceFile* primaryFile, const swift::Decl* lazyDeclaration) { + if (primaryFile) { + return TrapType::source; + } + if (lazyDeclaration) { + return TrapType::lazy_declarations; + } + return TrapType::module; +} + static std::unordered_set extractDeclarations( SwiftExtractorState& state, swift::CompilerInstance& compiler, swift::ModuleDecl& module, - swift::SourceFile* primaryFile = nullptr) { - auto filename = getFilename(module, primaryFile); + swift::SourceFile* primaryFile, + const swift::Decl* lazyDeclaration) { + auto filename = getFilename(module, primaryFile, lazyDeclaration); if (primaryFile) { state.sourceFiles.push_back(filename); } @@ -122,7 +119,7 @@ static std::unordered_set extractDeclarations( // The extractor can be called several times from different processes with // the same input file(s). Using `TargetFile` the first process will win, and the following // will just skip the work - const auto trapType = primaryFile ? TrapType::source : TrapType::module; + const auto trapType = getTrapType(primaryFile, lazyDeclaration); auto trap = createTargetTrapDomain(state, filename, trapType); if (!trap) { // another process arrived first, nothing to do for us @@ -143,9 +140,10 @@ static std::unordered_set extractDeclarations( SwiftLocationExtractor locationExtractor(*trap); locationExtractor.emitFile(primaryFile); - SwiftBodyEmissionStrategy bodyEmissionStrategy(module, primaryFile); - SwiftVisitor visitor(compiler.getSourceMgr(), *trap, locationExtractor, bodyEmissionStrategy); - auto topLevelDecls = getTopLevelDecls(module, primaryFile); + SwiftBodyEmissionStrategy bodyEmissionStrategy(module, primaryFile, lazyDeclaration); + SwiftVisitor visitor(compiler.getSourceMgr(), state, *trap, locationExtractor, + bodyEmissionStrategy); + auto topLevelDecls = getTopLevelDecls(module, primaryFile, lazyDeclaration); for (auto decl : topLevelDecls) { visitor.extract(decl); } @@ -198,10 +196,10 @@ void codeql::extractSwiftFiles(SwiftExtractorState& state, swift::CompilerInstan continue; } archiveFile(state.configuration, *sourceFile); - encounteredModules = extractDeclarations(state, compiler, *module, sourceFile); + encounteredModules = extractDeclarations(state, compiler, *module, sourceFile, nullptr); } if (!isFromSourceFile) { - encounteredModules = extractDeclarations(state, compiler, *module); + encounteredModules = extractDeclarations(state, compiler, *module, nullptr, nullptr); } for (auto encountered : encounteredModules) { if (state.encounteredModules.count(encountered) == 0) { @@ -211,3 +209,40 @@ void codeql::extractSwiftFiles(SwiftExtractorState& state, swift::CompilerInstan } } } + +static void cleanupPendingDeclarations(SwiftExtractorState& state) { + std::vector worklist; + std::copy(std::begin(state.pendingDeclarations), std::end(state.pendingDeclarations), + std::back_inserter(worklist)); + + for (auto decl : worklist) { + if (state.emittedDeclarations.count(decl)) { + state.pendingDeclarations.erase(decl); + } + } +} + +static void extractLazy(SwiftExtractorState& state, swift::CompilerInstance& compiler) { + cleanupPendingDeclarations(state); + std::vector worklist; + std::copy(std::begin(state.pendingDeclarations), std::end(state.pendingDeclarations), + std::back_inserter(worklist)); + + for (auto pending : worklist) { + extractDeclarations(state, compiler, *pending->getModuleContext(), nullptr, pending); + } +} + +void codeql::extractExtractLazyDeclarations(SwiftExtractorState& state, + swift::CompilerInstance& compiler) { + // Just in case + const int upperBound = 100; + int iteration = 0; + while (!state.pendingDeclarations.empty() && iteration++ < upperBound) { + extractLazy(state, compiler); + } + if (iteration >= upperBound) { + std::cerr << "Swift extractor reach upper bound while extracting lazy declarations\n"; + abort(); + } +} diff --git a/swift/extractor/SwiftExtractor.h b/swift/extractor/SwiftExtractor.h index b3cf98b6a59a..0847eb31d2da 100644 --- a/swift/extractor/SwiftExtractor.h +++ b/swift/extractor/SwiftExtractor.h @@ -7,4 +7,5 @@ namespace codeql { void extractSwiftFiles(SwiftExtractorState& state, swift::CompilerInstance& compiler); +void extractExtractLazyDeclarations(SwiftExtractorState& state, swift::CompilerInstance& compiler); } // namespace codeql diff --git a/swift/extractor/config/SwiftExtractorState.h b/swift/extractor/config/SwiftExtractorState.h index d244ec245392..8982e0b0aed1 100644 --- a/swift/extractor/config/SwiftExtractorState.h +++ b/swift/extractor/config/SwiftExtractorState.h @@ -24,6 +24,9 @@ struct SwiftExtractorState { // The path for the modules outputted by the underlying frontend run, ignoring path redirection std::vector originalOutputModules; + + std::unordered_set emittedDeclarations; + std::unordered_set pendingDeclarations; }; } // namespace codeql diff --git a/swift/extractor/infra/SwiftBodyEmissionStrategy.cpp b/swift/extractor/infra/SwiftBodyEmissionStrategy.cpp index 4713232a3458..7608379ac5fa 100644 --- a/swift/extractor/infra/SwiftBodyEmissionStrategy.cpp +++ b/swift/extractor/infra/SwiftBodyEmissionStrategy.cpp @@ -16,6 +16,9 @@ bool SwiftBodyEmissionStrategy::shouldEmitDeclBody(const swift::Decl& decl) { if (module != ¤tModule) { return false; } + if (currentLazyDeclaration && currentLazyDeclaration != &decl) { + return false; + } // ModuleDecl is a special case: if it passed the previous test, it is the current module // but it never has a source file, so we short circuit to emit it in any case if (!currentPrimarySourceFile || decl.getKind() == swift::DeclKind::Module) { diff --git a/swift/extractor/infra/SwiftBodyEmissionStrategy.h b/swift/extractor/infra/SwiftBodyEmissionStrategy.h index e67906405e0e..34eae944a554 100644 --- a/swift/extractor/infra/SwiftBodyEmissionStrategy.h +++ b/swift/extractor/infra/SwiftBodyEmissionStrategy.h @@ -8,13 +8,17 @@ namespace codeql { class SwiftBodyEmissionStrategy { public: SwiftBodyEmissionStrategy(swift::ModuleDecl& currentModule, - swift::SourceFile* currentPrimarySourceFile) - : currentModule(currentModule), currentPrimarySourceFile(currentPrimarySourceFile) {} + swift::SourceFile* currentPrimarySourceFile, + const swift::Decl* currentLazyDeclaration) + : currentModule(currentModule), + currentPrimarySourceFile(currentPrimarySourceFile), + currentLazyDeclaration(currentLazyDeclaration) {} bool shouldEmitDeclBody(const swift::Decl& decl); private: swift::ModuleDecl& currentModule; swift::SourceFile* currentPrimarySourceFile; + const swift::Decl* currentLazyDeclaration; }; } // namespace codeql diff --git a/swift/extractor/infra/SwiftDispatcher.h b/swift/extractor/infra/SwiftDispatcher.h index c6d90616a02d..3431ba3777e9 100644 --- a/swift/extractor/infra/SwiftDispatcher.h +++ b/swift/extractor/infra/SwiftDispatcher.h @@ -12,6 +12,7 @@ #include "swift/extractor/trap/generated/TrapClasses.h" #include "swift/extractor/infra/SwiftLocationExtractor.h" #include "swift/extractor/infra/SwiftBodyEmissionStrategy.h" +#include "swift/extractor/config/SwiftExtractorState.h" namespace codeql { @@ -45,10 +46,12 @@ class SwiftDispatcher { // all references and pointers passed as parameters to this constructor are supposed to outlive // the SwiftDispatcher SwiftDispatcher(const swift::SourceManager& sourceManager, + SwiftExtractorState& state, TrapDomain& trap, SwiftLocationExtractor& locationExtractor, SwiftBodyEmissionStrategy& bodyEmissionStrategy) : sourceManager{sourceManager}, + state{state}, trap{trap}, locationExtractor{locationExtractor}, bodyEmissionStrategy{bodyEmissionStrategy} {} @@ -57,6 +60,9 @@ class SwiftDispatcher { return std::move(encounteredModules); } + void extractedDeclaration(const swift::Decl* decl) { state.emittedDeclarations.insert(decl); } + void skippedDeclaration(const swift::Decl* decl) { state.pendingDeclarations.insert(decl); } + template void emit(Entry&& entry) { bool valid = true; @@ -302,6 +308,7 @@ class SwiftDispatcher { virtual void visit(const swift::CapturedValue* capture) = 0; const swift::SourceManager& sourceManager; + SwiftExtractorState& state; TrapDomain& trap; Store store; SwiftLocationExtractor& locationExtractor; diff --git a/swift/extractor/infra/TargetDomains.cpp b/swift/extractor/infra/TargetDomains.cpp index 4a1bc7633357..441a7ec70f04 100644 --- a/swift/extractor/infra/TargetDomains.cpp +++ b/swift/extractor/infra/TargetDomains.cpp @@ -13,6 +13,8 @@ static const char* typeToStr(TrapType type) { return "invocations"; case TrapType::linkage: return "linkage"; + case TrapType::lazy_declarations: + return "lazy_decls"; default: return ""; } diff --git a/swift/extractor/infra/TargetDomains.h b/swift/extractor/infra/TargetDomains.h index 654dd91eab31..f0a829098a4a 100644 --- a/swift/extractor/infra/TargetDomains.h +++ b/swift/extractor/infra/TargetDomains.h @@ -12,6 +12,7 @@ enum class TrapType { module, invocation, linkage, + lazy_declarations, }; std::filesystem::path getTrapPath(const SwiftExtractorState& state, diff --git a/swift/extractor/main.cpp b/swift/extractor/main.cpp index ffad31cbfe51..7547aff2e1f9 100644 --- a/swift/extractor/main.cpp +++ b/swift/extractor/main.cpp @@ -92,6 +92,7 @@ class Observer : public swift::FrontendObserver { void performedSemanticAnalysis(swift::CompilerInstance& compiler) override { codeql::extractSwiftFiles(state, compiler); codeql::extractSwiftInvocation(state, compiler, invocationTrap); + codeql::extractExtractLazyDeclarations(state, compiler); } private: diff --git a/swift/extractor/translators/DeclTranslator.h b/swift/extractor/translators/DeclTranslator.h index d6243d693c54..291a0a9f91c9 100644 --- a/swift/extractor/translators/DeclTranslator.h +++ b/swift/extractor/translators/DeclTranslator.h @@ -70,9 +70,11 @@ class DeclTranslator : public AstTranslatorBase { std::optional> entry; auto id = dispatcher.assignNewLabel(decl, mangledName(decl)); if (dispatcher.shouldEmitDeclBody(decl)) { + dispatcher.extractedDeclaration(&decl); entry.emplace(id); fillDecl(decl, *entry); } + dispatcher.skippedDeclaration(&decl); return entry; } From 276fec39fc1719b38a1216b6434fe52fb10e2597 Mon Sep 17 00:00:00 2001 From: Alex Denisov Date: Wed, 1 Mar 2023 15:41:49 +0100 Subject: [PATCH 2/5] Swift: consider lazy declaration emitted if the process lost in the race --- swift/extractor/SwiftExtractor.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/swift/extractor/SwiftExtractor.cpp b/swift/extractor/SwiftExtractor.cpp index 6a99a350e03e..657d3a0d0e60 100644 --- a/swift/extractor/SwiftExtractor.cpp +++ b/swift/extractor/SwiftExtractor.cpp @@ -123,6 +123,9 @@ static std::unordered_set extractDeclarations( auto trap = createTargetTrapDomain(state, filename, trapType); if (!trap) { // another process arrived first, nothing to do for us + if (lazyDeclaration) { + state.emittedDeclarations.insert(lazyDeclaration); + } return {}; } @@ -242,7 +245,7 @@ void codeql::extractExtractLazyDeclarations(SwiftExtractorState& state, extractLazy(state, compiler); } if (iteration >= upperBound) { - std::cerr << "Swift extractor reach upper bound while extracting lazy declarations\n"; + std::cerr << "Swift extractor reached upper bound while extracting lazy declarations\n"; abort(); } } From ffcb3827053463af1bf9cc0d704c982ebeb29051 Mon Sep 17 00:00:00 2001 From: Alex Denisov Date: Thu, 2 Mar 2023 15:59:14 +0100 Subject: [PATCH 3/5] Swift: only consider Builting and __ObjC declarations as lazy --- swift/extractor/SwiftExtractor.cpp | 2 -- swift/extractor/infra/SwiftDispatcher.h | 14 ++++++++++++-- swift/extractor/translators/DeclTranslator.h | 4 +++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/swift/extractor/SwiftExtractor.cpp b/swift/extractor/SwiftExtractor.cpp index 657d3a0d0e60..3125c2f5014a 100644 --- a/swift/extractor/SwiftExtractor.cpp +++ b/swift/extractor/SwiftExtractor.cpp @@ -224,13 +224,11 @@ static void cleanupPendingDeclarations(SwiftExtractorState& state) { } } } - static void extractLazy(SwiftExtractorState& state, swift::CompilerInstance& compiler) { cleanupPendingDeclarations(state); std::vector worklist; std::copy(std::begin(state.pendingDeclarations), std::end(state.pendingDeclarations), std::back_inserter(worklist)); - for (auto pending : worklist) { extractDeclarations(state, compiler, *pending->getModuleContext(), nullptr, pending); } diff --git a/swift/extractor/infra/SwiftDispatcher.h b/swift/extractor/infra/SwiftDispatcher.h index 3431ba3777e9..12c207e193fe 100644 --- a/swift/extractor/infra/SwiftDispatcher.h +++ b/swift/extractor/infra/SwiftDispatcher.h @@ -60,8 +60,18 @@ class SwiftDispatcher { return std::move(encounteredModules); } - void extractedDeclaration(const swift::Decl* decl) { state.emittedDeclarations.insert(decl); } - void skippedDeclaration(const swift::Decl* decl) { state.pendingDeclarations.insert(decl); } + void extractedDeclaration(const swift::Decl* decl) { + swift::ModuleDecl* module = decl->getModuleContext(); + if (module->isBuiltinModule() || module->getName().str() == "__ObjC") { + state.emittedDeclarations.insert(decl); + } + } + void skippedDeclaration(const swift::Decl* decl) { + swift::ModuleDecl* module = decl->getModuleContext(); + if (module->isBuiltinModule() || module->getName().str() == "__ObjC") { + state.pendingDeclarations.insert(decl); + } + } template void emit(Entry&& entry) { diff --git a/swift/extractor/translators/DeclTranslator.h b/swift/extractor/translators/DeclTranslator.h index 291a0a9f91c9..bda36ae40651 100644 --- a/swift/extractor/translators/DeclTranslator.h +++ b/swift/extractor/translators/DeclTranslator.h @@ -2,6 +2,7 @@ #include #include +#include #include "swift/extractor/translators/TranslatorBase.h" #include "swift/extractor/trap/generated/decl/TrapClasses.h" @@ -73,8 +74,9 @@ class DeclTranslator : public AstTranslatorBase { dispatcher.extractedDeclaration(&decl); entry.emplace(id); fillDecl(decl, *entry); + } else { + dispatcher.skippedDeclaration(&decl); } - dispatcher.skippedDeclaration(&decl); return entry; } From 60c15050976817eff3dbe1df3459c1e035f8f98e Mon Sep 17 00:00:00 2001 From: Alex Denisov Date: Fri, 3 Mar 2023 10:18:43 +0100 Subject: [PATCH 4/5] Swift: address review comments --- swift/extractor/SwiftExtractor.cpp | 39 +++++++++++--------- swift/extractor/config/SwiftExtractorState.h | 3 ++ swift/extractor/infra/SwiftDispatcher.h | 29 ++++++++------- swift/extractor/infra/TargetDomains.cpp | 2 +- swift/extractor/infra/TargetDomains.h | 2 +- swift/extractor/translators/DeclTranslator.h | 4 +- 6 files changed, 44 insertions(+), 35 deletions(-) diff --git a/swift/extractor/SwiftExtractor.cpp b/swift/extractor/SwiftExtractor.cpp index 3125c2f5014a..ebe82542f7c6 100644 --- a/swift/extractor/SwiftExtractor.cpp +++ b/swift/extractor/SwiftExtractor.cpp @@ -78,20 +78,22 @@ static fs::path getFilename(swift::ModuleDecl& module, return resolvePath(filename); } -static llvm::SmallVector getTopLevelDecls(swift::ModuleDecl& module, - swift::SourceFile* primaryFile, - const swift::Decl* lazyDeclaration) { - llvm::SmallVector ret; +static llvm::SmallVector getTopLevelDecls(swift::ModuleDecl& module, + swift::SourceFile* primaryFile, + const swift::Decl* lazyDeclaration) { + llvm::SmallVector ret; if (lazyDeclaration) { - ret.push_back(const_cast(lazyDeclaration)); + ret.push_back(lazyDeclaration); return ret; } ret.push_back(&module); + llvm::SmallVector topLevelDecls; if (primaryFile) { - primaryFile->getTopLevelDecls(ret); + primaryFile->getTopLevelDecls(topLevelDecls); } else { - module.getTopLevelDecls(ret); + module.getTopLevelDecls(topLevelDecls); } + ret.insert(ret.end(), topLevelDecls.data(), topLevelDecls.data() + topLevelDecls.size()); return ret; } @@ -100,7 +102,7 @@ static TrapType getTrapType(swift::SourceFile* primaryFile, const swift::Decl* l return TrapType::source; } if (lazyDeclaration) { - return TrapType::lazy_declarations; + return TrapType::lazy_declaration; } return TrapType::module; } @@ -199,10 +201,12 @@ void codeql::extractSwiftFiles(SwiftExtractorState& state, swift::CompilerInstan continue; } archiveFile(state.configuration, *sourceFile); - encounteredModules = extractDeclarations(state, compiler, *module, sourceFile, nullptr); + encounteredModules = + extractDeclarations(state, compiler, *module, sourceFile, /*lazy declaration*/ nullptr); } if (!isFromSourceFile) { - encounteredModules = extractDeclarations(state, compiler, *module, nullptr, nullptr); + encounteredModules = extractDeclarations(state, compiler, *module, /*source file*/ nullptr, + /*lazy declaration*/ nullptr); } for (auto encountered : encounteredModules) { if (state.encounteredModules.count(encountered) == 0) { @@ -214,23 +218,22 @@ void codeql::extractSwiftFiles(SwiftExtractorState& state, swift::CompilerInstan } static void cleanupPendingDeclarations(SwiftExtractorState& state) { - std::vector worklist; - std::copy(std::begin(state.pendingDeclarations), std::end(state.pendingDeclarations), - std::back_inserter(worklist)); - + std::vector worklist(std::begin(state.pendingDeclarations), + std::end(state.pendingDeclarations)); for (auto decl : worklist) { if (state.emittedDeclarations.count(decl)) { state.pendingDeclarations.erase(decl); } } } + static void extractLazy(SwiftExtractorState& state, swift::CompilerInstance& compiler) { cleanupPendingDeclarations(state); - std::vector worklist; - std::copy(std::begin(state.pendingDeclarations), std::end(state.pendingDeclarations), - std::back_inserter(worklist)); + std::vector worklist(std::begin(state.pendingDeclarations), + std::end(state.pendingDeclarations)); for (auto pending : worklist) { - extractDeclarations(state, compiler, *pending->getModuleContext(), nullptr, pending); + extractDeclarations(state, compiler, *pending->getModuleContext(), /*source file*/ nullptr, + pending); } } diff --git a/swift/extractor/config/SwiftExtractorState.h b/swift/extractor/config/SwiftExtractorState.h index 8982e0b0aed1..1855844d33d1 100644 --- a/swift/extractor/config/SwiftExtractorState.h +++ b/swift/extractor/config/SwiftExtractorState.h @@ -25,7 +25,10 @@ struct SwiftExtractorState { // The path for the modules outputted by the underlying frontend run, ignoring path redirection std::vector originalOutputModules; + // All lazy named declarations that were already emitted std::unordered_set emittedDeclarations; + + // Lazy named declarations that were not yet emitted and will be emitted each one separately std::unordered_set pendingDeclarations; }; diff --git a/swift/extractor/infra/SwiftDispatcher.h b/swift/extractor/infra/SwiftDispatcher.h index 12c207e193fe..b2c347e36e66 100644 --- a/swift/extractor/infra/SwiftDispatcher.h +++ b/swift/extractor/infra/SwiftDispatcher.h @@ -60,19 +60,6 @@ class SwiftDispatcher { return std::move(encounteredModules); } - void extractedDeclaration(const swift::Decl* decl) { - swift::ModuleDecl* module = decl->getModuleContext(); - if (module->isBuiltinModule() || module->getName().str() == "__ObjC") { - state.emittedDeclarations.insert(decl); - } - } - void skippedDeclaration(const swift::Decl* decl) { - swift::ModuleDecl* module = decl->getModuleContext(); - if (module->isBuiltinModule() || module->getName().str() == "__ObjC") { - state.pendingDeclarations.insert(decl); - } - } - template void emit(Entry&& entry) { bool valid = true; @@ -264,7 +251,23 @@ class SwiftDispatcher { locationExtractor.attachLocation(sourceManager, comment, entry.id); } + void extractedDeclaration(const swift::Decl& decl) { + if (isLazyDeclaration(decl)) { + state.emittedDeclarations.insert(&decl); + } + } + void skippedDeclaration(const swift::Decl& decl) { + if (isLazyDeclaration(decl)) { + state.pendingDeclarations.insert(&decl); + } + } + private: + bool isLazyDeclaration(const swift::Decl& decl) { + swift::ModuleDecl* module = decl.getModuleContext(); + return module->isBuiltinModule() || module->getName().str() == "__ObjC"; + } + template struct HasSize : std::false_type {}; diff --git a/swift/extractor/infra/TargetDomains.cpp b/swift/extractor/infra/TargetDomains.cpp index 441a7ec70f04..b43068589a18 100644 --- a/swift/extractor/infra/TargetDomains.cpp +++ b/swift/extractor/infra/TargetDomains.cpp @@ -13,7 +13,7 @@ static const char* typeToStr(TrapType type) { return "invocations"; case TrapType::linkage: return "linkage"; - case TrapType::lazy_declarations: + case TrapType::lazy_declaration: return "lazy_decls"; default: return ""; diff --git a/swift/extractor/infra/TargetDomains.h b/swift/extractor/infra/TargetDomains.h index f0a829098a4a..60ecd47db942 100644 --- a/swift/extractor/infra/TargetDomains.h +++ b/swift/extractor/infra/TargetDomains.h @@ -12,7 +12,7 @@ enum class TrapType { module, invocation, linkage, - lazy_declarations, + lazy_declaration, }; std::filesystem::path getTrapPath(const SwiftExtractorState& state, diff --git a/swift/extractor/translators/DeclTranslator.h b/swift/extractor/translators/DeclTranslator.h index bda36ae40651..e4713063d0af 100644 --- a/swift/extractor/translators/DeclTranslator.h +++ b/swift/extractor/translators/DeclTranslator.h @@ -71,11 +71,11 @@ class DeclTranslator : public AstTranslatorBase { std::optional> entry; auto id = dispatcher.assignNewLabel(decl, mangledName(decl)); if (dispatcher.shouldEmitDeclBody(decl)) { - dispatcher.extractedDeclaration(&decl); + dispatcher.extractedDeclaration(decl); entry.emplace(id); fillDecl(decl, *entry); } else { - dispatcher.skippedDeclaration(&decl); + dispatcher.skippedDeclaration(decl); } return entry; } From ae7a0c517cb2728402eb5818be44e58585f3d188 Mon Sep 17 00:00:00 2001 From: Alex Denisov Date: Fri, 3 Mar 2023 10:28:08 +0100 Subject: [PATCH 5/5] Swift: do not allocate mangler statically --- swift/extractor/SwiftExtractor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swift/extractor/SwiftExtractor.cpp b/swift/extractor/SwiftExtractor.cpp index ebe82542f7c6..9638ac2949b5 100644 --- a/swift/extractor/SwiftExtractor.cpp +++ b/swift/extractor/SwiftExtractor.cpp @@ -50,7 +50,7 @@ static fs::path getFilename(swift::ModuleDecl& module, return resolvePath(primaryFile->getFilename()); } if (lazyDeclaration) { - static SwiftMangler mangler; + SwiftMangler mangler; return mangler.mangledName(*lazyDeclaration); } // PCM clang module