diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index df9701e508aa0..f8bd5a140ee89 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -236,36 +236,23 @@ struct RestrictionOrFix { class ExpressionTimer { -public: - using AnchorType = llvm::PointerUnion; - -private: - AnchorType Anchor; - ASTContext &Context; + ConstraintSystem &CS; llvm::TimeRecord StartTime; /// The number of seconds from creation until /// this timer is considered expired. unsigned ThresholdInSecs; - bool PrintDebugTiming; bool PrintWarning; public: const static unsigned NoLimit = (unsigned) -1; - ExpressionTimer(AnchorType Anchor, ConstraintSystem &CS, - unsigned thresholdInSecs); + ExpressionTimer(ConstraintSystem &CS, unsigned thresholdInSecs); ~ExpressionTimer(); - AnchorType getAnchor() const { return Anchor; } - - SourceRange getAffectedRange() const; - - unsigned getWarnLimit() const { - return Context.TypeCheckerOpts.WarnLongExpressionTypeChecking; - } + unsigned getWarnLimit() const; llvm::TimeRecord startedAt() const { return StartTime; } /// Return the elapsed process time (including fractional seconds) @@ -2159,7 +2146,12 @@ struct ClosureIsolatedByPreconcurrency { /// solution of which assigns concrete types to each of the type variables. /// Constraint systems are typically generated given an (untyped) expression. class ConstraintSystem { +public: + using ExprOrLocator = llvm::PointerUnion; + +private: ASTContext &Context; + ExprOrLocator CurrentAnchor; public: DeclContext *DC; @@ -5394,6 +5386,9 @@ class ConstraintSystem { /// \returns The selected conjunction. Constraint *selectConjunction(); + void diagnoseTooComplex(SourceLoc fallbackLoc, + SolutionResult &result); + /// Solve the system of constraints generated from provided expression. /// /// \param target The target to generate constraints from. @@ -5491,6 +5486,8 @@ class ConstraintSystem { compareSolutions(ConstraintSystem &cs, ArrayRef solutions, const SolutionDiff &diff, unsigned idx1, unsigned idx2); + void startExpressionTimer(); + public: /// Increase the score of the given kind for the current (partial) solution /// along the current solver path. @@ -5528,7 +5525,6 @@ class ConstraintSystem { std::optional findBestSolution(SmallVectorImpl &solutions, bool minimize); -public: /// Apply a given solution to the target, producing a fully /// type-checked target or \c None if an error occurred. /// @@ -5581,7 +5577,17 @@ class ConstraintSystem { /// resolved before any others. void optimizeConstraints(Expr *e); - void startExpressionTimer(ExpressionTimer::AnchorType anchor); + /// Set the current sub-expression (of a multi-statement closure, etc) for + /// the purposes of diagnosing "reasonable time" errors. + void startExpression(ExprOrLocator anchor); + + /// Returns the above anchor. + ExprOrLocator getCurrentAnchor() const { + return CurrentAnchor; + } + + /// The source range of the above. + SourceRange getAffectedRange() const; /// Determine if we've already explored too many paths in an /// attempt to solve this expression. @@ -5594,53 +5600,7 @@ class ConstraintSystem { return range.isValid() ? range : std::optional(); } - bool isTooComplex(size_t solutionMemory) { - if (isAlreadyTooComplex.first) - return true; - - auto CancellationFlag = getASTContext().CancellationFlag; - if (CancellationFlag && CancellationFlag->load(std::memory_order_relaxed)) - return true; - - auto markTooComplex = [&](SourceRange range, StringRef reason) { - if (isDebugMode()) { - if (solverState) - llvm::errs().indent(solverState->getCurrentIndent()); - llvm::errs() << "(too complex: " << reason << ")\n"; - } - isAlreadyTooComplex = {true, range}; - return true; - }; - - auto used = getASTContext().getSolverMemory() + solutionMemory; - MaxMemory = std::max(used, MaxMemory); - auto threshold = getASTContext().TypeCheckerOpts.SolverMemoryThreshold; - if (MaxMemory > threshold) { - // No particular location for OoM problems. - return markTooComplex(SourceRange(), "exceeded memory limit"); - } - - if (Timer && Timer->isExpired()) { - // Disable warnings about expressions that go over the warning - // threshold since we're arbitrarily ending evaluation and - // emitting an error. - Timer->disableWarning(); - - return markTooComplex(Timer->getAffectedRange(), "exceeded time limit"); - } - - auto &opts = getASTContext().TypeCheckerOpts; - - // Bail out once we've looked at a really large number of choices. - if (opts.SolverScopeThreshold && NumSolverScopes > opts.SolverScopeThreshold) - return markTooComplex(SourceRange(), "exceeded scope limit"); - - // Bail out once we've taken a really large number of steps. - if (opts.SolverTrailThreshold && NumTrailSteps > opts.SolverTrailThreshold) - return markTooComplex(SourceRange(), "exceeded trail limit"); - - return false; - } + bool isTooComplex(size_t solutionMemory); bool isTooComplex(ArrayRef solutions) { if (isAlreadyTooComplex.first) diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index 326fb242476f9..a1e11d4dd4a98 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -1053,9 +1053,7 @@ TypeChecker::applyResultBuilderBodyTransform(FuncDecl *func, Type builderType) { case SolutionResult::Kind::TooComplex: reportSolutionsToSolutionCallback(salvagedResult); - func->diagnose(diag::expression_too_complex) - .highlight(func->getBodySourceRange()); - salvagedResult.markAsDiagnosed(); + cs.diagnoseTooComplex(func->getLoc(), salvagedResult); return nullptr; } diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index a17c7222a0bf9..95f61f4018cbd 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -829,9 +829,7 @@ bool ConstraintSystem::Candidate::solve( // Allocate new constraint system for sub-expression. ConstraintSystem cs(DC, std::nullopt); - - // Set up expression type checker timer for the candidate. - cs.startExpressionTimer(E); + cs.startExpression(E); // Generate constraints for the new system. if (auto generatedExpr = cs.generateConstraints(E, DC)) { @@ -1455,18 +1453,7 @@ ConstraintSystem::solve(SyntacticElementTarget &target, return std::nullopt; case SolutionResult::TooComplex: { - auto affectedRange = solution.getTooComplexAt(); - - // If affected range is unknown, let's use whole - // target. - if (!affectedRange) - affectedRange = target.getSourceRange(); - - getASTContext() - .Diags.diagnose(affectedRange->Start, diag::expression_too_complex) - .highlight(*affectedRange); - - solution.markAsDiagnosed(); + diagnoseTooComplex(target.getLoc(), solution); return std::nullopt; } @@ -1501,6 +1488,19 @@ ConstraintSystem::solve(SyntacticElementTarget &target, llvm_unreachable("Loop always returns"); } +void ConstraintSystem::diagnoseTooComplex(SourceLoc fallbackLoc, + SolutionResult &result) { + auto affectedRange = result.getTooComplexAt(); + + SourceLoc loc = (affectedRange ? affectedRange->Start : fallbackLoc); + auto diag = getASTContext().Diags.diagnose(loc, diag::expression_too_complex); + + if (affectedRange) + diag.highlight(*affectedRange); + + result.markAsDiagnosed(); +} + SolutionResult ConstraintSystem::solveImpl(SyntacticElementTarget &target, FreeTypeVariableBinding allowFreeTypeVariables) { @@ -1518,9 +1518,8 @@ ConstraintSystem::solveImpl(SyntacticElementTarget &target, assert(!solverState && "cannot be used directly"); - // Set up the expression type checker timer. if (Expr *expr = target.getAsExpr()) - startExpressionTimer(expr); + startExpression(expr); if (generateConstraints(target, allowFreeTypeVariables)) return SolutionResult::forError(); @@ -1701,8 +1700,7 @@ bool ConstraintSystem::solveForCodeCompletion( // Tell the constraint system what the contextual type is. setContextualInfo(expr, target.getExprContextualTypeInfo()); - // Set up the expression type checker timer. - startExpressionTimer(expr); + startExpression(expr); shrink(expr); } diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index 8b52fad07a6d5..c451357a27cbf 100644 --- a/lib/Sema/CSStep.cpp +++ b/lib/Sema/CSStep.cpp @@ -823,10 +823,10 @@ bool ConjunctionStep::attempt(const ConjunctionElement &element) { // (expression) gets a fresh time slice to get solved. This // is important for closures with large number of statements // in them. - if (CS.Timer) { + if (CS.Timer) CS.Timer.reset(); - CS.startExpressionTimer(element.getLocator()); - } + + CS.startExpression(element.getLocator()); auto success = element.attempt(CS); diff --git a/lib/Sema/CSStep.h b/lib/Sema/CSStep.h index a0b7a40cb03b9..df92850ea2b34 100644 --- a/lib/Sema/CSStep.h +++ b/lib/Sema/CSStep.h @@ -867,8 +867,7 @@ class ConjunctionStep : public BindingStep { /// The number of milliseconds until outer constraint system /// is considered "too complex" if timer is enabled. - std::optional> - OuterTimeRemaining = std::nullopt; + std::optional OuterTimeRemaining = std::nullopt; /// Conjunction constraint associated with this step. Constraint *Conjunction; @@ -910,7 +909,7 @@ class ConjunctionStep : public BindingStep { if (cs.Timer) { auto remainingTime = cs.Timer->getRemainingProcessTimeInSeconds(); - OuterTimeRemaining.emplace(cs.Timer->getAnchor(), remainingTime); + OuterTimeRemaining.emplace(remainingTime); } } @@ -925,11 +924,8 @@ class ConjunctionStep : public BindingStep { if (HadFailure) restoreBestScore(); - if (OuterTimeRemaining) { - auto anchor = OuterTimeRemaining->first; - auto remainingTime = OuterTimeRemaining->second; - CS.Timer.emplace(anchor, CS, remainingTime); - } + if (OuterTimeRemaining) + CS.Timer.emplace(CS, *OuterTimeRemaining); } StepResult resume(bool prevFailed) override; diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 7823676ee3e3b..bae73c2262e5e 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -54,43 +54,101 @@ using namespace inference; #define DEBUG_TYPE "ConstraintSystem" -ExpressionTimer::ExpressionTimer(AnchorType Anchor, ConstraintSystem &CS, - unsigned thresholdInSecs) - : Anchor(Anchor), Context(CS.getASTContext()), - StartTime(llvm::TimeRecord::getCurrentTime()), - ThresholdInSecs(thresholdInSecs), - PrintDebugTiming(CS.getASTContext().TypeCheckerOpts.DebugTimeExpressions), - PrintWarning(true) {} +void ConstraintSystem::startExpression(ConstraintSystem::ExprOrLocator anchor) { + CurrentAnchor = anchor; + + startExpressionTimer(); +} -SourceRange ExpressionTimer::getAffectedRange() const { +SourceRange ConstraintSystem::getAffectedRange() const { ASTNode anchor; - if (auto *locator = Anchor.dyn_cast()) { + if (auto *locator = CurrentAnchor.dyn_cast()) { anchor = simplifyLocatorToAnchor(locator); // If locator couldn't be simplified down to a single AST // element, let's use its root. if (!anchor) anchor = locator->getAnchor(); } else { - anchor = cast(Anchor); + anchor = cast(CurrentAnchor); } return anchor.getSourceRange(); } +bool ConstraintSystem::isTooComplex(size_t solutionMemory) { + if (isAlreadyTooComplex.first) + return true; + + auto CancellationFlag = getASTContext().CancellationFlag; + if (CancellationFlag && CancellationFlag->load(std::memory_order_relaxed)) + return true; + + auto markTooComplex = [&](SourceRange range, StringRef reason) { + if (isDebugMode()) { + if (solverState) + llvm::errs().indent(solverState->getCurrentIndent()); + llvm::errs() << "(too complex: " << reason << ")\n"; + } + isAlreadyTooComplex = {true, range}; + return true; + }; + + auto used = getASTContext().getSolverMemory() + solutionMemory; + MaxMemory = std::max(used, MaxMemory); + auto threshold = getASTContext().TypeCheckerOpts.SolverMemoryThreshold; + if (MaxMemory > threshold) { + // Bail once we've used too much constraint solver arena memory. + return markTooComplex(getAffectedRange(), "exceeded memory limit"); + } + + if (Timer && Timer->isExpired()) { + // Disable warnings about expressions that go over the warning + // threshold since we're arbitrarily ending evaluation and + // emitting an error. + Timer->disableWarning(); + + return markTooComplex(getAffectedRange(), "exceeded time limit"); + } + + auto &opts = getASTContext().TypeCheckerOpts; + + // Bail out once we've looked at a really large number of choices. + if (opts.SolverScopeThreshold && NumSolverScopes > opts.SolverScopeThreshold) + return markTooComplex(getAffectedRange(), "exceeded scope limit"); + + // Bail out once we've taken a really large number of steps. + if (opts.SolverTrailThreshold && NumTrailSteps > opts.SolverTrailThreshold) + return markTooComplex(getAffectedRange(), "exceeded trail limit"); + + return false; +} + +ExpressionTimer::ExpressionTimer(ConstraintSystem &CS, unsigned thresholdInSecs) + : CS(CS), + StartTime(llvm::TimeRecord::getCurrentTime()), + ThresholdInSecs(thresholdInSecs), + PrintWarning(true) {} + +unsigned ExpressionTimer::getWarnLimit() const { + return CS.getASTContext().TypeCheckerOpts.WarnLongExpressionTypeChecking; +} + ExpressionTimer::~ExpressionTimer() { auto elapsed = getElapsedProcessTimeInFractionalSeconds(); unsigned elapsedMS = static_cast(elapsed * 1000); + auto &ctx = CS.getASTContext(); - if (PrintDebugTiming) { + if (ctx.TypeCheckerOpts.DebugTimeExpressions) { // Round up to the nearest 100th of a millisecond. llvm::errs() << llvm::format("%0.2f", std::ceil(elapsed * 100000) / 100) << "ms\t"; - if (auto *E = Anchor.dyn_cast()) { - E->getLoc().print(llvm::errs(), Context.SourceMgr); + auto anchor = CS.getCurrentAnchor(); + if (auto *E = anchor.dyn_cast()) { + E->getLoc().print(llvm::errs(), ctx.SourceMgr); } else { - auto *locator = cast(Anchor); - locator->dump(&Context.SourceMgr, llvm::errs()); + auto *locator = cast(anchor); + locator->dump(&ctx.SourceMgr, llvm::errs()); } llvm::errs() << "\n"; } @@ -103,10 +161,10 @@ ExpressionTimer::~ExpressionTimer() { if (WarnLimit == 0 || elapsedMS < WarnLimit) return; - auto sourceRange = getAffectedRange(); + auto sourceRange = CS.getAffectedRange(); if (sourceRange.Start.isValid()) { - Context.Diags + ctx.Diags .diagnose(sourceRange.Start, diag::debug_long_expression, elapsedMS, WarnLimit) .highlight(sourceRange); @@ -140,7 +198,7 @@ ConstraintSystem::~ConstraintSystem() { } } -void ConstraintSystem::startExpressionTimer(ExpressionTimer::AnchorType anchor) { +void ConstraintSystem::startExpressionTimer() { ASSERT(!Timer); const auto &opts = getASTContext().TypeCheckerOpts; @@ -156,7 +214,7 @@ void ConstraintSystem::startExpressionTimer(ExpressionTimer::AnchorType anchor) timeout = ExpressionTimer::NoLimit; } - Timer.emplace(anchor, *this, timeout); + Timer.emplace(*this, timeout); } void ConstraintSystem::incrementScopeCounter() { diff --git a/test/Constraints/too_complex_source_location.swift b/test/Constraints/too_complex_source_location.swift new file mode 100644 index 0000000000000..24cf1258f8c86 --- /dev/null +++ b/test/Constraints/too_complex_source_location.swift @@ -0,0 +1,19 @@ +// RUN: %target-typecheck-verify-swift -solver-scope-threshold=10 + +// Note: the scope threshold is intentionally set low so that the expression will fail. +// +// The purpose of the test is to ensure the diagnostic points at the second statement in +// the closure, and not the closure itself. +// +// If the expression becomes very fast and we manage to type check it with fewer than +// 10 scopes, please *do not* remove the expected error! Instead, make the expression +// more complex again. + +let s = "" +let n = 0 + +let closure = { + let _ = 0 + let _ = "" + s + "" + s + "" + s + "" + n + "" // expected-error {{reasonable time}} + let _ = 0 +} diff --git a/validation-test/Sema/SwiftUI/too_complex_source_location.swift b/validation-test/Sema/SwiftUI/too_complex_source_location.swift new file mode 100644 index 0000000000000..470b5dbd61b47 --- /dev/null +++ b/validation-test/Sema/SwiftUI/too_complex_source_location.swift @@ -0,0 +1,43 @@ +// RUN: %target-typecheck-verify-swift -target %target-cpu-apple-macosx10.15 -swift-version 5 +// REQUIRES: objc_interop + +// https://forums.swift.org/t/roadmap-for-improving-the-type-checker/82952/9 +// +// The purpose of the test is to ensure the diagnostic points at the right statement in +// the function body, and not the function declaration itself. +// +// Ideally, we would produce a useful diagnostic here. Once we are able to do that, we +// will need to devise a new test which complains with 'reasonable time' to ensure the +// source location remains correct. + +import SwiftUI + +struct ContentView: View { + @State var selection = "" + + @State var a: Int? + @State var b: Int? + @State var c: Int? + + var body: some View { + ScrollView { + VStack { + Picker(selection: $selection) { + ForEach(["a", "b", "c"], id: \.self) { + Text($0) // expected-error {{reasonable time}} + .foregroundStyl(.red) // Typo is here + } + } label: { + } + .pickerStyle(.segmented) + } + .padding(.horizontal) + } + .onChange(of: a) { oldValue, newValue in + } + .onChange(of: b) { oldValue, newValue in + } + .onChange(of: c) { oldValue, newValue in + } + } +} diff --git a/validation-test/Sema/type_checker_perf/slow/rdar19612086.swift b/validation-test/Sema/type_checker_perf/slow/rdar19612086.swift index 1b85cdf490590..0510537e25013 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar19612086.swift +++ b/validation-test/Sema/type_checker_perf/slow/rdar19612086.swift @@ -11,8 +11,8 @@ struct rdar19612086 { let x = 1.0 var description : String { - return "\(i)" + Stringly(format: "%.2f", x) + // expected-error {{reasonable time}} - "\(i+1)" + Stringly(format: "%.2f", x) + + return "\(i)" + Stringly(format: "%.2f", x) + + "\(i+1)" + Stringly(format: "%.2f", x) + // expected-error {{reasonable time}} "\(i+2)" + Stringly(format: "%.2f", x) + "\(i+3)" + Stringly(format: "%.2f", x) + "\(i+4)" + Stringly(format: "%.2f", x) +