Skip to content

Commit 3c4469b

Browse files
committed
[QoI] Improve contextual error diagnostics related to calls
Currently some contextual errors are discovered too late which leads to diagnostics of unrelated problems like argument mismatches, these changes attempt to improve the situation and try to diagnose contextual errors related to calls before everything else. Resolves: SR-5045, rdar://problem/32934129 (cherry picked from commit 4e6677e)
1 parent 52a0d92 commit 3c4469b

File tree

7 files changed

+100
-25
lines changed

7 files changed

+100
-25
lines changed

lib/Sema/CSDiag.cpp

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2066,12 +2066,17 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
20662066

20672067
/// Attempt to produce a diagnostic for a mismatch between an expression's
20682068
/// type and its assumed contextual type.
2069-
bool diagnoseContextualConversionError();
2069+
bool diagnoseContextualConversionError(Expr *expr, Type contextualType);
20702070

20712071
/// For an expression being type checked with a CTP_CalleeResult contextual
20722072
/// type, try to diagnose a problem.
20732073
bool diagnoseCalleeResultContextualConversionError();
20742074

2075+
/// Attempt to produce a diagnostic for a mismatch between a call's
2076+
/// type and its assumed contextual type.
2077+
bool diagnoseCallContextualConversionErrors(ApplyExpr *callEpxr,
2078+
Type contextualType);
2079+
20752080
private:
20762081
/// Validate potential contextual type for type-checking one of the
20772082
/// sub-expressions, usually correct/valid types are the ones which
@@ -3863,10 +3868,10 @@ static bool tryDiagnoseNonEscapingParameterToEscaping(Expr *expr, Type srcType,
38633868
return true;
38643869
}
38653870

