From eac92e3df0f14e13453029d76fd804740e8882ca Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Mon, 11 May 2020 15:53:24 -0700 Subject: [PATCH 1/4] [NFC] Const-Qualify LangOptions Accesses Through ASTContext --- include/swift/AST/ASTContext.h | 2 +- include/swift/AST/DiagnosticsFrontend.def | 5 +++++ include/swift/AST/PlatformKind.h | 4 ++-- lib/AST/PlatformKind.cpp | 4 ++-- lib/ClangImporter/ClangImporter.cpp | 2 +- lib/ClangImporter/ImporterImpl.h | 2 +- lib/Frontend/CompilerInvocation.cpp | 5 +++++ lib/FrontendTool/FrontendTool.cpp | 7 +------ lib/IDE/CompletionInstance.cpp | 4 ++-- 9 files changed, 20 insertions(+), 15 deletions(-) diff --git a/include/swift/AST/ASTContext.h b/include/swift/AST/ASTContext.h index bdd257d9c71cd..901a46aac3732 100644 --- a/include/swift/AST/ASTContext.h +++ b/include/swift/AST/ASTContext.h @@ -238,7 +238,7 @@ class ASTContext final { UnifiedStatsReporter *Stats = nullptr; /// The language options used for translation. - LangOptions &LangOpts; + const LangOptions &LangOpts; /// The type checker options. TypeCheckerOptions &TypeCheckerOpts; diff --git a/include/swift/AST/DiagnosticsFrontend.def b/include/swift/AST/DiagnosticsFrontend.def index 6ad68ae2b6c65..4f72e3e17a7bb 100644 --- a/include/swift/AST/DiagnosticsFrontend.def +++ b/include/swift/AST/DiagnosticsFrontend.def @@ -379,5 +379,10 @@ ERROR(expectation_missing_opening_braces,none, ERROR(expectation_missing_closing_braces,none, "didn't find '}}' to match '{{' in expectation", ()) +WARNING(module_incompatible_with_skip_function_bodies,none, + "module '%0' cannot be built with " + "-experimental-skip-non-inlinable-function-bodies; this option has " + "been automatically disabled", (StringRef)) + #define UNDEFINE_DIAGNOSTIC_MACROS #include "DefineDiagnosticMacros.h" diff --git a/include/swift/AST/PlatformKind.h b/include/swift/AST/PlatformKind.h index 521167f6ce2d7..f03d5adc8f7bf 100644 --- a/include/swift/AST/PlatformKind.h +++ b/include/swift/AST/PlatformKind.h @@ -54,11 +54,11 @@ StringRef prettyPlatformString(PlatformKind platform); /// If ForTargetVariant is true then for zippered builds the target-variant /// triple will be used rather than the target to determine whether the /// platform is active. -bool isPlatformActive(PlatformKind Platform, LangOptions &LangOpts, +bool isPlatformActive(PlatformKind Platform, const LangOptions &LangOpts, bool ForTargetVariant = false); /// Returns the target platform for the given language options. -PlatformKind targetPlatform(LangOptions &LangOpts); +PlatformKind targetPlatform(const LangOptions &LangOpts); /// Returns true when availability attributes from the "parent" platform /// should also apply to the "child" platform for declarations without diff --git a/lib/AST/PlatformKind.cpp b/lib/AST/PlatformKind.cpp index 1119601d458c1..707ba4473b5fe 100644 --- a/lib/AST/PlatformKind.cpp +++ b/lib/AST/PlatformKind.cpp @@ -92,7 +92,7 @@ static bool isPlatformActiveForTarget(PlatformKind Platform, llvm_unreachable("bad PlatformKind"); } -bool swift::isPlatformActive(PlatformKind Platform, LangOptions &LangOpts, +bool swift::isPlatformActive(PlatformKind Platform, const LangOptions &LangOpts, bool ForTargetVariant) { llvm::Triple TT = LangOpts.Target; @@ -105,7 +105,7 @@ bool swift::isPlatformActive(PlatformKind Platform, LangOptions &LangOpts, LangOpts.EnableAppExtensionRestrictions); } -PlatformKind swift::targetPlatform(LangOptions &LangOpts) { +PlatformKind swift::targetPlatform(const LangOptions &LangOpts) { if (LangOpts.Target.isMacOSX()) { return (LangOpts.EnableAppExtensionRestrictions ? PlatformKind::OSXApplicationExtension diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 4e0673d910c30..5d5fdb30326e4 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -1877,7 +1877,7 @@ ClangImporter::getWrapperForModule(const clang::Module *mod, return clangUnit->getParentModule(); } -PlatformAvailability::PlatformAvailability(LangOptions &langOpts) +PlatformAvailability::PlatformAvailability(const LangOptions &langOpts) : platformKind(targetPlatform(langOpts)) { switch (platformKind) { case PlatformKind::iOS: diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h index 7a8d40f4bc3f1..72de3a47bcde7 100644 --- a/lib/ClangImporter/ImporterImpl.h +++ b/lib/ClangImporter/ImporterImpl.h @@ -255,7 +255,7 @@ struct PlatformAvailability { /// API is now unavailable. std::string deprecatedAsUnavailableMessage; - PlatformAvailability(LangOptions &opts); + PlatformAvailability(const LangOptions &opts); private: PlatformAvailability(const PlatformAvailability&) = delete; diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index 4fe5d3a4abdad..863dae629163d 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -629,6 +629,11 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args, } } + if (FrontendOpts.RequestedAction == FrontendOptions::ActionType::EmitSyntax) { + Opts.BuildSyntaxTree = true; + Opts.VerifySyntaxTree = true; + } + return HadError || UnsupportedOS || UnsupportedArch; } diff --git a/lib/FrontendTool/FrontendTool.cpp b/lib/FrontendTool/FrontendTool.cpp index 316df9056c85b..ed17a1f6378b3 100644 --- a/lib/FrontendTool/FrontendTool.cpp +++ b/lib/FrontendTool/FrontendTool.cpp @@ -1200,12 +1200,7 @@ static bool performCompile(CompilerInstance &Instance, FrontendObserver *observer) { const auto &Invocation = Instance.getInvocation(); const auto &opts = Invocation.getFrontendOptions(); - FrontendOptions::ActionType Action = opts.RequestedAction; - - if (Action == FrontendOptions::ActionType::EmitSyntax) { - Instance.getASTContext().LangOpts.BuildSyntaxTree = true; - Instance.getASTContext().LangOpts.VerifySyntaxTree = true; - } + const FrontendOptions::ActionType Action = opts.RequestedAction; // We've been asked to precompile a bridging header or module; we want to // avoid touching any other inputs and just parse, emit and exit. diff --git a/lib/IDE/CompletionInstance.cpp b/lib/IDE/CompletionInstance.cpp index 3dcbf77883faf..27da23c6c82cd 100644 --- a/lib/IDE/CompletionInstance.cpp +++ b/lib/IDE/CompletionInstance.cpp @@ -317,6 +317,8 @@ bool CompletionInstance::performCachedOperationIfPossible( LangOptions langOpts = CI.getASTContext().LangOpts; langOpts.DisableParserLookup = true; + // Ensure all non-function-body tokens are hashed into the interface hash + langOpts.EnableTypeFingerprints = false; TypeCheckerOptions typeckOpts = CI.getASTContext().TypeCheckerOpts; SearchPathOptions searchPathOpts = CI.getASTContext().SearchPathOpts; DiagnosticEngine tmpDiags(tmpSM); @@ -331,8 +333,6 @@ bool CompletionInstance::performCachedOperationIfPossible( SourceFile(*tmpM, oldSF->Kind, tmpBufferID, /*KeepParsedTokens=*/false, /*BuildSyntaxTree=*/false, oldSF->getParsingOptions()); tmpSF->enableInterfaceHash(); - // Ensure all non-function-body tokens are hashed into the interface hash - tmpCtx->LangOpts.EnableTypeFingerprints = false; // FIXME: Since we don't setup module loaders on the temporary AST context, // 'canImport()' conditional compilation directive always fails. That causes From 52ddcea0d2cb93aaabaa4f38f4fd629fa65af3f0 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Mon, 11 May 2020 15:54:35 -0700 Subject: [PATCH 2/4] Diagnose Building SwiftONoneSupport with -experimental-skip-function-bodies This allows us to push the validation and, importantly, the mutation of this flag into the compiler invocation where it belongs. --- lib/Frontend/CompilerInvocation.cpp | 23 +++++++++++++++---- lib/Sema/TypeChecker.cpp | 7 ------ .../skip-non-inlinable-function-bodies.swift | 5 +++- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index 863dae629163d..d26dd923dfbbe 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -682,6 +682,17 @@ static bool ParseTypeCheckerArgs(TypeCheckerOptions &Opts, ArgList &Args, // body skipping. Opts.SkipNonInlinableFunctionBodies |= Args.hasArg(OPT_tbd_is_installapi); + if (Opts.SkipNonInlinableFunctionBodies && + FrontendOpts.ModuleName == SWIFT_ONONE_SUPPORT) { + // Disable this optimization if we're compiling SwiftOnoneSupport, because + // we _definitely_ need to look inside every declaration to figure out + // what gets prespecialized. + Opts.SkipNonInlinableFunctionBodies = false; + Diags.diagnose(SourceLoc(), + diag::module_incompatible_with_skip_function_bodies, + SWIFT_ONONE_SUPPORT); + } + Opts.DisableConstraintSolverPerformanceHacks |= Args.hasArg(OPT_disable_constraint_solver_performance_hacks); @@ -908,12 +919,14 @@ void parseExclusivityEnforcementOptions(const llvm::opt::Arg *A, static bool ParseSILArgs(SILOptions &Opts, ArgList &Args, IRGenOptions &IRGenOpts, - FrontendOptions &FEOpts, + const FrontendOptions &FEOpts, + const TypeCheckerOptions &TCOpts, DiagnosticEngine &Diags, const llvm::Triple &Triple, ClangImporterOptions &ClangOpts) { using namespace options; + if (const Arg *A = Args.getLastArg(OPT_sil_inline_threshold)) { if (StringRef(A->getValue()).getAsInteger(10, Opts.InlineThreshold)) { Diags.diagnose(SourceLoc(), diag::error_invalid_arg_value, @@ -956,8 +969,9 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args, if (Args.hasArg(OPT_sil_merge_partial_modules)) Opts.MergePartialModules = true; - if (Args.hasArg(OPT_experimental_skip_non_inlinable_function_bodies)) - Opts.SkipNonInlinableFunctionBodies = true; + // Propagate the typechecker's understanding of + // -experimental-skip-non-inlinable-function-bodies to SIL. + Opts.SkipNonInlinableFunctionBodies = TCOpts.SkipNonInlinableFunctionBodies; // Parse the optimization level. // Default to Onone settings if no option is passed. @@ -1629,7 +1643,8 @@ bool CompilerInvocation::parseArgs( return true; } - if (ParseSILArgs(SILOpts, ParsedArgs, IRGenOpts, FrontendOpts, Diags, + if (ParseSILArgs(SILOpts, ParsedArgs, IRGenOpts, FrontendOpts, + TypeCheckerOpts, Diags, LangOpts.Target, ClangImporterOpts)) { return true; } diff --git a/lib/Sema/TypeChecker.cpp b/lib/Sema/TypeChecker.cpp index edbd80a4027ec..1be236f1bb27f 100644 --- a/lib/Sema/TypeChecker.cpp +++ b/lib/Sema/TypeChecker.cpp @@ -332,13 +332,6 @@ TypeCheckSourceFileRequest::evaluate(Evaluator &eval, SourceFile *SF) const { FrontendStatsTracer tracer(Ctx.Stats, "Type checking and Semantic analysis"); - if (Ctx.TypeCheckerOpts.SkipNonInlinableFunctionBodies) - // Disable this optimization if we're compiling SwiftOnoneSupport, because - // we _definitely_ need to look inside every declaration to figure out - // what gets prespecialized. - if (SF->getParentModule()->isOnoneSupportModule()) - Ctx.TypeCheckerOpts.SkipNonInlinableFunctionBodies = false; - if (!Ctx.LangOpts.DisableAvailabilityChecking) { // Build the type refinement hierarchy for the primary // file before type checking. diff --git a/test/Frontend/skip-non-inlinable-function-bodies.swift b/test/Frontend/skip-non-inlinable-function-bodies.swift index bc342f4a9ceee..db53577cd7234 100644 --- a/test/Frontend/skip-non-inlinable-function-bodies.swift +++ b/test/Frontend/skip-non-inlinable-function-bodies.swift @@ -1,10 +1,13 @@ // RUN: %empty-directory(%t) -// 1. Make sure you can't -emit-ir or -c when you're skipping non-inlinable function bodies +// 1a. Make sure you can't -emit-ir or -c when you're skipping non-inlinable function bodies +// 1b. Also make sure we warn when you try to build SwiftONoneSupport with this option enabled // RUN: not %target-swift-frontend -emit-ir %s -experimental-skip-non-inlinable-function-bodies %s 2>&1 | %FileCheck %s --check-prefix ERROR // RUN: not %target-swift-frontend -c %s -experimental-skip-non-inlinable-function-bodies %s 2>&1 | %FileCheck %s --check-prefix ERROR +// RUN: %target-swift-frontend -typecheck -experimental-skip-non-inlinable-function-bodies -module-name SwiftOnoneSupport %s 2>&1 | %FileCheck %s --check-prefix WARNING // ERROR: -experimental-skip-non-inlinable-function-bodies does not support emitting IR +// WARNING: module 'SwiftOnoneSupport' cannot be built with -experimental-skip-non-inlinable-function-bodies; this option has been automatically disabled // 2. Emit the SIL for a module and check it against the expected strings in function bodies. // If we're doing the right skipping, then the strings that are CHECK-NOT'd won't have been typechecked or SILGen'd. From a5dcb1d400a8d0d6d341b19d0579e8280c41fdbf Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Mon, 11 May 2020 16:44:28 -0700 Subject: [PATCH 3/4] [NFC] Add a "Main Module" Bit The constraint system will need this if it wants to disable debug mode for serialized modules --- include/swift/AST/Decl.h | 7 +++++-- include/swift/AST/Module.h | 11 +++++++++++ lib/AST/Module.cpp | 10 ++++++++++ lib/Frontend/Frontend.cpp | 3 ++- 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 288a6f805ad73..eb6af822e7ab7 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -575,7 +575,7 @@ class alignas(1 << DeclAlignInBits) Decl { HasAnyUnavailableValues : 1 ); - SWIFT_INLINE_BITFIELD(ModuleDecl, TypeDecl, 1+1+1+1+1+1+1+1, + SWIFT_INLINE_BITFIELD(ModuleDecl, TypeDecl, 1+1+1+1+1+1+1+1+1, /// If the module was or is being compiled with `-enable-testing`. TestingEnabled : 1, @@ -601,7 +601,10 @@ class alignas(1 << DeclAlignInBits) Decl { /// Whether the module was imported from Clang (or, someday, maybe another /// language). - IsNonSwiftModule : 1 + IsNonSwiftModule : 1, + + /// Whether this module is the main module. + IsMainModule : 1 ); SWIFT_INLINE_BITFIELD(PrecedenceGroupDecl, Decl, 1+2, diff --git a/include/swift/AST/Module.h b/include/swift/AST/Module.h index 936bf83f5df50..c5a70a6e66773 100644 --- a/include/swift/AST/Module.h +++ b/include/swift/AST/Module.h @@ -347,6 +347,13 @@ class ModuleDecl : public DeclContext, public TypeDecl { return new (ctx) ModuleDecl(name, ctx, importInfo); } + static ModuleDecl * + createMainModule(ASTContext &ctx, Identifier name, ImplicitImportInfo iinfo) { + auto *Mod = ModuleDecl::create(name, ctx, iinfo); + Mod->Bits.ModuleDecl.IsMainModule = true; + return Mod; + } + using Decl::getASTContext; /// Retrieves information about which modules are implicitly imported by @@ -542,6 +549,10 @@ class ModuleDecl : public DeclContext, public TypeDecl { Bits.ModuleDecl.IsNonSwiftModule = flag; } + bool isMainModule() const { + return Bits.ModuleDecl.IsMainModule; + } + /// Retrieve the top-level module. If this module is already top-level, this /// returns itself. If this is a submodule such as \c Foo.Bar.Baz, this /// returns the module \c Foo. diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index a766aafebf39b..c8a667ebe38ab 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -475,6 +475,16 @@ ModuleDecl::ModuleDecl(Identifier name, ASTContext &ctx, setInterfaceType(ModuleType::get(this)); setAccess(AccessLevel::Public); + + Bits.ModuleDecl.TestingEnabled = 0; + Bits.ModuleDecl.FailedToLoad = 0; + Bits.ModuleDecl.RawResilienceStrategy = 0; + Bits.ModuleDecl.HasResolvedImports = 0; + Bits.ModuleDecl.PrivateImportsEnabled = 0; + Bits.ModuleDecl.ImplicitDynamicEnabled = 0; + Bits.ModuleDecl.IsSystemModule = 0; + Bits.ModuleDecl.IsNonSwiftModule = 0; + Bits.ModuleDecl.IsMainModule = 0; } ArrayRef ModuleDecl::getImplicitImports() const { diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp index ce81e02c40d0b..b9192a13c45e1 100644 --- a/lib/Frontend/Frontend.cpp +++ b/lib/Frontend/Frontend.cpp @@ -717,7 +717,8 @@ ImplicitImportInfo CompilerInstance::getImplicitImportInfo() const { ModuleDecl *CompilerInstance::getMainModule() const { if (!MainModule) { Identifier ID = Context->getIdentifier(Invocation.getModuleName()); - MainModule = ModuleDecl::create(ID, *Context, getImplicitImportInfo()); + MainModule = ModuleDecl::createMainModule(*Context, ID, + getImplicitImportInfo()); if (Invocation.getFrontendOptions().EnableTesting) MainModule->setTestingEnabled(); if (Invocation.getFrontendOptions().EnablePrivateImports) From 2bca0134579c3737f453a7d7cf55f5a381c31db2 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Mon, 11 May 2020 16:48:37 -0700 Subject: [PATCH 4/4] Move "isDebugMode" into ConstraintSystem This eliminates the final source of mutation of the TypeCheckerFlags on the ASTContext. --- include/swift/AST/ASTContext.h | 2 +- lib/Sema/CSApply.cpp | 2 +- lib/Sema/CSBindings.cpp | 2 +- lib/Sema/CSGen.cpp | 2 +- lib/Sema/CSRanking.cpp | 18 +++++++------- lib/Sema/CSSimplify.cpp | 4 +-- lib/Sema/CSSolver.cpp | 41 +++++++++++++++++-------------- lib/Sema/CSStep.cpp | 16 ++++++------ lib/Sema/CSStep.h | 12 +++------ lib/Sema/ConstraintGraph.cpp | 4 +-- lib/Sema/ConstraintSystem.cpp | 12 ++++++--- lib/Sema/ConstraintSystem.h | 25 +++++++++++++------ lib/Sema/SourceLoader.cpp | 4 --- lib/Sema/TypeCheckConstraints.cpp | 4 +-- 14 files changed, 80 insertions(+), 68 deletions(-) diff --git a/include/swift/AST/ASTContext.h b/include/swift/AST/ASTContext.h index 901a46aac3732..160b905be6626 100644 --- a/include/swift/AST/ASTContext.h +++ b/include/swift/AST/ASTContext.h @@ -241,7 +241,7 @@ class ASTContext final { const LangOptions &LangOpts; /// The type checker options. - TypeCheckerOptions &TypeCheckerOpts; + const TypeCheckerOptions &TypeCheckerOpts; /// The search path options used by this AST context. SearchPathOptions &SearchPathOpts; diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 6bbc088b4bcbd..b6db0facd2475 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -8321,7 +8321,7 @@ ExprWalker::rewriteTarget(SolutionApplicationTarget target) { result.setExpr(resultExpr); auto &ctx = cs.getASTContext(); - if (ctx.TypeCheckerOpts.DebugConstraintSolver) { + if (cs.isDebugMode()) { auto &log = ctx.TypeCheckerDebug->getStream(); log << "---Type-checked expression---\n"; resultExpr->dump(log); diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index f485bd4f6c1c8..0dc6432c717a3 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -107,7 +107,7 @@ ConstraintSystem::determineBestBindings() { inferTransitiveSupertypeBindings(cache, bindings); - if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { auto &log = getASTContext().TypeCheckerDebug->getStream(); bindings.dump(typeVar, log, solverState->depth * 2); } diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 2bbd2f839b97b..61b324534394c 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -4364,7 +4364,7 @@ bool ConstraintSystem::generateConstraints( target = *resultTarget; } - if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { auto &log = getASTContext().TypeCheckerDebug->getStream(); log << "---Initial constraints for the given expression---\n"; print(log, expr); diff --git a/lib/Sema/CSRanking.cpp b/lib/Sema/CSRanking.cpp index d42a49b6eefe3..86dafa1f24723 100644 --- a/lib/Sema/CSRanking.cpp +++ b/lib/Sema/CSRanking.cpp @@ -35,7 +35,7 @@ void ConstraintSystem::increaseScore(ScoreKind kind, unsigned value) { unsigned index = static_cast(kind); CurrentScore.Data[index] += value; - if (getASTContext().TypeCheckerOpts.DebugConstraintSolver && value > 0) { + if (isDebugMode() && value > 0) { auto &log = getASTContext().TypeCheckerDebug->getStream(); if (solverState) log.indent(solverState->depth * 2); @@ -102,7 +102,7 @@ bool ConstraintSystem::worseThanBestSolution() const { CurrentScore <= *solverState->BestScore) return false; - if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { auto &log = getASTContext().TypeCheckerDebug->getStream(); log.indent(solverState->depth * 2) << "(solution is worse than the best solution)\n"; @@ -386,7 +386,9 @@ bool CompareDeclSpecializationRequest::evaluate( Evaluator &eval, DeclContext *dc, ValueDecl *decl1, ValueDecl *decl2, bool isDynamicOverloadComparison) const { auto &C = decl1->getASTContext(); - if (C.TypeCheckerOpts.DebugConstraintSolver) { + // Construct a constraint system to compare the two declarations. + ConstraintSystem cs(dc, ConstraintSystemOptions()); + if (cs.isDebugMode()) { auto &log = C.TypeCheckerDebug->getStream(); log << "Comparing declarations\n"; decl1->print(log); @@ -397,8 +399,8 @@ bool CompareDeclSpecializationRequest::evaluate( log << ")\n"; } - auto completeResult = [&C](bool result) { - if (C.TypeCheckerOpts.DebugConstraintSolver) { + auto completeResult = [&C, &cs](bool result) { + if (cs.isDebugMode()) { auto &log = C.TypeCheckerDebug->getStream(); log << "comparison result: " << (result ? "better" : "not better") << "\n"; @@ -499,8 +501,6 @@ bool CompareDeclSpecializationRequest::evaluate( return cs.openType(type, replacements); }; - // Construct a constraint system to compare the two declarations. - ConstraintSystem cs(dc, ConstraintSystemOptions()); bool knownNonSubtype = false; auto *locator = cs.getConstraintLocator({}); @@ -737,7 +737,7 @@ static void addKeyPathDynamicMemberOverloads( SolutionCompareResult ConstraintSystem::compareSolutions( ConstraintSystem &cs, ArrayRef solutions, const SolutionDiff &diff, unsigned idx1, unsigned idx2) { - if (cs.getASTContext().TypeCheckerOpts.DebugConstraintSolver) { + if (cs.isDebugMode()) { auto &log = cs.getASTContext().TypeCheckerDebug->getStream(); log.indent(cs.solverState->depth * 2) << "comparing solutions " << idx1 << " and " << idx2 <<"\n"; @@ -1261,7 +1261,7 @@ ConstraintSystem::findBestSolution(SmallVectorImpl &viable, if (viable.size() == 1) return 0; - if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { auto &log = getASTContext().TypeCheckerDebug->getStream(); log.indent(solverState->depth * 2) << "Comparing " << viable.size() << " viable solutions\n"; diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index a48564aa074e3..6c5607f0deaac 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -8148,7 +8148,7 @@ bool ConstraintSystem::simplifyAppliedOverloadsImpl( // If we have a common result type, bind the expected result type to it. if (commonResultType && !commonResultType->is()) { ASTContext &ctx = getASTContext(); - if (ctx.TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { auto &log = ctx.TypeCheckerDebug->getStream(); log.indent(solverState ? solverState->depth * 2 : 0) << "(common result type for $T" << fnTypeVar->getID() << " is " @@ -9290,7 +9290,7 @@ static bool isAugmentingFix(ConstraintFix *fix) { bool ConstraintSystem::recordFix(ConstraintFix *fix, unsigned impact) { auto &ctx = getASTContext(); - if (ctx.TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { auto &log = ctx.TypeCheckerDebug->getStream(); log.indent(solverState ? solverState->depth * 2 : 0) << "(attempting fix "; diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index b010ef5ac298f..53ae160860a01 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -375,11 +375,10 @@ ConstraintSystem::SolverState::SolverState( // If we're supposed to debug a specific constraint solver attempt, // turn on debugging now. ASTContext &ctx = CS.getASTContext(); - auto &tyOpts = ctx.TypeCheckerOpts; - OldDebugConstraintSolver = tyOpts.DebugConstraintSolver; + const auto &tyOpts = ctx.TypeCheckerOpts; if (tyOpts.DebugConstraintSolverAttempt && tyOpts.DebugConstraintSolverAttempt == SolutionAttempt) { - tyOpts.DebugConstraintSolver = true; + CS.Options |= ConstraintSystemFlags::DebugConstraints; llvm::raw_ostream &dbgOut = ctx.TypeCheckerDebug->getStream(); dbgOut << "---Constraint system #" << SolutionAttempt << "---\n"; CS.print(dbgOut); @@ -420,9 +419,14 @@ ConstraintSystem::SolverState::~SolverState() { CS.activateConstraint(constraint); } - // Restore debugging state. - TypeCheckerOptions &tyOpts = CS.getASTContext().TypeCheckerOpts; - tyOpts.DebugConstraintSolver = OldDebugConstraintSolver; + // If global constraing debugging is off and we are finished logging the + // current solution attempt, switch debugging back off. + const auto &tyOpts = CS.getASTContext().TypeCheckerOpts; + if (!tyOpts.DebugConstraintSolver && + tyOpts.DebugConstraintSolverAttempt && + tyOpts.DebugConstraintSolverAttempt == SolutionAttempt) { + CS.Options -= ConstraintSystemFlags::DebugConstraints; + } // Write our local statistics back to the overall statistics. #define CS_STATISTIC(Name, Description) JOIN2(Overall,Name) += Name; @@ -627,8 +631,8 @@ bool ConstraintSystem::Candidate::solve( return false; auto &ctx = cs.getASTContext(); - if (ctx.TypeCheckerOpts.DebugConstraintSolver) { - auto &log = cs.getASTContext().TypeCheckerDebug->getStream(); + if (cs.isDebugMode()) { + auto &log = ctx.TypeCheckerDebug->getStream(); log << "--- Solving candidate for shrinking at "; auto R = E->getSourceRange(); if (R.isValid()) { @@ -664,8 +668,8 @@ bool ConstraintSystem::Candidate::solve( cs.solveImpl(solutions); } - if (ctx.TypeCheckerOpts.DebugConstraintSolver) { - auto &log = cs.getASTContext().TypeCheckerDebug->getStream(); + if (cs.isDebugMode()) { + auto &log = ctx.TypeCheckerDebug->getStream(); if (solutions.empty()) { log << "--- No Solutions ---\n"; } else { @@ -1130,14 +1134,15 @@ Optional> ConstraintSystem::solve( SolutionApplicationTarget &target, FreeTypeVariableBinding allowFreeTypeVariables ) { - llvm::SaveAndRestore debugForExpr( - getASTContext().TypeCheckerOpts.DebugConstraintSolver, - debugConstraintSolverForTarget(getASTContext(), target)); + llvm::SaveAndRestore debugForExpr(Options); + if (debugConstraintSolverForTarget(getASTContext(), target)) { + Options |= ConstraintSystemFlags::DebugConstraints; + } /// Dump solutions for debugging purposes. auto dumpSolutions = [&](const SolutionResult &result) { // Debug-print the set of solutions. - if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { auto &log = getASTContext().TypeCheckerDebug->getStream(); if (result.getKind() == SolutionResult::Success) { log << "---Solution---\n"; @@ -1224,7 +1229,7 @@ Optional> ConstraintSystem::solve( SolutionResult ConstraintSystem::solveImpl(SolutionApplicationTarget &target, FreeTypeVariableBinding allowFreeTypeVariables) { - if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { auto &log = getASTContext().TypeCheckerDebug->getStream(); log << "---Constraint solving at "; auto R = target.getSourceRange(); @@ -1272,7 +1277,7 @@ bool ConstraintSystem::solve(SmallVectorImpl &solutions, // Solve the system. solveImpl(solutions); - if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { auto &log = getASTContext().TypeCheckerDebug->getStream(); log << "---Solver statistics---\n"; log << "Total number of scopes explored: " << solverState->NumStatesExplored << "\n"; @@ -1409,7 +1414,7 @@ ConstraintSystem::filterDisjunction( continue; } - if (ctx.TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { auto &log = ctx.TypeCheckerDebug->getStream(); log.indent(solverState ? solverState->depth * 2 : 0) << "(disabled disjunction term "; @@ -1470,7 +1475,7 @@ ConstraintSystem::filterDisjunction( recordDisjunctionChoice(disjunction->getLocator(), choiceIdx); } - if (ctx.TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { auto &log = ctx.TypeCheckerDebug->getStream(); log.indent(solverState ? solverState->depth * 2 : 0) << "(introducing single enabled disjunction term "; diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index 95687f104cc06..8717fd15d469f 100644 --- a/lib/Sema/CSStep.cpp +++ b/lib/Sema/CSStep.cpp @@ -100,7 +100,7 @@ void SplitterStep::computeFollowupSteps( return; } - if (isDebugMode()) { + if (CS.isDebugMode()) { auto &log = getDebugLogger(); // Verify that the constraint graph is valid. CG.verify(); @@ -238,7 +238,7 @@ bool SplitterStep::mergePartialSolutions() const { // Finalize this solution. auto solution = CS.finalize(); solutionMemory += solution.getTotalMemory(); - if (isDebugMode()) + if (CS.isDebugMode()) getDebugLogger() << "(composed solution " << CS.CurrentScore << ")\n"; // Save this solution. @@ -391,7 +391,7 @@ StepResult ComponentStep::take(bool prevFailed) { } auto solution = CS.finalize(); - if (isDebugMode()) + if (CS.isDebugMode()) getDebugLogger() << "(found solution " << getCurrentScore() << ")\n"; Solutions.push_back(std::move(solution)); @@ -408,7 +408,7 @@ StepResult ComponentStep::finalize(bool isSuccess) { // Rewind all modifications done to constraint system. ComponentScope.reset(); - if (isDebugMode()) { + if (CS.isDebugMode()) { auto &log = getDebugLogger(); log << (isSuccess ? "finished" : "failed") << " component #" << Index << ")\n"; @@ -440,7 +440,7 @@ StepResult ComponentStep::finalize(bool isSuccess) { void TypeVariableStep::setup() { ++CS.solverState->NumTypeVariablesBound; - if (isDebugMode()) { + if (CS.isDebugMode()) { PrintOptions PO; PO.PrintTypesForDebugging = true; auto &log = getDebugLogger(); @@ -478,7 +478,7 @@ StepResult TypeVariableStep::resume(bool prevFailed) { // Rewind back all of the changes made to constraint system. ActiveChoice.reset(); - if (isDebugMode()) + if (CS.isDebugMode()) getDebugLogger() << ")\n"; // Let's check if we should stop right before @@ -517,7 +517,7 @@ StepResult DisjunctionStep::resume(bool prevFailed) { // Rewind back the constraint system information. ActiveChoice.reset(); - if (isDebugMode()) + if (CS.isDebugMode()) getDebugLogger() << ")\n"; // Attempt next disjunction choice (if any left). @@ -530,7 +530,7 @@ bool DisjunctionStep::shouldSkip(const DisjunctionChoice &choice) const { bool attemptFixes = CS.shouldAttemptFixes(); // Enable all disabled choices in "diagnostic" mode. if (!attemptFixes && choice.isDisabled()) { - if (isDebugMode()) { + if (CS.isDebugMode()) { auto &log = getDebugLogger(); log << "(skipping "; choice.print(log, &ctx.SourceMgr); diff --git a/lib/Sema/CSStep.h b/lib/Sema/CSStep.h index 8fd3b8bca027c..330981d7d07a2 100644 --- a/lib/Sema/CSStep.h +++ b/lib/Sema/CSStep.h @@ -231,12 +231,6 @@ class SolverStep { CS.filterSolutions(solutions, minimize); } - /// Check whether constraint solver is running in "debug" mode, - /// which should output diagnostic information. - bool isDebugMode() const { - return CS.getASTContext().TypeCheckerOpts.DebugConstraintSolver; - } - llvm::raw_ostream &getDebugLogger(bool indent = true) const { auto &log = CS.getASTContext().TypeCheckerDebug->getStream(); return indent ? log.indent(CS.solverState->depth * 2) : log; @@ -471,7 +465,7 @@ class ComponentStep final : public SolverStep { if (IsSingle) return; - if (isDebugMode()) + if (CS.isDebugMode()) getDebugLogger() << "(solving component #" << Index << '\n'; ComponentScope = std::make_unique(*this); @@ -515,7 +509,7 @@ template class BindingStep : public SolverStep { if (shouldStopAt(*choice)) break; - if (isDebugMode()) { + if (CS.isDebugMode()) { auto &log = getDebugLogger(); log << "(attempting "; choice->print(log, &CS.getASTContext().SourceMgr); @@ -530,7 +524,7 @@ template class BindingStep : public SolverStep { } } - if (isDebugMode()) + if (CS.isDebugMode()) getDebugLogger() << ")\n"; // If this binding didn't match, let's check if we've attempted diff --git a/lib/Sema/ConstraintGraph.cpp b/lib/Sema/ConstraintGraph.cpp index 0288a9a40f323..510cfd421cf8b 100644 --- a/lib/Sema/ConstraintGraph.cpp +++ b/lib/Sema/ConstraintGraph.cpp @@ -872,7 +872,7 @@ namespace { contractedCycle = false; for (const auto &edge : cycleEdges) { if (unionSets(edge.first, edge.second)) { - if (ctx.TypeCheckerOpts.DebugConstraintSolver) { + if (cs.isDebugMode()) { auto &log = ctx.TypeCheckerDebug->getStream(); if (cs.solverState) log.indent(cs.solverState->depth * 2); @@ -1155,7 +1155,7 @@ bool ConstraintGraph::contractEdges() { rep2->getImpl().canBindToLValue()) || // Allow l-value contractions when binding parameter types. isParamBindingConstraint)) { - if (CS.getASTContext().TypeCheckerOpts.DebugConstraintSolver) { + if (CS.isDebugMode()) { auto &log = CS.getASTContext().TypeCheckerDebug->getStream(); if (CS.solverState) log.indent(CS.solverState->depth * 2); diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index f92a85935a67d..1cb63aa77438b 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -74,6 +74,12 @@ ConstraintSystem::ConstraintSystem(DeclContext *dc, CG(*new ConstraintGraph(*this)) { assert(DC && "context required"); + // Respect the global debugging flag, but turn off debugging while + // parsing and loading other modules. + if (Context.TypeCheckerOpts.DebugConstraintSolver && + DC->getParentModule()->isMainModule()) { + Options |= ConstraintSystemFlags::DebugConstraints; + } } ConstraintSystem::~ConstraintSystem() { @@ -2408,7 +2414,7 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator, verifyThatArgumentIsHashable); } - if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { PrintOptions PO; PO.PrintTypesForDebugging = true; auto &log = getASTContext().TypeCheckerDebug->getStream(); @@ -2571,7 +2577,7 @@ bool OverloadChoice::isImplicitlyUnwrappedValueOrReturnValue() const { SolutionResult ConstraintSystem::salvage() { auto &ctx = getASTContext(); - if (ctx.TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { auto &log = ctx.TypeCheckerDebug->getStream(); log << "---Attempting to salvage and emit diagnostics---\n"; } @@ -2619,7 +2625,7 @@ SolutionResult ConstraintSystem::salvage() { // If there are multiple solutions, try to diagnose an ambiguity. if (viable.size() > 1) { - if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { auto &log = getASTContext().TypeCheckerDebug->getStream(); log << "---Ambiguity error: " << viable.size() << " solutions found---\n"; diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index a68185f4825f8..b0de352fd5cc6 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -1089,6 +1089,16 @@ enum class ConstraintSystemFlags { /// If set, constraint system always reuses type of pre-typechecked /// expression, and doesn't dig into its subexpressions. ReusePrecheckedType = 0x08, + + /// If set, verbose output is enabled for this constraint system. + /// + /// Note that this flag is automatically applied to all constraint systems + /// when \c DebugConstraintSolver is set in \c TypeCheckerOptions. It can be + /// automatically enabled for select constraint solving attempts by setting + /// \c DebugConstraintSolverAttempt. Finally, it be automatically enabled + /// for a pre-configured set of expressions on line numbers by setting + /// \c DebugConstraintSolverOnLines. + DebugConstraints = 0x10, }; /// Options that affect the constraint system as a whole. @@ -1895,11 +1905,6 @@ class ConstraintSystem { FreeTypeVariableBinding AllowFreeTypeVariables; - /// Old value of DebugConstraintSolver. - /// FIXME: Move the "debug constraint solver" bit into the constraint - /// system itself. - bool OldDebugConstraintSolver; - /// Depth of the solution stack. unsigned depth = 0; @@ -2320,6 +2325,12 @@ class ConstraintSystem { /// variables. bool hasFreeTypeVariables(); + /// Check whether constraint solver is running in "debug" mode, + /// which should output diagnostic information. + bool isDebugMode() const { + return Options.contains(ConstraintSystemFlags::DebugConstraints); + } + private: /// Finalize this constraint system; we're done attempting to solve /// it. @@ -2946,7 +2957,7 @@ class ConstraintSystem { /// Whether we should record the failure of a constraint. bool shouldRecordFailedConstraint() const { // If we're debugging, always note a failure so we can print it out. - if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) + if (isDebugMode()) return true; // Otherwise, only record it if we don't already have a failed constraint. @@ -2961,7 +2972,7 @@ class ConstraintSystem { if (!failedConstraint) failedConstraint = constraint; - if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) { + if (isDebugMode()) { auto &log = getASTContext().TypeCheckerDebug->getStream(); log.indent(solverState ? solverState->depth * 2 : 0) << "(failed constraint "; diff --git a/lib/Sema/SourceLoader.cpp b/lib/Sema/SourceLoader.cpp index 133553fdadfd8..b535a6986391d 100644 --- a/lib/Sema/SourceLoader.cpp +++ b/lib/Sema/SourceLoader.cpp @@ -104,10 +104,6 @@ ModuleDecl *SourceLoader::loadModule(SourceLoc importLoc, dependencyTracker->addDependency(inputFile->getBufferIdentifier(), /*isSystem=*/false); - // Turn off debugging while parsing other modules. - llvm::SaveAndRestore - turnOffDebug(Ctx.TypeCheckerOpts.DebugConstraintSolver, false); - unsigned bufferID; if (auto BufID = Ctx.SourceMgr.getIDForBufferIdentifier(inputFile->getBufferIdentifier())) diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 05a11aea80e7a..7ebd29e5b5fa8 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -2214,7 +2214,7 @@ getTypeOfCompletionOperatorImpl(DeclContext *DC, Expr *expr, if (!expr) return nullptr; - if (Context.TypeCheckerOpts.DebugConstraintSolver) { + if (CS.isDebugMode()) { auto &log = Context.TypeCheckerDebug->getStream(); log << "---Initial constraints for the given expression---\n"; expr->dump(log); @@ -2228,7 +2228,7 @@ getTypeOfCompletionOperatorImpl(DeclContext *DC, Expr *expr, return nullptr; auto &solution = viable[0]; - if (Context.TypeCheckerOpts.DebugConstraintSolver) { + if (CS.isDebugMode()) { auto &log = Context.TypeCheckerDebug->getStream(); log << "---Solution---\n"; solution.dump(log);