From ad1ad43c6e4c6862870f6a095630f2e1ffba5da7 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 27 Jan 2020 21:08:36 -0800 Subject: [PATCH 01/12] Generalize SolutionApplicationTarget and use it more widely. Make this type suitable for representing the result of solution application, and do so. --- lib/Sema/CSApply.cpp | 53 ++++++++++++++++++++----------------- lib/Sema/ConstraintSystem.h | 48 +++++++++++++++++++++++++-------- 2 files changed, 65 insertions(+), 36 deletions(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 770b79c72a9c2..4195ef7b474e6 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -7231,14 +7231,14 @@ bool ConstraintSystem::applySolutionFixes(const Solution &solution) { /// Apply a given solution to the expression, producing a fully /// type-checked expression. -llvm::PointerUnion ConstraintSystem::applySolutionImpl( +Optional ConstraintSystem::applySolutionImpl( Solution &solution, SolutionApplicationTarget target, Type convertType, bool discardedExpr, bool performingDiagnostics) { // If any fixes needed to be applied to arrive at this solution, resolve // them to specific expressions. if (!solution.Fixes.empty()) { if (shouldSuppressDiagnostics()) - return nullptr; + return None; bool diagnosedErrorsViaFixes = applySolutionFixes(solution); // If all of the available fixes would result in a warning, @@ -7248,12 +7248,12 @@ llvm::PointerUnion ConstraintSystem::applySolutionImpl( })) { // If we already diagnosed any errors via fixes, that's it. if (diagnosedErrorsViaFixes) - return nullptr; + return None; // If we didn't manage to diagnose anything well, so fall back to // diagnosing mining the system to construct a reasonable error message. diagnoseFailureFor(target); - return nullptr; + return None; } } @@ -7261,9 +7261,13 @@ llvm::PointerUnion ConstraintSystem::applySolutionImpl( ExprWalker walker(rewriter); // Apply the solution to the target. - llvm::PointerUnion result; + SolutionApplicationTarget result = target; if (auto expr = target.getAsExpr()) { - result = expr->walk(walker); + Expr *rewrittenExpr = expr->walk(walker); + if (!rewrittenExpr) + return None; + + result.setExpr(rewrittenExpr); } else { auto fn = *target.getAsFunction(); @@ -7284,14 +7288,11 @@ llvm::PointerUnion ConstraintSystem::applySolutionImpl( }); if (!newBody) - return result; + return None; - result = newBody; + result.setFunctionBody(newBody); } - if (result.isNull()) - return result; - // If we're re-typechecking an expression for diagnostics, don't // visit closures that have non-single expression bodies. if (!performingDiagnostics) { @@ -7309,10 +7310,10 @@ llvm::PointerUnion ConstraintSystem::applySolutionImpl( // If any of them failed to type check, bail. if (hadError) - return nullptr; + return None; } - if (auto resultExpr = result.dyn_cast()) { + if (auto resultExpr = result.getAsExpr()) { Expr *expr = target.getAsExpr(); assert(expr && "Can't have expression result without expression target"); // We are supposed to use contextual type only if it is present and @@ -7328,19 +7329,20 @@ llvm::PointerUnion ConstraintSystem::applySolutionImpl( // If we're supposed to convert the expression to some particular type, // do so now. if (shouldCoerceToContextualType()) { - result = rewriter.coerceToType(resultExpr, convertType, - getConstraintLocator(expr)); - if (!result) - return nullptr; + resultExpr = rewriter.coerceToType(resultExpr, convertType, + getConstraintLocator(expr)); } else if (getType(resultExpr)->hasLValueType() && !discardedExpr) { // We referenced an lvalue. Load it. - result = rewriter.coerceToType(resultExpr, - getType(resultExpr)->getRValueType(), - getConstraintLocator(expr)); + resultExpr = rewriter.coerceToType(resultExpr, + getType(resultExpr)->getRValueType(), + getConstraintLocator(expr)); } - if (resultExpr) - solution.setExprTypes(resultExpr); + if (!resultExpr) + return None; + + solution.setExprTypes(resultExpr); + result.setExpr(resultExpr); } rewriter.finalize(); @@ -7512,13 +7514,14 @@ MutableArrayRef SolutionResult::takeAmbiguousSolutions() && { return MutableArrayRef(solutions, numSolutions); } -llvm::PointerUnion SolutionApplicationTarget::walk( - ASTWalker &walker) { +SolutionApplicationTarget SolutionApplicationTarget::walk(ASTWalker &walker) { switch (kind) { case Kind::expression: return getAsExpr()->walk(walker); case Kind::function: - return getAsFunction()->getBody()->walk(walker); + return SolutionApplicationTarget( + *getAsFunction(), + cast_or_null(getFunctionBody()->walk(walker))); } } diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 472174e6de469..6b4e14edc5ad2 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -1147,7 +1147,11 @@ class SolutionApplicationTarget { union { Expr *expression; - AnyFunctionRef function; + + struct { + AnyFunctionRef function; + BraceStmt *body; + } function; }; public: @@ -1156,9 +1160,13 @@ class SolutionApplicationTarget { expression = expr; } - SolutionApplicationTarget(AnyFunctionRef fn) { + SolutionApplicationTarget(AnyFunctionRef fn) + : SolutionApplicationTarget(fn, fn.getBody()) { } + + SolutionApplicationTarget(AnyFunctionRef fn, BraceStmt *body) { kind = Kind::function; - function = fn; + function.function = fn; + function.body = body; } Expr *getAsExpr() const { @@ -1171,18 +1179,32 @@ class SolutionApplicationTarget { } } + void setExpr(Expr *expr) { + assert(kind == Kind::expression); + expression = expr; + } + Optional getAsFunction() const { switch (kind) { case Kind::expression: return None; case Kind::function: - return function; + return function.function; } } + BraceStmt *getFunctionBody() const { + assert(kind == Kind::function); + return function.body; + } + + void setFunctionBody(BraceStmt *stmt) { + assert(kind == Kind::function); + function.body = stmt; + } /// Walk the contents of the application target. - llvm::PointerUnion walk(ASTWalker &walker); + SolutionApplicationTarget walk(ASTWalker &walker); }; enum class ConstraintSystemPhase { @@ -4147,7 +4169,7 @@ class ConstraintSystem { bool minimize); private: - llvm::PointerUnion applySolutionImpl( + Optional applySolutionImpl( Solution &solution, SolutionApplicationTarget target, Type convertType, bool discardedExpr, bool performingDiagnostics); @@ -4165,15 +4187,19 @@ class ConstraintSystem { Type convertType, bool discardedExpr, bool performingDiagnostics) { - return applySolutionImpl(solution, expr, convertType, discardedExpr, - performingDiagnostics).get(); + auto result = applySolutionImpl( + solution, expr, convertType, discardedExpr, performingDiagnostics); + if (result) + return result->getAsExpr(); + return nullptr; } /// Apply a given solution to the body of the given function. BraceStmt *applySolutionToBody(Solution &solution, AnyFunctionRef fn) { - return cast_or_null( - applySolutionImpl(solution, fn, Type(), false, false) - .dyn_cast()); + auto result = applySolutionImpl(solution, fn, Type(), false, false); + if (result) + return result->getFunctionBody(); + return nullptr; } /// Reorder the disjunctive clauses for a given expression to From 98db6e642262ea04b3d34b4903c3daaf15c109c0 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 27 Jan 2020 21:08:53 -0800 Subject: [PATCH 02/12] =?UTF-8?q?[Constraint=20solver]=20Don=E2=80=99t=20m?= =?UTF-8?q?ark=20a=20moved-from=20instance=20as=20=E2=80=9Cdiagnosed?= =?UTF-8?q?=E2=80=9D.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/Sema/CSApply.cpp | 1 + lib/Sema/CSSolver.cpp | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 4195ef7b474e6..1f13de2f7e0b1 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -7511,6 +7511,7 @@ ArrayRef SolutionResult::getAmbiguousSolutions() const { MutableArrayRef SolutionResult::takeAmbiguousSolutions() && { assert(getKind() == Ambiguous); + markAsDiagnosed(); return MutableArrayRef(solutions, numSolutions); } diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index fbae75e344bc2..20699a855fd48 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1172,7 +1172,6 @@ bool ConstraintSystem::solve(Expr *&expr, solutions.assign(std::make_move_iterator(ambiguousSolutions.begin()), std::make_move_iterator(ambiguousSolutions.end())); dumpSolutions(); - solution.markAsDiagnosed(); return false; } From e3124dcb5f41a9b908adf3d9360a97203c55adee Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 28 Jan 2020 09:32:54 -0800 Subject: [PATCH 03/12] [Constraint system] Expand SolutionApplicationTarget for expressions. Add the final conversion type and the flag indicating whether a given expression is discarded to SolutionApplicationTarget, rather than separating the arguments to the solver implementation. --- lib/Sema/CSApply.cpp | 16 +++++++++++----- lib/Sema/CSSolver.cpp | 4 +++- lib/Sema/ConstraintSystem.h | 37 +++++++++++++++++++++++++++++-------- 3 files changed, 43 insertions(+), 14 deletions(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 1f13de2f7e0b1..8fdcc55d12af1 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -7232,8 +7232,8 @@ bool ConstraintSystem::applySolutionFixes(const Solution &solution) { /// Apply a given solution to the expression, producing a fully /// type-checked expression. Optional ConstraintSystem::applySolutionImpl( - Solution &solution, SolutionApplicationTarget target, Type convertType, - bool discardedExpr, bool performingDiagnostics) { + Solution &solution, SolutionApplicationTarget target, + bool performingDiagnostics) { // If any fixes needed to be applied to arrive at this solution, resolve // them to specific expressions. if (!solution.Fixes.empty()) { @@ -7316,9 +7316,11 @@ Optional ConstraintSystem::applySolutionImpl( if (auto resultExpr = result.getAsExpr()) { Expr *expr = target.getAsExpr(); assert(expr && "Can't have expression result without expression target"); + // We are supposed to use contextual type only if it is present and // this expression doesn't represent the implicit return of the single // expression function which got deduced to be `Never`. + Type convertType = target.getExprConversionType(); auto shouldCoerceToContextualType = [&]() { return convertType && !(getType(resultExpr)->isUninhabited() && @@ -7331,7 +7333,8 @@ Optional ConstraintSystem::applySolutionImpl( if (shouldCoerceToContextualType()) { resultExpr = rewriter.coerceToType(resultExpr, convertType, getConstraintLocator(expr)); - } else if (getType(resultExpr)->hasLValueType() && !discardedExpr) { + } else if (getType(resultExpr)->hasLValueType() && + !target.isDiscardedExpr()) { // We referenced an lvalue. Load it. resultExpr = rewriter.coerceToType(resultExpr, getType(resultExpr)->getRValueType(), @@ -7517,8 +7520,11 @@ MutableArrayRef SolutionResult::takeAmbiguousSolutions() && { SolutionApplicationTarget SolutionApplicationTarget::walk(ASTWalker &walker) { switch (kind) { - case Kind::expression: - return getAsExpr()->walk(walker); + case Kind::expression: { + SolutionApplicationTarget result = *this; + result.setExpr(getAsExpr()->walk(walker)); + return result; + } case Kind::function: return SolutionApplicationTarget( diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 20699a855fd48..9be997703876d 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1184,7 +1184,9 @@ bool ConstraintSystem::solve(Expr *&expr, } if (stage == 1) { - diagnoseFailureFor(expr); + diagnoseFailureFor( + SolutionApplicationTarget(expr, convertType, + /*isDiscarded=*/false)); solution.markAsDiagnosed(); return true; } diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 6b4e14edc5ad2..c88c8b3541478 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -1146,7 +1146,15 @@ class SolutionApplicationTarget { } kind; union { - Expr *expression; + struct { + Expr *expression; + + /// The type to which the expression should be converted. + Type convertType; + + /// Whether the expression result will be discarded at the end. + bool isDiscarded; + } expression; struct { AnyFunctionRef function; @@ -1155,9 +1163,11 @@ class SolutionApplicationTarget { }; public: - SolutionApplicationTarget(Expr *expr) { + SolutionApplicationTarget(Expr *expr, Type convertType, bool isDiscarded) { kind = Kind::expression; - expression = expr; + expression.expression = expr; + expression.convertType = convertType; + expression.isDiscarded = isDiscarded; } SolutionApplicationTarget(AnyFunctionRef fn) @@ -1172,16 +1182,26 @@ class SolutionApplicationTarget { Expr *getAsExpr() const { switch (kind) { case Kind::expression: - return expression; + return expression.expression; case Kind::function: return nullptr; } } + Type getExprConversionType() const { + assert(kind == Kind::expression); + return expression.convertType; + } + + bool isDiscardedExpr() const { + assert(kind == Kind::expression); + return expression.isDiscarded; + } + void setExpr(Expr *expr) { assert(kind == Kind::expression); - expression = expr; + expression.expression = expr; } Optional getAsFunction() const { @@ -4171,7 +4191,7 @@ class ConstraintSystem { private: Optional applySolutionImpl( Solution &solution, SolutionApplicationTarget target, - Type convertType, bool discardedExpr, bool performingDiagnostics); + bool performingDiagnostics); public: /// Apply a given solution to the expression, producing a fully @@ -4188,7 +4208,8 @@ class ConstraintSystem { bool discardedExpr, bool performingDiagnostics) { auto result = applySolutionImpl( - solution, expr, convertType, discardedExpr, performingDiagnostics); + solution, SolutionApplicationTarget(expr, convertType, discardedExpr), + performingDiagnostics); if (result) return result->getAsExpr(); return nullptr; @@ -4196,7 +4217,7 @@ class ConstraintSystem { /// Apply a given solution to the body of the given function. BraceStmt *applySolutionToBody(Solution &solution, AnyFunctionRef fn) { - auto result = applySolutionImpl(solution, fn, Type(), false, false); + auto result = applySolutionImpl(solution, fn, false); if (result) return result->getFunctionBody(); return nullptr; From eb2862ec1e0899c86aa7a01e767e993552d03e96 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 28 Jan 2020 09:54:39 -0800 Subject: [PATCH 04/12] [Constraint system] Collapse applySolutionImpl() into applySolution(). Now that we have a unified notion of a SolutionApplicationTarget, use it to collapse 3 applySolution-ish functions into one. --- lib/Sema/BuilderTransform.cpp | 10 +++++++-- lib/Sema/CSApply.cpp | 2 +- lib/Sema/ConstraintSystem.h | 36 ++++++------------------------- lib/Sema/TypeCheckConstraints.cpp | 15 +++++++------ 4 files changed, 24 insertions(+), 39 deletions(-) diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index d2f21354ab52e..0dff01b97cc29 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -1107,8 +1107,14 @@ Optional TypeChecker::applyFunctionBuilderBodyTransform( } // Apply the solution to the function body. - return cast_or_null( - cs.applySolutionToBody(solutions.front(), func)); + if (auto result = cs.applySolution( + solutions.front(), + SolutionApplicationTarget(func), + /*performingDiagnostics=*/false)) { + return result->getFunctionBody(); + } + + return nullptr; } ConstraintSystem::TypeMatchResult ConstraintSystem::matchFunctionBuilder( diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 8fdcc55d12af1..fa7d40e7b28c6 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -7231,7 +7231,7 @@ bool ConstraintSystem::applySolutionFixes(const Solution &solution) { /// Apply a given solution to the expression, producing a fully /// type-checked expression. -Optional ConstraintSystem::applySolutionImpl( +Optional ConstraintSystem::applySolution( Solution &solution, SolutionApplicationTarget target, bool performingDiagnostics) { // If any fixes needed to be applied to arrive at this solution, resolve diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index c88c8b3541478..4ea147deb4d39 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -4188,40 +4188,16 @@ class ConstraintSystem { findBestSolution(SmallVectorImpl &solutions, bool minimize); -private: - Optional applySolutionImpl( - Solution &solution, SolutionApplicationTarget target, - bool performingDiagnostics); - public: - /// Apply a given solution to the expression, producing a fully - /// type-checked expression. + /// Apply a given solution to the target, producing a fully + /// type-checked target or \c None if an error occurred. /// - /// \param convertType the contextual type to which the - /// expression should be converted, if any. - /// \param discardedExpr if true, the result of the expression - /// is contextually ignored. + /// \param target the target to which the solution will be applied. /// \param performingDiagnostics if true, don't descend into bodies of /// non-single expression closures, or build curry thunks. - Expr *applySolution(Solution &solution, Expr *expr, - Type convertType, - bool discardedExpr, - bool performingDiagnostics) { - auto result = applySolutionImpl( - solution, SolutionApplicationTarget(expr, convertType, discardedExpr), - performingDiagnostics); - if (result) - return result->getAsExpr(); - return nullptr; - } - - /// Apply a given solution to the body of the given function. - BraceStmt *applySolutionToBody(Solution &solution, AnyFunctionRef fn) { - auto result = applySolutionImpl(solution, fn, false); - if (result) - return result->getFunctionBody(); - return nullptr; - } + Optional applySolution( + Solution &solution, SolutionApplicationTarget target, + bool performingDiagnostics); /// Reorder the disjunctive clauses for a given expression to /// increase the likelihood that a favored constraint will be successfully diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index de6d54af3adce..19d1d18153064 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -2267,16 +2267,19 @@ Type TypeChecker::typeCheckExpressionImpl(Expr *&expr, DeclContext *dc, cs.applySolution(solution); // Apply the solution to the expression. - result = cs.applySolution( - solution, result, convertType.getType(), - options.contains(TypeCheckExprFlags::IsDiscarded), - options.contains(TypeCheckExprFlags::SubExpressionDiagnostics)); - - if (!result) { + SolutionApplicationTarget target( + result, convertType.getType(), + options.contains(TypeCheckExprFlags::IsDiscarded)); + bool performingDiagnostics = + options.contains(TypeCheckExprFlags::SubExpressionDiagnostics); + auto resultTarget = cs.applySolution(solution, target, performingDiagnostics); + if (!resultTarget) { listener.applySolutionFailed(solution, expr); + // Failure already diagnosed, above, as part of applying the solution. return Type(); } + result = resultTarget->getAsExpr(); // Notify listener that we've applied the solution. result = listener.appliedSolution(solution, result); From 56aeac5d0a80935d170a08ed8f3b3e8c5675f3b3 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 28 Jan 2020 10:40:45 -0800 Subject: [PATCH 05/12] [Constraint system] Push SolutionApplicationTarget into the solveImpl Baby steps toward generalizing the expression-centric solver core. --- lib/Sema/CSSolver.cpp | 15 +++++++++------ lib/Sema/ConstraintSystem.h | 17 +++++++++++++---- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 9be997703876d..961dce47a94ad 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1136,9 +1136,10 @@ bool ConstraintSystem::solve(Expr *&expr, // Take up to two attempts at solving the system. The first attempts to // solve a system that is expected to be well-formed, the second kicks in // when there is an error and attempts to salvage an ill-formed expression. + SolutionApplicationTarget target(expr, convertType, /*isDiscarded=*/false); for (unsigned stage = 0; stage != 2; ++stage) { auto solution = (stage == 0) - ? solveImpl(expr, convertType, listener, allowFreeTypeVariables) + ? solveImpl(target, listener, allowFreeTypeVariables) : salvage(); switch (solution.getKind()) { @@ -1201,14 +1202,13 @@ bool ConstraintSystem::solve(Expr *&expr, } SolutionResult -ConstraintSystem::solveImpl(Expr *&expr, - Type convertType, +ConstraintSystem::solveImpl(SolutionApplicationTarget &target, ExprTypeCheckListener *listener, FreeTypeVariableBinding allowFreeTypeVariables) { if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) { auto &log = getASTContext().TypeCheckerDebug->getStream(); - log << "---Constraint solving for the expression at "; - auto R = expr->getSourceRange(); + log << "---Constraint solving at "; + auto R = target.getSourceRange(); if (R.isValid()) { R.print(log, getASTContext().SourceMgr, /*PrintText=*/ false); } else { @@ -1220,6 +1220,7 @@ ConstraintSystem::solveImpl(Expr *&expr, assert(!solverState && "cannot be used directly"); // Set up the expression type checker timer. + Expr *expr = target.getAsExpr(); Timer.emplace(expr, *this); Expr *origExpr = expr; @@ -1240,7 +1241,7 @@ ConstraintSystem::solveImpl(Expr *&expr, // If there is a type that we're expected to convert to, add the conversion // constraint. - if (convertType) { + if (Type convertType = target.getExprConversionType()) { // Determine whether we know more about the contextual type. ContextualTypePurpose ctp = CTP_Unused; bool isOpaqueReturnType = false; @@ -1287,6 +1288,8 @@ ConstraintSystem::solveImpl(Expr *&expr, if (getExpressionTooComplex(solutions)) return SolutionResult::forTooComplex(); + target.setExpr(expr); + switch (solutions.size()) { case 0: return SolutionResult::forUndiagnosedError(); diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 4ea147deb4d39..7d3846e646f7a 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -1223,6 +1223,17 @@ class SolutionApplicationTarget { assert(kind == Kind::function); function.body = stmt; } + + /// Retrieve the source range of the target. + SourceRange getSourceRange() const { + switch (kind) { + case Kind::expression: + return expression.expression->getSourceRange(); + + case Kind::function: + return function.body->getSourceRange(); + } + } /// Walk the contents of the application target. SolutionApplicationTarget walk(ASTWalker &walker); }; @@ -4078,13 +4089,11 @@ class ConstraintSystem { /// Solve the system of constraints generated from provided expression. /// - /// \param expr The expression to generate constraints from. - /// \param convertType The expected type of the expression. + /// \param target The target to generate constraints from. /// \param listener The callback to check solving progress. /// \param allowFreeTypeVariables How to bind free type variables in /// the solution. - SolutionResult solveImpl(Expr *&expr, - Type convertType, + SolutionResult solveImpl(SolutionApplicationTarget &target, ExprTypeCheckListener *listener, FreeTypeVariableBinding allowFreeTypeVariables = FreeTypeVariableBinding::Disallow); From 54ac78d28f4e9fd39148a3dcc2722d6661ac085a Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 28 Jan 2020 10:48:40 -0800 Subject: [PATCH 06/12] [Constrsint solver] Remove ExprTypeCheckListener::constraintGenerationFailed() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function isn’t needed, because we can report the failure to the caller directly rather than go through a callback. --- lib/Sema/CSSolver.cpp | 2 -- lib/Sema/TypeCheckConstraints.cpp | 7 ------- lib/Sema/TypeChecker.h | 4 ---- 3 files changed, 13 deletions(-) diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 961dce47a94ad..223fd0e3d6793 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1234,8 +1234,6 @@ ConstraintSystem::solveImpl(SolutionApplicationTarget &target, if (auto generatedExpr = generateConstraints(expr, DC)) expr = generatedExpr; else { - if (listener) - listener->constraintGenerationFailed(expr); return SolutionResult::forError(); } diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 19d1d18153064..4f8bbace048f8 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -2036,7 +2036,6 @@ Expr *ExprTypeCheckListener::appliedSolution(Solution &solution, Expr *expr) { } void ExprTypeCheckListener::preCheckFailed(Expr *expr) {} -void ExprTypeCheckListener::constraintGenerationFailed(Expr *expr) {} void ExprTypeCheckListener::applySolutionFailed(Solution &solution, Expr *expr) {} @@ -2104,12 +2103,6 @@ class FallbackDiagnosticListener : public ExprTypeCheckListener { maybeProduceFallbackDiagnostic(expr); } - void constraintGenerationFailed(Expr *expr) override { - if (BaseListener) - BaseListener->constraintGenerationFailed(expr); - maybeProduceFallbackDiagnostic(expr); - } - void applySolutionFailed(Solution &solution, Expr *expr) override { if (BaseListener) BaseListener->applySolutionFailed(solution, expr); diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index c4df6c61bb254..09068a8443e18 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -325,10 +325,6 @@ class ExprTypeCheckListener { /// be correctly processed by the constraint solver. virtual void preCheckFailed(Expr *expr); - /// Callback invoked if constraint system failed to generate - /// constraints for a given expression. - virtual void constraintGenerationFailed(Expr *expr); - /// Callback invoked if application of chosen solution to /// expression has failed. virtual void applySolutionFailed(constraints::Solution &solution, Expr *expr); From 754afff034df6f5cbcf9c4a4dfc8f40268f83f01 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 28 Jan 2020 10:53:45 -0800 Subject: [PATCH 07/12] [Constraint solver] Remove ExprTypeCheckListener::preCheckFailed(). Failures will be reported to clients; this callback is unnecessary. --- lib/Sema/TypeCheckConstraints.cpp | 8 -------- lib/Sema/TypeChecker.h | 4 ---- 2 files changed, 12 deletions(-) diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 4f8bbace048f8..834e00b6fcf80 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -2035,7 +2035,6 @@ Expr *ExprTypeCheckListener::appliedSolution(Solution &solution, Expr *expr) { return expr; } -void ExprTypeCheckListener::preCheckFailed(Expr *expr) {} void ExprTypeCheckListener::applySolutionFailed(Solution &solution, Expr *expr) {} @@ -2097,12 +2096,6 @@ class FallbackDiagnosticListener : public ExprTypeCheckListener { return BaseListener ? BaseListener->appliedSolution(solution, expr) : expr; } - void preCheckFailed(Expr *expr) override { - if (BaseListener) - BaseListener->preCheckFailed(expr); - maybeProduceFallbackDiagnostic(expr); - } - void applySolutionFailed(Solution &solution, Expr *expr) override { if (BaseListener) BaseListener->applySolutionFailed(solution, expr); @@ -2169,7 +2162,6 @@ Type TypeChecker::typeCheckExpressionImpl(Expr *&expr, DeclContext *dc, // First, pre-check the expression, validating any types that occur in the // expression and folding sequence expressions. if (ConstraintSystem::preCheckExpression(expr, dc, baseCS)) { - listener.preCheckFailed(expr); return Type(); } diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index 09068a8443e18..27fc91a4570fe 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -321,10 +321,6 @@ class ExprTypeCheckListener { virtual Expr *appliedSolution(constraints::Solution &solution, Expr *expr); - /// Callback invoked if expression is structurally unsound and can't - /// be correctly processed by the constraint solver. - virtual void preCheckFailed(Expr *expr); - /// Callback invoked if application of chosen solution to /// expression has failed. virtual void applySolutionFailed(constraints::Solution &solution, Expr *expr); From f8fd19729528f33f59ca17909c33725d4be41910 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 28 Jan 2020 10:59:45 -0800 Subject: [PATCH 08/12] [Constraint solver] Remove ExprTypeCheckListener::applySolutionFailed. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don’t need this fallback path. --- lib/Sema/TypeCheckConstraints.cpp | 44 ------------------------------- lib/Sema/TypeChecker.h | 4 --- 2 files changed, 48 deletions(-) diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 834e00b6fcf80..29d7dd96c3066 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -2035,9 +2035,6 @@ Expr *ExprTypeCheckListener::appliedSolution(Solution &solution, Expr *expr) { return expr; } -void ExprTypeCheckListener::applySolutionFailed(Solution &solution, - Expr *expr) {} - void ParentConditionalConformance::diagnoseConformanceStack( DiagnosticEngine &diags, SourceLoc loc, ArrayRef conformances) { @@ -2095,45 +2092,6 @@ class FallbackDiagnosticListener : public ExprTypeCheckListener { Expr *appliedSolution(Solution &solution, Expr *expr) override { return BaseListener ? BaseListener->appliedSolution(solution, expr) : expr; } - - void applySolutionFailed(Solution &solution, Expr *expr) override { - if (BaseListener) - BaseListener->applySolutionFailed(solution, expr); - - if (hadAnyErrors()) - return; - - // If solution involves invalid or incomplete conformances that's - // a probable cause of failure to apply it without producing an error, - // which is going to be diagnosed later, so let's not produce - // fallback diagnostic in this case. - if (llvm::any_of( - solution.Conformances, - [](const std::pair - &conformance) -> bool { - auto &ref = conformance.second; - return ref.isConcrete() && ref.getConcrete()->isInvalid(); - })) - return; - - maybeProduceFallbackDiagnostic(expr); - } - -private: - bool hadAnyErrors() const { return Context.Diags.hadAnyError(); } - - void maybeProduceFallbackDiagnostic(Expr *expr) const { - if (Options.contains(TypeCheckExprFlags::SubExpressionDiagnostics) || - DiagnosticSuppression::isEnabled(Context.Diags)) - return; - - // Before producing fatal error here, let's check if there are any "error" - // diagnostics already emitted or waiting to be emitted. Because they are - // a better indication of the problem. - if (!(hadAnyErrors() || Context.hasDelayedConformanceErrors())) - Context.Diags.diagnose(expr->getLoc(), - diag::failed_to_produce_diagnostic); - } }; #pragma mark High-level entry points @@ -2259,8 +2217,6 @@ Type TypeChecker::typeCheckExpressionImpl(Expr *&expr, DeclContext *dc, options.contains(TypeCheckExprFlags::SubExpressionDiagnostics); auto resultTarget = cs.applySolution(solution, target, performingDiagnostics); if (!resultTarget) { - listener.applySolutionFailed(solution, expr); - // Failure already diagnosed, above, as part of applying the solution. return Type(); } diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index 27fc91a4570fe..71dab9dc6f570 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -320,10 +320,6 @@ class ExprTypeCheckListener { /// failure. virtual Expr *appliedSolution(constraints::Solution &solution, Expr *expr); - - /// Callback invoked if application of chosen solution to - /// expression has failed. - virtual void applySolutionFailed(constraints::Solution &solution, Expr *expr); }; /// A conditional conformance that implied some other requirements. That is, \c From 5c6608f5ca1b0d7faef732a7350ddc4c68cdccf1 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 28 Jan 2020 11:05:00 -0800 Subject: [PATCH 09/12] [Constraint solver] Remove the FallbackDiagnosticListener. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It doesn’t do anything any more, because we’re handling error diagnostics locally without callbacks. Also collapse typeCheckExpressionImpl() into typeCheckExpression(). --- lib/Sema/TypeCheckConstraints.cpp | 52 +++---------------------------- lib/Sema/TypeChecker.h | 9 ------ 2 files changed, 5 insertions(+), 56 deletions(-) diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 29d7dd96c3066..74b6424a0da22 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -2062,38 +2062,6 @@ bool GenericRequirementsCheckListener::diagnoseUnsatisfiedRequirement( return false; } -/// Sometimes constraint solver fails without producing any diagnostics, -/// that leads to crashes down the line in AST Verifier or SILGen -/// which, as a result, are much harder to figure out. -/// -/// This class is intended to guard against situations like that by -/// keeping track of failures of different type-check phases, and -/// emitting fallback fatal error if any of them fail without producing -/// error diagnostic, and there were no errors emitted or scheduled to be -/// emitted previously. -class FallbackDiagnosticListener : public ExprTypeCheckListener { - ASTContext &Context; - TypeCheckExprOptions Options; - ExprTypeCheckListener *BaseListener; - -public: - FallbackDiagnosticListener(ASTContext &ctx, TypeCheckExprOptions options, - ExprTypeCheckListener *base) - : Context(ctx), Options(options), BaseListener(base) {} - - bool builtConstraints(ConstraintSystem &cs, Expr *expr) override { - return BaseListener ? BaseListener->builtConstraints(cs, expr) : false; - } - - Expr *foundSolution(Solution &solution, Expr *expr) override { - return BaseListener ? BaseListener->foundSolution(solution, expr) : expr; - } - - Expr *appliedSolution(Solution &solution, Expr *expr) override { - return BaseListener ? BaseListener->appliedSolution(solution, expr) : expr; - } -}; - #pragma mark High-level entry points Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc, TypeLoc convertType, @@ -2102,18 +2070,6 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc, ExprTypeCheckListener *listener, ConstraintSystem *baseCS) { auto &Context = dc->getASTContext(); - FallbackDiagnosticListener diagListener(Context, options, listener); - return typeCheckExpressionImpl(expr, dc, convertType, convertTypePurpose, - options, diagListener, baseCS); -} - -Type TypeChecker::typeCheckExpressionImpl(Expr *&expr, DeclContext *dc, - TypeLoc convertType, - ContextualTypePurpose convertTypePurpose, - TypeCheckExprOptions options, - ExprTypeCheckListener &listener, - ConstraintSystem *baseCS) { - auto &Context = dc->getASTContext(); FrontendStatsTracer StatsTracer(Context.Stats, "typecheck-expr", expr); PrettyStackTraceExpr stackTrace(Context, "type-checking", expr); @@ -2188,7 +2144,7 @@ Type TypeChecker::typeCheckExpressionImpl(Expr *&expr, DeclContext *dc, SmallVector viable; // Attempt to solve the constraint system. - if (cs.solve(expr, convertTo, &listener, viable, allowFreeTypeVariables)) + if (cs.solve(expr, convertTo, listener, viable, allowFreeTypeVariables)) return Type(); // If the client allows the solution to have unresolved type expressions, @@ -2202,7 +2158,8 @@ Type TypeChecker::typeCheckExpressionImpl(Expr *&expr, DeclContext *dc, auto result = expr; auto &solution = viable[0]; - result = listener.foundSolution(solution, result); + if (listener) + result = listener->foundSolution(solution, result); if (!result) return Type(); @@ -2223,7 +2180,8 @@ Type TypeChecker::typeCheckExpressionImpl(Expr *&expr, DeclContext *dc, result = resultTarget->getAsExpr(); // Notify listener that we've applied the solution. - result = listener.appliedSolution(solution, result); + if (listener) + result = listener->appliedSolution(solution, result); if (!result) return Type(); diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index 71dab9dc6f570..c82063b17044a 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -856,15 +856,6 @@ class TypeChecker final { TypeCheckExprOptions(), listener); } -private: - static Type typeCheckExpressionImpl(Expr *&expr, DeclContext *dc, - TypeLoc convertType, - ContextualTypePurpose convertTypePurpose, - TypeCheckExprOptions options, - ExprTypeCheckListener &listener, - constraints::ConstraintSystem *baseCS); - -public: /// Type check the given expression and return its type without /// applying the solution. /// From 40ca1ef1647d72de6ff6bf91c8ab5e83e280e12e Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 28 Jan 2020 11:13:55 -0800 Subject: [PATCH 10/12] [Constraint solver] Eliminate ExprTypeCheckListener::foundSolution() Only one client was using this, and its logic is trivially inlined into appliedSolution(). --- lib/Sema/TypeCheckConstraints.cpp | 21 ++++++--------------- lib/Sema/TypeChecker.h | 7 ------- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 74b6424a0da22..a204332a66607 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -2027,10 +2027,6 @@ bool ExprTypeCheckListener::builtConstraints(ConstraintSystem &cs, Expr *expr) { return false; } -Expr *ExprTypeCheckListener::foundSolution(Solution &solution, Expr *expr) { - return expr; -} - Expr *ExprTypeCheckListener::appliedSolution(Solution &solution, Expr *expr) { return expr; } @@ -2158,8 +2154,6 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc, auto result = expr; auto &solution = viable[0]; - if (listener) - result = listener->foundSolution(solution, result); if (!result) return Type(); @@ -2578,16 +2572,13 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer, return false; } - Expr *foundSolution(Solution &solution, Expr *expr) override { - // Figure out what type the constraints decided on. - auto ty = solution.simplifyType(initType); - initType = ty->getRValueType()->reconstituteSugar(/*recursive =*/false); - - // Just keep going. - return expr; - } - Expr *appliedSolution(Solution &solution, Expr *expr) override { + { + // Figure out what type the constraints decided on. + auto ty = solution.simplifyType(initType); + initType = ty->getRValueType()->reconstituteSugar(/*recursive =*/false); + } + // Convert the initializer to the type of the pattern. expr = solution.coerceToType(expr, initType, Locator); if (!expr) diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index c82063b17044a..3c643d706e25e 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -305,13 +305,6 @@ class ExprTypeCheckListener { /// constraint system, or false otherwise. virtual bool builtConstraints(constraints::ConstraintSystem &cs, Expr *expr); - /// Callback invoked once a solution has been found. - /// - /// The callback may further alter the expression, returning either a - /// new expression (to replace the result) or a null pointer to indicate - /// failure. - virtual Expr *foundSolution(constraints::Solution &solution, Expr *expr); - /// Callback invokes once the chosen solution has been applied to the /// expression. /// From e0702d9f2e333ad1c9f0a5a88dec67caca99cc62 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 30 Jan 2020 21:51:57 -0800 Subject: [PATCH 11/12] [Constraint solver] Generalize solve() for arbitrary solution targets. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Start cleaning up the main “solve” entry point for solving an expression and applying the solution, so it handles arbitrary solution targets. This is another small step that doesn’t do much on its own, but will help with unifying the various places in the code base where we run the solver. --- lib/Sema/CSApply.cpp | 3 +- lib/Sema/CSSolver.cpp | 81 ++++++++++++++++--------------- lib/Sema/ConstraintSystem.h | 42 ++++++++++------ lib/Sema/TypeCheckConstraints.cpp | 44 +++++++++-------- 4 files changed, 93 insertions(+), 77 deletions(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index fa7d40e7b28c6..443f67d35fd29 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -7331,7 +7331,8 @@ Optional ConstraintSystem::applySolution( // If we're supposed to convert the expression to some particular type, // do so now. if (shouldCoerceToContextualType()) { - resultExpr = rewriter.coerceToType(resultExpr, convertType, + resultExpr = rewriter.coerceToType(resultExpr, + simplifyType(convertType), getConstraintLocator(expr)); } else if (getType(resultExpr)->hasLValueType() && !target.isDiscardedExpr()) { diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 223fd0e3d6793..2d07b9245ba8b 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1074,7 +1074,8 @@ void ConstraintSystem::shrink(Expr *expr) { } } -static bool debugConstraintSolverForExpr(ASTContext &C, Expr *expr) { +static bool debugConstraintSolverForTarget( + ASTContext &C, SolutionApplicationTarget target) { if (C.TypeCheckerOpts.DebugConstraintSolver) return true; @@ -1082,14 +1083,13 @@ static bool debugConstraintSolverForExpr(ASTContext &C, Expr *expr) { // No need to compute the line number to find out it's not present. return false; - // Get the lines on which the expression starts and ends. + // Get the lines on which the target starts and ends. unsigned startLine = 0, endLine = 0; - if (expr->getSourceRange().isValid()) { - auto range = - Lexer::getCharSourceRangeFromSourceRange(C.SourceMgr, - expr->getSourceRange()); - startLine = C.SourceMgr.getLineNumber(range.getStart()); - endLine = C.SourceMgr.getLineNumber(range.getEnd()); + SourceRange range = target.getSourceRange(); + if (range.isValid()) { + auto charRange = Lexer::getCharSourceRangeFromSourceRange(C.SourceMgr, range); + startLine = C.SourceMgr.getLineNumber(charRange.getStart()); + endLine = C.SourceMgr.getLineNumber(charRange.getEnd()); } assert(startLine <= endLine && "expr ends before it starts?"); @@ -1107,25 +1107,26 @@ static bool debugConstraintSolverForExpr(ASTContext &C, Expr *expr) { return startBound != endBound; } -bool ConstraintSystem::solve(Expr *&expr, - Type convertType, - ExprTypeCheckListener *listener, - SmallVectorImpl &solutions, - FreeTypeVariableBinding allowFreeTypeVariables) { +Optional> ConstraintSystem::solve( + SolutionApplicationTarget &target, + ExprTypeCheckListener *listener, + FreeTypeVariableBinding allowFreeTypeVariables +) { llvm::SaveAndRestore debugForExpr( getASTContext().TypeCheckerOpts.DebugConstraintSolver, - debugConstraintSolverForExpr(getASTContext(), expr)); + debugConstraintSolverForTarget(getASTContext(), target)); /// Dump solutions for debugging purposes. - auto dumpSolutions = [&] { + auto dumpSolutions = [&](const SolutionResult &result) { // Debug-print the set of solutions. if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) { auto &log = getASTContext().TypeCheckerDebug->getStream(); - if (solutions.size() == 1) { + if (result.getKind() == SolutionResult::Success) { log << "---Solution---\n"; - solutions[0].dump(log); - } else { - for (unsigned i = 0, e = solutions.size(); i != e; ++i) { + result.getSolution().dump(log); + } else if (result.getKind() == SolutionResult::Ambiguous) { + auto solutions = result.getAmbiguousSolutions(); + for (unsigned i : indices(solutions)) { log << "--- Solution #" << i << " ---\n"; solutions[i].dump(log); } @@ -1135,45 +1136,47 @@ bool ConstraintSystem::solve(Expr *&expr, // Take up to two attempts at solving the system. The first attempts to // solve a system that is expected to be well-formed, the second kicks in - // when there is an error and attempts to salvage an ill-formed expression. - SolutionApplicationTarget target(expr, convertType, /*isDiscarded=*/false); + // when there is an error and attempts to salvage an ill-formed program. for (unsigned stage = 0; stage != 2; ++stage) { auto solution = (stage == 0) ? solveImpl(target, listener, allowFreeTypeVariables) : salvage(); switch (solution.getKind()) { - case SolutionResult::Success: + case SolutionResult::Success: { // Return the successful solution. - solutions.clear(); - solutions.push_back(std::move(solution).takeSolution()); - dumpSolutions(); - return false; + dumpSolutions(solution); + std::vector result; + result.push_back(std::move(solution).takeSolution()); + return std::move(result); + } case SolutionResult::Error: - return true; + return None; case SolutionResult::TooComplex: - getASTContext().Diags.diagnose(expr->getLoc(), diag::expression_too_complex) - .highlight(expr->getSourceRange()); + getASTContext().Diags.diagnose( + target.getLoc(), diag::expression_too_complex) + .highlight(target.getSourceRange()); solution.markAsDiagnosed(); - return true; + return None; case SolutionResult::Ambiguous: // If salvaging produced an ambiguous result, it has already been // diagnosed. if (stage == 1) { solution.markAsDiagnosed(); - return true; + return None; } if (Options.contains( ConstraintSystemFlags::AllowUnresolvedTypeVariables)) { + dumpSolutions(solution); auto ambiguousSolutions = std::move(solution).takeAmbiguousSolutions(); - solutions.assign(std::make_move_iterator(ambiguousSolutions.begin()), - std::make_move_iterator(ambiguousSolutions.end())); - dumpSolutions(); - return false; + std::vector result( + std::make_move_iterator(ambiguousSolutions.begin()), + std::make_move_iterator(ambiguousSolutions.end())); + return std::move(result); } LLVM_FALLTHROUGH; @@ -1181,15 +1184,13 @@ bool ConstraintSystem::solve(Expr *&expr, case SolutionResult::UndiagnosedError: if (shouldSuppressDiagnostics()) { solution.markAsDiagnosed(); - return true; + return None; } if (stage == 1) { - diagnoseFailureFor( - SolutionApplicationTarget(expr, convertType, - /*isDiscarded=*/false)); + diagnoseFailureFor(target); solution.markAsDiagnosed(); - return true; + return None; } // Loop again to try to salvage. diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 7d3846e646f7a..e5383f0a7e518 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -1194,6 +1194,11 @@ class SolutionApplicationTarget { return expression.convertType; } + void setExprConversionType(Type type) { + assert(kind == Kind::expression); + expression.convertType = type; + } + bool isDiscardedExpr() const { assert(kind == Kind::expression); return expression.isDiscarded; @@ -1234,6 +1239,18 @@ class SolutionApplicationTarget { return function.body->getSourceRange(); } } + + /// Retrieve the source location for the target. + SourceLoc getLoc() const { + switch (kind) { + case Kind::expression: + return expression.expression->getLoc(); + + case Kind::function: + return function.function.getLoc(); + } + } + /// Walk the contents of the application target. SolutionApplicationTarget walk(ASTWalker &walker); }; @@ -4104,26 +4121,21 @@ class ConstraintSystem { static bool preCheckExpression(Expr *&expr, DeclContext *dc, ConstraintSystem *baseCS = nullptr); - /// Solve the system of constraints generated from provided expression. - /// - /// The expression should have already been pre-checked with - /// preCheckExpression(). + /// Solve the system of constraints generated from provided target. /// - /// \param expr The expression to generate constraints from. - /// \param convertType The expected type of the expression. + /// \param target The target that we'll generate constraints from, which + /// may be updated by the solving process. /// \param listener The callback to check solving progress. - /// \param solutions The set of solutions to the system of constraints. /// \param allowFreeTypeVariables How to bind free type variables in /// the solution. /// - /// \returns true is an error occurred, false is system is consistent - /// and solutions were found. - bool solve(Expr *&expr, - Type convertType, - ExprTypeCheckListener *listener, - SmallVectorImpl &solutions, - FreeTypeVariableBinding allowFreeTypeVariables - = FreeTypeVariableBinding::Disallow); + /// \returns the set of solutions, if any were found, or \c None if an + /// error occurred. When \c None, an error has been emitted. + Optional> solve( + SolutionApplicationTarget &target, + ExprTypeCheckListener *listener, + FreeTypeVariableBinding allowFreeTypeVariables + = FreeTypeVariableBinding::Disallow); /// Solve the system of constraints. /// diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index a204332a66607..0a9f90509187a 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -2138,22 +2138,25 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc, convertTo = getOptionalType(expr->getLoc(), var); } - SmallVector viable; // Attempt to solve the constraint system. - if (cs.solve(expr, convertTo, listener, viable, allowFreeTypeVariables)) + SolutionApplicationTarget target( + expr, convertTo, + options.contains(TypeCheckExprFlags::IsDiscarded)); + auto viable = cs.solve(target, listener, allowFreeTypeVariables); + if (!viable) return Type(); // If the client allows the solution to have unresolved type expressions, // check for them now. We cannot apply the solution with unresolved TypeVars, // because they will leak out into arbitrary places in the resultant AST. if (options.contains(TypeCheckExprFlags::AllowUnresolvedTypeVariables) && - (viable.size() != 1 || + (viable->size() != 1 || (convertType.getType() && convertType.getType()->hasUnresolvedType()))) { return ErrorType::get(Context); } - auto result = expr; - auto &solution = viable[0]; + auto result = target.getAsExpr(); + auto &solution = (*viable)[0]; if (!result) return Type(); @@ -2161,11 +2164,10 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc, cs.applySolution(solution); // Apply the solution to the expression. - SolutionApplicationTarget target( - result, convertType.getType(), - options.contains(TypeCheckExprFlags::IsDiscarded)); bool performingDiagnostics = options.contains(TypeCheckExprFlags::SubExpressionDiagnostics); + // FIXME: HACK! + target.setExprConversionType(convertType.getType()); auto resultTarget = cs.applySolution(solution, target, performingDiagnostics); if (!resultTarget) { // Failure already diagnosed, above, as part of applying the solution. @@ -2244,7 +2246,6 @@ getTypeOfExpressionWithoutApplying(Expr *&expr, DeclContext *dc, ConstraintSystem cs(dc, ConstraintSystemFlags::SuppressDiagnostics); // Attempt to solve the constraint system. - SmallVector viable; const Type originalType = expr->getType(); const bool needClearType = originalType && originalType->hasError(); const auto recoverOriginalType = [&] () { @@ -2256,14 +2257,16 @@ getTypeOfExpressionWithoutApplying(Expr *&expr, DeclContext *dc, // re-check. if (needClearType) expr->setType(Type()); - if (cs.solve(expr, /*convertType*/Type(), listener, viable, - allowFreeTypeVariables)) { + SolutionApplicationTarget target(expr, Type(), /*isDiscarded=*/false); + auto viable = cs.solve(target, listener, allowFreeTypeVariables); + if (!viable) { recoverOriginalType(); return Type(); } // Get the expression's simplified type. - auto &solution = viable[0]; + expr = target.getAsExpr(); + auto &solution = (*viable)[0]; auto &solutionCS = solution.getConstraintSystem(); Type exprType = solution.simplifyType(solutionCS.getType(expr)); @@ -2330,19 +2333,18 @@ void TypeChecker::getPossibleTypesOfExpressionWithoutApplying( ConstraintSystem cs(dc, options); // Attempt to solve the constraint system. - SmallVector viable; - const Type originalType = expr->getType(); if (originalType && originalType->hasError()) expr->setType(Type()); - cs.solve(expr, /*convertType*/ Type(), listener, viable, - allowFreeTypeVariables); - - for (auto &solution : viable) { - auto exprType = solution.simplifyType(cs.getType(expr)); - assert(exprType && !exprType->hasTypeVariable()); - types.insert(exprType.getPointer()); + SolutionApplicationTarget target(expr, Type(), /*isDiscarded=*/false); + if (auto viable = cs.solve(target, listener, allowFreeTypeVariables)) { + expr = target.getAsExpr(); + for (auto &solution : *viable) { + auto exprType = solution.simplifyType(cs.getType(expr)); + assert(exprType && !exprType->hasTypeVariable()); + types.insert(exprType.getPointer()); + } } } From ce357312b6b8164e2e9d8015d17215f9a5d0b6fd Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 30 Jan 2020 21:59:39 -0800 Subject: [PATCH 12/12] [Constraint solver] Reinstate the fallback diagnostic, just in case. --- lib/Sema/CSSolver.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 2d07b9245ba8b..9e8c3678a7f2f 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1107,6 +1107,24 @@ static bool debugConstraintSolverForTarget( return startBound != endBound; } +/// If we aren't certain that we've emitted a diagnostic, emit a fallback +/// diagnostic. +static void maybeProduceFallbackDiagnostic( + ConstraintSystem &cs, SolutionApplicationTarget target) { + if (cs.Options.contains(ConstraintSystemFlags::SubExpressionDiagnostics) || + cs.Options.contains(ConstraintSystemFlags::SuppressDiagnostics)) + return; + + // Before producing fatal error here, let's check if there are any "error" + // diagnostics already emitted or waiting to be emitted. Because they are + // a better indication of the problem. + ASTContext &ctx = cs.getASTContext(); + if (ctx.Diags.hadAnyError() || ctx.hasDelayedConformanceErrors()) + return; + + ctx.Diags.diagnose(target.getLoc(), diag::failed_to_produce_diagnostic); +} + Optional> ConstraintSystem::solve( SolutionApplicationTarget &target, ExprTypeCheckListener *listener, @@ -1152,6 +1170,7 @@ Optional> ConstraintSystem::solve( } case SolutionResult::Error: + maybeProduceFallbackDiagnostic(*this, target); return None; case SolutionResult::TooComplex: