From fcb1dcaa4c9f3059ad8f5f471f0c86795ad12966 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 6 May 2020 16:20:36 -0700 Subject: [PATCH 01/14] Requestify SIL parsing Replace `parseSourceFileSIL` with ParseSILModuleRequest, and add some convenience members to SILGenDescriptor to retrieve the SourceFile to be parsed. --- include/swift/AST/SILGenRequests.h | 29 ++++++++++++++++- include/swift/AST/SILGenTypeIDZone.def | 3 ++ include/swift/Subsystems.h | 3 -- lib/SIL/Parser/ParseSIL.cpp | 21 ++++++++----- lib/SILGen/SILGenRequests.cpp | 43 ++++++++++++++++++++++++++ 5 files changed, 88 insertions(+), 11 deletions(-) diff --git a/include/swift/AST/SILGenRequests.h b/include/swift/AST/SILGenRequests.h index c01443393062d..c20c014838308 100644 --- a/include/swift/AST/SILGenRequests.h +++ b/include/swift/AST/SILGenRequests.h @@ -41,7 +41,7 @@ void reportEvaluatedRequest(UnifiedStatsReporter &stats, const Request &request); struct SILGenDescriptor { - llvm::PointerUnion context; + llvm::PointerUnion context; Lowering::TypeConverter &conv; const SILOptions &opts; @@ -73,6 +73,17 @@ struct SILGenDescriptor { const SILOptions &opts) { return SILGenDescriptor{mod, conv, opts}; } + + /// For a single file input, returns a single element array containing that + /// file. Otherwise returns an array of each file in the module. + ArrayRef getFiles() const; + + /// If the module or file contains SIL that needs parsing, returns the file + /// to be parsed, or \c nullptr if parsing isn't required. + SourceFile *getSourceFileToParse() const; + + /// Whether the SIL is being emitted for a whole module. + bool isWholeModule() const; }; void simple_display(llvm::raw_ostream &out, const SILGenDescriptor &d); @@ -120,6 +131,22 @@ class SILGenWholeModuleRequest : bool isCached() const { return true; } }; +/// Parses a .sil file into a SILModule. +class ParseSILModuleRequest + : public SimpleRequest(SILGenDescriptor), + RequestFlags::Uncached> { +public: + using SimpleRequest::SimpleRequest; + +private: + friend SimpleRequest; + + // Evaluation. + std::unique_ptr + evaluate(Evaluator &evaluator, SILGenDescriptor desc) const; +}; + /// The zone number for SILGen. #define SWIFT_TYPEID_ZONE SILGen #define SWIFT_TYPEID_HEADER "swift/AST/SILGenTypeIDZone.def" diff --git a/include/swift/AST/SILGenTypeIDZone.def b/include/swift/AST/SILGenTypeIDZone.def index f1cc214b6977a..7f6c49392724d 100644 --- a/include/swift/AST/SILGenTypeIDZone.def +++ b/include/swift/AST/SILGenTypeIDZone.def @@ -20,3 +20,6 @@ SWIFT_REQUEST(SILGen, SILGenSourceFileRequest, SWIFT_REQUEST(SILGen, SILGenWholeModuleRequest, std::unique_ptr(SILGenDescriptor), Uncached, NoLocationInfo) +SWIFT_REQUEST(SILGen, ParseSILModuleRequest, + std::unique_ptr(SILGenDescriptor), + Uncached, NoLocationInfo) diff --git a/include/swift/Subsystems.h b/include/swift/Subsystems.h index ea277abc2c5fd..23bd0b34546db 100644 --- a/include/swift/Subsystems.h +++ b/include/swift/Subsystems.h @@ -102,9 +102,6 @@ namespace swift { /// @} - /// Parse a source file's SIL declarations into a given SIL module. - void parseSourceFileSIL(SourceFile &SF, SILParserState *sil); - /// Finish the code completion. void performCodeCompletionSecondPass(SourceFile &SF, CodeCompletionCallbacksFactory &Factory); diff --git a/lib/SIL/Parser/ParseSIL.cpp b/lib/SIL/Parser/ParseSIL.cpp index 4857b84e44bca..74ed927e269bc 100644 --- a/lib/SIL/Parser/ParseSIL.cpp +++ b/lib/SIL/Parser/ParseSIL.cpp @@ -17,6 +17,7 @@ #include "swift/AST/NameLookup.h" #include "swift/AST/NameLookupRequests.h" #include "swift/AST/ProtocolConformance.h" +#include "swift/AST/SILGenRequests.h" #include "swift/AST/SourceFile.h" #include "swift/AST/TypeCheckRequests.h" #include "swift/Basic/Defer.h" @@ -105,17 +106,23 @@ SILParserState::SILParserState(SILModule *M) SILParserState::~SILParserState() = default; -void swift::parseSourceFileSIL(SourceFile &SF, SILParserState *sil) { - auto bufferID = SF.getBufferID(); +std::unique_ptr +ParseSILModuleRequest::evaluate(Evaluator &evaluator, + SILGenDescriptor desc) const { + auto *SF = desc.getSourceFileToParse(); + assert(SF); + + auto bufferID = SF->getBufferID(); assert(bufferID); - FrontendStatsTracer tracer(SF.getASTContext().Stats, - "Parsing SIL"); - Parser parser(*bufferID, SF, sil->Impl.get(), - /*persistentParserState*/ nullptr, - /*syntaxTreeCreator*/ nullptr); + auto *mod = SF->getParentModule(); + auto silMod = SILModule::createEmptyModule(mod, desc.conv, desc.opts, + desc.isWholeModule()); + SILParserState parserState(silMod.get()); + Parser parser(*bufferID, *SF, parserState.Impl.get()); PrettyStackTraceParser StackTrace(parser); parser.parseTopLevelSIL(); + return silMod; } //===----------------------------------------------------------------------===// diff --git a/lib/SILGen/SILGenRequests.cpp b/lib/SILGen/SILGenRequests.cpp index 3c077efcd1ed0..96a17359ec328 100644 --- a/lib/SILGen/SILGenRequests.cpp +++ b/lib/SILGen/SILGenRequests.cpp @@ -56,6 +56,49 @@ evaluator::DependencySource SILGenSourceFileRequest::readDependencySource( }; } +ArrayRef SILGenDescriptor::getFiles() const { + if (auto *mod = context.dyn_cast()) + return mod->getFiles(); + + // For a single file, we can form an ArrayRef that points at its storage in + // the union. + return llvm::makeArrayRef(*context.getAddrOfPtr1()); +} + +bool SILGenDescriptor::isWholeModule() const { + return context.is(); +} + +SourceFile *SILGenDescriptor::getSourceFileToParse() const { +#ifndef NDEBUG + auto sfCount = llvm::count_if(getFiles(), [](FileUnit *file) { + return isa(file); + }); + auto silFileCount = llvm::count_if(getFiles(), [](FileUnit *file) { + auto *SF = dyn_cast(file); + return SF && SF->Kind == SourceFileKind::SIL; + }); + assert(silFileCount == 0 || (silFileCount == 1 && sfCount == 1) && + "Cannot currently mix a .sil file with other SourceFiles"); +#endif + + for (auto *file : getFiles()) { + // Skip other kinds of files. + auto *SF = dyn_cast(file); + if (!SF) + continue; + + // Given the above precondition that a .sil file isn't mixed with other + // SourceFiles, we can return a SIL file if we have it, or return nullptr. + if (SF->Kind == SourceFileKind::SIL) { + return SF; + } else { + return nullptr; + } + } + return nullptr; +} + // Define request evaluation functions for each of the SILGen requests. static AbstractRequestFunction *silGenRequestFunctions[] = { #define SWIFT_REQUEST(Zone, Name, Sig, Caching, LocOptions) \ From 11d8f70dec8075882d7f2f5cc024d88db2241926 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 6 May 2020 16:20:37 -0700 Subject: [PATCH 02/14] Trigger SIL parsing from performSILGeneration Rather than eagerly parsing an input .sil file in `performSemaUpTo`, trigger it from `performSILGeneration`. This will allow us to remove the SILModule stored on the CompilerInstance and will eventually allow the various SIL tools to just call into `performSILGeneration` without needing to call `performSema`. --- lib/Frontend/Frontend.cpp | 7 ------- lib/FrontendTool/FrontendTool.cpp | 10 ++-------- lib/SILGen/SILGen.cpp | 10 ++++++++++ 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp index cfb6c808c4dd4..6d2e26ea74d74 100644 --- a/lib/Frontend/Frontend.cpp +++ b/lib/Frontend/Frontend.cpp @@ -814,13 +814,6 @@ void CompilerInstance::performSemaUpTo(SourceFile::ASTStage_t LimitStage) { forEachFileToTypeCheck([&](SourceFile &SF) { performTypeChecking(SF); - - // Parse the SIL decls if needed. - // TODO: Requestify SIL parsing. - if (TheSILModule) { - SILParserState SILContext(TheSILModule.get()); - parseSourceFileSIL(SF, &SILContext); - } }); finishTypeChecking(); diff --git a/lib/FrontendTool/FrontendTool.cpp b/lib/FrontendTool/FrontendTool.cpp index 113928024ea2d..f42e4a98f00b0 100644 --- a/lib/FrontendTool/FrontendTool.cpp +++ b/lib/FrontendTool/FrontendTool.cpp @@ -1066,17 +1066,11 @@ static bool performCompileStepsPostSILGen(CompilerInstance &Instance, static bool performCompileStepsPostSema(CompilerInstance &Instance, int &ReturnValue, FrontendObserver *observer) { - auto mod = Instance.getMainModule(); - if (auto SM = Instance.takeSILModule()) { - const PrimarySpecificPaths PSPs = - Instance.getPrimarySpecificPathsForAtMostOnePrimary(); - return performCompileStepsPostSILGen(Instance, std::move(SM), mod, PSPs, - ReturnValue, observer); - } - const auto &Invocation = Instance.getInvocation(); const SILOptions &SILOpts = Invocation.getSILOptions(); const FrontendOptions &opts = Invocation.getFrontendOptions(); + + auto *mod = Instance.getMainModule(); if (!opts.InputsAndOutputs.hasPrimaryInputs()) { // If there are no primary inputs the compiler is in WMO mode and builds one // SILModule for the entire module. diff --git a/lib/SILGen/SILGen.cpp b/lib/SILGen/SILGen.cpp index 9e47438e3d134..7beb6644996d0 100644 --- a/lib/SILGen/SILGen.cpp +++ b/lib/SILGen/SILGen.cpp @@ -1860,6 +1860,11 @@ class SILGenModuleRAII { std::unique_ptr SILGenSourceFileRequest::evaluate(Evaluator &evaluator, SILGenDescriptor desc) const { + // If we have a .sil file to parse, defer to the parsing request. + if (desc.getSourceFileToParse()) { + return llvm::cantFail(evaluator(ParseSILModuleRequest{desc})); + } + auto *unit = desc.context.get(); auto *mod = unit->getParentModule(); auto M = std::unique_ptr( @@ -1879,6 +1884,11 @@ SILGenSourceFileRequest::evaluate(Evaluator &evaluator, std::unique_ptr SILGenWholeModuleRequest::evaluate(Evaluator &evaluator, SILGenDescriptor desc) const { + // If we have a .sil file to parse, defer to the parsing request. + if (desc.getSourceFileToParse()) { + return llvm::cantFail(evaluator(ParseSILModuleRequest{desc})); + } + auto *mod = desc.context.get(); auto M = std::unique_ptr( new SILModule(mod, desc.conv, desc.opts, mod, /*wholeModule*/ true)); From d92374c2fcb0d739238c3dc4e7aa19f98b52223f Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 6 May 2020 16:20:37 -0700 Subject: [PATCH 03/14] [ParseSIL] Return empty SILModule on error Because we were previously performing SIL parsing during `performSema`, we were relying on the pipeline being stopped before reaching the SIL pipeline passes. However under a lazy evaluation model, we can't rely on that. Instead, just return an empty SILModule if we encounter a parsing error. --- include/swift/Parse/Parser.h | 3 ++- lib/Parse/ParseDecl.cpp | 6 +++++- lib/SIL/Parser/ParseSIL.cpp | 9 ++++++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/include/swift/Parse/Parser.h b/include/swift/Parse/Parser.h index 219bfd88fcc2f..e7c46332260c5 100644 --- a/include/swift/Parse/Parser.h +++ b/include/swift/Parse/Parser.h @@ -888,7 +888,8 @@ class Parser { void parseTopLevel(SmallVectorImpl &decls); /// Parse the top-level SIL decls into the SIL module. - void parseTopLevelSIL(); + /// \returns \c true if there was a parsing error. + bool parseTopLevelSIL(); /// Flags that control the parsing of declarations. enum ParseDeclFlags { diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 99b540e8854c8..fffd40535dc34 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -241,7 +241,7 @@ void Parser::parseTopLevel(SmallVectorImpl &decls) { TokReceiver->finalize(); } -void Parser::parseTopLevelSIL() { +bool Parser::parseTopLevelSIL() { assert(SIL && isInSILMode()); // Prime the lexer. @@ -253,6 +253,7 @@ void Parser::parseTopLevelSIL() { skipSingle(); }; + auto hadError = false; while (!Tok.is(tok::eof)) { // If we run into a Swift decl, skip over until we find the next SIL decl. if (isStartOfSwiftDecl()) { @@ -269,6 +270,7 @@ void Parser::parseTopLevelSIL() { if (SIL->parse##NAME(*this)) { \ Lexer::SILBodyRAII sbr(*L); \ skipToNextSILDecl(); \ + hadError = true; \ } \ break; \ } @@ -288,9 +290,11 @@ void Parser::parseTopLevelSIL() { // or a SIL decl. Emit an error and skip ahead to the next SIL decl. diagnose(Tok, diag::expected_sil_keyword); skipToNextSILDecl(); + hadError = true; break; } } + return hadError; } ParserResult Parser::parseExtendedAvailabilitySpecList( diff --git a/lib/SIL/Parser/ParseSIL.cpp b/lib/SIL/Parser/ParseSIL.cpp index 74ed927e269bc..5b520f663f5d4 100644 --- a/lib/SIL/Parser/ParseSIL.cpp +++ b/lib/SIL/Parser/ParseSIL.cpp @@ -121,7 +121,14 @@ ParseSILModuleRequest::evaluate(Evaluator &evaluator, SILParserState parserState(silMod.get()); Parser parser(*bufferID, *SF, parserState.Impl.get()); PrettyStackTraceParser StackTrace(parser); - parser.parseTopLevelSIL(); + + auto hadError = parser.parseTopLevelSIL(); + if (hadError) { + // The rest of the SIL pipeline expects well-formed SIL, so if we encounter + // a parsing error, just return an empty SIL module. + return SILModule::createEmptyModule(mod, desc.conv, desc.opts, + desc.isWholeModule()); + } return silMod; } From e249a71ead56f10ac56abfd1bcbbdbf02a6dd57f Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 6 May 2020 16:20:38 -0700 Subject: [PATCH 04/14] Update SIL tooling Have the tools call into `performSILGeneration` in order to retrieve the SILModule rather than getting it from the CompilerInstance. --- .../SILFunctionExtractor.cpp | 30 +++++----- tools/sil-llvm-gen/SILLLVMGen.cpp | 38 ++++++------ tools/sil-nm/SILNM.cpp | 24 ++++---- tools/sil-opt/SILOpt.cpp | 60 +++++++++---------- 4 files changed, 74 insertions(+), 78 deletions(-) diff --git a/tools/sil-func-extractor/SILFunctionExtractor.cpp b/tools/sil-func-extractor/SILFunctionExtractor.cpp index 43c0dcd0b25bd..f3524e55892da 100644 --- a/tools/sil-func-extractor/SILFunctionExtractor.cpp +++ b/tools/sil-func-extractor/SILFunctionExtractor.cpp @@ -272,16 +272,16 @@ int main(int argc, char **argv) { if (CI.getASTContext().hadError()) return 1; - // Load the SIL if we have a module. We have to do this after SILParse - // creating the unfortunate double if statement. - if (Invocation.hasSerializedAST()) { - assert(!CI.hasSILModule() && - "performSema() should not create a SILModule."); - CI.createSILModule(); - std::unique_ptr SL = SerializedSILLoader::create( - CI.getASTContext(), CI.getSILModule(), nullptr); - - if (extendedInfo.isSIB() || DisableSILLinking) + auto SILMod = performSILGeneration(CI.getMainModule(), CI.getSILTypes(), + CI.getSILOptions()); + + // Load the SIL if we have a non-SIB serialized module. SILGen handles SIB for + // us. + if (Invocation.hasSerializedAST() && !extendedInfo.isSIB()) { + auto SL = SerializedSILLoader::create( + CI.getASTContext(), SILMod.get(), nullptr); + + if (DisableSILLinking) SL->getAllForModule(CI.getMainModule()->getName(), nullptr); else SL->getAll(); @@ -329,7 +329,7 @@ int main(int argc, char **argv) { llvm::errs() << " " << str << '\n'; })); - removeUnwantedFunctions(CI.getSILModule(), MangledNames, DemangledNames); + removeUnwantedFunctions(SILMod.get(), MangledNames, DemangledNames); if (EmitSIB) { llvm::SmallString<128> OutputFile; @@ -350,7 +350,7 @@ int main(int argc, char **argv) { serializationOpts.SerializeAllSIL = true; serializationOpts.IsSIB = true; - serialize(CI.getMainModule(), serializationOpts, CI.getSILModule()); + serialize(CI.getMainModule(), serializationOpts, SILMod.get()); } else { const StringRef OutputFile = OutputFilename.size() ? StringRef(OutputFilename) : "-"; @@ -360,8 +360,7 @@ int main(int argc, char **argv) { SILOpts.EmitSortedSIL = EnableSILSortOutput; if (OutputFile == "-") { - CI.getSILModule()->print(llvm::outs(), CI.getMainModule(), SILOpts, - !DisableASTDump); + SILMod->print(llvm::outs(), CI.getMainModule(), SILOpts, !DisableASTDump); } else { std::error_code EC; llvm::raw_fd_ostream OS(OutputFile, EC, llvm::sys::fs::F_None); @@ -370,8 +369,7 @@ int main(int argc, char **argv) { << '\n'; return 1; } - CI.getSILModule()->print(OS, CI.getMainModule(), SILOpts, - !DisableASTDump); + SILMod->print(OS, CI.getMainModule(), SILOpts, !DisableASTDump); } } } diff --git a/tools/sil-llvm-gen/SILLLVMGen.cpp b/tools/sil-llvm-gen/SILLLVMGen.cpp index 54b2efafbb426..3ae077d4a62d2 100644 --- a/tools/sil-llvm-gen/SILLLVMGen.cpp +++ b/tools/sil-llvm-gen/SILLLVMGen.cpp @@ -184,26 +184,28 @@ int main(int argc, char **argv) { if (CI.getASTContext().hadError()) return 1; - // Load the SIL if we have a module. We have to do this after SILParse - // creating the unfortunate double if statement. - if (Invocation.hasSerializedAST()) { - assert(!CI.hasSILModule() && - "performSema() should not create a SILModule."); - CI.createSILModule(); - std::unique_ptr SL = SerializedSILLoader::create( - CI.getASTContext(), CI.getSILModule(), nullptr); - - if (extendedInfo.isSIB()) - SL->getAllForModule(CI.getMainModule()->getName(), nullptr); - else - SL->getAll(); + auto *mod = CI.getMainModule(); + assert(mod->getFiles().size() == 1); + + std::unique_ptr SILMod; + if (PerformWMO) { + SILMod = performSILGeneration(mod, CI.getSILTypes(), CI.getSILOptions()); + } else { + SILMod = performSILGeneration(*mod->getFiles()[0], CI.getSILTypes(), + CI.getSILOptions()); + } + + // Load the SIL if we have a non-SIB serialized module. SILGen handles SIB for + // us. + if (Invocation.hasSerializedAST() && !extendedInfo.isSIB()) { + auto SL = SerializedSILLoader::create( + CI.getASTContext(), SILMod.get(), nullptr); + SL->getAll(); } const PrimarySpecificPaths PSPs(OutputFilename, InputFilename); - auto Mod = - performIRGeneration(Opts, CI.getMainModule(), CI.takeSILModule(), - CI.getMainModule()->getName().str(), - PSPs, - ArrayRef()); + auto Mod = performIRGeneration(Opts, CI.getMainModule(), std::move(SILMod), + CI.getMainModule()->getName().str(), PSPs, + ArrayRef()); return CI.getASTContext().hadError(); } diff --git a/tools/sil-nm/SILNM.cpp b/tools/sil-nm/SILNM.cpp index 04c754faebb3b..c477ca5fc7929 100644 --- a/tools/sil-nm/SILNM.cpp +++ b/tools/sil-nm/SILNM.cpp @@ -186,22 +186,18 @@ int main(int argc, char **argv) { if (CI.getASTContext().hadError()) return 1; - // Load the SIL if we have a module. We have to do this after SILParse - // creating the unfortunate double if statement. - if (Invocation.hasSerializedAST()) { - assert(!CI.hasSILModule() && - "performSema() should not create a SILModule."); - CI.createSILModule(); - std::unique_ptr SL = SerializedSILLoader::create( - CI.getASTContext(), CI.getSILModule(), nullptr); - - if (extendedInfo.isSIB()) - SL->getAllForModule(CI.getMainModule()->getName(), nullptr); - else - SL->getAll(); + auto SILMod = performSILGeneration(CI.getMainModule(), CI.getSILTypes(), + CI.getSILOptions()); + + // Load the SIL if we have a non-SIB serialized module. SILGen handles SIB for + // us. + if (Invocation.hasSerializedAST() && !extendedInfo.isSIB()) { + auto SL = SerializedSILLoader::create( + CI.getASTContext(), SILMod.get(), nullptr); + SL->getAll(); } - nmModule(CI.getSILModule()); + nmModule(SILMod.get()); return CI.getASTContext().hadError(); } diff --git a/tools/sil-opt/SILOpt.cpp b/tools/sil-opt/SILOpt.cpp index 3a160c34fa107..961edd7335dcf 100644 --- a/tools/sil-opt/SILOpt.cpp +++ b/tools/sil-opt/SILOpt.cpp @@ -443,24 +443,29 @@ int main(int argc, char **argv) { if (HadError) return finishDiagProcessing(1); - // Load the SIL if we have a module. We have to do this after SILParse - // creating the unfortunate double if statement. - if (Invocation.hasSerializedAST()) { - assert(!CI.hasSILModule() && - "performSema() should not create a SILModule."); - CI.createSILModule(); - std::unique_ptr SL = SerializedSILLoader::create( - CI.getASTContext(), CI.getSILModule(), nullptr); - - if (extendedInfo.isSIB() || DisableSILLinking) + auto *mod = CI.getMainModule(); + assert(mod->getFiles().size() == 1); + + std::unique_ptr SILMod; + if (PerformWMO) { + SILMod = performSILGeneration(mod, CI.getSILTypes(), CI.getSILOptions()); + } else { + SILMod = performSILGeneration(*mod->getFiles()[0], CI.getSILTypes(), + CI.getSILOptions()); + } + SILMod->setSerializeSILAction([]{}); + + // Load the SIL if we have a non-SIB serialized module. SILGen handles SIB for + // us. + if (Invocation.hasSerializedAST() && !extendedInfo.isSIB()) { + auto SL = SerializedSILLoader::create( + CI.getASTContext(), SILMod.get(), nullptr); + if (DisableSILLinking) SL->getAllForModule(CI.getMainModule()->getName(), nullptr); else SL->getAll(); } - if (CI.getSILModule()) - CI.getSILModule()->setSerializeSILAction([]{}); - if (!RemarksFilename.empty()) { llvm::Expected formatOrErr = llvm::remarks::parseFormat(RemarksFormat); @@ -474,24 +479,21 @@ int main(int argc, char **argv) { SILOpts.OptRecordFormat = *formatOrErr; } - CI.getSILModule()->installSILRemarkStreamer(); + SILMod->installSILRemarkStreamer(); } if (OptimizationGroup == OptGroup::Diagnostics) { - runSILDiagnosticPasses(*CI.getSILModule()); + runSILDiagnosticPasses(*SILMod.get()); } else if (OptimizationGroup == OptGroup::Performance) { - runSILOptimizationPasses(*CI.getSILModule()); + runSILOptimizationPasses(*SILMod.get()); } else if (OptimizationGroup == OptGroup::Lowering) { - runSILLoweringPasses(*CI.getSILModule()); + runSILLoweringPasses(*SILMod.get()); } else { - auto *SILMod = CI.getSILModule(); - { - auto T = irgen::createIRGenModule( - SILMod, Invocation.getOutputFilenameForAtMostOnePrimary(), - Invocation.getMainInputFilenameForDebugInfoForAtMostOnePrimary(), ""); - runCommandLineSelectedPasses(SILMod, T.second); - irgen::deleteIRGenModule(T); - } + auto T = irgen::createIRGenModule( + SILMod.get(), Invocation.getOutputFilenameForAtMostOnePrimary(), + Invocation.getMainInputFilenameForDebugInfoForAtMostOnePrimary(), ""); + runCommandLineSelectedPasses(SILMod.get(), T.second); + irgen::deleteIRGenModule(T); } if (EmitSIB) { @@ -513,7 +515,7 @@ int main(int argc, char **argv) { serializationOpts.SerializeAllSIL = true; serializationOpts.IsSIB = true; - serialize(CI.getMainModule(), serializationOpts, CI.getSILModule()); + serialize(CI.getMainModule(), serializationOpts, SILMod.get()); } else { const StringRef OutputFile = OutputFilename.size() ? StringRef(OutputFilename) : "-"; @@ -521,8 +523,7 @@ int main(int argc, char **argv) { SILOpts.EmitVerboseSIL = EmitVerboseSIL; SILOpts.EmitSortedSIL = EnableSILSortOutput; if (OutputFile == "-") { - CI.getSILModule()->print(llvm::outs(), CI.getMainModule(), - SILOpts, !DisableASTDump); + SILMod->print(llvm::outs(), CI.getMainModule(), SILOpts, !DisableASTDump); } else { std::error_code EC; llvm::raw_fd_ostream OS(OutputFile, EC, llvm::sys::fs::F_None); @@ -531,8 +532,7 @@ int main(int argc, char **argv) { << EC.message() << '\n'; return finishDiagProcessing(1); } - CI.getSILModule()->print(OS, CI.getMainModule(), SILOpts, - !DisableASTDump); + SILMod->print(OS, CI.getMainModule(), SILOpts, !DisableASTDump); } } From 70abfd3252ec9410467550d4c9e62012ad41ef0e Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 6 May 2020 16:20:38 -0700 Subject: [PATCH 05/14] [Frontend] Remove TheSILModule Now that SIL parsing is handled lazily, the CompilerInstance no longer needs to hang onto a SILModule. --- include/swift/Frontend/Frontend.h | 16 ----------- lib/Frontend/Frontend.cpp | 22 --------------- lib/FrontendTool/FrontendTool.cpp | 47 +++++++++---------------------- 3 files changed, 13 insertions(+), 72 deletions(-) diff --git a/include/swift/Frontend/Frontend.h b/include/swift/Frontend/Frontend.h index be15d866627e5..09dc478b67bf6 100644 --- a/include/swift/Frontend/Frontend.h +++ b/include/swift/Frontend/Frontend.h @@ -425,7 +425,6 @@ class CompilerInstance { DiagnosticEngine Diagnostics{SourceMgr}; std::unique_ptr Context; std::unique_ptr TheSILTypes; - std::unique_ptr TheSILModule; std::unique_ptr DiagVerifier; /// Null if no tracker. @@ -498,8 +497,6 @@ class CompilerInstance { Lowering::TypeConverter &getSILTypes(); - void createSILModule(); - void addDiagnosticConsumer(DiagnosticConsumer *DC) { Diagnostics.addConsumer(*DC); } @@ -517,16 +514,6 @@ class CompilerInstance { UnifiedStatsReporter *getStatsReporter() const { return Stats.get(); } - SILModule *getSILModule() { - return TheSILModule.get(); - } - - std::unique_ptr takeSILModule(); - - bool hasSILModule() { - return static_cast(TheSILModule); - } - ModuleDecl *getMainModule() const; MemoryBufferSerializedModuleLoader * @@ -659,9 +646,6 @@ class CompilerInstance { public: void freeASTContext(); - /// Frees up the SILModule that this instance is holding on to. - void freeSILModule(); - private: /// Load stdlib & return true if should continue, i.e. no error bool loadStdlib(); diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp index 6d2e26ea74d74..a87fd6c868921 100644 --- a/lib/Frontend/Frontend.cpp +++ b/lib/Frontend/Frontend.cpp @@ -186,14 +186,6 @@ Lowering::TypeConverter &CompilerInstance::getSILTypes() { return *tc; } -void CompilerInstance::createSILModule() { - assert(MainModule && "main module not created yet"); - // Assume WMO if a -primary-file option was not provided. - TheSILModule = SILModule::createEmptyModule( - getMainModule(), getSILTypes(), Invocation.getSILOptions(), - Invocation.getFrontendOptions().InputsAndOutputs.isWholeModule()); -} - void CompilerInstance::recordPrimaryInputBuffer(unsigned BufID) { PrimaryBufferIDs.insert(BufID); } @@ -679,10 +671,6 @@ CompilerInstance::openModuleDoc(const InputFile &input) { return None; } -std::unique_ptr CompilerInstance::takeSILModule() { - return std::move(TheSILModule); -} - /// Implicitly import the SwiftOnoneSupport module in non-optimized /// builds. This allows for use of popular specialized functions /// from the standard library, which makes the non-optimized builds @@ -760,14 +748,6 @@ void CompilerInstance::performSemaUpTo(SourceFile::ASTStage_t LimitStage) { ModuleDecl *mainModule = getMainModule(); Context->LoadedModules[mainModule->getName()] = mainModule; - if (Invocation.getInputKind() == InputFileKind::SIL) { - assert(!InputSourceCodeBufferIDs.empty()); - assert(InputSourceCodeBufferIDs.size() == 1); - assert(MainBufferID != NO_SUCH_BUFFER); - assert(isPrimaryInput(MainBufferID) || isWholeModuleCompilation()); - createSILModule(); - } - if (Invocation.getImplicitStdlibKind() == ImplicitStdlibKind::Stdlib) { if (!loadStdlib()) return; @@ -989,8 +969,6 @@ void CompilerInstance::freeASTContext() { PrimarySourceFiles.clear(); } -void CompilerInstance::freeSILModule() { TheSILModule.reset(); } - /// Perform "stable" optimizations that are invariant across compiler versions. static bool performMandatorySILPasses(CompilerInvocation &Invocation, SILModule *SM) { diff --git a/lib/FrontendTool/FrontendTool.cpp b/lib/FrontendTool/FrontendTool.cpp index f42e4a98f00b0..78005108e7463 100644 --- a/lib/FrontendTool/FrontendTool.cpp +++ b/lib/FrontendTool/FrontendTool.cpp @@ -1461,30 +1461,24 @@ static bool validateTBDIfNeeded(const CompilerInvocation &Invocation, } } -enum class DeallocatableResources { - None, - SILModule, - SILModuleAndASTContext, -}; -static DeallocatableResources -computeDeallocatableResources(const CompilerInstance &Instance) { - // If the stats reporter is installed, we need the ASTContext and SILModule - // to live through the entire compilation process. +static void freeASTContextIfPossible(CompilerInstance &Instance) { + // If the stats reporter is installed, we need the ASTContext to live through + // the entire compilation process. if (Instance.getASTContext().Stats) { - return DeallocatableResources::None; + return; } // If we're going to dump the API of the module, we cannot tear down // the ASTContext, as that would cause the module to be freed prematurely. const auto &opts = Instance.getInvocation().getFrontendOptions(); if (!opts.DumpAPIPath.empty()) { - return DeallocatableResources::SILModule; + return; } // Verifying incremental dependencies relies on access to the Swift Module's - // source files. We can still free the SIL module, though. + // source files. if (opts.EnableIncrementalDependencyVerifier) { - return DeallocatableResources::SILModule; + return; } // If there are multiple primary inputs it is too soon to free @@ -1493,29 +1487,14 @@ computeDeallocatableResources(const CompilerInstance &Instance) { // unlikely to reduce the peak heap size. So, only optimize the // single-primary-case (or WMO). if (opts.InputsAndOutputs.hasMultiplePrimaryInputs()) { - return DeallocatableResources::SILModule; + return; } - return DeallocatableResources::SILModuleAndASTContext; -} - -static void freeDeallocatableResourcesIfPossible(CompilerInstance &Instance) { - switch (computeDeallocatableResources(Instance)) { - case DeallocatableResources::None: - break; - case DeallocatableResources::SILModule: - Instance.freeSILModule(); - break; - case DeallocatableResources::SILModuleAndASTContext: - Instance.freeSILModule(); - - // Make sure we emit dependencies now, because we can't do it after the - // context is gone. - emitReferenceDependenciesForAllPrimaryInputsIfNeeded(Instance); + // Make sure we emit dependencies now, because we can't do it after the + // context is gone. + emitReferenceDependenciesForAllPrimaryInputsIfNeeded(Instance); - Instance.freeASTContext(); - break; - } + Instance.freeASTContext(); } static bool generateCode(CompilerInstance &Instance, StringRef OutputFilename, @@ -1528,7 +1507,7 @@ static bool generateCode(CompilerInstance &Instance, StringRef OutputFilename, Instance.getASTContext().LangOpts.EffectiveLanguageVersion; // Free up some compiler resources now that we have an IRModule. - freeDeallocatableResourcesIfPossible(Instance); + freeASTContextIfPossible(Instance); // Now that we have a single IR Module, hand it over to performLLVM. return performLLVM(opts, Instance.getDiags(), nullptr, HashGlobal, IRModule, From b619d309086f0db6fe65eba02dd5cbe02263b8bc Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Tue, 28 Apr 2020 16:37:38 -0700 Subject: [PATCH 06/14] [ownership] Track /all/ non consuming uses and emit errors for all of them instead of relying on jsut the last one in a block. Beyond allowing us to emit better errors, this will allow me to (in a subsequent commit) count the amount of uses that are "outside" of the linear lifetime. I can then compare that against a passed in set of "non consuming uses". If the count of the number of uses "outside" of the linear lifetime equals the count of the passed in set of "non consuming uses", then I know that /all/ non consuming uses that I am testing against are not co-incident with the linear lifetime, meaning that they can not effect (in a local, direct sense) the linear lifetime. I am going to use that information to determine when it is safe to convert an inout form a load [copy] to a load_borrow in the face of local mutations. I can pass the set of local mutations as non-consuming uses to a linear lifetime consisting of the load [copy]/destroy_values and thus prove that no writes occur in between the load [copy]/destroy_value meaning it is safe to conver them to borrow form. NOTE: The aforementioned optimization is an extension of an optimization already in tree that just bails if we have any writes to an inout locally, which is so unfortunate. --- include/swift/Basic/FrozenMultiMap.h | 2 + lib/SIL/Verifier/LinearLifetimeChecker.cpp | 135 ++++++++---------- test/SIL/ownership-verifier/arguments.sil | 110 +++++++------- .../borrow_scope_introducing_operands.sil | 49 ++++--- 4 files changed, 141 insertions(+), 155 deletions(-) diff --git a/include/swift/Basic/FrozenMultiMap.h b/include/swift/Basic/FrozenMultiMap.h index b8f44f13f5f4d..540c138a4d9f5 100644 --- a/include/swift/Basic/FrozenMultiMap.h +++ b/include/swift/Basic/FrozenMultiMap.h @@ -231,6 +231,8 @@ class FrozenMultiMap { /// Return a range of (key, ArrayRef) pairs. The keys are guaranteed to /// be in key sorted order and the ArrayRef are in insertion order. + /// + /// The range skips all erased (key, ArrayRef) entries. RangeType getRange() const { assert(isFrozen() && "Can not create range until data structure is frozen?!"); diff --git a/lib/SIL/Verifier/LinearLifetimeChecker.cpp b/lib/SIL/Verifier/LinearLifetimeChecker.cpp index b0510de4e7926..8aa4130895b33 100644 --- a/lib/SIL/Verifier/LinearLifetimeChecker.cpp +++ b/lib/SIL/Verifier/LinearLifetimeChecker.cpp @@ -22,6 +22,9 @@ #define DEBUG_TYPE "sil-linear-lifetime-checker" #include "LinearLifetimeCheckerPrivate.h" +#include "swift/Basic/BlotMapVector.h" +#include "swift/Basic/Defer.h" +#include "swift/Basic/FrozenMultiMap.h" #include "swift/SIL/BasicBlockUtils.h" #include "swift/SIL/OwnershipUtils.h" #include "swift/SIL/SILBasicBlock.h" @@ -80,7 +83,13 @@ struct State { /// The set of blocks with non-consuming uses and the associated /// non-consuming use SILInstruction. - llvm::SmallDenseMap blocksWithNonConsumingUses; + /// + /// NOTE: This is initialized in initializeAllNonConsumingUses after which it + /// is frozen. Before this is frozen, one can only add new (Key, Value) pairs + /// to the map. Once frozen, map operations and blot-erase operations can be + /// performed. Additionally it provides a getRange() operation that can be + /// used to iterate over all (Key, [Value]) pairs ignoring erased keys. + SmallFrozenMultiMap blocksWithNonConsumingUses; /// The worklist that we use when performing our block dataflow. SmallVector worklist; @@ -168,58 +177,39 @@ struct State { void State::initializeAllNonConsumingUses( ArrayRef nonConsumingUsers) { + SWIFT_DEFER { + // Once we have finished initializing blocksWithNonConsumingUses, we need to + // freeze it. By using a defer here, we can make sure we don't forget to do + // this below. + blocksWithNonConsumingUses.setFrozen(); + }; + for (Operand *use : nonConsumingUsers) { auto *userBlock = use->getUser()->getParent(); - // First check if this non consuming user is in our definition block. If so, - // validate that it is strictly after our defining instruction. If so, just - // emit an error now and exit. - if (userBlock == getBeginBlock()) { - if (std::find_if(beginInst->getIterator(), userBlock->end(), - [&use](const SILInstruction &inst) -> bool { - return use->getUser() == &inst; - }) == userBlock->end()) { - - errorBuilder.handleUseAfterFree([&] { - llvm::errs() << "Found use before def?!\n" - << "Value: "; - if (auto v = value) { - llvm::errs() << *v; - } else { - llvm::errs() << "N/A. \n"; - } - }); - return; - } - } - - // First try to associate User with User->getParent(). - auto result = - blocksWithNonConsumingUses.insert(std::make_pair(userBlock, use)); - - // If the insertion succeeds, then we know that there is no more work to - // be done, so process the next use. - if (result.second) - continue; - - // If the insertion fails, then we have at least two non-consuming uses in - // the same block. Since we are performing a liveness type of dataflow, we - // only need the last non-consuming use to show that all consuming uses post - // dominate both. Since we do not need to deal with cond_br that have - // non-trivial values, we now can check if Use is after Result.first->second - // in the use list. If Use is not later, then we wish to keep the already - // mapped value, not use, so continue. - if (std::find_if(result.first->second->getUser()->getIterator(), - userBlock->end(), + // Make sure that this non consuming user is in either not in our definition + // block or is strictly after our defining instruction. If so, stash the use + // and continue. + if (userBlock != getBeginBlock() || + std::find_if(beginInst->getIterator(), userBlock->end(), [&use](const SILInstruction &inst) -> bool { return use->getUser() == &inst; - }) == userBlock->end()) { + }) != userBlock->end()) { + blocksWithNonConsumingUses.insert(userBlock, use); continue; } - // At this point, we know that user is later in the Block than - // result.first->second, so store user instead. - result.first->second = use; + // Otherwise, we emit an error since we found a use before our def. We do + // not bail early here since we want to gather up /all/ that we find. + errorBuilder.handleUseAfterFree([&] { + llvm::errs() << "Found use before def?!\n" + << "Value: "; + if (auto v = value) { + llvm::errs() << *v; + } else { + llvm::errs() << "N/A. \n"; + } + }); } } @@ -284,20 +274,27 @@ void State::checkForSameBlockUseAfterFree(Operand *consumingUse, // If we do not have any consuming uses in the same block as our // consuming user, then we can not have a same block use-after-free. auto iter = blocksWithNonConsumingUses.find(userBlock); - if (iter == blocksWithNonConsumingUses.end()) + if (!iter.hasValue()) { return; + } - Operand *nonConsumingUse = iter->second; + auto nonConsumingUsesInBlock = *iter; - // Make sure that the non-consuming use is before the consuming + // Make sure that all of our non-consuming uses are before the consuming // use. Otherwise, we have a use after free. Since we do not allow for cond_br // anymore, we know that both of our users are non-cond branch users and thus // must be instructions in the given block. Make sure that the non consuming // user is strictly before the consuming user. - if (std::find_if(consumingUse->getUser()->getIterator(), userBlock->end(), - [&nonConsumingUse](const SILInstruction &i) -> bool { - return nonConsumingUse->getUser() == &i; - }) != userBlock->end()) { + for (auto *nonConsumingUse : nonConsumingUsesInBlock) { + if (std::find_if(consumingUse->getUser()->getIterator(), userBlock->end(), + [&nonConsumingUse](const SILInstruction &i) -> bool { + return nonConsumingUse->getUser() == &i; + }) == userBlock->end()) { + continue; + } + + // NOTE: We do not exit here since we want to catch /all/ errors that we can + // find. errorBuilder.handleUseAfterFree([&] { llvm::errs() << "Found use after free?!\n" << "Value: "; @@ -307,15 +304,14 @@ void State::checkForSameBlockUseAfterFree(Operand *consumingUse, llvm::errs() << "N/A. \n"; } llvm::errs() << "Consuming User: " << *consumingUse->getUser() - << "Non Consuming User: " << *iter->second->getUser() + << "Non Consuming User: " << *nonConsumingUse->getUser() << "Block: bb" << userBlock->getDebugID() << "\n\n"; }); - return; } - // Erase the use since we know that it is properly joint post-dominated. - blocksWithNonConsumingUses.erase(iter); - return; + // Erase the use since we know that it is either properly joint post-dominated + // or it was not and we emitted use after free errors. + blocksWithNonConsumingUses.erase(userBlock); } void State::checkPredsForDoubleConsume(Operand *consumingUse, @@ -485,22 +481,16 @@ void State::checkDataflowEndState(DeadEndBlocks &deBlocks) { // wants us to tell it where to insert compensating destroys. } - // Make sure that we do not have any lifetime ending uses left to visit that - // are not transitively unreachable blocks.... so return early. - if (blocksWithNonConsumingUses.empty()) { - return; - } - // If we do have remaining blocks, then these non lifetime ending uses must be // outside of our "alive" blocks implying a use-after free. - for (auto &pair : blocksWithNonConsumingUses) { - if (deBlocks.isDeadEnd(pair.first)) { + for (auto pair : blocksWithNonConsumingUses.getRange()) { + auto *block = pair.first; + if (deBlocks.isDeadEnd(block)) { continue; } errorBuilder.handleUseAfterFree([&] { - llvm::errs() << "Found use after free due to unvisited non lifetime " - "ending uses?!\n" + llvm::errs() << "Found outside of lifetime uses!\n" << "Value: "; if (auto v = value) { llvm::errs() << *v; @@ -508,12 +498,13 @@ void State::checkDataflowEndState(DeadEndBlocks &deBlocks) { llvm::errs() << "N/A. \n"; } - llvm::errs() << "Remaining Users:\n"; - for (auto &pair : blocksWithNonConsumingUses) { - llvm::errs() << "User:" << *pair.second->getUser() << "Block: bb" - << pair.first->getDebugID() << "\n"; + auto uses = pair.second; + llvm::errs() << "User List:\n"; + for (auto *op : uses) { + llvm::errs() << "User:" << *op->getUser() << "Block: bb" + << block->getDebugID() << "\n"; + llvm::errs() << "\n"; } - llvm::errs() << "\n"; }); } } diff --git a/test/SIL/ownership-verifier/arguments.sil b/test/SIL/ownership-verifier/arguments.sil index a4b77942ab28e..c7d28d086fcf5 100644 --- a/test/SIL/ownership-verifier/arguments.sil +++ b/test/SIL/ownership-verifier/arguments.sil @@ -244,16 +244,6 @@ bb5: // CHECK: Block: bb2 // CHECK: Error#: 0. End Error in Function: 'bad_order' // -// TODO: Should we really flag this twice? -// -// CHECK-LABEL: Error#: 1. Begin Error in Function: 'bad_order' -// CHECK: Found use after free due to unvisited non lifetime ending uses?! -// CHECK: Value: %1 = begin_borrow %0 : $Builtin.NativeObject -// CHECK: Remaining Users: -// CHECK: User: end_borrow %5 : $Builtin.NativeObject -// CHECK: Block: bb2 -// CHECK: Error#: 1. End Error in Function: 'bad_order' -// // CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'bad_order' sil [ossa] @bad_order : $@convention(thin) (@owned Builtin.NativeObject) -> () { bb0(%0 : @owned $Builtin.NativeObject): @@ -301,6 +291,15 @@ bb5: // CHECK: end_borrow %1 : $Builtin.NativeObject // id: %19 // CHECK: Error#: 1. End Error in Function: 'bad_order_add_a_level' // +// This block is reported as leaking block since given our partial-cfg: +// +// /---> BB3 +// BB1 -> BB2 ------> BB5 +// +// we create the borrow at BB1 and end its lifetime at BB2 and BB5. This causes +// us to walk up, identify they double consume at BB2 from BB5. Since we skipped +// BB3 that is identified as a leaking block appropriately. +// // CHECK-LABEL: Error#: 2. Begin Error in Function: 'bad_order_add_a_level' // CHECK: Error! Found a leak due to a consuming post-dominance failure! // CHECK: Value: %1 = begin_borrow %0 : $Builtin.NativeObject // users: %19, %14, %6, %3 @@ -308,18 +307,7 @@ bb5: // CHECK: bb3 // CHECK: Error#: 2. End Error in Function: 'bad_order_add_a_level' // -// This error is a "use after free" error b/c we are accessing the argument -// /after/ we end the parent borrow. We /could/ improve the error message here. -// // CHECK-LABEL: Error#: 3. Begin Error in Function: 'bad_order_add_a_level' -// CHECK: Found use after free due to unvisited non lifetime ending uses?! -// CHECK: Value: %1 = begin_borrow %0 : $Builtin.NativeObject // users: %19, %14, %6, %3 -// CHECK: Remaining Users: -// CHECK: User: end_borrow %5 : $Builtin.NativeObject // id: %7 -// CHECK: Block: bb2 -// CHECK: Error#: 3. End Error in Function: 'bad_order_add_a_level' -// -// CHECK-LABEL: Error#: 4. Begin Error in Function: 'bad_order_add_a_level' // CHECK: Found over consume?! // CHECK: Value: %5 = argument of bb2 : $Builtin.NativeObject // users: %13, %9, %7 // CHECK: User: end_borrow %5 : $Builtin.NativeObject // id: %13 @@ -327,27 +315,31 @@ bb5: // CHECK: Consuming Users: // CHECK: end_borrow %5 : $Builtin.NativeObject // id: %7 // CHECK: end_borrow %5 : $Builtin.NativeObject // id: %13 -// CHECK: Error#: 4. End Error in Function: 'bad_order_add_a_level' +// CHECK: Error#: 3. End Error in Function: 'bad_order_add_a_level' // -// CHECK-LABEL: Error#: 5. Begin Error in Function: 'bad_order_add_a_level' +// CHECK-LABEL: Error#: 4. Begin Error in Function: 'bad_order_add_a_level' // CHECK: Error! Found a leak due to a consuming post-dominance failure! // CHECK: Value: %5 = argument of bb2 : $Builtin.NativeObject // users: %13, %9, %7 // CHECK: Post Dominating Failure Blocks: // CHECK: bb3 -// CHECK: Error#: 5. End Error in Function: 'bad_order_add_a_level' +// CHECK: Error#: 4. End Error in Function: 'bad_order_add_a_level' // -// CHECK-LABEL: Error#: 6. Begin Error in Function: 'bad_order_add_a_level' -// CHECK: Found use after free due to unvisited non lifetime ending uses?! +// We found a use that we didn't visit. For our purposes, we consider this to be +// an outside lifetime use rather than use after freer to unite it with errors +// from uses that are not reachable from the definition of the value. +// +// CHECK-LABEL: Error#: 5. Begin Error in Function: 'bad_order_add_a_level' +// CHECK: Found outside of lifetime uses! // CHECK: Value: %5 = argument of bb2 : $Builtin.NativeObject // users: %13, %9, %7 -// CHECK: Remaining Users: +// CHECK: User List: // CHECK: User: %9 = begin_borrow %5 : $Builtin.NativeObject // user: %10 // CHECK: Block: bb3 -// CHECK: Error#: 6. End Error in Function: 'bad_order_add_a_level' +// CHECK: Error#: 5. End Error in Function: 'bad_order_add_a_level' // -// CHECK-LABEL: Error#: 7. Begin Error in Function: 'bad_order_add_a_level' +// CHECK-LABEL: Error#: 6. Begin Error in Function: 'bad_order_add_a_level' // CHECK: Non trivial values, non address values, and non guaranteed function args must have at least one lifetime ending use?! // CHECK: Value: %11 = argument of bb4 : $Builtin.NativeObject -// CHECK: Error#: 7. End Error in Function: 'bad_order_add_a_level' +// CHECK: Error#: 6. End Error in Function: 'bad_order_add_a_level' // // CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'bad_order_add_a_level' sil [ossa] @bad_order_add_a_level : $@convention(thin) (@owned Builtin.NativeObject) -> () { @@ -442,34 +434,20 @@ bb7: // CHECK: end_borrow %1 : $Builtin.NativeObject // id: %22 // CHECK: Error#: 4. End Error in Function: 'bad_order_add_a_level_2' // -// These are handled via other checks. -// // CHECK-LABEL: Error#: 5. Begin Error in Function: 'bad_order_add_a_level_2' -// CHECK-NEXT: Found use after free due to unvisited non lifetime ending uses?! -// CHECK: Error#: 5. End Error in Function: 'bad_order_add_a_level_2' -// -// CHECK-LABEL: Error#: 6. Begin Error in Function: 'bad_order_add_a_level_2' -// CHECK-NEXT: Found use after free due to unvisited non lifetime ending uses?! -// CHECK: Error#: 6. End Error in Function: 'bad_order_add_a_level_2' -// -// CHECK-LABEL: Error#: 7. Begin Error in Function: 'bad_order_add_a_level_2' -// CHECK-NEXT: Found use after free due to unvisited non lifetime ending uses?! -// CHECK: Error#: 7. End Error in Function: 'bad_order_add_a_level_2' -// -// CHECK-LABEL: Error#: 8. Begin Error in Function: 'bad_order_add_a_level_2' // CHECK-NEXT: Found use after free?! // CHECK-NEXT: Value: %5 = argument of bb2 : $Builtin.NativeObject // CHECK-NEXT: Consuming User: end_borrow %5 : $Builtin.NativeObject // CHECK-NEXT: Non Consuming User: end_borrow %11 : $Builtin.NativeObject // CHECK-NEXT: Block: bb4 -// CHECK: Error#: 8. End Error in Function: 'bad_order_add_a_level_2' +// CHECK: Error#: 5. End Error in Function: 'bad_order_add_a_level_2' // -// CHECK-LABEL: Error#: 9. Begin Error in Function: 'bad_order_add_a_level_2' +// CHECK-LABEL: Error#: 6. Begin Error in Function: 'bad_order_add_a_level_2' // CHECK-NEXT: Found over consume?! // CHECK-NEXT: Value: %5 = argument of bb2 : $Builtin.NativeObject // CHECK-NEXT: User: end_borrow %5 : $Builtin.NativeObject // CHECK-NEXT: Block: bb2 -// CHECK: Error#: 9. End Error in Function: 'bad_order_add_a_level_2' +// CHECK: Error#: 6. End Error in Function: 'bad_order_add_a_level_2' // // NOTE: There are 2-3 errors here we are not pattern matching. We should add // patterns for them so we track if they are changed. @@ -683,19 +661,29 @@ bb3: // CHECK-NEXT: br bb2(%1 : $Builtin.NativeObject, %1 : $Builtin.NativeObject) // id: %3 // CHECK: Error#: 0. End Error in Function: 'simple_loop_carry_over_consume' // +// This error is a little non-sensical since we have malformed SIL here. The +// end_borrow is considered to be a read only use of itself since we borrow %5 +// and then pass %8 (the result of the borrow) around the loop. This then +// becomes %5 and then our end_borrow is treated as the end of the lfietime of +// the end_borrow and thus a liveness requiring use of the thing that %8 was +// borrowed from, %5. +// // CHECK-LABEL: Error#: 1. Begin Error in Function: 'simple_loop_carry_over_consume' // CHECK: Found use after free?! // CHECK: Value: %5 = argument of bb2 : $Builtin.NativeObject // users: %11, %10, %8 // CHECK: Consuming User: end_borrow %5 : $Builtin.NativeObject // id: %11 -// CHECK: Non Consuming User: end_borrow %4 : $Builtin.NativeObject // id: %12 +// CHECK: Non Consuming User: end_borrow %5 : $Builtin.NativeObject // id: %11 // CHECK: Block: bb5 // CHECK: Error#: 1. End Error in Function: 'simple_loop_carry_over_consume' // +// %5 is passed around the loop as an incoming value to %4. So we get a certain +// amount of circularity here. The important thing is we flag it. +// // CHECK-LABEL: Error#: 2. Begin Error in Function: 'simple_loop_carry_over_consume' -// CHECK: Found use after free due to unvisited non lifetime ending uses?! +// CHECK: Found use after free?! // CHECK: Value: %5 = argument of bb2 : $Builtin.NativeObject // users: %11, %10, %8 -// CHECK: Remaining Users: -// CHECK: User: end_borrow %4 : $Builtin.NativeObject // id: %12 +// CHECK: Consuming User: end_borrow %5 : $Builtin.NativeObject // id: %11 +// CHECK: Non Consuming User: end_borrow %4 : $Builtin.NativeObject // id: %12 // CHECK: Block: bb5 // CHECK: Error#: 2. End Error in Function: 'simple_loop_carry_over_consume' // @@ -756,24 +744,26 @@ bb3: return %9999 : $() } -// This first test fails since the %2a is considered dead at bb2a's -// terminator. So the end_borrow of %2a in bb3 is considered a liveness use of -// %2 due to it consuming %3 via the loop carry. +// This is slightly non-sensical since we are feeding the verifier broken SIL. // // CHECK-LABEL: Error#: 0. Begin Error in Function: 'simple_loop_carry_borrows_do_not_loop_carry' // CHECK-NEXT: Found use after free?! // CHECK-NEXT: Value: %5 = argument of bb1 : $Builtin.NativeObject // users: %11, %10, %8 // CHECK-NEXT: Consuming User: end_borrow %5 : $Builtin.NativeObject // id: %11 -// CHECK-NEXT: Non Consuming User: end_borrow %4 : $Builtin.NativeObject // id: %12 +// CHECK-NEXT: Non Consuming User: end_borrow %5 : $Builtin.NativeObject // id: %11 // CHECK-NEXT: Block: bb4 // CHECK: Error#: 0. End Error in Function: 'simple_loop_carry_borrows_do_not_loop_carry' // +// This test fails since the %2a is considered dead at bb2a's terminator. So the +// end_borrow of %2a in bb3 is considered a liveness use of %2 due to it +// consuming %3 via the loop carry. +// // CHECK-LABEL: Error#: 1. Begin Error in Function: 'simple_loop_carry_borrows_do_not_loop_carry' -// CHECK-NEXT: Found use after free due to unvisited non lifetime ending uses?! -// CHECK-NEXT: Value: %5 = argument of bb1 : $Builtin.NativeObject // users: %11, %10, %8 -// CHECK-NEXT: Remaining Users: -// CHECK-NEXT: User: end_borrow %4 : $Builtin.NativeObject // id: %12 -// CHECK-NEXT: Block: bb4 +// CHECK: Found use after free?! +// CHECK: Value: %5 = argument of bb1 : $Builtin.NativeObject // users: %11, %10, %8 +// CHECK: Consuming User: end_borrow %5 : $Builtin.NativeObject // id: %11 +// CHECK: Non Consuming User: end_borrow %4 : $Builtin.NativeObject // id: %12 +// CHECK: Block: bb4 // CHECK: Error#: 1. End Error in Function: 'simple_loop_carry_borrows_do_not_loop_carry' // // CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'simple_loop_carry_borrows_do_not_loop_carry' diff --git a/test/SIL/ownership-verifier/borrow_scope_introducing_operands.sil b/test/SIL/ownership-verifier/borrow_scope_introducing_operands.sil index 2aa91496230c3..a93e2cc705a9b 100644 --- a/test/SIL/ownership-verifier/borrow_scope_introducing_operands.sil +++ b/test/SIL/ownership-verifier/borrow_scope_introducing_operands.sil @@ -19,12 +19,13 @@ bb2: unwind } -// CHECK-LABEL: Function: 'destroy_value_before_end_borrow' +// CHECK-LABEL: Error#: 0. Begin Error in Function: 'destroy_value_before_end_borrow' // CHECK: Found use after free?! // CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject // CHECK: Consuming User: destroy_value %0 : $Builtin.NativeObject // CHECK: Non Consuming User: end_borrow %1 : $Builtin.NativeObject // CHECK: Block: bb0 +// CHECK: Error#: 0. End Error in Function: 'destroy_value_before_end_borrow' sil [ossa] @destroy_value_before_end_borrow : $@convention(thin) (@owned Builtin.NativeObject) -> () { bb0(%0 : @owned $Builtin.NativeObject): %1 = begin_borrow %0 : $Builtin.NativeObject @@ -34,12 +35,15 @@ bb0(%0 : @owned $Builtin.NativeObject): return %9999 : $() } -// CHECK-LABEL: Function: 'destroy_value_before_end_borrow_coroutine' +// CHECK-LABEL: Error#: 0. Begin Error in Function: 'destroy_value_before_end_borrow_coroutine' // CHECK: Found use after free?! // CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject // CHECK: Consuming User: destroy_value %0 : $Builtin.NativeObject // CHECK: Non Consuming User: end_apply %2 // CHECK: Block: bb0 +// CHECK: Error#: 0. End Error in Function: 'destroy_value_before_end_borrow_coroutine' +// +// CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'destroy_value_before_end_borrow_coroutine' sil [ossa] @destroy_value_before_end_borrow_coroutine : $@convention(thin) (@owned Builtin.NativeObject) -> () { bb0(%0 : @owned $Builtin.NativeObject): %coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> () @@ -50,12 +54,15 @@ bb0(%0 : @owned $Builtin.NativeObject): return %r : $() } -// CHECK-LABEL: Function: 'destroy_value_before_end_borrow_coroutine_2' +// CHECK-LABEL: Error#: 0. Begin Error in Function: 'destroy_value_before_end_borrow_coroutine_2' // CHECK: Found use after free?! // CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject // CHECK: Consuming User: destroy_value %0 : $Builtin.NativeObject // CHECK: Non Consuming User: abort_apply %2 // CHECK: Block: bb0 +// CHECK: Error#: 0. End Error in Function: 'destroy_value_before_end_borrow_coroutine_2' +// +// CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'destroy_value_before_end_borrow_coroutine_2' sil [ossa] @destroy_value_before_end_borrow_coroutine_2 : $@convention(thin) (@owned Builtin.NativeObject) -> () { bb0(%0 : @owned $Builtin.NativeObject): %coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> () @@ -66,20 +73,15 @@ bb0(%0 : @owned $Builtin.NativeObject): return %r : $() } -// CHECK-LABEL: Function: 'destroy_value_before_end_borrow_coroutine_3' +// CHECK-LABEL: Error#: 0. Begin Error in Function: 'destroy_value_before_end_borrow_coroutine_3' // CHECK: Found use after free?! // CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject // CHECK: Consuming User: destroy_value %0 : $Builtin.NativeObject // CHECK: Non Consuming User: abort_apply %2 // CHECK: Block: bb1 - -// CHECK-LABEL: Function: 'destroy_value_before_end_borrow_coroutine_3' -// CHECK: Found use after free due to unvisited non lifetime ending uses?! -// CHECK: Value: %0 = argument of bb0 : $Builtin.NativeObject -// CHECK: Remaining Users: -// CHECK: User: abort_apply %2 -// CHECK: Block: bb1 - +// CHECK: Error#: 0. End Error in Function: 'destroy_value_before_end_borrow_coroutine_3' +// +// CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'destroy_value_before_end_borrow_coroutine_3' sil [ossa] @destroy_value_before_end_borrow_coroutine_3 : $@convention(thin) (@owned Builtin.NativeObject) -> () { bb0(%0 : @owned $Builtin.NativeObject): %coro = function_ref @coroutine_callee : $@yield_once @convention(thin) (@guaranteed Builtin.NativeObject) -> () @@ -101,12 +103,15 @@ bb3: return %r : $() } -// CHECK-LABEL: Function: 'parent_borrow_scope_end_before_end_borrow_coroutine' +// CHECK-LABEL: Error#: 0. Begin Error in Function: 'parent_borrow_scope_end_before_end_borrow_coroutine' // CHECK: Found use after free?! // CHECK: Value: %1 = begin_borrow %0 : $Builtin.NativeObject // CHECK: Consuming User: end_borrow %1 : $Builtin.NativeObject // CHECK: Non Consuming User: end_apply %3 // CHECK: Block: bb0 +// CHECK: Error#: 0. End Error in Function: 'parent_borrow_scope_end_before_end_borrow_coroutine' +// +// CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'parent_borrow_scope_end_before_end_borrow_coroutine' sil [ossa] @parent_borrow_scope_end_before_end_borrow_coroutine : $@convention(thin) (@owned Builtin.NativeObject) -> () { bb0(%0 : @owned $Builtin.NativeObject): %1 = begin_borrow %0 : $Builtin.NativeObject @@ -119,12 +124,15 @@ bb0(%0 : @owned $Builtin.NativeObject): return %r : $() } -// CHECK-LABEL: Function: 'parent_borrow_scope_end_before_end_borrow_coroutine_2' +// CHECK-LABEL: Error#: 0. Begin Error in Function: 'parent_borrow_scope_end_before_end_borrow_coroutine_2' // CHECK: Found use after free?! // CHECK: Value: %1 = begin_borrow %0 : $Builtin.NativeObject // CHECK: Consuming User: end_borrow %1 : $Builtin.NativeObject // CHECK: Non Consuming User: abort_apply %3 // CHECK: Block: bb0 +// CHECK: Error#: 0. End Error in Function: 'parent_borrow_scope_end_before_end_borrow_coroutine_2' +// +// CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'parent_borrow_scope_end_before_end_borrow_coroutine_2' sil [ossa] @parent_borrow_scope_end_before_end_borrow_coroutine_2 : $@convention(thin) (@owned Builtin.NativeObject) -> () { bb0(%0 : @owned $Builtin.NativeObject): %1 = begin_borrow %0 : $Builtin.NativeObject @@ -137,20 +145,15 @@ bb0(%0 : @owned $Builtin.NativeObject): return %r : $() } -// CHECK-LABEL: Function: 'parent_borrow_scope_end_before_end_borrow_coroutine_3' +// CHECK-LABEL: Error#: 0. Begin Error in Function: 'parent_borrow_scope_end_before_end_borrow_coroutine_3' // CHECK: Found use after free?! // CHECK: Value: %1 = begin_borrow %0 : $Builtin.NativeObject // CHECK: Consuming User: end_borrow %1 : $Builtin.NativeObject // CHECK: Non Consuming User: abort_apply %3 // CHECK: Block: bb1 - -// CHECK-LABEL: Function: 'parent_borrow_scope_end_before_end_borrow_coroutine_3' -// CHECK: Found use after free due to unvisited non lifetime ending uses?! -// CHECK: Value: %1 = begin_borrow %0 : $Builtin.NativeObject -// CHECK: Remaining Users: -// CHECK: User: abort_apply %3 -// CHECK: Block: bb1 - +// CHECK: Error#: 0. End Error in Function: 'parent_borrow_scope_end_before_end_borrow_coroutine_3' +// +// CHECK-NOT: Error#: {{[0-9][0-9]*}}. End Error in Function: 'parent_borrow_scope_end_before_end_borrow_coroutine_3' sil [ossa] @parent_borrow_scope_end_before_end_borrow_coroutine_3 : $@convention(thin) (@owned Builtin.NativeObject) -> () { bb0(%0 : @owned $Builtin.NativeObject): %1 = begin_borrow %0 : $Builtin.NativeObject From d611b4c4f59b501d055247f78c3177ad292bd205 Mon Sep 17 00:00:00 2001 From: Michael Forster Date: Thu, 7 May 2020 16:57:26 +0200 Subject: [PATCH 07/14] Synthesize memberwise initializers despite AccessSpecDecl Previously the mere presence of `public:` or `private:` inhibited the synthesis of memberwise initializers. --- lib/ClangImporter/ImportDecl.cpp | 7 +++ .../Cxx/class/Inputs/memberwise-initializer.h | 44 +++++++++++++++++++ .../Interop/Cxx/class/Inputs/module.modulemap | 4 ++ .../member-variables-module-interface.swift | 1 + ...berwise-initializer-module-interface.swift | 34 ++++++++++++++ .../memberwise-initializer-typechecker.swift | 15 +++++++ 6 files changed, 105 insertions(+) create mode 100644 test/Interop/Cxx/class/Inputs/memberwise-initializer.h create mode 100644 test/Interop/Cxx/class/memberwise-initializer-module-interface.swift create mode 100644 test/Interop/Cxx/class/memberwise-initializer-typechecker.swift diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 2690b577657b6..b5aa91293e81f 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -45,6 +45,7 @@ #include "swift/Config.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Attr.h" +#include "clang/AST/DeclCXX.h" #include "clang/Basic/CharInfo.h" #include "swift/Basic/Statistic.h" #include "clang/Basic/TargetInfo.h" @@ -3311,6 +3312,12 @@ namespace { // it is nested in a struct. for (auto m : decl->decls()) { + if (isa(m)) { + // The presence of AccessSpecDecls themselves does not influence + // whether we can generate a member-wise initializer. + continue; + } + auto nd = dyn_cast(m); if (!nd) { // We couldn't import the member, so we can't reference it in Swift. diff --git a/test/Interop/Cxx/class/Inputs/memberwise-initializer.h b/test/Interop/Cxx/class/Inputs/memberwise-initializer.h new file mode 100644 index 0000000000000..9b69f29706117 --- /dev/null +++ b/test/Interop/Cxx/class/Inputs/memberwise-initializer.h @@ -0,0 +1,44 @@ +#ifndef TEST_INTEROP_CXX_CLASS_INPUTS_MEMBERWISE_INITIALIZER_H +#define TEST_INTEROP_CXX_CLASS_INPUTS_MEMBERWISE_INITIALIZER_H + +struct StructPrivateOnly { +private: + int varPrivate; +}; + +struct StructPublicOnly { + int varPublic; +}; + +struct StructEmptyPrivateSection { + int varPublic; +private: +}; + +struct StructPublicAndPrivate { + int varPublic; +private: + int varPrivate; +}; + +class ClassPrivateOnly { + int varPrivate; +}; + +class ClassPublicOnly { +public: + int varPublic; +}; + +class ClassEmptyPublicSection { + int varPrivate; +public: +}; + +class ClassPrivateAndPublic { + int varPrivate; +public: + int varPublic; +}; + +#endif diff --git a/test/Interop/Cxx/class/Inputs/module.modulemap b/test/Interop/Cxx/class/Inputs/module.modulemap index 1b740175317d4..3e3aab62a3129 100644 --- a/test/Interop/Cxx/class/Inputs/module.modulemap +++ b/test/Interop/Cxx/class/Inputs/module.modulemap @@ -2,6 +2,10 @@ module AccessSpecifiers { header "access-specifiers.h" } +module MemberwiseInitializer { + header "memberwise-initializer.h" +} + module MemoryLayout { header "memory-layout.h" } diff --git a/test/Interop/Cxx/class/member-variables-module-interface.swift b/test/Interop/Cxx/class/member-variables-module-interface.swift index bb523e26a52a2..731be7d3a703d 100644 --- a/test/Interop/Cxx/class/member-variables-module-interface.swift +++ b/test/Interop/Cxx/class/member-variables-module-interface.swift @@ -3,4 +3,5 @@ // CHECK: struct MyClass { // CHECK-NEXT: var const_member: Int32 { get } // CHECK-NEXT: init() +// CHECK-NEXT: init(const_member: Int32) // CHECK-NEXT: } diff --git a/test/Interop/Cxx/class/memberwise-initializer-module-interface.swift b/test/Interop/Cxx/class/memberwise-initializer-module-interface.swift new file mode 100644 index 0000000000000..86e16e169075e --- /dev/null +++ b/test/Interop/Cxx/class/memberwise-initializer-module-interface.swift @@ -0,0 +1,34 @@ +// RUN: %target-swift-ide-test -print-module -module-to-print=MemberwiseInitializer -I %S/Inputs -source-filename=x -enable-cxx-interop | %FileCheck %s + +// CHECK: struct StructPrivateOnly { +// CHECK-NEXT: init() +// CHECK-NEXT: } +// CHECK-NEXT: struct StructPublicOnly { +// CHECK-NEXT: var varPublic: Int32 +// CHECK-NEXT: init() +// CHECK-NEXT: init(varPublic: Int32) +// CHECK-NEXT: } +// CHECK-NEXT: struct StructEmptyPrivateSection { +// CHECK-NEXT: var varPublic: Int32 +// CHECK-NEXT: init() +// CHECK-NEXT: init(varPublic: Int32) +// CHECK-NEXT: } +// CHECK-NEXT: struct StructPublicAndPrivate { +// CHECK-NEXT: var varPublic: Int32 +// CHECK-NEXT: init() +// CHECK-NEXT: } +// CHECK-NEXT: struct ClassPrivateOnly { +// CHECK-NEXT: init() +// CHECK-NEXT: } +// CHECK-NEXT: struct ClassPublicOnly { +// CHECK-NEXT: var varPublic: Int32 +// CHECK-NEXT: init() +// CHECK-NEXT: init(varPublic: Int32) +// CHECK-NEXT: } +// CHECK-NEXT: struct ClassEmptyPublicSection { +// CHECK-NEXT: init() +// CHECK-NEXT: } +// CHECK-NEXT: struct ClassPrivateAndPublic { +// CHECK-NEXT: var varPublic: Int32 +// CHECK-NEXT: init() +// CHECK-NEXT: } diff --git a/test/Interop/Cxx/class/memberwise-initializer-typechecker.swift b/test/Interop/Cxx/class/memberwise-initializer-typechecker.swift new file mode 100644 index 0000000000000..508b43cc14c24 --- /dev/null +++ b/test/Interop/Cxx/class/memberwise-initializer-typechecker.swift @@ -0,0 +1,15 @@ +// RUN: %target-typecheck-verify-swift -I %S/Inputs -enable-cxx-interop + +import MemberwiseInitializer + +let structPrivateOnly = StructPrivateOnly(varPrivate: 42) // expected-error {{argument passed to call that takes no arguments}} +let structPublicOnly = StructPublicOnly(varPublic: 42) +let structEmptyPrivateSetion = StructEmptyPrivateSection(varPublic: 42) +let structPublicAndPrivate1 = StructPublicAndPrivate(varPublic: 42) // expected-error {{argument passed to call that takes no arguments}} +let structPublicAndPrivate2 = StructPublicAndPrivate(varPublic: 42, varPrivate: 23) // expected-error {{argument passed to call that takes no arguments}} + +let classPrivateOnly = ClassPrivateOnly(varPrivate: 42) // expected-error {{argument passed to call that takes no arguments}} +let classPublicOnly = ClassPublicOnly(varPublic: 42) +let classEmptyPublicSetion = ClassEmptyPublicSection(varPrivate: 42) // expected-error {{argument passed to call that takes no arguments}} +let classPublicAndPrivate1 = ClassPrivateAndPublic(varPublic: 23) // expected-error {{argument passed to call that takes no arguments}} +let classPublicAndPrivate2 = ClassPrivateAndPublic(varPrivate: 42, varPublic: 23) // expected-error {{argument passed to call that takes no arguments}} From 09975d125354304ab54d1375610ef0e686de9ca3 Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Thu, 7 May 2020 11:04:17 -0700 Subject: [PATCH 08/14] sprinkle `llvm_unreachable` for covered switches (NFC) Annotate the covered switches with `llvm_unreachable` to avoid the MSVC warning which does not recognise the covered switches. This allows us to avoid a spew of warnings. --- include/swift/AST/Decl.h | 1 + include/swift/AST/EvaluatorDependencies.h | 2 ++ include/swift/AST/Stmt.h | 1 + lib/AST/ASTDemangler.cpp | 1 + lib/AST/AutoDiff.cpp | 3 +++ lib/AST/ClangTypeConverter.cpp | 1 + lib/AST/Decl.cpp | 2 ++ lib/AST/ModuleDependencies.cpp | 2 ++ lib/AST/Type.cpp | 1 + lib/Driver/Compilation.cpp | 2 ++ lib/Frontend/DependencyVerifier.cpp | 1 + lib/IDE/Formatting.cpp | 1 + lib/IRGen/ProtocolInfo.h | 1 + lib/Parse/ParseDecl.cpp | 1 + lib/Sema/BuilderTransform.cpp | 1 + lib/Sema/CSApply.cpp | 1 + lib/Sema/CSDiagnostics.cpp | 1 + lib/Sema/ConstraintSystem.cpp | 2 ++ lib/Sema/ConstraintSystem.h | 7 +++++++ lib/Sema/DerivedConformanceAdditiveArithmetic.cpp | 1 + lib/Sema/SolutionResult.h | 1 + lib/Sema/TypeCheckConstraints.cpp | 1 + lib/Sema/TypeCheckDecl.cpp | 1 + lib/Sema/TypeCheckProtocol.cpp | 1 + lib/Sema/TypeCheckType.cpp | 1 + lib/SymbolGraphGen/DeclarationFragmentPrinter.cpp | 1 + lib/SymbolGraphGen/Symbol.cpp | 1 + 27 files changed, 40 insertions(+) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index a481d6df1427e..64cbedf0f8dcb 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -7187,6 +7187,7 @@ class OperatorDecl : public Decl { case DeclKind::PostfixOperator: return OperatorFixity::Postfix; } + llvm_unreachable("inavlid decl kind"); } SourceLoc getOperatorLoc() const { return OperatorLoc; } diff --git a/include/swift/AST/EvaluatorDependencies.h b/include/swift/AST/EvaluatorDependencies.h index 3759ec940eba6..ceb16440c67bf 100644 --- a/include/swift/AST/EvaluatorDependencies.h +++ b/include/swift/AST/EvaluatorDependencies.h @@ -94,6 +94,7 @@ inline DependencyScope getScopeForAccessLevel(AccessLevel l) { case AccessLevel::Open: return DependencyScope::Cascading; } + llvm_unreachable("invalid access level kind"); } // A \c DependencySource is currently defined to be a parent source file and @@ -258,6 +259,7 @@ struct DependencyCollector { case Mode::ExperimentalPrivateDependencies: return false; } + llvm_unreachable("invalid mode"); } }; } // end namespace evaluator diff --git a/include/swift/AST/Stmt.h b/include/swift/AST/Stmt.h index d57f2fe5dcc3f..a9bece28862f5 100644 --- a/include/swift/AST/Stmt.h +++ b/include/swift/AST/Stmt.h @@ -1015,6 +1015,7 @@ class CaseStmt final return SourceRange(getLoc(), getLoc()); } } + llvm_unreachable("invalid parent kind"); } bool isDefault() { return getCaseLabelItems()[0].isDefault(); } diff --git a/lib/AST/ASTDemangler.cpp b/lib/AST/ASTDemangler.cpp index 9b81a8e681aa1..558452d3af46e 100644 --- a/lib/AST/ASTDemangler.cpp +++ b/lib/AST/ASTDemangler.cpp @@ -452,6 +452,7 @@ getParameterDifferentiability(ImplParameterDifferentiability diffKind) { case ImplParameterDifferentiability::NotDifferentiable: return SILParameterDifferentiability::NotDifferentiable; } + llvm_unreachable("unknown differentiability kind"); } static ResultConvention getResultConvention(ImplResultConvention conv) { diff --git a/lib/AST/AutoDiff.cpp b/lib/AST/AutoDiff.cpp index 25c6a19945289..5282f501af5e8 100644 --- a/lib/AST/AutoDiff.cpp +++ b/lib/AST/AutoDiff.cpp @@ -62,6 +62,7 @@ NormalDifferentiableFunctionTypeComponent::getAsDerivativeFunctionKind() const { case VJP: return {AutoDiffDerivativeFunctionKind::VJP}; } + llvm_unreachable("invalid derivative kind"); } LinearDifferentiableFunctionTypeComponent:: @@ -93,6 +94,7 @@ DifferentiabilityWitnessFunctionKind::getAsDerivativeFunctionKind() const { case Transpose: return None; } + llvm_unreachable("invalid derivative kind"); } void SILAutoDiffIndices::print(llvm::raw_ostream &s) const { @@ -375,6 +377,7 @@ Type TangentSpace::getType() const { case Kind::Tuple: return value.tupleType; } + llvm_unreachable("invalid tangent space kind"); } CanType TangentSpace::getCanonicalType() const { diff --git a/lib/AST/ClangTypeConverter.cpp b/lib/AST/ClangTypeConverter.cpp index a535950cf82bb..eb639bf06e478 100644 --- a/lib/AST/ClangTypeConverter.cpp +++ b/lib/AST/ClangTypeConverter.cpp @@ -155,6 +155,7 @@ const clang::Type *ClangTypeConverter::getFunctionType( case AnyFunctionType::Representation::Thin: llvm_unreachable("Expected a C-compatible representation."); } + llvm_unreachable("invalid representation"); } clang::QualType ClangTypeConverter::convertMemberType(NominalTypeDecl *DC, diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index ea072f6537180..068b0b41badcb 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -645,6 +645,7 @@ static_assert(sizeof(checkSourceLocType(&ID##Decl::getLoc)) == 2, \ case FileUnitKind::DWARFModule: return SourceLoc(); } + llvm_unreachable("invalid file kind"); } Expr *AbstractFunctionDecl::getSingleExpressionBody() const { @@ -6297,6 +6298,7 @@ bool ParamDecl::hasCallerSideDefaultExpr() const { case DefaultArgumentKind::EmptyDictionary: return true; } + llvm_unreachable("invalid default argument kind"); } Expr *ParamDecl::getTypeCheckedDefaultExpr() const { diff --git a/lib/AST/ModuleDependencies.cpp b/lib/AST/ModuleDependencies.cpp index d1e40663968a6..be1f849c7a282 100644 --- a/lib/AST/ModuleDependencies.cpp +++ b/lib/AST/ModuleDependencies.cpp @@ -106,6 +106,7 @@ ModuleDependenciesCache::getDependenciesMap(ModuleDependenciesKind kind) { case ModuleDependenciesKind::Clang: return ClangModuleDependencies; } + llvm_unreachable("invalid dependency kind"); } const llvm::StringMap & @@ -117,6 +118,7 @@ ModuleDependenciesCache::getDependenciesMap(ModuleDependenciesKind kind) const { case ModuleDependenciesKind::Clang: return ClangModuleDependencies; } + llvm_unreachable("invalid dependency kind"); } bool ModuleDependenciesCache::hasDependencies( diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 3e29c740aa6c1..8cdf5a681377c 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -2969,6 +2969,7 @@ static bool canSubstituteTypeInto(Type ty, const DeclContext *dc, return typeDecl->getEffectiveAccess() > AccessLevel::Internal; } + llvm_unreachable("invalid subsitution kind"); } Type ReplaceOpaqueTypesWithUnderlyingTypes:: diff --git a/lib/Driver/Compilation.cpp b/lib/Driver/Compilation.cpp index 3ef8473f73cc3..1718f0ba233a2 100644 --- a/lib/Driver/Compilation.cpp +++ b/lib/Driver/Compilation.cpp @@ -1079,6 +1079,7 @@ namespace driver { forRanges, "file is up-to-date and output exists"); return false; } + llvm_unreachable("invalid job condition"); } bool isCascadingJobAccordingToCondition( @@ -1092,6 +1093,7 @@ namespace driver { case Job::Condition::CheckDependencies: return false; } + llvm_unreachable("invalid job condition"); } void forEachOutOfDateExternalDependency( diff --git a/lib/Frontend/DependencyVerifier.cpp b/lib/Frontend/DependencyVerifier.cpp index 8c1bf56254b12..bfa77ad0126d5 100644 --- a/lib/Frontend/DependencyVerifier.cpp +++ b/lib/Frontend/DependencyVerifier.cpp @@ -239,6 +239,7 @@ struct Obligation { case Expectation::Scope::Cascading: return "cascading"; } + llvm_unreachable("invalid expectation scope"); } StringRef renderAsFixit(ASTContext &Ctx) const { diff --git a/lib/IDE/Formatting.cpp b/lib/IDE/Formatting.cpp index f86117f64ac13..cd6c494bce74e 100644 --- a/lib/IDE/Formatting.cpp +++ b/lib/IDE/Formatting.cpp @@ -875,6 +875,7 @@ class OutdentChecker: protected RangeWalker { return !SM.isBeforeInBuffer(L, CheckRange.Start) && (R.isInvalid() || !SM.isBeforeInBuffer(CheckRange.End, R)); } + llvm_unreachable("invalid range kind"); } public: diff --git a/lib/IRGen/ProtocolInfo.h b/lib/IRGen/ProtocolInfo.h index d0a192ad39e98..3be67a0c87dc0 100644 --- a/lib/IRGen/ProtocolInfo.h +++ b/lib/IRGen/ProtocolInfo.h @@ -194,6 +194,7 @@ class WitnessTableEntry { left.AssociatedConformanceEntry.Protocol == right.AssociatedConformanceEntry.Protocol; } + llvm_unreachable("invalid witness kind"); } }; diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index ddbe58fc22265..c1f33bfe3c06a 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -1855,6 +1855,7 @@ bool Parser::parseNewDeclAttribute(DeclAttributes &Attributes, SourceLoc AtLoc, return makeParserError(); } } + llvm_unreachable("invalid next segment kind"); }).isError() || SuppressLaterDiags) { return false; } diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index 09aba51a554a5..6e3a62c02e515 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -1007,6 +1007,7 @@ class BuilderClosureRewriter // Execute the expression. return rewriteExpr(capturedExpr); } + llvm_unreachable("invalid function builder target"); } /// Declare the given temporary variable, adding the appropriate diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 2a2673dfbce2b..6bbc088b4bcbd 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -8544,4 +8544,5 @@ SolutionApplicationTarget SolutionApplicationTarget::walk(ASTWalker &walker) { return *this; } + llvm_unreachable("invalid target kind"); } diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index e0cb18112854b..0434172c9b4a4 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -5880,6 +5880,7 @@ void NonEphemeralConversionFailure::emitSuggestionNotes() const { case PTK_AutoreleasingUnsafeMutablePointer: return None; } + llvm_unreachable("invalid pointer kind"); }; // First emit a note about the implicit conversion only lasting for the diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 44a44f15f39db..4db3d1f64c888 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -4077,6 +4077,7 @@ ConstraintSystem::isConversionEphemeral(ConversionRestrictionKind conversion, // parameter. return ConversionEphemeralness::NonEphemeral; } + llvm_unreachable("invalid conversion restriction kind"); } Expr *ConstraintSystem::buildAutoClosureExpr(Expr *expr, @@ -4381,6 +4382,7 @@ bool SolutionApplicationTarget::contextualTypeIsOnlyAHint() const { case CTP_CannotFail: return false; } + llvm_unreachable("invalid contextual type"); } /// Given a specific expression and the remnants of the failed constraint diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index c0fa1adcc7d75..a68185f4825f8 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -1351,6 +1351,7 @@ class SolutionApplicationTarget { case Kind::caseLabelItem: return nullptr; } + llvm_unreachable("invalid expression type"); } DeclContext *getDeclContext() const { @@ -1367,6 +1368,7 @@ class SolutionApplicationTarget { case Kind::caseLabelItem: return caseLabelItem.dc; } + llvm_unreachable("invalid decl context type"); } ContextualTypePurpose getExprContextualTypePurpose() const { @@ -1520,6 +1522,7 @@ class SolutionApplicationTarget { case Kind::function: return function.function; } + llvm_unreachable("invalid function kind"); } Optional getAsStmtCondition() const { @@ -1532,6 +1535,7 @@ class SolutionApplicationTarget { case Kind::stmtCondition: return stmtCondition.stmtCondition; } + llvm_unreachable("invalid statement kind"); } Optional getAsCaseLabelItem() const { @@ -1544,6 +1548,7 @@ class SolutionApplicationTarget { case Kind::caseLabelItem: return caseLabelItem.caseLabelItem; } + llvm_unreachable("invalid case label type"); } BraceStmt *getFunctionBody() const { @@ -1572,6 +1577,7 @@ class SolutionApplicationTarget { case Kind::caseLabelItem: return caseLabelItem.caseLabelItem->getSourceRange(); } + llvm_unreachable("invalid target type"); } /// Retrieve the source location for the target. @@ -1589,6 +1595,7 @@ class SolutionApplicationTarget { case Kind::caseLabelItem: return caseLabelItem.caseLabelItem->getStartLoc(); } + llvm_unreachable("invalid target type"); } /// Walk the contents of the application target. diff --git a/lib/Sema/DerivedConformanceAdditiveArithmetic.cpp b/lib/Sema/DerivedConformanceAdditiveArithmetic.cpp index af8d6a3c7bdf3..8f38f6e6b144d 100644 --- a/lib/Sema/DerivedConformanceAdditiveArithmetic.cpp +++ b/lib/Sema/DerivedConformanceAdditiveArithmetic.cpp @@ -51,6 +51,7 @@ static StringRef getMathOperatorName(MathOperator op) { case Subtract: return "-"; } + llvm_unreachable("invalid math operator kind"); } bool DerivedConformance::canDeriveAdditiveArithmetic(NominalTypeDecl *nominal, diff --git a/lib/Sema/SolutionResult.h b/lib/Sema/SolutionResult.h index e0e73382303f8..04bac3b6dc8dd 100644 --- a/lib/Sema/SolutionResult.h +++ b/lib/Sema/SolutionResult.h @@ -135,6 +135,7 @@ class SolutionResult { case TooComplex: return true; } + llvm_unreachable("invalid diagnostic kind"); } /// Note that the failure has been diagnosed. diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 8c57b244177b2..05a11aea80e7a 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -3450,6 +3450,7 @@ CheckedCastKind TypeChecker::typeCheckCheckedCast(Type fromType, case CheckedCastKind::Unresolved: return failed(); } + llvm_unreachable("invalid cast type"); }; // Check for casts between specific concrete types that cannot succeed. diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index b30da7cf18537..e7c00c73fa87b 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -2351,6 +2351,7 @@ InterfaceTypeRequest::evaluate(Evaluator &eval, ValueDecl *D) const { return resultTy; } } + llvm_unreachable("invalid decl kind"); } NamedPattern * diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 0361f02511988..d0c04836216d9 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -5556,6 +5556,7 @@ ValueDecl *TypeChecker::deriveProtocolRequirement(DeclContext *DC, llvm_unreachable( "When possible, OptionSet is derived via memberwise init synthesis"); } + llvm_unreachable("unknown derivable protocol kind"); } Type TypeChecker::deriveTypeWitness(DeclContext *DC, diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index ce1afd50fc1de..81a62cf392ea0 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -694,6 +694,7 @@ static Type checkContextualRequirements(Type type, case RequirementCheckResult::Success: return type; } + llvm_unreachable("invalid requirement check type"); } static void diagnoseUnboundGenericType(Type ty, SourceLoc loc); diff --git a/lib/SymbolGraphGen/DeclarationFragmentPrinter.cpp b/lib/SymbolGraphGen/DeclarationFragmentPrinter.cpp index 6fa95a900224e..36a40a8c174ca 100644 --- a/lib/SymbolGraphGen/DeclarationFragmentPrinter.cpp +++ b/lib/SymbolGraphGen/DeclarationFragmentPrinter.cpp @@ -54,6 +54,7 @@ DeclarationFragmentPrinter::getKindSpelling(FragmentKind Kind) const { case FragmentKind::None: llvm_unreachable("Fragment kind of 'None' has no spelling"); } + llvm_unreachable("invalid fragment kind"); } void DeclarationFragmentPrinter::closeFragment() { diff --git a/lib/SymbolGraphGen/Symbol.cpp b/lib/SymbolGraphGen/Symbol.cpp index 78ef93e44e632..32ebd6226190e 100644 --- a/lib/SymbolGraphGen/Symbol.cpp +++ b/lib/SymbolGraphGen/Symbol.cpp @@ -371,6 +371,7 @@ Symbol::getDomain(PlatformAgnosticAvailabilityKind AgnosticKind, case swift::PlatformKind::none: return None; } + llvm_unreachable("invalid platform kind"); } void Symbol::serializeAvailabilityMixin(llvm::json::OStream &OS) const { From 8e31f37d60b32bf2f312e14eac14e66cd65f379b Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Thu, 7 May 2020 12:14:48 -0700 Subject: [PATCH 09/14] [docs] Add a table of contents to DebuggingTheCompiler.md and re-organize slightly the layout to make things clearer. I also improved some titles. --- docs/DebuggingTheCompiler.md | 303 ++++++++++++++++++----------------- 1 file changed, 160 insertions(+), 143 deletions(-) diff --git a/docs/DebuggingTheCompiler.md b/docs/DebuggingTheCompiler.md index d51633208d154..01efb58dcdbc0 100644 --- a/docs/DebuggingTheCompiler.md +++ b/docs/DebuggingTheCompiler.md @@ -1,14 +1,52 @@ -Debugging the Swift Compiler -============================ -Abstract --------- - -This document contains some useful information for debugging the -Swift compiler and Swift compiler output. - -Basic Utilities ---------------- +

Debugging The Compiler

+ +This document contains some useful information for debugging: + +* The Swift compiler. +* Intermediate output of the Swift Compiler. +* Swift applications at runtime. + +Please feel free add any useful tips that one finds to this document for the +benefit of all swift developers. + +**Table of Contents** + +- [Debugging the Compiler Itself](#debugging-the-compiler-itself) + - [Basic Utilities](#basic-utilities) + - [Printing the Intermediate Representations](#printing-the-intermediate-representations) + - [Debugging Diagnostic Emission](#debugging-diagnostic-emission) + - [Asserting on first emitted Warning/Assert Diagnostic](#asserting-on-first-emitted-warningassert-diagnostic) + - [Finding Diagnostic Names](#finding-diagnostic-names) + - [Debugging the Type Checker](#debugging-the-type-checker) + - [Enabling Logging](#enabling-logging) + - [Debugging on SIL Level](#debugging-on-sil-level) + - [Options for Dumping the SIL](#options-for-dumping-the-sil) + - [Getting CommandLine for swift stdlib from Ninja to enable dumping stdlib SIL](#getting-commandline-for-swift-stdlib-from-ninja-to-enable-dumping-stdlib-sil) + - [Dumping the SIL and other Data in LLDB](#dumping-the-sil-and-other-data-in-lldb) + - [Debugging and Profiling on SIL level](#debugging-and-profiling-on-sil-level) + - [SIL source level profiling using -gsil](#sil-source-level-profiling-using--gsil) + - [ViewCFG: Regex based CFG Printer for SIL/LLVM-IR](#viewcfg-regex-based-cfg-printer-for-silllvm-ir) + - [Debugging the Compiler using advanced LLDB Breakpoints](#debugging-the-compiler-using-advanced-lldb-breakpoints) + - [Debugging the Compiler using LLDB Scripts](#debugging-the-compiler-using-lldb-scripts) + - [Custom LLDB Commands](#custom-lldb-commands) + - [Bisecting Compiler Errors](#bisecting-compiler-errors) + - [Bisecting on SIL optimizer pass counts to identify optimizer bugs](#bisecting-on-sil-optimizer-pass-counts-to-identify-optimizer-bugs) + - [Using git-bisect in the presence of branch forwarding/feature branches](#using-git-bisect-in-the-presence-of-branch-forwardingfeature-branches) + - [Reducing SIL test cases using bug_reducer](#reducing-sil-test-cases-using-bugreducer) +- [Debugging Swift Executables](#debugging-swift-executables) + - [Determining the mangled name of a function in LLDB](#determining-the-mangled-name-of-a-function-in-lldb) + - [Manually symbolication using LLDB](#manually-symbolication-using-lldb) +- [Debugging LLDB failures](#debugging-lldb-failures) + - ["Types" Log](#types-log) + - ["Expression" Log](#expression-log) + - [Multiple Logs at a Time](#multiple-logs-at-a-time) +- [Compiler Tools/Options for Bug Hunting](#compiler-toolsoptions-for-bug-hunting) + - [Using `clang-tidy` to run the Static Analyzer](#using-clang-tidy-to-run-the-static-analyzer) + +# Debugging the Compiler Itself + +## Basic Utilities Often, the first step to debug a compiler problem is to re-run the compiler with a command line, which comes from a crash trace or a build log. @@ -16,8 +54,7 @@ with a command line, which comes from a crash trace or a build log. The script `split-cmdline` in `utils/dev-scripts` splits a command line into multiple lines. This is helpful to understand and edit long command lines. -Printing the Intermediate Representations ------------------------------------------ +## Printing the Intermediate Representations The most important thing when debugging the compiler is to examine the IR. Here is how to dump the IR after the main phases of the Swift compiler @@ -75,11 +112,9 @@ print the SIL *and* the LLVM IR, you have to run the compiler twice. The output of all these dump options (except `-dump-ast`) can be redirected with an additional `-o ` option. -Debugging Diagnostic Emission ------------------------------ +## Debugging Diagnostic Emission -Asserting on first emitted Warning/Assert Diagnostic ----------------------------------------------------- +### Asserting on first emitted Warning/Assert Diagnostic When changing the type checker and various SIL passes, one can cause a series of cascading diagnostics (errors/warnings) to be emitted. Since Swift does not by @@ -94,19 +129,16 @@ diagnostic engine to assert on the first error/warning: These allow one to dump a stack trace of where the diagnostic is being emitted (if run without a debugger) or drop into the debugger if a debugger is attached. -Finding Diagnostic Names ----------------------------------------------------- +### Finding Diagnostic Names Some diagnostics rely heavily on format string arguments, so it can be difficult to find their implementation by searching for parts of the emitted message in the codebase. To print the corresponding diagnostic name at the end of each emitted message, use the `-Xfrontend -debug-diagnostic-names` argument. -Debugging the Type Checker --------------------------- +## Debugging the Type Checker -Enabling Logging ----------------- +### Enabling Logging To enable logging in the type checker, use the following argument: `-Xfrontend -debug-constraints`. This will cause the typechecker to log its internal state as it solves @@ -168,11 +200,9 @@ typing `:constraints debug on`: *** Type ':help' for assistance. *** (swift) :constraints debug on -Debugging on SIL Level ----------------------- +## Debugging on SIL Level -Options for Dumping the SIL ---------------------------- +### Options for Dumping the SIL Often it is not sufficient to dump the SIL at the beginning or end of the optimization pipeline. The SILPassManager supports useful options @@ -223,8 +253,7 @@ optimization pass runs easily via formulations like: NOTE: This may emit a lot of text to stderr, so be sure to pipe the output to a file. -Getting CommandLine for swift stdlib from Ninja ------------------------------------------------ +### Getting CommandLine for swift stdlib from Ninja to enable dumping stdlib SIL If one builds swift using ninja and wants to dump the SIL of the stdlib using some of the SIL dumping options from the previous @@ -235,8 +264,7 @@ section, one can use the following one-liner: This should give one a single command line that one can use for Swift.o, perfect for applying the previous sections options to. -Dumping the SIL and other Data in LLDB --------------------------------------- +### Dumping the SIL and other Data in LLDB When debugging the Swift compiler with LLDB (or Xcode, of course), there is even a more powerful way to examine the data in the compiler, e.g. the SIL. @@ -273,8 +301,9 @@ with the proper attributes to ensure they'll be available in the debugger. In particular, if you see `SWIFT_DEBUG_DUMP` in a class declaration, that class has a `dump()` method you can call. -Debugging and Profiling on SIL level ------------------------------------- +## Debugging and Profiling on SIL level + +### SIL source level profiling using -gsil The compiler provides a way to debug and profile on SIL level. To enable SIL debugging add the front-end option -gsil together with -g. Example: @@ -289,8 +318,7 @@ For details see the SILDebugInfoGenerator pass. To enable SIL debugging and profiling for the Swift standard library, use the build-script-impl option `--build-sil-debugging-stdlib`. -ViewCFG: Regex based CFG Printer --------------------------------- +### ViewCFG: Regex based CFG Printer for SIL/LLVM-IR ViewCFG (`./utils/viewcfg`) is a script that parses a textual CFG (e.g. a llvm or sil function) and displays a .dot file of the CFG. Since the parsing is done @@ -336,8 +364,7 @@ used with viewcfg: $ blockifyasm < file.s | viewcfg -Using Breakpoints ------------------ +### Debugging the Compiler using advanced LLDB Breakpoints LLDB has very powerful breakpoints, which can be utilized in many ways to debug the compiler and Swift executables. The examples in this section show the LLDB @@ -424,8 +451,7 @@ Then lldb would add 38 to the offset of foo and break there. This is really useful in contexts where one wants to set a breakpoint at an assembly address that is stable across multiple different invocations of lldb. -LLDB Scripts ------------- +### Debugging the Compiler using LLDB Scripts LLDB has powerful capabilities of scripting in Python among other languages. An often overlooked, but very useful technique is the -s command to lldb. This @@ -463,8 +489,7 @@ Then by running `lldb test -s test.lldb`, lldb will: Using LLDB scripts can enable one to use complex debugger workflows without needing to retype the various commands perfectly every time. -Custom LLDB Commands --------------------- +### Custom LLDB Commands If you've ever found yourself repeatedly entering a complex sequence of commands within a debug session, consider using custom lldb commands. Custom @@ -510,42 +535,9 @@ to define custom commands using just other lldb commands. For example, (lldb) command alias cs sequence p/x $rax; stepi -Reducing SIL test cases using bug_reducer ------------------------------------------ - -There is functionality provided in ./swift/utils/bug_reducer/bug_reducer.py for -reducing SIL test cases by: - -1. Producing intermediate sib files that only require some of the passes to - trigger the crasher. -2. Reducing the size of the sil test case by extracting functions or - partitioning a module into unoptimized and optimized modules. - -For more information and a high level example, see: -./swift/utils/bug_reducer/README.md. - -Using `clang-tidy` to run the Static Analyzer ------------------------------------------------ - -Recent versions of LLVM package the tool `clang-tidy`. This can be used in -combination with a json compilation database to run static analyzer checks as -well as cleanups/modernizations on a code-base. Swift's cmake invocation by -default creates one of these json databases at the root path of the swift host -build, for example on macOS: - - $PATH_TO_BUILD/swift-macosx-x86_64/compile_commands.json - -Using this file, one invokes `clang-tidy` on a specific file in the codebase -as follows: - - clang-tidy -p=$PATH_TO_BUILD/swift-macosx-x86_64/compile_commands.json $FULL_PATH_TO_FILE - -One can also use shell regex to visit multiple files in the same directory. Example: - - clang-tidy -p=$PATH_TO_BUILD/swift-macosx-x86_64/compile_commands.json $FULL_PATH_TO_DIR/*.cpp +## Bisecting Compiler Errors -Identifying an optimizer bug ----------------------------- +### Bisecting on SIL optimizer pass counts to identify optimizer bugs If a compiled executable is crashing when built with optimizations, but not crashing when built with -Onone, it's most likely one of the SIL optimizations @@ -588,16 +580,87 @@ it's quite easy to do this manually: did wrong. To simplify the comparison, it's sometimes helpful to replace all SIL values (e.g. `%27`) with a constant string (e.g. `%x`). +### Using git-bisect in the presence of branch forwarding/feature branches + +`git-bisect` is a useful tool for finding where a regression was +introduced. Sadly `git-bisect` does not handle long lived branches +and will in fact choose commits from upstream branches that may be +missing important content from the downstream branch. As an example, +consider a situation where one has the following straw man commit flow +graph: + + github/master -> github/tensorflow + +In this case if one attempts to use `git-bisect` on +github/tensorflow, `git-bisect` will sometimes choose commits from +github/master resulting in one being unable to compile/test specific +tensorflow code that has not been upstreamed yet. Even worse, what if +we are trying to bisect in between two that were branched from +github/tensorflow and have had subsequent commits cherry-picked on +top. Without any loss of generality, lets call those two tags +`tag-tensorflow-bad` and `tag-tensorflow-good`. Since both of +these tags have had commits cherry-picked on top, they are technically +not even on the github/tensorflow branch, but rather in a certain +sense are a tag of a feature branch from master/tensorflow. So, +`git-bisect` doesn't even have a clear history to bisect on in +multiple ways. + +With those constraints in mind, we can bisect! We just need to be +careful how we do it. Lets assume that we have a test script called +`test.sh` that indicates error by the error code. With that in hand, +we need to compute the least common ancestor of the good/bad +commits. This is traditionally called the "merge base" of the +commits. We can compute this as so: + + TAG_MERGE_BASE=$(git merge-base tags/tag-tensorflow-bad tags/tag-tensorflow-good) + +Given that both tags were taken from the feature branch, the reader +can prove to themselves that this commit is guaranteed to be on +`github/tensorflow` and not `github/master` since all commits from +`github/master` are forwarded using git merges. + +Then lets assume that we checked out `$TAG_MERGE_BASE` and then ran +`test.sh` and did not hit any error. Ok, we can not bisect. Sadly, +as mentioned above if we run git-bisect in between `$TAG_MERGE_BASE` +and `tags/tag-tensorflow-bad`, `git-bisect` will sometimes choose +commits from `github/master` which would cause `test.sh` to fail +if we are testing tensorflow specific code! To work around this +problem, we need to start our bisect and then tell `git-bisect` to +ignore those commits by using the skip sub command: + + git bisect start tags/tag-tensorflow-bad $TAG_MERGE_BASE + for rev in $(git rev-list $TAG_MERGE_BASE..tags/tag-tensorflow-bad --merges --first-parent); do + git rev-list $rev^2 --not $rev^ + done | xargs git bisect skip + +Once this has been done, one uses `git-bisect` normally. One thing +to be aware of is that `git-bisect` will return a good/bad commits +on the feature branch and if one of those commits is a merge from the +upstream branch, one will need to analyze the range of commits from +upstream for the bad commit afterwards. The commit range in the merge +should be relatively small though compared with the large git history +one just bisected. + +### Reducing SIL test cases using bug_reducer -Debugging Swift Executables -=========================== +There is functionality provided in ./swift/utils/bug_reducer/bug_reducer.py for +reducing SIL test cases by: + +1. Producing intermediate sib files that only require some of the passes to + trigger the crasher. +2. Reducing the size of the sil test case by extracting functions or + partitioning a module into unoptimized and optimized modules. + +For more information and a high level example, see: +./swift/utils/bug_reducer/README.md. + +# Debugging Swift Executables One can use the previous tips for debugging the Swift compiler with Swift executables as well. Here are some additional useful techniques that one can use in Swift executables. -Determining the mangled name of a function in LLDB --------------------------------------------------- +## Determining the mangled name of a function in LLDB One problem that often comes up when debugging Swift code in LLDB is that LLDB shows the demangled name instead of the mangled name. This can lead to mistakes @@ -611,15 +674,13 @@ function in the current frame: Module: file = "/Volumes/Files/work/solon/build/build-swift/validation-test-macosx-x86_64/stdlib/Output/CollectionType.swift.gyb.tmp/CollectionType3", arch = "x86_64" Symbol: id = {0x0000008c}, range = [0x0000000100004db0-0x00000001000056f0), name="ext.CollectionType3.CollectionType3.MutableCollectionType2.(subscript.materializeForSet : (Swift.Range) -> Swift.MutableSlice).(closure #1)", mangled="_TFFeRq_15CollectionType322MutableCollectionType2_S_S0_m9subscriptFGVs5Rangeqq_s16MutableIndexable5Index_GVs12MutableSliceq__U_FTBpRBBRQPS0_MS4__T_" -Manually symbolication using LLDB ---------------------------------- +## Manually symbolication using LLDB One can perform manual symbolication of a crash log or an executable using LLDB without running the actual executable. For a detailed guide on how to do this, see: https://lldb.llvm.org/symbolication.html. -Debugging LLDB failures -======================= +# Debugging LLDB failures Sometimes one needs to be able to while debugging actually debug LLDB and its interaction with Swift itself. Some examples of problems where this can come up @@ -639,8 +700,7 @@ For more details about any of the information below, please run: (lldb) help log enable -"Types" Log ------------ +## "Types" Log The "types" log reports on LLDB's process of constructing SwiftASTContexts and errors that may occur. The two main tasks here are: @@ -670,8 +730,7 @@ That will write the types log to the file passed to the -f option. This will ensure that the type import command is run before /any/ modules are imported. -"Expression" Log ----------------- +## "Expression" Log The "expression" log reports on the process of wrapping, parsing, SILGen'ing, JITing, and inserting an expression into the current Swift module. Since this can @@ -698,72 +757,30 @@ especially if one evaluates multiple expressions with the logging enabled. In such a situation, run all expressions before the bad expression, turn on the logging, and only then run the bad expression. -Multiple Logs at a Time ------------------------ +## Multiple Logs at a Time Note, you can also turn on more than one log at a time as well, e.x.: (lldb) log enable -f /tmp/lldb-types-log.txt lldb types expression -Using git-bisect in the presence of branch forwarding/feature branches -====================================================================== - +# Compiler Tools/Options for Bug Hunting -`git-bisect` is a useful tool for finding where a regression was -introduced. Sadly `git-bisect` does not handle long lived branches -and will in fact choose commits from upstream branches that may be -missing important content from the downstream branch. As an example, -consider a situation where one has the following straw man commit flow -graph: +## Using `clang-tidy` to run the Static Analyzer - github/master -> github/tensorflow - -In this case if one attempts to use `git-bisect` on -github/tensorflow, `git-bisect` will sometimes choose commits from -github/master resulting in one being unable to compile/test specific -tensorflow code that has not been upstreamed yet. Even worse, what if -we are trying to bisect in between two that were branched from -github/tensorflow and have had subsequent commits cherry-picked on -top. Without any loss of generality, lets call those two tags -`tag-tensorflow-bad` and `tag-tensorflow-good`. Since both of -these tags have had commits cherry-picked on top, they are technically -not even on the github/tensorflow branch, but rather in a certain -sense are a tag of a feature branch from master/tensorflow. So, -`git-bisect` doesn't even have a clear history to bisect on in -multiple ways. +Recent versions of LLVM package the tool `clang-tidy`. This can be used in +combination with a json compilation database to run static analyzer checks as +well as cleanups/modernizations on a code-base. Swift's cmake invocation by +default creates one of these json databases at the root path of the swift host +build, for example on macOS: -With those constraints in mind, we can bisect! We just need to be -careful how we do it. Lets assume that we have a test script called -`test.sh` that indicates error by the error code. With that in hand, -we need to compute the least common ancestor of the good/bad -commits. This is traditionally called the "merge base" of the -commits. We can compute this as so: + $PATH_TO_BUILD/swift-macosx-x86_64/compile_commands.json - TAG_MERGE_BASE=$(git merge-base tags/tag-tensorflow-bad tags/tag-tensorflow-good) +Using this file, one invokes `clang-tidy` on a specific file in the codebase +as follows: -Given that both tags were taken from the feature branch, the reader -can prove to themselves that this commit is guaranteed to be on -`github/tensorflow` and not `github/master` since all commits from -`github/master` are forwarded using git merges. + clang-tidy -p=$PATH_TO_BUILD/swift-macosx-x86_64/compile_commands.json $FULL_PATH_TO_FILE -Then lets assume that we checked out `$TAG_MERGE_BASE` and then ran -`test.sh` and did not hit any error. Ok, we can not bisect. Sadly, -as mentioned above if we run git-bisect in between `$TAG_MERGE_BASE` -and `tags/tag-tensorflow-bad`, `git-bisect` will sometimes choose -commits from `github/master` which would cause `test.sh` to fail -if we are testing tensorflow specific code! To work around this -problem, we need to start our bisect and then tell `git-bisect` to -ignore those commits by using the skip sub command: +One can also use shell regex to visit multiple files in the same directory. Example: - git bisect start tags/tag-tensorflow-bad $TAG_MERGE_BASE - for rev in $(git rev-list $TAG_MERGE_BASE..tags/tag-tensorflow-bad --merges --first-parent); do - git rev-list $rev^2 --not $rev^ - done | xargs git bisect skip + clang-tidy -p=$PATH_TO_BUILD/swift-macosx-x86_64/compile_commands.json $FULL_PATH_TO_DIR/*.cpp -Once this has been done, one uses `git-bisect` normally. One thing -to be aware of is that `git-bisect` will return a good/bad commits -on the feature branch and if one of those commits is a merge from the -upstream branch, one will need to analyze the range of commits from -upstream for the bad commit afterwards. The commit range in the merge -should be relatively small though compared with the large git history -one just bisected. From 84a7882419207d1db9acc987926e0d837c1c4a36 Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Thu, 7 May 2020 12:14:41 -0700 Subject: [PATCH 10/14] build: add file dependencies for gyb Add dependencies on the files for gyb to ensure that updates gyb are treated as triggers to regenerate the files. This is suspected to have caused issues for incremental builds. --- cmake/modules/AddSwift.cmake | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/cmake/modules/AddSwift.cmake b/cmake/modules/AddSwift.cmake index 0fc5c47cf0bc9..88a11b34f66a5 100644 --- a/cmake/modules/AddSwift.cmake +++ b/cmake/modules/AddSwift.cmake @@ -5,6 +5,18 @@ include(SwiftWindowsSupport) include(SwiftAndroidSupport) function(_swift_gyb_target_sources target scope) + file(GLOB GYB_UNICODE_DATA ${SWIFT_SOURCE_DIR}/utils/UnicodeData/*) + file(GLOB GYB_STDLIB_SUPPORT ${SWIFT_SOURCE_DIR}/utils/gyb_stdlib_support.py) + file(GLOB GYB_SYNTAX_SUPPORT ${SWIFT_SOURCE_DIR}/utils/gyb_syntax_support/*) + file(GLOB GYB_SOURCEKIT_SUPPORT ${SWIFT_SOURCE_DIR}/utils/gyb_sourcekit_support/*) + set(GYB_SOURCES + ${SWIFT_SOURCE_DIR}/utils/GYBUnicodeDataUtils.py + ${SWIFT_SOURCE_DIR}/utils/SwiftIntTypes.py + ${GYB_UNICODE_DATA} + ${GYB_STDLIB_SUPPORT} + ${GYB_SYNTAX_SUPPORT} + ${GYB_SOURCEKIT_SUPPORT}) + foreach(source ${ARGN}) get_filename_component(generated ${source} NAME_WLE) get_filename_component(absolute ${source} REALPATH) @@ -17,6 +29,7 @@ function(_swift_gyb_target_sources target scope) COMMAND ${CMAKE_COMMAND} -E remove ${CMAKE_CURRENT_BINARY_DIR}/${generated}.tmp DEPENDS + ${GYB_SOURCES} ${absolute}) set_source_files_properties(${CMAKE_CURRENT_BINARY_DIR}/${generated} PROPERTIES GENERATED TRUE) From 70bd271e99309e0b8664e1de8232ce461e36914c Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Thu, 7 May 2020 12:42:33 -0700 Subject: [PATCH 11/14] runtime: remove dependency on `LLVM_ENABLE_THREADS` This removes the LLVMSupport usage for thread local. This effectively is a no-op as the runtime requires that it is built with clang, which provides the C++11 `thread_local` support on all the targets. In the case that the target does not support `thread_local`, clang would fallback to `__declspec(thread)` on MSVC and `__thread` on other modes. However, because the runtime is always built in C++14 mode, the `thread_local` support should be assumed to be present. Remove the unnecessary condition and inline the single use of the macro for `thread_local`. --- stdlib/public/runtime/Exclusivity.cpp | 4 ++-- stdlib/public/runtime/ThreadLocalStorage.h | 13 +++---------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/stdlib/public/runtime/Exclusivity.cpp b/stdlib/public/runtime/Exclusivity.cpp index 39ba0ce6b9678..aa6993d29284f 100644 --- a/stdlib/public/runtime/Exclusivity.cpp +++ b/stdlib/public/runtime/Exclusivity.cpp @@ -270,10 +270,10 @@ static SwiftTLSContext &getTLSContext() { return *ctx; } -#elif SWIFT_TLS_HAS_THREADLOCAL +#elif __has_feature(cxx_thread_local) // Second choice is direct language support for thread-locals. -static LLVM_THREAD_LOCAL SwiftTLSContext TLSContext; +static thread_local SwiftTLSContext TLSContext; static SwiftTLSContext &getTLSContext() { return TLSContext; diff --git a/stdlib/public/runtime/ThreadLocalStorage.h b/stdlib/public/runtime/ThreadLocalStorage.h index 09b870c5c426a..bb2fb0b2ff564 100644 --- a/stdlib/public/runtime/ThreadLocalStorage.h +++ b/stdlib/public/runtime/ThreadLocalStorage.h @@ -26,16 +26,6 @@ # define SWIFT_TLS_HAS_RESERVED_PTHREAD_SPECIFIC 1 #endif -// If we're using Clang, and Clang claims not to support thread_local, -// it must be because we're on a platform that doesn't support it. -#if __clang__ && !__has_feature(cxx_thread_local) -// No thread_local. -#else -// Otherwise, we do have thread_local. -# define SWIFT_TLS_HAS_THREADLOCAL 1 -static_assert(LLVM_ENABLE_THREADS, "LLVM_THREAD_LOCAL will use a global?"); -#endif - #if SWIFT_TLS_HAS_RESERVED_PTHREAD_SPECIFIC // Use reserved TSD keys. # if __has_include() @@ -92,6 +82,9 @@ typedef unsigned long __swift_thread_key_t; # include # define WIN32_LEAN_AND_MEAN # include + +#include + static_assert(std::is_same<__swift_thread_key_t, DWORD>::value, "__swift_thread_key_t is not a DWORD"); From c5863ac0f3e8d849054ffe3c499bdb9bc78b1a4d Mon Sep 17 00:00:00 2001 From: Joe Groff Date: Thu, 7 May 2020 13:20:38 -0700 Subject: [PATCH 12/14] SILOptimizer: Constant fold the _kvcKeyPathString of literal key paths. Eliminate the intermediate key path object when a literal key path is passed to a function that just wants its KVC string to pass down to an ObjC API. --- include/swift/AST/ASTContext.h | 4 + include/swift/AST/Attr.h | 13 +++ include/swift/AST/Decl.h | 8 +- include/swift/AST/SemanticAttrs.def | 2 + .../SILOptimizer/Utils/KeyPathProjector.h | 6 ++ lib/AST/ASTContext.cpp | 8 ++ lib/SILGen/SILGenApply.cpp | 8 +- lib/SILOptimizer/SILCombiner/SILCombiner.h | 4 +- .../SILCombiner/SILCombinerApplyVisitors.cpp | 100 +++++++++++++++++- lib/SILOptimizer/Utils/KeyPathProjector.cpp | 13 ++- lib/Sema/ConstantnessSemaDiagnostics.cpp | 6 +- stdlib/public/core/KeyPath.swift | 7 +- test/SILOptimizer/optimize_keypath_objc.swift | 33 ++++++ 13 files changed, 183 insertions(+), 29 deletions(-) create mode 100644 test/SILOptimizer/optimize_keypath_objc.swift diff --git a/include/swift/AST/ASTContext.h b/include/swift/AST/ASTContext.h index 215d22ccfde71..27e1e85c82b36 100644 --- a/include/swift/AST/ASTContext.h +++ b/include/swift/AST/ASTContext.h @@ -1027,6 +1027,10 @@ class ASTContext final { /// Retrieve the IRGen specific SIL passes. SILTransformCtors getIRGenSILTransforms() const; + + /// Check whether a given string would be considered "pure ASCII" by the + /// standard library's String implementation. + bool isASCIIString(StringRef s) const; private: friend Decl; diff --git a/include/swift/AST/Attr.h b/include/swift/AST/Attr.h index 7cb616a4a6ed1..fde3102843bd5 100644 --- a/include/swift/AST/Attr.h +++ b/include/swift/AST/Attr.h @@ -2209,6 +2209,19 @@ class DeclAttributes { make_range(begin(), end()), ToAttributeKind()); } + /// Return the range of semantics attributes attached to this attribute set. + auto getSemanticsAttrs() const + -> decltype(getAttributes()) { + return getAttributes(); + } + + /// Return whether this attribute set includes the given semantics attribute. + bool hasSemanticsAttr(StringRef attrValue) const { + return llvm::any_of(getSemanticsAttrs(), [&](const SemanticsAttr *attr) { + return attrValue.equals(attr->Value); + }); + } + // Remove the given attribute from the list of attributes. Used when // the attribute was semantically invalid. void removeAttribute(const DeclAttribute *attr) { diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index a481d6df1427e..871343e30a4a5 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -3524,14 +3524,12 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext { /// Return the range of semantics attributes attached to this NominalTypeDecl. auto getSemanticsAttrs() const - -> decltype(getAttrs().getAttributes()) { - return getAttrs().getAttributes(); + -> decltype(getAttrs().getSemanticsAttrs()) { + return getAttrs().getSemanticsAttrs(); } bool hasSemanticsAttr(StringRef attrValue) const { - return llvm::any_of(getSemanticsAttrs(), [&](const SemanticsAttr *attr) { - return attrValue.equals(attr->Value); - }); + return getAttrs().hasSemanticsAttr(attrValue); } /// Whether this declaration has a synthesized memberwise initializer. diff --git a/include/swift/AST/SemanticAttrs.def b/include/swift/AST/SemanticAttrs.def index fb61ba12b4b95..81615e37d9c10 100644 --- a/include/swift/AST/SemanticAttrs.def +++ b/include/swift/AST/SemanticAttrs.def @@ -87,5 +87,7 @@ SEMANTICS_ATTR(SLOWPATH, "slowpath") SEMANTICS_ATTR(PROGRAMTERMINATION_POINT, "programtermination_point") SEMANTICS_ATTR(CONVERT_TO_OBJECTIVE_C, "convertToObjectiveC") +SEMANTICS_ATTR(KEYPATH_KVC_KEY_PATH_STRING, "keypath.kvcKeyPathString") + #undef SEMANTICS_ATTR diff --git a/include/swift/SILOptimizer/Utils/KeyPathProjector.h b/include/swift/SILOptimizer/Utils/KeyPathProjector.h index 6c64c469bf776..a956560695a9c 100644 --- a/include/swift/SILOptimizer/Utils/KeyPathProjector.h +++ b/include/swift/SILOptimizer/Utils/KeyPathProjector.h @@ -24,6 +24,8 @@ namespace swift { +class KeyPathInst; + /// Projects a statically known key path expression to /// a direct property access. class KeyPathProjector { @@ -53,6 +55,10 @@ class KeyPathProjector { static std::unique_ptr create(SILValue keyPath, SILValue root, SILLocation loc, SILBuilder &builder); + /// Extract the literal KeyPathInst underlying a value, or return null if there is none. + static KeyPathInst * + getLiteralKeyPath(SILValue keyPath); + /// Projects the key path to an address. Sets up the projection, /// invokes the callback, then tears down the projection. /// \param accessType The access type of the projected address. diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 97b4b4a4128a5..038fa40cb790f 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -4827,3 +4827,11 @@ llvm::LLVMContext &ASTContext::getIntrinsicScratchContext() const { #endif } +bool ASTContext::isASCIIString(StringRef s) const { + for (unsigned char c : s) { + if (c > 127) { + return false; + } + } + return true; +} diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 172447aba7727..0a101dd2f0c76 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -1614,13 +1614,7 @@ static PreparedArguments emitStringLiteral(SILGenFunction &SGF, Expr *E, StringRef Str, SGFContext C, StringLiteralExpr::Encoding encoding) { uint64_t Length; - bool isASCII = true; - for (unsigned char c : Str) { - if (c > 127) { - isASCII = false; - break; - } - } + bool isASCII = SGF.getASTContext().isASCIIString(Str); StringLiteralInst::Encoding instEncoding; switch (encoding) { case StringLiteralExpr::UTF8: diff --git a/lib/SILOptimizer/SILCombiner/SILCombiner.h b/lib/SILOptimizer/SILCombiner/SILCombiner.h index 713d80aec74c5..31585fee4ee03 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombiner.h +++ b/lib/SILOptimizer/SILCombiner/SILCombiner.h @@ -241,7 +241,9 @@ class SILCombiner : bool tryOptimizeKeypath(ApplyInst *AI); bool tryOptimizeInoutKeypath(BeginApplyInst *AI); - + bool tryOptimizeKeypathApplication(ApplyInst *AI, SILFunction *callee); + bool tryOptimizeKeypathKVCString(ApplyInst *AI, SILDeclRef callee); + // Optimize concatenation of string literals. // Constant-fold concatenation of string literals known at compile-time. SILInstruction *optimizeConcatenationOfStringLiterals(ApplyInst *AI); diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index f25de2558ad73..d18d165842e7d 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -226,11 +226,8 @@ SILCombiner::optimizeApplyOfConvertFunctionInst(FullApplySite AI, /// %addr = struct_element_addr/ref_element_addr %root_object /// ... /// load/store %addr -bool SILCombiner::tryOptimizeKeypath(ApplyInst *AI) { - SILFunction *callee = AI->getReferencedFunctionOrNull(); - if (!callee) - return false; - +bool SILCombiner::tryOptimizeKeypathApplication(ApplyInst *AI, + SILFunction *callee) { if (AI->getNumArguments() != 3) return false; @@ -274,6 +271,99 @@ bool SILCombiner::tryOptimizeKeypath(ApplyInst *AI) { return true; } +/// Try to optimize a keypath KVC string access on a literal key path. +/// +/// Replace: +/// %kp = keypath (objc "blah", ...) +/// %string = apply %keypath_kvcString_method(%kp) +/// With: +/// %string = string_literal "blah" +bool SILCombiner::tryOptimizeKeypathKVCString(ApplyInst *AI, + SILDeclRef callee) { + if (AI->getNumArguments() != 1) { + return false; + } + if (!callee.hasDecl()) { + return false; + } + auto calleeFn = dyn_cast(callee.getDecl()); + if (!calleeFn) + return false; + + if (!calleeFn->getAttrs() + .hasSemanticsAttr(semantics::KEYPATH_KVC_KEY_PATH_STRING)) + return false; + + // Method should return `String?` + auto &C = calleeFn->getASTContext(); + auto objTy = AI->getType().getOptionalObjectType(); + if (!objTy || objTy.getStructOrBoundGenericStruct() != C.getStringDecl()) + return false; + + KeyPathInst *kp + = KeyPathProjector::getLiteralKeyPath(AI->getArgument(0)); + if (!kp || !kp->hasPattern()) + return false; + + auto objcString = kp->getPattern()->getObjCString(); + + SILValue literalValue; + if (objcString.empty()) { + // Replace with a nil String value. + literalValue = Builder.createEnum(AI->getLoc(), SILValue(), + C.getOptionalNoneDecl(), + AI->getType()); + } else { + // Construct a literal String from the ObjC string. + auto init = C.getStringBuiltinInitDecl(C.getStringDecl()); + if (!init) + return false; + auto initRef = SILDeclRef(init.getDecl(), SILDeclRef::Kind::Allocator); + auto initFn = AI->getModule().findFunction(initRef.mangle(), + SILLinkage::PublicExternal); + if (!initFn) + return false; + + auto stringValue = Builder.createStringLiteral(AI->getLoc(), objcString, + StringLiteralInst::Encoding::UTF8); + auto stringLen = Builder.createIntegerLiteral(AI->getLoc(), + SILType::getBuiltinWordType(C), + objcString.size()); + auto isAscii = Builder.createIntegerLiteral(AI->getLoc(), + SILType::getBuiltinIntegerType(1, C), + C.isASCIIString(objcString)); + auto metaTy = + CanMetatypeType::get(objTy.getASTType(), MetatypeRepresentation::Thin); + auto self = Builder.createMetatype(AI->getLoc(), + SILType::getPrimitiveObjectType(metaTy)); + + auto initFnRef = Builder.createFunctionRef(AI->getLoc(), initFn); + auto string = Builder.createApply(AI->getLoc(), + initFnRef, {}, + {stringValue, stringLen, isAscii, self}); + + literalValue = Builder.createEnum(AI->getLoc(), string, + C.getOptionalSomeDecl(), AI->getType()); + } + + AI->replaceAllUsesWith(literalValue); + eraseInstFromFunction(*AI); + ++NumOptimizedKeypaths; + return true; +} + +bool SILCombiner::tryOptimizeKeypath(ApplyInst *AI) { + if (SILFunction *callee = AI->getReferencedFunctionOrNull()) { + return tryOptimizeKeypathApplication(AI, callee); + } + + if (auto method = dyn_cast(AI->getCallee())) { + return tryOptimizeKeypathKVCString(AI, method->getMember()); + } + + return false; +} + /// Try to optimize a keypath application with an apply instruction. /// /// Replaces (simplified SIL): diff --git a/lib/SILOptimizer/Utils/KeyPathProjector.cpp b/lib/SILOptimizer/Utils/KeyPathProjector.cpp index 3fac6fcf6f9ae..e6a40b3f55d9a 100644 --- a/lib/SILOptimizer/Utils/KeyPathProjector.cpp +++ b/lib/SILOptimizer/Utils/KeyPathProjector.cpp @@ -656,14 +656,19 @@ class CompleteKeyPathProjector : public KeyPathProjector { } }; +KeyPathInst * +KeyPathProjector::getLiteralKeyPath(SILValue keyPath) { + if (auto *upCast = dyn_cast(keyPath)) + keyPath = upCast->getOperand(); + // TODO: Look through other conversions, copies, etc.? + return dyn_cast(keyPath); +} + std::unique_ptr KeyPathProjector::create(SILValue keyPath, SILValue root, SILLocation loc, SILBuilder &builder) { - if (auto *upCast = dyn_cast(keyPath)) - keyPath = upCast->getOperand(); - // Is it a keypath instruction at all? - auto *kpInst = dyn_cast(keyPath); + auto *kpInst = getLiteralKeyPath(keyPath); if (!kpInst || !kpInst->hasPattern()) return nullptr; diff --git a/lib/Sema/ConstantnessSemaDiagnostics.cpp b/lib/Sema/ConstantnessSemaDiagnostics.cpp index 49f9334697c55..cd60bcff27ef2 100644 --- a/lib/Sema/ConstantnessSemaDiagnostics.cpp +++ b/lib/Sema/ConstantnessSemaDiagnostics.cpp @@ -35,11 +35,7 @@ using namespace swift; /// Check whether a given \p decl has a @_semantics attribute with the given /// attribute name \c attrName. static bool hasSemanticsAttr(ValueDecl *decl, StringRef attrName) { - for (auto semantics : decl->getAttrs().getAttributes()) { - if (semantics->Value.equals(attrName)) - return true; - } - return false; + return decl->getAttrs().hasSemanticsAttr(attrName); } /// Return true iff the given \p structDecl has a name that matches one of the diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift index ce528f1c87d50..dcedf5aa4c79d 100644 --- a/stdlib/public/core/KeyPath.swift +++ b/stdlib/public/core/KeyPath.swift @@ -122,9 +122,12 @@ public class AnyKeyPath: Hashable, _AppendKeyPath { // SPI for the Foundation overlay to allow interop with KVC keypath-based // APIs. public var _kvcKeyPathString: String? { - guard let ptr = _kvcKeyPathStringPtr else { return nil } + @_semantics("keypath.kvcKeyPathString") + get { + guard let ptr = _kvcKeyPathStringPtr else { return nil } - return String(validatingUTF8: ptr) + return String(validatingUTF8: ptr) + } } // MARK: Implementation details diff --git a/test/SILOptimizer/optimize_keypath_objc.swift b/test/SILOptimizer/optimize_keypath_objc.swift new file mode 100644 index 0000000000000..d8b931f2774a3 --- /dev/null +++ b/test/SILOptimizer/optimize_keypath_objc.swift @@ -0,0 +1,33 @@ +// RUN: %empty-directory(%t) +// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -primary-file %s -O -sil-verify-all -emit-sil >%t/output.sil +// RUN: %FileCheck %s < %t/output.sil + +// REQUIRES: objc_interop + +import Foundation + +class Foo: NSObject { + @objc(doesIndeedHaveAKVCString) var hasKVCString: Int = 0 + + var noKVCString: Int? = 0 +} + +// CHECK-LABEL: sil hidden @{{.*}}21optimize_keypath_objc12hasKVCString +func hasKVCString() -> String? { + // CHECK-NOT: = keypath + // CHECK: string_literal utf8 "doesIndeedHaveAKVCString" + // CHECK-NOT: = keypath + // CHECK: [[RESULT:%.*]] = enum $Optional, #Optional.some + // CHECK-NEXT: return [[RESULT:%.*]] + // CHECK-NEXT: } + return (\Foo.hasKVCString)._kvcKeyPathString +} + +// CHECK-LABEL: sil hidden @{{.*}}21optimize_keypath_objc11noKVCString +func noKVCString() -> String? { + // CHECK-NOT: = keypath + // CHECK: [[RESULT:%.*]] = enum $Optional, #Optional.none + // CHECK-NEXT: return [[RESULT:%.*]] + // CHECK-NEXT: } + return (\Foo.noKVCString)._kvcKeyPathString +} From 24e8aa900b9950ce75eee216e5c049451304d483 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Thu, 7 May 2020 16:32:06 -0700 Subject: [PATCH 13/14] [test] Add regression test for rdar://62890683 --- test/Constraints/metatypes.swift | 8 ++++++++ test/Constraints/rdar62890683.swift | 11 +++++++++++ 2 files changed, 19 insertions(+) create mode 100644 test/Constraints/rdar62890683.swift diff --git a/test/Constraints/metatypes.swift b/test/Constraints/metatypes.swift index f1d2f193e1999..45d0ced160194 100644 --- a/test/Constraints/metatypes.swift +++ b/test/Constraints/metatypes.swift @@ -21,3 +21,11 @@ acceptMeta(A) // expected-error {{expected member name or constructor call after acceptMeta((A) -> Void) // expected-error {{expected member name or constructor call after type name}} // expected-note@-1 {{use '.self' to reference the type object}} + +func id(_ x: T.Type) -> T.Type { x } + +// rdar://62890683: Don't allow arbitrary subtyping for a metatype's instance type. +let _: A?.Type = B.self // expected-error {{cannot convert value of type 'B.Type' to specified type 'A?.Type'}} +let _: A?.Type = id(B.self) // expected-error {{cannot convert value of type 'B.Type' to specified type 'A?.Type'}} +let _: S?.Type = id(S.self) // expected-error {{cannot convert value of type 'S.Type' to specified type 'S?.Type'}} + diff --git a/test/Constraints/rdar62890683.swift b/test/Constraints/rdar62890683.swift new file mode 100644 index 0000000000000..f27bd23bef02c --- /dev/null +++ b/test/Constraints/rdar62890683.swift @@ -0,0 +1,11 @@ +// RUN: %target-typecheck-verify-swift + +class C { + init!() {} +} + +func foo(_: T.Type, _ fn: () -> T) {} + +func test() { + foo(C.self) { C() } +} From 9a4ca42e7c3ceadfd9bfe8ab6eb49fa297a602ff Mon Sep 17 00:00:00 2001 From: Stephen Canon Date: Thu, 7 May 2020 19:33:32 -0400 Subject: [PATCH 14/14] Replace "unchecked" arithmetic with "wrapping" in optimization guide. (#31463) * Replace "unchecked" arithmetic with "wrapping" in optimization guide. Previously, this section talked about "unchecked" arithmetic operations; that's not what &+ and friends are. They are wrapping operations with fully-defined behavior (unlike nsw llvm ops, which are actually "unchecked"). * Update docs/OptimizationTips.rst --- docs/OptimizationTips.rst | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/docs/OptimizationTips.rst b/docs/OptimizationTips.rst index e24f02cbc5a0c..7640507778274 100644 --- a/docs/OptimizationTips.rst +++ b/docs/OptimizationTips.rst @@ -283,18 +283,19 @@ through the usage of ``inout`` parameters: var a = [1, 2, 3] append_one_in_place(&a) -Unchecked operations +Wrapping operations ==================== Swift eliminates integer overflow bugs by checking for overflow when performing -normal arithmetic. These checks are not appropriate in high performance code -where one knows that no memory safety issues can result. +normal arithmetic. These checks may not be appropriate in high performance code +if one either knows that overflow cannot occur, or that the result of +allowing the operation to wrap around is correct. -Advice: Use unchecked integer arithmetic when you can prove that overflow cannot occur +Advice: Use wrapping integer arithmetic when you can prove that overflow cannot occur --------------------------------------------------------------------------------------- -In performance-critical code you can elide overflow checks if you know it is -safe. +In performance-critical code you can use wrapping arithmetic to avoid overflow +checks if you know it is safe. :: @@ -302,11 +303,17 @@ safe. b: [Int] c: [Int] - // Precondition: for all a[i], b[i]: a[i] + b[i] does not overflow! + // Precondition: for all a[i], b[i]: a[i] + b[i] either does not overflow, + // or the result of wrapping is desired. for i in 0 ... n { c[i] = a[i] &+ b[i] } +It's important to note that the behavior of the ``&+``, ``&-``, and ``&*`` +operators is fully-defined; the result simply wraps around if it would overflow. +Thus, ``Int.max &+ 1`` is guaranteed to be ``Int.min`` (unlike in C, where +``INT_MAX + 1`` is undefined behavior). + Generics ========