From d9dcf06b64476e1b7c7f2b44af0430ecda01892e Mon Sep 17 00:00:00 2001 From: Ethan Smith Date: Thu, 18 May 2023 15:53:47 -0700 Subject: [PATCH 1/8] Desugar ElabroatedType in isNSUInteger if present. Re-enable PrintAsObjC/availability-real-sdk and PrintAsObjC/override, they are now passing. --- lib/PrintAsClang/DeclAndTypePrinter.cpp | 3 +++ test/PrintAsObjC/availability-real-sdk.swift | 2 -- test/PrintAsObjC/override.swift | 2 -- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/PrintAsClang/DeclAndTypePrinter.cpp b/lib/PrintAsClang/DeclAndTypePrinter.cpp index 4c691332a8b8a..ca28788a639e2 100644 --- a/lib/PrintAsClang/DeclAndTypePrinter.cpp +++ b/lib/PrintAsClang/DeclAndTypePrinter.cpp @@ -959,6 +959,9 @@ class DeclAndTypePrinter::Implementation /// Returns true if \p clangTy is the typedef for NSUInteger. bool isNSUInteger(clang::QualType clangTy) { + if (const auto* elaboratedTy = dyn_cast(clangTy)) { + clangTy = elaboratedTy->desugar(); + } const auto *typedefTy = dyn_cast(clangTy); if (!typedefTy) return false; diff --git a/test/PrintAsObjC/availability-real-sdk.swift b/test/PrintAsObjC/availability-real-sdk.swift index 9f375aa9c8c8d..a264137b6b228 100644 --- a/test/PrintAsObjC/availability-real-sdk.swift +++ b/test/PrintAsObjC/availability-real-sdk.swift @@ -49,8 +49,6 @@ // CHECK-NEXT: - (nullable instancetype)initWithCoder:(NSCoder * _Nonnull){{[a-zA-Z]+}} OBJC_DESIGNATED_INITIALIZER; // CHECK-NEXT: @end -// REQUIRES: rdar102629628 - import Foundation diff --git a/test/PrintAsObjC/override.swift b/test/PrintAsObjC/override.swift index 92dea80eea2a8..c6f8dd6d92028 100644 --- a/test/PrintAsObjC/override.swift +++ b/test/PrintAsObjC/override.swift @@ -15,8 +15,6 @@ // REQUIRES: objc_interop -// REQUIRES: rdar102629628 - import OverrideBase // No errors from Clang until we get to the FixMe class. // CLANG-NOT: error From 0c974e0bf00fd4dcceb3c59e653338ecb1dbee5b Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 19 May 2023 11:26:30 +0100 Subject: [PATCH 2/8] [Build] Make sure lldb API tests run against newly built libcxx Currently any buildbot that runs LLDB API tests does so against the system SDK. However, we explicitly need users to run the tests against a newly built libcxx. We recently added a new `LLDB_TEST_LIBCXX_ROOT_DIR` CMake variable to LLDB to allow standalone builds to point their tests to a custom libcxx directory. This patch makes sure the relevant LLDB presets build libcxx and then sets above CMake variable. --- utils/build-presets.ini | 4 ++++ utils/build-script-impl | 8 +++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/utils/build-presets.ini b/utils/build-presets.ini index 36901a2a24993..9b48a925ec883 100644 --- a/utils/build-presets.ini +++ b/utils/build-presets.ini @@ -291,6 +291,8 @@ skip-test-watchos-host # This is a mixin preset which builds and smoke-tests lldb. [preset: lldb-smoketest,tools=RA] +# Build libcxx for tests +libcxx # Build LLDB lldb @@ -301,6 +303,8 @@ lldb-test-swift-only lldb-assertions [preset: lldb-pull-request] +# Build libcxx for tests +libcxx lldb lit-args=-v diff --git a/utils/build-script-impl b/utils/build-script-impl index acdbab7390e35..92f6eb285a85e 100755 --- a/utils/build-script-impl +++ b/utils/build-script-impl @@ -1709,7 +1709,7 @@ for host in "${ALL_HOSTS[@]}"; do libcxx) build_targets=(cxx) cmake_options=( - -DLLVM_ENABLE_RUNTIMES="libcxx" -DLIBCXX_INSTALL_LIBRARY=OFF + -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi" -DLIBCXX_INSTALL_LIBRARY=OFF "${cmake_options[@]}" "${llvm_cmake_options[@]}" ) @@ -2126,6 +2126,7 @@ for host in "${ALL_HOSTS[@]}"; do cmark_build_dir=$(build_directory ${host} cmark) lldb_build_dir=$(build_directory ${host} lldb) swift_build_dir=$(build_directory ${host} swift) + libcxx_build_dir=$(build_directory ${host} libcxx) # Add any lldb extra cmake arguments here. @@ -2205,6 +2206,7 @@ for host in "${ALL_HOSTS[@]}"; do -DLLDB_ENABLE_LZMA=OFF -DLLDB_ENABLE_LUA=OFF -DLLDB_INCLUDE_TESTS:BOOL=$(false_true ${BUILD_TOOLCHAIN_ONLY}) + -DLLDB_TEST_LIBCXX_ROOT_DIR:STRING="${libcxx_build_dir}" -DLLDB_TEST_USER_ARGS="${DOTEST_ARGS}" ) @@ -2650,10 +2652,6 @@ for host in "${ALL_HOSTS[@]}"; do call env "${EXTRA_DISTCC_OPTIONS[@]}" "${CMAKE}" "${cmake_options[@]}" "${EXTRA_CMAKE_OPTIONS[@]}" "${source_dir}" fi - if [[ "${product}" == "libcxx" ]]; then - continue - fi - # Build. if [[ $(not ${SKIP_BUILD}) ]]; then if [[ "${CMAKE_GENERATOR}" == "Xcode" ]] ; then From 237f552e8fed3e627b622c8b8100976689f7ee3e Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Mon, 22 May 2023 11:07:23 +0200 Subject: [PATCH 3/8] add missing %empty-directory in test --- .../distributed_actor_thunk_doesnt_leak_class_arguments.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Distributed/distributed_actor_thunk_doesnt_leak_class_arguments.swift b/test/Distributed/distributed_actor_thunk_doesnt_leak_class_arguments.swift index 9b2ee1d7e330b..8e4dbfc8f2c3b 100644 --- a/test/Distributed/distributed_actor_thunk_doesnt_leak_class_arguments.swift +++ b/test/Distributed/distributed_actor_thunk_doesnt_leak_class_arguments.swift @@ -1,3 +1,4 @@ +// RUN: %empty-directory(%t) // RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/Inputs/FakeDistributedActorSystems.swift // RUN: %target-swift-frontend -module-name no_to_arg_leaks -emit-irgen -disable-availability-checking -I %t 2>&1 %s | %IRGenFileCheck %s -check-prefix CHECK-%target-import-type From ffde684143ad3f82a7dcefd1d970d19738aa763b Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Mon, 22 May 2023 14:55:17 +0100 Subject: [PATCH 4/8] [Stats] Force the printing of 0 counters This matches the behavior of `printAlwaysOnStatsAndTimers`, which we use in a release build. This fixes the diverging behavior, and ensures process-stats-dir can handle comparing deltas between runs where one of the runs had a 0 counter. --- lib/Basic/Statistic.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/Basic/Statistic.cpp b/lib/Basic/Statistic.cpp index 08451423165eb..04b0bc83a4e65 100644 --- a/lib/Basic/Statistic.cpp +++ b/lib/Basic/Statistic.cpp @@ -419,11 +419,14 @@ UnifiedStatsReporter::noteCurrentProcessExitStatus(int status) { void UnifiedStatsReporter::publishAlwaysOnStatsToLLVM() { + // NOTE: We do `Stat = 0` below to force LLVM to register the statistic, + // ensuring we print counters, even if 0. if (FrontendCounters) { auto &C = getFrontendCounters(); #define FRONTEND_STATISTIC(TY, NAME) \ do { \ static Statistic Stat = {#TY, #NAME, #NAME}; \ + Stat = 0; \ Stat += (C).NAME; \ } while (0); #include "swift/Basic/Statistics.def" @@ -434,6 +437,7 @@ UnifiedStatsReporter::publishAlwaysOnStatsToLLVM() { #define DRIVER_STATISTIC(NAME) \ do { \ static Statistic Stat = {"Driver", #NAME, #NAME}; \ + Stat = 0; \ Stat += (C).NAME; \ } while (0); #include "swift/Basic/Statistics.def" From 4ce49bd4d20f375fe6056e4eba468f1bac2ff929 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Mon, 22 May 2023 14:55:18 +0100 Subject: [PATCH 5/8] [Parse] Sink `registerParseRequestFunctions` call into ParserUnit --- lib/Parse/Parser.cpp | 2 ++ tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp | 1 - tools/swift-ide-test/swift-ide-test.cpp | 2 -- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp index c96c0a5bfa7a9..0e163d85d8dd8 100644 --- a/lib/Parse/Parser.cpp +++ b/lib/Parse/Parser.cpp @@ -1115,6 +1115,8 @@ struct ParserUnit::Implementation { TypeCheckerOpts(TyOpts), SILOpts(silOpts), Diags(SM), Ctx(*ASTContext::get(LangOpts, TypeCheckerOpts, SILOpts, SearchPathOpts, clangImporterOpts, symbolGraphOpts, SM, Diags)) { + registerParseRequestFunctions(Ctx.evaluator); + auto parsingOpts = SourceFile::getDefaultParsingOptions(LangOpts); parsingOpts |= ParsingFlags::DisableDelayedBodies; parsingOpts |= ParsingFlags::DisablePoundIfEvaluation; diff --git a/tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp b/tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp index bb03379842331..2fe71aa7b8eda 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp +++ b/tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp @@ -775,7 +775,6 @@ class SwiftDocumentSyntaxInfo { CompInv.getTypeCheckerOptions(), CompInv.getSILOptions(), CompInv.getModuleName())); - registerParseRequestFunctions(Parser->getParser().Context.evaluator); registerTypeCheckerRequestFunctions( Parser->getParser().Context.evaluator); registerClangImporterRequestFunctions(Parser->getParser().Context.evaluator); diff --git a/tools/swift-ide-test/swift-ide-test.cpp b/tools/swift-ide-test/swift-ide-test.cpp index ad996e47f9f99..985b40212da4e 100644 --- a/tools/swift-ide-test/swift-ide-test.cpp +++ b/tools/swift-ide-test/swift-ide-test.cpp @@ -2007,7 +2007,6 @@ static int doSyntaxColoring(const CompilerInvocation &InitInvok, Invocation.getTypeCheckerOptions(), Invocation.getSILOptions(), Invocation.getModuleName()); - registerParseRequestFunctions(Parser.getParser().Context.evaluator); registerTypeCheckerRequestFunctions(Parser.getParser().Context.evaluator); registerClangImporterRequestFunctions(Parser.getParser().Context.evaluator); @@ -2236,7 +2235,6 @@ static int doStructureAnnotation(const CompilerInvocation &InitInvok, Invocation.getTypeCheckerOptions(), Invocation.getSILOptions(), Invocation.getModuleName()); - registerParseRequestFunctions(Parser.getParser().Context.evaluator); registerTypeCheckerRequestFunctions( Parser.getParser().Context.evaluator); registerClangImporterRequestFunctions(Parser.getParser().Context.evaluator); From 58d66947838609f6916df8af018ecae819372f28 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Mon, 22 May 2023 14:55:19 +0100 Subject: [PATCH 6/8] Requestify ExportedSourceFile parsing Avoid parsing the syntax tree up-front, and instead only parse it when required, which happens when either: 1. ASTGen parsing is enabled (currently disabled by default) 2. Round trip checking is enabled for a primary file (enabled by default in a debug build, except when dep scanning or doing an IDE operation) 3. We need to evaluate a macro in that file This change therefore means that we now no longer need to parse the syntax tree for secondary files by default unless we specifically need to evaluate a macro in them (e.g if we need to lookup a member on a decl with an attached macro). And the same for primaries in release builds. rdar://109283847 --- include/swift/AST/ParseRequests.h | 18 ++ include/swift/AST/ParseTypeIDZone.def | 3 + include/swift/AST/SourceFile.h | 16 +- lib/AST/Module.cpp | 9 + lib/Frontend/Frontend.cpp | 36 ++-- lib/Parse/ParseDecl.cpp | 181 +++++++++++------- lib/Sema/TypeCheckMacros.cpp | 17 +- lib/Serialization/ModuleDependencyScanner.cpp | 3 - test/Macros/lazy_parsing.swift | 44 +++++ test/ScanDependencies/ensure_no_astgen.swift | 41 ++++ 10 files changed, 265 insertions(+), 103 deletions(-) create mode 100644 test/Macros/lazy_parsing.swift create mode 100644 test/ScanDependencies/ensure_no_astgen.swift diff --git a/include/swift/AST/ParseRequests.h b/include/swift/AST/ParseRequests.h index feec6fb325b59..f9affe82ecb4e 100644 --- a/include/swift/AST/ParseRequests.h +++ b/include/swift/AST/ParseRequests.h @@ -116,6 +116,24 @@ class ParseSourceFileRequest readDependencySource(const evaluator::DependencyRecorder &) const; }; +/// Parse the ExportedSourceFile for a given SourceFile. +class ExportedSourceFileRequest + : public SimpleRequest { +public: + using SimpleRequest::SimpleRequest; + +private: + friend SimpleRequest; + + void *evaluate(Evaluator &evaluator, const SourceFile *SF) const; + +public: + // Cached. + bool isCached() const { return true; } +}; + /// Parse the top-level items of a SourceFile. class ParseTopLevelDeclsRequest : public SimpleRequest< diff --git a/include/swift/AST/ParseTypeIDZone.def b/include/swift/AST/ParseTypeIDZone.def index 417c74531eaaf..9966033cade8d 100644 --- a/include/swift/AST/ParseTypeIDZone.def +++ b/include/swift/AST/ParseTypeIDZone.def @@ -28,3 +28,6 @@ SWIFT_REQUEST(Parse, ParseSourceFileRequest, SWIFT_REQUEST(Parse, ParseTopLevelDeclsRequest, ArrayRef(SourceFile *), Cached, NoLocationInfo) +SWIFT_REQUEST(Parse, ExportedSourceFileRequest, + void *(const SourceFile *), Cached, + NoLocationInfo) diff --git a/include/swift/AST/SourceFile.h b/include/swift/AST/SourceFile.h index d50b17f5cb014..0e9185d81befc 100644 --- a/include/swift/AST/SourceFile.h +++ b/include/swift/AST/SourceFile.h @@ -91,9 +91,11 @@ class SourceFile final : public FileUnit { /// files, as they get parsed multiple times. SuppressWarnings = 1 << 4, - /// Whether to disable the Swift Parser ASTGen - /// e.g. in dependency scanning, where an AST is not needed. - DisableSwiftParserASTGen = 1 << 5, + /// Ensure that the SwiftSyntax tree round trips correctly. + RoundTrip = 1 << 5, + + /// Validate the new SwiftSyntax parser diagnostics. + ValidateNewParserDiagnostics = 1 << 6, }; using ParsingOptions = OptionSet; @@ -262,6 +264,10 @@ class SourceFile final : public FileUnit { /// code for it. Note this method returns \c false in WMO. bool isPrimary() const { return IsPrimary; } + /// Retrieve the \c ExportedSourceFile instance produced by ASTGen, which + /// includes the SourceFileSyntax node corresponding to this source file. + void *getExportedSourceFile() const; + /// The list of local type declarations in the source file. llvm::SetVector LocalTypeDecls; @@ -334,10 +340,6 @@ class SourceFile final : public FileUnit { /// this source file. llvm::SmallVector, 0> VirtualFilePaths; - /// The \c ExportedSourceFile instance produced by ASTGen, which includes - /// the SourceFileSyntax node corresponding to this source file. - void *exportedSourceFile = nullptr; - /// Returns information about the file paths used for diagnostics and magic /// identifiers in this source file, including virtual filenames introduced by /// \c #sourceLocation(file:) declarations. diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index ee39904386951..6623179e0f718 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -3687,6 +3687,10 @@ SourceFile::getDefaultParsingOptions(const LangOptions &langOpts) { opts |= ParsingFlags::DisablePoundIfEvaluation; if (langOpts.CollectParsedToken) opts |= ParsingFlags::CollectParsedTokens; + if (langOpts.hasFeature(Feature::ParserRoundTrip)) + opts |= ParsingFlags::RoundTrip; + if (langOpts.hasFeature(Feature::ParserValidation)) + opts |= ParsingFlags::ValidateNewParserDiagnostics; return opts; } @@ -3762,6 +3766,11 @@ ArrayRef SourceFile::getHoistedDecls() const { return Hoisted; } +void *SourceFile::getExportedSourceFile() const { + auto &eval = getASTContext().evaluator; + return evaluateOrDefault(eval, ExportedSourceFileRequest{this}, nullptr); +} + void SourceFile::addDeclWithRuntimeDiscoverableAttrs(ValueDecl *decl) { assert(!decl->getRuntimeDiscoverableAttrs().empty()); DeclsWithRuntimeDiscoverableAttrs.insert(decl); diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp index c36a8ada13fbd..9061086eba8a2 100644 --- a/lib/Frontend/Frontend.cpp +++ b/lib/Frontend/Frontend.cpp @@ -1513,6 +1513,9 @@ void CompilerInstance::finishTypeChecking() { SourceFile::ParsingOptions CompilerInstance::getSourceFileParsingOptions(bool forPrimary) const { + using ActionType = FrontendOptions::ActionType; + using ParsingFlags = SourceFile::ParsingFlags; + const auto &frontendOpts = Invocation.getFrontendOptions(); const auto action = frontendOpts.RequestedAction; @@ -1521,34 +1524,37 @@ CompilerInstance::getSourceFileParsingOptions(bool forPrimary) const { // Generally in a parse-only invocation, we want to disable #if evaluation. // However, there are a couple of modes where we need to know which clauses // are active. - if (action != FrontendOptions::ActionType::EmitImportedModules && - action != FrontendOptions::ActionType::ScanDependencies) { - opts |= SourceFile::ParsingFlags::DisablePoundIfEvaluation; + if (action != ActionType::EmitImportedModules && + action != ActionType::ScanDependencies) { + opts |= ParsingFlags::DisablePoundIfEvaluation; } // If we need to dump the parse tree, disable delayed bodies as we want to // show everything. - if (action == FrontendOptions::ActionType::DumpParse) - opts |= SourceFile::ParsingFlags::DisableDelayedBodies; + if (action == ActionType::DumpParse) + opts |= ParsingFlags::DisableDelayedBodies; } - auto typeOpts = getASTContext().TypeCheckerOpts; - if (forPrimary || isWholeModuleCompilation()) { + const auto &typeOpts = getASTContext().TypeCheckerOpts; + const auto isEffectivelyPrimary = forPrimary || isWholeModuleCompilation(); + if (isEffectivelyPrimary) { // Disable delayed body parsing for primaries and in WMO, unless // forcefully skipping function bodies if (typeOpts.SkipFunctionBodies == FunctionBodySkipping::None) - opts |= SourceFile::ParsingFlags::DisableDelayedBodies; + opts |= ParsingFlags::DisableDelayedBodies; } else { // Suppress parse warnings for non-primaries, as they'll get parsed multiple // times. - opts |= SourceFile::ParsingFlags::SuppressWarnings; + opts |= ParsingFlags::SuppressWarnings; } - // Dependency scanning does not require an AST, so disable Swift Parser - // ASTGen parsing completely. - if (frontendOpts.RequestedAction == - FrontendOptions::ActionType::ScanDependencies) - opts |= SourceFile::ParsingFlags::DisableSwiftParserASTGen; + // Turn off round-trip checking for secondary files, and for dependency + // scanning and IDE inspection. + if (!isEffectivelyPrimary || SourceMgr.hasIDEInspectionTargetBuffer() || + frontendOpts.RequestedAction == ActionType::ScanDependencies) { + opts -= ParsingFlags::RoundTrip; + opts -= ParsingFlags::ValidateNewParserDiagnostics; + } // Enable interface hash computation for primaries or emit-module-separately, // but not in WMO, as it's only currently needed for incremental mode. @@ -1556,7 +1562,7 @@ CompilerInstance::getSourceFileParsingOptions(bool forPrimary) const { typeOpts.SkipFunctionBodies == FunctionBodySkipping::NonInlinableWithoutTypes || frontendOpts.ReuseFrontendForMultipleCompilations) { - opts |= SourceFile::ParsingFlags::EnableInterfaceHash; + opts |= ParsingFlags::EnableInterfaceHash; } return opts; } diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 9936ff0833c89..07e7942d3cccd 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -209,9 +209,7 @@ extern "C" void swift_ASTGen_buildTopLevelASTNodes(void *sourceFile, void Parser::parseTopLevelItems(SmallVectorImpl &items) { #if SWIFT_SWIFT_PARSER Optional existingParsingTransaction; - if (!SF.getParsingOptions() - .contains(SourceFile::ParsingFlags::DisableSwiftParserASTGen)) - parseSourceFileViaASTGen(items, existingParsingTransaction); + parseSourceFileViaASTGen(items, existingParsingTransaction); #endif // Prime the lexer. @@ -261,92 +259,135 @@ void Parser::parseTopLevelItems(SmallVectorImpl &items) { } #if SWIFT_SWIFT_PARSER - if (!SF.getParsingOptions().contains( - SourceFile::ParsingFlags::DisableSwiftParserASTGen)) { - if (existingParsingTransaction) - existingParsingTransaction->abort(); - - // Perform round-trip and/or validation checking. - if ((Context.LangOpts.hasFeature(Feature::ParserRoundTrip) || - Context.LangOpts.hasFeature(Feature::ParserValidation)) && - SF.exportedSourceFile && - !SourceMgr.hasIDEInspectionTargetBuffer()) { - if (Context.LangOpts.hasFeature(Feature::ParserRoundTrip) && - swift_ASTGen_roundTripCheck(SF.exportedSourceFile)) { - SourceLoc loc; - if (auto bufferID = SF.getBufferID()) { - loc = Context.SourceMgr.getLocForBufferStart(*bufferID); - } - diagnose(loc, diag::parser_round_trip_error); - } else if (Context.LangOpts.hasFeature(Feature::ParserValidation) && - !Context.Diags.hadAnyError() && - swift_ASTGen_emitParserDiagnostics( - &Context.Diags, SF.exportedSourceFile, - /*emitOnlyErrors=*/true, - /*downgradePlaceholderErrorsToWarnings=*/ - Context.LangOpts.Playground || - Context.LangOpts.WarnOnEditorPlaceholder)) { - // We might have emitted warnings in the C++ parser but no errors, in - // which case we still have `hadAnyError() == false`. To avoid emitting - // the same warnings from SwiftParser, only emit errors from SwiftParser - SourceLoc loc; - if (auto bufferID = SF.getBufferID()) { + if (existingParsingTransaction) + existingParsingTransaction->abort(); + + using ParsingFlags = SourceFile::ParsingFlags; + const auto parsingOpts = SF.getParsingOptions(); + + // If we don't need to validate anything, we're done. + if (!parsingOpts.contains(ParsingFlags::RoundTrip) && + !parsingOpts.contains(ParsingFlags::ValidateNewParserDiagnostics)) { + return; + } + + auto *exportedSourceFile = SF.getExportedSourceFile(); + if (!exportedSourceFile) + return; + + // Perform round-trip and/or validation checking. + if (parsingOpts.contains(ParsingFlags::RoundTrip) && + swift_ASTGen_roundTripCheck(exportedSourceFile)) { + SourceLoc loc; + if (auto bufferID = SF.getBufferID()) { + loc = Context.SourceMgr.getLocForBufferStart(*bufferID); + } + diagnose(loc, diag::parser_round_trip_error); + return; + } + if (parsingOpts.contains(ParsingFlags::ValidateNewParserDiagnostics) && + !Context.Diags.hadAnyError()) { + auto hadSyntaxError = swift_ASTGen_emitParserDiagnostics( + &Context.Diags, exportedSourceFile, + /*emitOnlyErrors=*/true, + /*downgradePlaceholderErrorsToWarnings=*/ + Context.LangOpts.Playground || + Context.LangOpts.WarnOnEditorPlaceholder); + if (hadSyntaxError) { + // We might have emitted warnings in the C++ parser but no errors, in + // which case we still have `hadAnyError() == false`. To avoid + // emitting the same warnings from SwiftParser, only emit errors from + // SwiftParser + SourceLoc loc; + if (auto bufferID = SF.getBufferID()) { loc = Context.SourceMgr.getLocForBufferStart(*bufferID); - } - diagnose(loc, diag::parser_new_parser_errors); } + diagnose(loc, diag::parser_new_parser_errors); } } #endif } +void *ExportedSourceFileRequest::evaluate(Evaluator &evaluator, + const SourceFile *SF) const { +#if SWIFT_SWIFT_PARSER + // The SwiftSyntax parser doesn't (yet?) handle SIL. + if (SF->Kind == SourceFileKind::SIL) + return nullptr; + + auto &ctx = SF->getASTContext(); + auto &SM = ctx.SourceMgr; + + auto bufferID = SF->getBufferID(); + if (!bufferID) + return nullptr; + + StringRef contents = SM.extractText(SM.getRangeForBuffer(*bufferID)); + + // Parse the source file. + auto exportedSourceFile = swift_ASTGen_parseSourceFile( + contents.begin(), contents.size(), + SF->getParentModule()->getName().str().str().c_str(), + SF->getFilename().str().c_str()); + + ctx.addCleanup([exportedSourceFile] { + swift_ASTGen_destroySourceFile(exportedSourceFile); + }); + return exportedSourceFile; +#else + return nullptr; +#endif +} + void Parser::parseSourceFileViaASTGen(SmallVectorImpl &items, Optional &transaction, bool suppressDiagnostics) { #if SWIFT_SWIFT_PARSER - Optional existingParsingTransaction; - if (SF.Kind != SourceFileKind::SIL) { - StringRef contents = - SourceMgr.extractText(SourceMgr.getRangeForBuffer(L->getBufferID())); - - // Parse the source file. - auto exportedSourceFile = swift_ASTGen_parseSourceFile( - contents.begin(), contents.size(), - SF.getParentModule()->getName().str().str().c_str(), - SF.getFilename().str().c_str()); - SF.exportedSourceFile = exportedSourceFile; - Context.addCleanup([exportedSourceFile] { - swift_ASTGen_destroySourceFile(exportedSourceFile); - }); + using ParsingFlags = SourceFile::ParsingFlags; + const auto parsingOpts = SF.getParsingOptions(); + const auto &langOpts = Context.LangOpts; + + // We only need to do parsing if we either have ASTGen enabled, or want the + // new parser diagnostics. + auto needToParse = [&]() { + if (langOpts.hasFeature(Feature::ParserASTGen)) + return true; + if (!suppressDiagnostics && + langOpts.hasFeature(Feature::ParserDiagnostics)) { + return true; + } + return false; + }(); + if (!needToParse) + return; - // If we're supposed to emit diagnostics from the parser, do so now. - if ((Context.LangOpts.hasFeature(Feature::ParserDiagnostics) || - Context.LangOpts.hasFeature(Feature::ParserASTGen)) && - !suppressDiagnostics && - swift_ASTGen_emitParserDiagnostics( - &Context.Diags, SF.exportedSourceFile, /*emitOnlyErrors=*/false, - /*downgradePlaceholderErrorsToWarnings=*/ - Context.LangOpts.Playground || - Context.LangOpts.WarnOnEditorPlaceholder) && - Context.Diags.hadAnyError() && - !Context.LangOpts.hasFeature(Feature::ParserASTGen)) { + auto *exportedSourceFile = SF.getExportedSourceFile(); + if (!exportedSourceFile) + return; + + // If we're supposed to emit diagnostics from the parser, do so now. + if (!suppressDiagnostics) { + auto hadSyntaxError = swift_ASTGen_emitParserDiagnostics( + &Context.Diags, exportedSourceFile, /*emitOnlyErrors=*/false, + /*downgradePlaceholderErrorsToWarnings=*/langOpts.Playground || + langOpts.WarnOnEditorPlaceholder); + if (hadSyntaxError && Context.Diags.hadAnyError() && + !langOpts.hasFeature(Feature::ParserASTGen)) { // Errors were emitted, and we're still using the C++ parser, so // disable diagnostics from the C++ parser. transaction.emplace(Context.Diags); } + } - // If we want to do ASTGen, do so now. - if (Context.LangOpts.hasFeature(Feature::ParserASTGen)) { - swift_ASTGen_buildTopLevelASTNodes( - exportedSourceFile, CurDeclContext, &Context, &items, appendToVector); - - // Spin the C++ parser to the end; we won't be using it. - while (!Tok.is(tok::eof)) { - consumeToken(); - } + // If we want to do ASTGen, do so now. + if (langOpts.hasFeature(Feature::ParserASTGen)) { + swift_ASTGen_buildTopLevelASTNodes(exportedSourceFile, CurDeclContext, + &Context, &items, appendToVector); - return; + // Spin the C++ parser to the end; we won't be using it. + while (!Tok.is(tok::eof)) { + consumeToken(); } } #endif diff --git a/lib/Sema/TypeCheckMacros.cpp b/lib/Sema/TypeCheckMacros.cpp index 09d3f210fea37..c607afc182499 100644 --- a/lib/Sema/TypeCheckMacros.cpp +++ b/lib/Sema/TypeCheckMacros.cpp @@ -185,9 +185,9 @@ MacroDefinition MacroDefinitionRequest::evaluate( ptrdiff_t *replacements; ptrdiff_t numReplacements; auto checkResult = swift_ASTGen_checkMacroDefinition( - &ctx.Diags, sourceFile->exportedSourceFile, macro->getLoc().getOpaquePointerValue(), - &externalMacroNamePtr, &externalMacroNameLength, - &replacements, &numReplacements); + &ctx.Diags, sourceFile->getExportedSourceFile(), + macro->getLoc().getOpaquePointerValue(), &externalMacroNamePtr, + &externalMacroNameLength, &replacements, &numReplacements); // Clean up after the call. SWIFT_DEFER { @@ -781,7 +781,7 @@ swift::expandMacroExpr(MacroExpansionExpr *mee) { PrettyStackTraceExpr debugStack(ctx, "expanding macro", mee); // Builtin macros are handled via ASTGen. - auto astGenSourceFile = sourceFile->exportedSourceFile; + auto *astGenSourceFile = sourceFile->getExportedSourceFile(); if (!astGenSourceFile) return None; @@ -979,7 +979,7 @@ swift::expandFreestandingMacro(MacroExpansionDecl *med) { PrettyStackTraceDecl debugStack("expanding declaration macro", med); // Builtin macros are handled via ASTGen. - auto astGenSourceFile = sourceFile->exportedSourceFile; + auto *astGenSourceFile = sourceFile->getExportedSourceFile(); if (!astGenSourceFile) return None; @@ -1175,18 +1175,19 @@ evaluateAttachedMacro(MacroDecl *macro, Decl *attachedTo, CustomAttr *attr, #if SWIFT_SWIFT_PARSER PrettyStackTraceDecl debugStack("expanding attached macro", attachedTo); - auto astGenAttrSourceFile = attrSourceFile->exportedSourceFile; + auto *astGenAttrSourceFile = attrSourceFile->getExportedSourceFile(); if (!astGenAttrSourceFile) return nullptr; - auto astGenDeclSourceFile = declSourceFile->exportedSourceFile; + auto *astGenDeclSourceFile = declSourceFile->getExportedSourceFile(); if (!astGenDeclSourceFile) return nullptr; void *astGenParentDeclSourceFile = nullptr; const void *parentDeclLoc = nullptr; if (passParentContext) { - astGenParentDeclSourceFile = parentDeclSourceFile->exportedSourceFile; + astGenParentDeclSourceFile = + parentDeclSourceFile->getExportedSourceFile(); if (!astGenParentDeclSourceFile) return nullptr; diff --git a/lib/Serialization/ModuleDependencyScanner.cpp b/lib/Serialization/ModuleDependencyScanner.cpp index a74950f424083..2d3de471e0247 100644 --- a/lib/Serialization/ModuleDependencyScanner.cpp +++ b/lib/Serialization/ModuleDependencyScanner.cpp @@ -167,10 +167,7 @@ ErrorOr ModuleDependencyScanner::scanInterfaceFile( unsigned bufferID = Ctx.SourceMgr.addNewSourceBuffer(std::move(interfaceBuf.get())); auto moduleDecl = ModuleDecl::create(realModuleName, Ctx); - // Dependency scanning does not require an AST, so disable Swift Parser - // ASTGen parsing completely. SourceFile::ParsingOptions parsingOpts; - parsingOpts |= SourceFile::ParsingFlags::DisableSwiftParserASTGen; auto sourceFile = new (Ctx) SourceFile( *moduleDecl, SourceFileKind::Interface, bufferID, parsingOpts); moduleDecl->addAuxiliaryFile(*sourceFile); diff --git a/test/Macros/lazy_parsing.swift b/test/Macros/lazy_parsing.swift new file mode 100644 index 0000000000000..47de85a59a6ba --- /dev/null +++ b/test/Macros/lazy_parsing.swift @@ -0,0 +1,44 @@ +// REQUIRES: swift_swift_parser + +// This test ensures that we only attempt to parse the syntax tree of a +// secondary file if the primary file needs it. + +// RUN: %empty-directory(%t) +// RUN: %empty-directory(%t/stats-no-lookup) +// RUN: %empty-directory(%t/stats-lookup) +// RUN: split-file %s %t + +// RUN: %host-build-swift -swift-version 5 -emit-library -o %t/%target-library-name(MacroDefinition) -parse-as-library -module-name=MacroDefinition %S/Inputs/syntax_macro_definitions.swift -g -no-toolchain-stdlib-rpath + +// RUN: %target-swift-frontend -typecheck -swift-version 5 -load-plugin-library %t/%target-library-name(MacroDefinition) -stats-output-dir %t/stats-no-lookup -primary-file %t/b.swift %t/a.swift +// RUN: %target-swift-frontend -typecheck -swift-version 5 -load-plugin-library %t/%target-library-name(MacroDefinition) -stats-output-dir %t/stats-lookup -primary-file %t/c.swift %t/a.swift + +// We use '<=' here instead of '==' to take account of the fact that in debug +// builds we'll be doing round-trip checking, which will parse the syntax tree +// for the primary. +// RUN: %{python} %utils/process-stats-dir.py --evaluate 'ExportedSourceFileRequest <= 1' %t/stats-no-lookup +// RUN: %{python} %utils/process-stats-dir.py --evaluate 'ExportedSourceFileRequest <= 2' %t/stats-lookup +// RUN: %{python} %utils/process-stats-dir.py --evaluate-delta 'ExportedSourceFileRequest == 1' %t/stats-no-lookup %t/stats-lookup + +//--- a.swift + +@attached( + member, + names: named(init), named(Storage), named(storage), named(getStorage()), named(method), named(init(other:)) +) +macro addMembers() = #externalMacro(module: "MacroDefinition", type: "AddMembers") + +@addMembers +struct S {} + +//--- b.swift + +// No lookup into S, so we don't need to evaluate any macros. +func foo(_ x: S) {} + +//--- c.swift + +// A lookup into S, we need to parse the syntax tree. +func foo(_ x: S) { + _ = x.getStorage() +} diff --git a/test/ScanDependencies/ensure_no_astgen.swift b/test/ScanDependencies/ensure_no_astgen.swift new file mode 100644 index 0000000000000..b449fb3bc37b2 --- /dev/null +++ b/test/ScanDependencies/ensure_no_astgen.swift @@ -0,0 +1,41 @@ +// REQUIRES: swift_swift_parser + +// RUN: %empty-directory(%t) +// RUN: %empty-directory(%t/stats) +// RUN: %empty-directory(%t/deps) +// RUN: split-file %s %t +// RUN: mv %t/Foo.swiftinterface %t/deps/ + +// This test ensures we don't attempt to do syntax tree parsing for dependency +// scanning. + +// RUN: %target-swift-frontend -scan-dependencies -o %t/deps.json %t/a.swift %t/b.swift -I %t/deps -stats-output-dir %t/stats +// RUN: %{python} %utils/process-stats-dir.py --evaluate 'ExportedSourceFileRequest == 0' %t/stats + +//--- Foo.swiftinterface +// swift-interface-format-version: 1.0 +// swift-module-flags: -module-name Foo + +@attached( + member, + names: named(init), named(Storage), named(storage), named(getStorage()), named(method), named(init(other:)) +) +public macro addMembers() = #externalMacro(module: "MacroDefinition", type: "AddMembers") + +@addMembers +public struct S1 {} + +//--- a.swift + +import Foo + +@addMembers +struct S2 {} + +//--- b.swift + +@freestanding(expression) macro myError(_ message: String) = #externalMacro(module: "MacroDefinition", type: "ErrorMacro") + +func foo(_ x: S) { + _ = x.getStorage() +} From e120a82e0c6b1b167178e5bfa41d7de87e02cb5d Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Mon, 22 May 2023 09:27:58 -0600 Subject: [PATCH 7/8] [SymbolGraphGen] refactor protocol conformance inheritance checking (#66012) rdar://109418762 --- lib/SymbolGraphGen/SymbolGraphASTWalker.cpp | 77 ++++--------------- .../Symbols/ProtocolClassInheritance.swift | 42 ++++++++++ 2 files changed, 58 insertions(+), 61 deletions(-) create mode 100644 test/SymbolGraph/Symbols/ProtocolClassInheritance.swift diff --git a/lib/SymbolGraphGen/SymbolGraphASTWalker.cpp b/lib/SymbolGraphGen/SymbolGraphASTWalker.cpp index a48d7ddf715d7..98b2fad193240 100644 --- a/lib/SymbolGraphGen/SymbolGraphASTWalker.cpp +++ b/lib/SymbolGraphGen/SymbolGraphASTWalker.cpp @@ -13,6 +13,7 @@ #include "llvm/ADT/StringSwitch.h" #include "swift/AST/Decl.h" #include "swift/AST/Module.h" +#include "swift/AST/ProtocolConformance.h" #include "swift/Serialization/SerializedModuleLoader.h" #include "swift/SymbolGraphGen/SymbolGraphGen.h" @@ -181,79 +182,33 @@ bool SymbolGraphASTWalker::walkToDeclPre(Decl *D, CharSourceRange Range) { // We want to add conformsTo relationships for all protocols implicitly // implied by those explicitly stated on the extension. // - // Thus, we have to expand two syntactic constructs: - // * `protocol A: B, C { ... }` declarations, where those that still have - // to be expanded are stored in `UnexpandedProtocols` - // that still have to be expanded - // * `typealias A = B & C` declarations, which are directly expanded to - // unexpanded protocols in `HandleProtocolOrComposition` - // - // The expansion adds the base protocol to `Protocols` and calls - // `HandleProtocolOrComposition` for the implied protocols. This process - // continues until there is nothing left to expand (`UnexpandedProtocols` - // is empty), because `HandleProtocolOrComposition` didn't add any new - // unexpanded protocols. At that point, all direct and indirect - // conformances are stored in `Protocols`. - - SmallVector Protocols; + // We start by collecting the conformances declared on the extension with + // `getLocalConformances`. From there, we inspect each protocol for any + // other protocols it inherits (whether stated explicitly or via a + // composed protocol type alias) with `getInheritedProtocols`. Each new + // protocol is added to `UnexpandedProtocols` until there are no new + // protocols to add. At that point, all direct and indirect conformances + // are stored in `Protocols`. + + SmallPtrSet Protocols; SmallVector UnexpandedProtocols; - // Unwrap `UnexpandedCompositions` and add all unexpanded protocols to the - // `UnexpandedProtocols` list for expansion. - auto HandleProtocolOrComposition = [&](Type Ty) { - if (const auto *Proto = - dyn_cast_or_null(Ty->getAnyNominal())) { - UnexpandedProtocols.push_back(Proto); - return; - } - - SmallVector UnexpandedCompositions; - - if (const auto *Comp = Ty->getAs()) { - UnexpandedCompositions.push_back(Comp); - } else { - llvm_unreachable("Expected ProtocolDecl or ProtocolCompositionType"); - } - - while (!UnexpandedCompositions.empty()) { - const auto *Comp = UnexpandedCompositions.pop_back_val(); - for (const auto &Member : Comp->getMembers()) { - if (const auto *Proto = - dyn_cast_or_null(Member->getAnyNominal())) { - Protocols.push_back(Proto); - UnexpandedProtocols.push_back(Proto); - } else if (const auto *Comp = - Member->getAs()) { - UnexpandedCompositions.push_back(Comp); - } else { - abort(); - } - } - } - }; - // Start the process with the conformances stated // explicitly on the extension. - for (const auto &InheritedLoc : Extension->getInherited()) { - auto InheritedTy = InheritedLoc.getType(); - if (!InheritedTy) { - continue; - } - HandleProtocolOrComposition(InheritedTy); + for (const auto *Conformance : Extension->getLocalConformances()) { + UnexpandedProtocols.push_back(Conformance->getProtocol()); } // "Recursively" expand the unexpanded list and populate // the expanded `Protocols` list (in an iterative manner). while (!UnexpandedProtocols.empty()) { const auto *Proto = UnexpandedProtocols.pop_back_val(); - for (const auto &InheritedEntry : Proto->getInherited()) { - auto InheritedTy = InheritedEntry.getType(); - if (!InheritedTy) { - continue; + if (!Protocols.contains(Proto)) { + for (const auto *InheritedProtocol : Proto->getInheritedProtocols()) { + UnexpandedProtocols.push_back(InheritedProtocol); } - HandleProtocolOrComposition(InheritedTy); + Protocols.insert(Proto); } - Protocols.push_back(Proto); } // Record the expanded list of protocols. diff --git a/test/SymbolGraph/Symbols/ProtocolClassInheritance.swift b/test/SymbolGraph/Symbols/ProtocolClassInheritance.swift new file mode 100644 index 0000000000000..2fbd0398b83da --- /dev/null +++ b/test/SymbolGraph/Symbols/ProtocolClassInheritance.swift @@ -0,0 +1,42 @@ +// RUN: %empty-directory(%t) +// RUN: %target-build-swift %s -module-name ProtocolClassInheritance -emit-module -emit-module-path %t/ +// RUN: %target-swift-symbolgraph-extract -module-name ProtocolClassInheritance -I %t -output-dir %t +// RUN: %FileCheck %s --input-file %t/ProtocolClassInheritance.symbols.json + +// When a protocol that declares a class inheritance requirement is added by an extension, make sure +// that SymbolGraphGen does not crash (rdar://109418762) + +public class ClassOne {} +public protocol ProtoOne: ClassOne {} + +public class ClassTwo: ClassOne {} +extension ClassTwo: ProtoOne {} + +// Same for a generic class inheritance requirement + +public class ClassThree {} +public protocol ProtoTwo: ClassThree {} + +public class ClassFour: ClassThree {} +extension ClassFour: ProtoTwo {} + +// Same for a protocol with a primary associated type + +public protocol ProtoThree { + associatedtype T +} +public protocol ProtoFour: ProtoThree {} + +public class ClassFive: ProtoThree { + public typealias T = Int +} +extension ClassFive: ProtoFour {} + +// ClassTwo conforms to ProtoOne +// CHECK-DAG: {"kind":"conformsTo","source":"s:24ProtocolClassInheritance0B3TwoC","target":"s:24ProtocolClassInheritance8ProtoOneP"} + +// ClassFour conforms to ProtoTwo +// CHECK-DAG: {"kind":"conformsTo","source":"s:24ProtocolClassInheritance0B4FourC","target":"s:24ProtocolClassInheritance8ProtoTwoP"} + +// ClassFive conforms to ProtoFour +// CHECK-DAG: {"kind":"conformsTo","source":"s:24ProtocolClassInheritance0B4FiveC","target":"s:24ProtocolClassInheritance9ProtoFourP"} From cae740d902442aae9ba01d62a8ee205eef978eb3 Mon Sep 17 00:00:00 2001 From: Dario Rexin Date: Mon, 22 May 2023 12:55:09 -0700 Subject: [PATCH 8/8] [Runtime] Use MetadataAllocator to allocate runtime instantiated layout strings (#66055) rdar://109660582 This will create more visibility into layout string allocations when using swift-inspect. --- stdlib/public/runtime/Metadata.cpp | 4 ++-- stdlib/public/runtime/MetadataAllocatorTags.def | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/stdlib/public/runtime/Metadata.cpp b/stdlib/public/runtime/Metadata.cpp index 5da6c753618cd..6a9c51985a0d2 100644 --- a/stdlib/public/runtime/Metadata.cpp +++ b/stdlib/public/runtime/Metadata.cpp @@ -2729,8 +2729,8 @@ void swift::swift_initStructMetadataWithLayoutString( const size_t fixedLayoutStringSize = layoutStringHeaderSize + sizeof(uint64_t) * 2; - uint8_t *layoutStr = (uint8_t *)malloc(fixedLayoutStringSize + - refCountBytes); + uint8_t *layoutStr = (uint8_t *)MetadataAllocator(LayoutStringTag) + .Allocate(fixedLayoutStringSize + refCountBytes, alignof(uint8_t)); *((size_t*)(layoutStr + sizeof(uint64_t))) = refCountBytes; diff --git a/stdlib/public/runtime/MetadataAllocatorTags.def b/stdlib/public/runtime/MetadataAllocatorTags.def index ea314dd375922..0184d1ce7fb6c 100644 --- a/stdlib/public/runtime/MetadataAllocatorTags.def +++ b/stdlib/public/runtime/MetadataAllocatorTags.def @@ -48,5 +48,6 @@ TAG(ExtendedExistentialTypes, 22) TAG(ExtendedExistentialTypeShapes, 23) TAG(MetadataPack, 24) TAG(WitnessTablePack, 25) +TAG(LayoutString, 26) #undef TAG