Skip to content

Commit 35202ab

Browse files
committed
[TypeChecker] Always emit a fallback error if type-check failed without producing one
Sometimes constraint solver fails without producing any diagnostics, it could happen during different phases e.g. pre-check, constraint generation, or even while attempting to apply solution. Such behavior leads to crashes down the line in AST Verifier or SILGen which are hard to diagnose. Let's guard against that by tracking if solver produced any diagnostics upon its failure and if no errors were or are scheduled to be produced, let's produce a fallback fatal error pointing at affected expression. Resolves: rdar://problem/38885760
1 parent 709aee7 commit 35202ab

File tree

7 files changed

+169
-16
lines changed

7 files changed

+169
-16
lines changed

include/swift/AST/ASTContext.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -758,6 +758,10 @@ class ASTContext final {
758758
bool IsError;
759759
};
760760

761+
/// Check whether current context has any errors associated with
762+
/// ill-formed protocol conformances which haven't been produced yet.
763+
bool hasDelayedConformanceErrors() const;
764+
761765
/// Add a delayed diagnostic produced while type-checking a
762766
/// particular protocol conformance.
763767
void addDelayedConformanceDiag(NormalProtocolConformance *conformance,

include/swift/AST/DiagnosticsSema.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2767,6 +2767,10 @@ ERROR(type_of_expression_is_ambiguous,none,
27672767
ERROR(specific_type_of_expression_is_ambiguous,none,
27682768
"expression type %0 is ambiguous without more context", (Type))
27692769

2770+
ERROR(failed_to_produce_diagnostic,Fatal,
2771+
"failed to produce diagnostic for expression; "
2772+
"please file a bug report with your project", ())
2773+
27702774

27712775
ERROR(missing_protocol,none,
27722776
"missing protocol %0", (Identifier))

lib/AST/ASTContext.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1999,6 +1999,19 @@ LazyGenericContextData *ASTContext::getOrCreateLazyGenericContextData(
19991999
lazyLoader);
20002000
}
20012001

2002+
bool ASTContext::hasDelayedConformanceErrors() const {
2003+
for (const auto &entry : getImpl().DelayedConformanceDiags) {
2004+
auto &diagnostics = entry.getSecond();
2005+
if (std::any_of(diagnostics.begin(), diagnostics.end(),
2006+
[](const ASTContext::DelayedConformanceDiag &diag) {
2007+
return diag.IsError;
2008+
}))
2009+
return true;
2010+
}
2011+
2012+
return false;
2013+
}
2014+
20022015
void ASTContext::addDelayedConformanceDiag(
20032016
NormalProtocolConformance *conformance,
20042017
DelayedConformanceDiag fn) {

lib/Sema/CSDiag.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1892,6 +1892,10 @@ Expr *FailureDiagnosis::typeCheckChildIndependently(
18921892
// the context is missing).
18931893
TypeCheckExprOptions TCEOptions = TypeCheckExprFlags::DisableStructuralChecks;
18941894

1895+
// Make sure that typechecker knows that this is an attempt
1896+
// to diagnose a problem.
1897+
TCEOptions |= TypeCheckExprFlags::SubExpressionDiagnostics;
1898+
18951899
// Don't walk into non-single expression closure bodies, because
18961900
// ExprTypeSaver and TypeNullifier skip them too.
18971901
TCEOptions |= TypeCheckExprFlags::SkipMultiStmtClosures;

lib/Sema/CSSolver.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,6 +1089,8 @@ ConstraintSystem::solveImpl(Expr *&expr,
10891089
if (auto generatedExpr = generateConstraints(expr))
10901090
expr = generatedExpr;
10911091
else {
1092+
if (listener)
1093+
listener->constraintGenerationFailed(expr);
10921094
return SolutionKind::Error;
10931095
}
10941096

lib/Sema/TypeCheckConstraints.cpp

Lines changed: 114 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "swift/AST/NameLookup.h"
3030
#include "swift/AST/ParameterList.h"
3131
#include "swift/AST/PrettyStackTrace.h"
32+
#include "swift/AST/ProtocolConformance.h"
3233
#include "swift/AST/SubstitutionMap.h"
3334
#include "swift/AST/TypeCheckerDebugConsumer.h"
3435
#include "swift/Basic/Statistic.h"
@@ -1914,6 +1915,11 @@ Expr *ExprTypeCheckListener::appliedSolution(Solution &solution, Expr *expr) {
19141915
return expr;
19151916
}
19161917

1918+
void ExprTypeCheckListener::preCheckFailed(Expr *expr) {}
1919+
void ExprTypeCheckListener::constraintGenerationFailed(Expr *expr) {}
1920+
void ExprTypeCheckListener::applySolutionFailed(Solution &solution,
1921+
Expr *expr) {}
1922+
19171923
void ParentConditionalConformance::diagnoseConformanceStack(
19181924
DiagnosticEngine &diags, SourceLoc loc,
19191925
ArrayRef<ParentConditionalConformance> conformances) {
@@ -1941,20 +1947,116 @@ bool GenericRequirementsCheckListener::diagnoseUnsatisfiedRequirement(
19411947
return false;
19421948
}
19431949

1950+
/// Sometimes constraint solver fails without producing any diagnostics,
1951+
/// that leads to crashes down the line in AST Verifier or SILGen
1952+
/// which, as a result, are much harder to figure out.
1953+
///
1954+
/// This class is intended to guard against situations like that by
1955+
/// keeping track of failures of different type-check phases, and
1956+
/// emitting fallback fatal error if any of them fail without producing
1957+
/// error diagnostic, and there were no errors emitted or scheduled to be
1958+
/// emitted previously.
1959+
class FallbackDiagnosticListener : public ExprTypeCheckListener {
1960+
TypeChecker &TC;
1961+
TypeCheckExprOptions Options;
1962+
ExprTypeCheckListener *BaseListener;
1963+
1964+
public:
1965+
FallbackDiagnosticListener(TypeChecker &TC, TypeCheckExprOptions options,
1966+
ExprTypeCheckListener *base)
1967+
: TC(TC), Options(options), BaseListener(base) {}
1968+
1969+
bool builtConstraints(ConstraintSystem &cs, Expr *expr) override {
1970+
return BaseListener ? BaseListener->builtConstraints(cs, expr) : false;
1971+
}
1972+
1973+
Expr *foundSolution(Solution &solution, Expr *expr) override {
1974+
return BaseListener ? BaseListener->foundSolution(solution, expr) : expr;
1975+
}
1976+
1977+
Expr *appliedSolution(Solution &solution, Expr *expr) override {
1978+
return BaseListener ? BaseListener->appliedSolution(solution, expr) : expr;
1979+
}
1980+
1981+
void preCheckFailed(Expr *expr) override {
1982+
if (BaseListener)
1983+
BaseListener->preCheckFailed(expr);
1984+
maybeProduceFallbackDiagnostic(expr);
1985+
}
1986+
1987+
void constraintGenerationFailed(Expr *expr) override {
1988+
if (BaseListener)
1989+
BaseListener->constraintGenerationFailed(expr);
1990+
maybeProduceFallbackDiagnostic(expr);
1991+
}
1992+
1993+
void applySolutionFailed(Solution &solution, Expr *expr) override {
1994+
if (BaseListener)
1995+
BaseListener->applySolutionFailed(solution, expr);
1996+
1997+
if (hadAnyErrors())
1998+
return;
1999+
2000+
// If solution involves invalid or incomplete conformances that's
2001+
// a probable cause of failure to apply it without producing an error,
2002+
// which is going to be diagnosed later, so let's not produce
2003+
// fallback diagnostic in this case.
2004+
if (llvm::any_of(
2005+
solution.Conformances,
2006+
[](const std::pair<ConstraintLocator *, ProtocolConformanceRef>
2007+
&conformance) -> bool {
2008+
auto &ref = conformance.second;
2009+
return ref.isConcrete() && (ref.getConcrete()->isInvalid() ||
2010+
ref.getConcrete()->isIncomplete());
2011+
}))
2012+
return;
2013+
2014+
maybeProduceFallbackDiagnostic(expr);
2015+
}
2016+
2017+
private:
2018+
bool hadAnyErrors() const { return TC.Context.Diags.hadAnyError(); }
2019+
2020+
void maybeProduceFallbackDiagnostic(Expr *expr) const {
2021+
if (Options.contains(TypeCheckExprFlags::SubExpressionDiagnostics) ||
2022+
Options.contains(TypeCheckExprFlags::SuppressDiagnostics))
2023+
return;
2024+
2025+
// Before producing fatal error here, let's check if there are any "error"
2026+
// diagnostics already emitted or waiting to be emitted. Because they are
2027+
// a better indication of the problem.
2028+
if (!(hadAnyErrors() || TC.Context.hasDelayedConformanceErrors()))
2029+
TC.diagnose(expr->getLoc(), diag::failed_to_produce_diagnostic);
2030+
}
2031+
};
2032+
19442033
#pragma mark High-level entry points
19452034
Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
19462035
TypeLoc convertType,
19472036
ContextualTypePurpose convertTypePurpose,
19482037
TypeCheckExprOptions options,
19492038
ExprTypeCheckListener *listener,
19502039
ConstraintSystem *baseCS) {
2040+
FallbackDiagnosticListener diagListener(*this, options, listener);
2041+
return typeCheckExpressionImpl(expr, dc, convertType, convertTypePurpose,
2042+
options, diagListener, baseCS);
2043+
}
2044+
2045+
Type TypeChecker::typeCheckExpressionImpl(Expr *&expr, DeclContext *dc,
2046+
TypeLoc convertType,
2047+
ContextualTypePurpose convertTypePurpose,
2048+
TypeCheckExprOptions options,
2049+
ExprTypeCheckListener &listener,
2050+
ConstraintSystem *baseCS) {
19512051
FrontendStatsTracer StatsTracer(Context.Stats, "typecheck-expr", expr);
19522052
PrettyStackTraceExpr stackTrace(Context, "type-checking", expr);
19532053

19542054
// First, pre-check the expression, validating any types that occur in the
19552055
// expression and folding sequence expressions.
1956-
if (preCheckExpression(expr, dc))
2056+
if (preCheckExpression(expr, dc)) {
2057+
listener.preCheckFailed(expr);
19572058
return Type();
2059+
}
19582060

19592061
// Construct a constraint system from this expression.
19602062
ConstraintSystemOptions csOptions = ConstraintSystemFlags::AllowFixes;
@@ -2009,10 +2111,9 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
20092111
convertTo = getOptionalType(expr->getLoc(), var);
20102112
}
20112113

2012-
// Attempt to solve the constraint system.
20132114
SmallVector<Solution, 4> viable;
2014-
if (cs.solve(expr, convertTo, listener, viable,
2015-
allowFreeTypeVariables))
2115+
// Attempt to solve the constraint system.
2116+
if (cs.solve(expr, convertTo, &listener, viable, allowFreeTypeVariables))
20162117
return Type();
20172118

20182119
// If the client allows the solution to have unresolved type expressions,
@@ -2026,29 +2127,26 @@ Type TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
20262127

20272128
auto result = expr;
20282129
auto &solution = viable[0];
2029-
if (listener) {
2030-
result = listener->foundSolution(solution, result);
2031-
if (!result)
2032-
return Type();
2033-
}
2130+
result = listener.foundSolution(solution, result);
2131+
if (!result)
2132+
return Type();
20342133

20352134
// Apply the solution to the expression.
20362135
result = cs.applySolution(
20372136
solution, result, convertType.getType(),
20382137
options.contains(TypeCheckExprFlags::IsDiscarded),
20392138
options.contains(TypeCheckExprFlags::SkipMultiStmtClosures));
2139+
20402140
if (!result) {
2141+
listener.applySolutionFailed(solution, expr);
20412142
// Failure already diagnosed, above, as part of applying the solution.
20422143
return Type();
20432144
}
20442145

2045-
// If there's a listener, notify it that we've applied the solution.
2046-
if (listener) {
2047-
result = listener->appliedSolution(solution, result);
2048-
if (!result) {
2049-
return Type();
2050-
}
2051-
}
2146+
// Notify listener that we've applied the solution.
2147+
result = listener.appliedSolution(solution, result);
2148+
if (!result)
2149+
return Type();
20522150

20532151
if (getLangOpts().DebugConstraintSolver) {
20542152
auto &log = Context.TypeCheckerDebug->getStream();

lib/Sema/TypeChecker.h

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,14 @@ enum class TypeCheckExprFlags {
263263
/// If set, a conversion constraint should be specified so that the result of
264264
/// the expression is an optional type.
265265
ExpressionTypeMustBeOptional = 0x200,
266+
267+
/// FIXME(diagnostics): Once diagnostics are completely switched to new
268+
/// framework, this flag could be removed as obsolete.
269+
///
270+
/// If set, this is a sub-expression, and it is being re-typechecked
271+
/// as part of the expression diagnostics, which is attempting to narrow
272+
/// down failure location.
273+
SubExpressionDiagnostics = 0x400,
266274
};
267275

268276
using TypeCheckExprOptions = OptionSet<TypeCheckExprFlags>;
@@ -381,6 +389,18 @@ class ExprTypeCheckListener {
381389
/// failure.
382390
virtual Expr *appliedSolution(constraints::Solution &solution,
383391
Expr *expr);
392+
393+
/// Callback invoked if expression is structurally unsound and can't
394+
/// be correctly processed by the constraint solver.
395+
virtual void preCheckFailed(Expr *expr);
396+
397+
/// Callback invoked if constraint system failed to generate
398+
/// constraints for a given expression.
399+
virtual void constraintGenerationFailed(Expr *expr);
400+
401+
/// Callback invoked if application of chosen solution to
402+
/// expression has failed.
403+
virtual void applySolutionFailed(constraints::Solution &solution, Expr *expr);
384404
};
385405

386406
/// A conditional conformance that implied some other requirements. That is, \c
@@ -1330,7 +1350,15 @@ class TypeChecker final : public LazyResolver {
13301350
TypeCheckExprOptions(), listener);
13311351
}
13321352

1353+
private:
1354+
Type typeCheckExpressionImpl(Expr *&expr, DeclContext *dc,
1355+
TypeLoc convertType,
1356+
ContextualTypePurpose convertTypePurpose,
1357+
TypeCheckExprOptions options,
1358+
ExprTypeCheckListener &listener,
1359+
constraints::ConstraintSystem *baseCS);
13331360

1361+
public:
13341362
/// Type check the given expression and return its type without
13351363
/// applying the solution.
13361364
///

0 commit comments

Comments
 (0)