3866-
bool FailureDiagnosis::diagnoseContextualConversionError() {
3871+
bool FailureDiagnosis::diagnoseContextualConversionError(Expr *expr,
3872+
Type contextualType) {
38673873
// If the constraint system has a contextual type, then we can test to see if
38683874
// this is the problem that prevents us from solving the system.
3869-
Type contextualType = CS->getContextualType();
38703875
if (!contextualType) {
38713876
// This contextual conversion constraint doesn't install an actual type.
38723877
if (CS->getContextualTypePurpose() == CTP_CalleeResult)
@@ -6066,7 +6071,7 @@ bool FailureDiagnosis::diagnoseTrailingClosureErrors(ApplyExpr *callExpr) {
60666071
// related to function type, let's try to diagnose it.
60676072
if (possibleTypes.empty() && contextualType &&
60686073
!contextualType->hasUnresolvedType())
6069-
return diagnoseContextualConversionError();
6074+
return diagnoseContextualConversionError(callExpr, contextualType);
60706075

60716076
auto currentType = fnExpr->getType();
60726077
if (currentType && currentType->is<ErrorType>())
@@ -6166,6 +6171,49 @@ bool FailureDiagnosis::diagnoseTrailingClosureErrors(ApplyExpr *callExpr) {
61666171
return false;
61676172
}
61686173

6174+
/// Check if there failure associated with expresssion is related
6175+
/// to given contextual type.
6176+
bool FailureDiagnosis::diagnoseCallContextualConversionErrors(
6177+
ApplyExpr *callExpr, Type contextualType) {
6178+
if (!contextualType || contextualType->hasUnresolvedType())
6179+
return false;
6180+
6181+
auto &TC = CS->TC;
6182+
auto *DC = CS->DC;
6183+
6184+
auto typeCheckExpr = [](TypeChecker &TC, Expr *expr, DeclContext *DC,
6185+
SmallVectorImpl<Type> &types,
6186+
Type contextualType = Type()) {
6187+
CalleeListener listener(contextualType);
6188+
TC.getPossibleTypesOfExpressionWithoutApplying(
6189+
expr, DC, types, FreeTypeVariableBinding::Disallow, &listener);
6190+
};
6191+
6192+
// First let's type-check expression without contextual type, and
6193+
// see if that's going to produce a type, if so, let's type-check
6194+
// again, this time using given contextual type.
6195+
SmallVector<Type, 4> withoutContextual;
6196+
typeCheckExpr(TC, callExpr, DC, withoutContextual);
6197+
6198+
// If there are no types returned, it means that problem was
6199+
// nothing to do with contextual information, probably parameter/argument
6200+
// mismatch.
6201+
if (withoutContextual.empty())
6202+
return false;
6203+
6204+
SmallVector<Type, 4> withContextual;
6205+
typeCheckExpr(TC, callExpr, DC, withContextual, contextualType);
6206+
// If type-checking with contextual type didn't produce any results
6207+
// it means that we have a contextual mismatch.
6208+
if (withContextual.empty())
6209+
return diagnoseContextualConversionError(callExpr, contextualType);
6210+
6211+
// If call produces a single type when type-checked with contextual
6212+
// expression, it means that the problem is elsewhere, any other
6213+
// outcome is ambiguous.
6214+
return false;
6215+
}
6216+
61696217
bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
61706218
// If this call involves trailing closure as an argument,
61716219
// let's treat it specially, because re-typecheck of the
@@ -6174,6 +6222,9 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
61746222
if (diagnoseTrailingClosureErrors(callExpr))
61756223
return true;
61766224

6225+
if (diagnoseCallContextualConversionErrors(callExpr, CS->getContextualType()))
6226+
return true;
6227+
61776228
// Type check the function subexpression to resolve a type for it if
61786229
// possible.
61796230
auto fnExpr = typeCheckChildIndependently(callExpr->getFn());
@@ -8518,7 +8569,7 @@ void ConstraintSystem::diagnoseFailureForExpr(Expr *expr) {
85188569
return;
85198570

85208571
// If this is a contextual conversion problem, dig out some information.
8521-
if (diagnosis.diagnoseContextualConversionError())
8572+
if (diagnosis.diagnoseContextualConversionError(expr, getContextualType()))
85228573
return;
85238574

85248575
// If we can diagnose a problem based on the constraints left laying around in

lib/Sema/TypeCheckConstraints.cpp

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1980,40 +1980,37 @@ void TypeChecker::getPossibleTypesOfExpressionWithoutApplying(
19801980
ExprTypeCheckListener *listener) {
19811981
PrettyStackTraceExpr stackTrace(Context, "type-checking", expr);
19821982

1983+
ExprCleaner cleaner(expr);
1984+
19831985
// Construct a constraint system from this expression.
19841986
ConstraintSystemOptions options;
19851987
options |= ConstraintSystemFlags::AllowFixes;
19861988
options |= ConstraintSystemFlags::ReturnAllDiscoveredSolutions;
19871989

19881990
ConstraintSystem cs(*this, dc, options);
1989-
CleanupIllFormedExpressionRAII cleanup(Context, expr);
19901991

19911992
// Attempt to solve the constraint system.
19921993
SmallVector<Solution, 4> viable;
1993-
const Type originalType = expr->getType();
1994-
const bool needClearType = originalType && originalType->hasError();
1995-
const auto recoverOriginalType = [&]() {
1996-
if (needClearType)
1997-
expr->setType(originalType);
1998-
};
19991994

20001995
// If the previous checking gives the expr error type,
20011996
// clear the result and re-check.
2002-
if (needClearType)
2003-
expr->setType(Type());
1997+
{
1998+
CleanupIllFormedExpressionRAII cleanup(Context, expr);
20041999

2005-
solveForExpression(expr, dc, /*convertType*/ Type(), allowFreeTypeVariables,
2006-
listener, cs, viable,
2007-
TypeCheckExprFlags::SuppressDiagnostics);
2000+
const Type originalType = expr->getType();
2001+
if (originalType && originalType->hasError())
2002+
expr->setType(Type());
2003+
2004+
solveForExpression(expr, dc, /*convertType*/ Type(), allowFreeTypeVariables,
2005+
listener, cs, viable,
2006+
TypeCheckExprFlags::SuppressDiagnostics);
2007+
}
20082008

20092009
for (auto &solution : viable) {
20102010
auto exprType = solution.simplifyType(cs.getType(expr));
20112011
assert(exprType && !exprType->hasTypeVariable());
20122012
types.push_back(exprType);
20132013
}
2014-
2015-
// Recover the original type if needed.
2016-
recoverOriginalType();
20172014
}
20182015

20192016
bool TypeChecker::typeCheckCompletionSequence(Expr *&expr, DeclContext *DC) {

test/Constraints/array_literal.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ useDoubleList([1.0,2,3])
5050
useDoubleList([1.0,2.0,3.0])
5151

5252
useIntDict(["Niners" => 31, "Ravens" => 34])
53-
useIntDict(["Niners" => 31, "Ravens" => 34.0]) // expected-error{{cannot convert value of type 'Double' to expected argument type 'Int'}}
53+
useIntDict(["Niners" => 31, "Ravens" => 34.0]) // expected-error{{cannot convert value of type '(String, Double)' to expected element type '(String, Int)'}}
5454
// <rdar://problem/22333090> QoI: Propagate contextual information in a call to operands
5555
useDoubleDict(["Niners" => 31, "Ravens" => 34.0])
5656
useDoubleDict(["Niners" => 31.0, "Ravens" => 34])

test/Constraints/diagnostics.swift

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -997,3 +997,30 @@ func rdar32726044() -> Float {
997997
return f
998998
}
999999

1000+
// SR-5045 - Attempting to return result of reduce(_:_:) in a method with no return produces ambiguous error
1001+
func sr5045() {
1002+
let doubles: [Double] = [1, 2, 3]
1003+
return doubles.reduce(0, +)
1004+
// expected-error@-1 {{unexpected non-void return value in void function}}
1005+
}
1006+
1007+
// rdar://problem/32934129 - QoI: misleading diagnostic
1008+
class L_32934129<T : Comparable> {
1009+
init(_ value: T) { self.value = value }
1010+
init(_ value: T, _ next: L_32934129<T>?) {
1011+
self.value = value
1012+
self.next = next
1013+
}
1014+
1015+
var value: T
1016+
var next: L_32934129<T>? = nil
1017+
1018+
func length() -> Int {
1019+
func inner(_ list: L_32934129<T>?, _ count: Int) {
1020+
guard let list = list else { return count } // expected-error {{unexpected non-void return value in void function}}
1021+
return inner(list.next, count + 1)
1022+
}
1023+
1024+
return inner(self, 0) // expected-error {{cannot convert return expression of type '()' to return type 'Int'}}
1025+
}
1026+
}

test/Constraints/fixes.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,4 +143,4 @@ struct S1116 {
143143
}
144144

145145
let a1116: [S1116] = []
146-
var s1116 = Set(1...10).subtracting(a1116.map({ $0.s })) // expected-error {{'map' produces '[T]', not the expected contextual result type 'Set<Int>'}}
146+
var s1116 = Set(1...10).subtracting(a1116.map({ $0.s })) // expected-error {{cannot convert value of type '[Int?]' to expected argument type 'Set<Int>'}}

test/Generics/deduction.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func useIdentity(_ x: Int, y: Float, i32: Int32) {
2222

2323
// Deduction where the result type and input type can get different results
2424
var xx : X, yy : Y
25-
xx = identity(yy) // expected-error{{cannot convert value of type 'Y' to expected argument type 'X'}}
25+
xx = identity(yy) // expected-error{{cannot assign value of type 'Y' to type 'X'}}
2626
xx = identity2(yy) // expected-error{{cannot convert value of type 'Y' to expected argument type 'X'}}
2727
}
2828

test/Serialization/Recovery/typedefs.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ public func testVTableBuilding(user: User) {
3838
} // CHECK-IR: ret void
3939

4040
#if VERIFY
41-
let _: String = useAssoc(ImportedType.self) // expected-error {{cannot convert call result type '_.Assoc?' to expected type 'String'}}
41+
let _: String = useAssoc(ImportedType.self) // expected-error {{cannot convert value of type 'Int32?' to specified type 'String'}}
4242
let _: Bool? = useAssoc(ImportedType.self) // expected-error {{cannot convert value of type 'Int32?' to specified type 'Bool?'}}
4343
let _: Int32? = useAssoc(ImportedType.self)
4444

45-
let _: String = useAssoc(AnotherType.self) // expected-error {{cannot convert call result type '_.Assoc?' to expected type 'String'}}
45+
let _: String = useAssoc(AnotherType.self) // expected-error {{cannot convert value of type 'AnotherType.Assoc?' (aka 'Optional<Int32>') to specified type 'String'}}
4646
let _: Bool? = useAssoc(AnotherType.self) // expected-error {{cannot convert value of type 'AnotherType.Assoc?' (aka 'Optional<Int32>') to specified type 'Bool?'}}
4747
let _: Int32? = useAssoc(AnotherType.self)
4848

0 commit comments

Comments
 (0)