Skip to content

Commit 4e6677e

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
1 parent 5520ca2 commit 4e6677e

File tree

7 files changed

+101
-25
lines changed

7 files changed

+101
-25
lines changed

lib/Sema/CSDiag.cpp

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

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

20582058
/// For an expression being type checked with a CTP_CalleeResult contextual
20592059
/// type, try to diagnose a problem.
20602060
bool diagnoseCalleeResultContextualConversionError();
20612061

2062+
/// Attempt to produce a diagnostic for a mismatch between a call's
2063+
/// type and its assumed contextual type.
2064+
bool diagnoseCallContextualConversionErrors(ApplyExpr *callEpxr,
2065+
Type contextualType);
2066+
20622067
private:
20632068
/// Validate potential contextual type for type-checking one of the
20642069
/// sub-expressions, usually correct/valid types are the ones which
@@ -3856,10 +3861,10 @@ static bool tryDiagnoseNonEscapingParameterToEscaping(Expr *expr, Type srcType,
38563861
return true;
38573862
}
38583863

3859-
bool FailureDiagnosis::diagnoseContextualConversionError() {
3864+
bool FailureDiagnosis::diagnoseContextualConversionError(Expr *expr,
3865+
Type contextualType) {
38603866
// If the constraint system has a contextual type, then we can test to see if
38613867
// this is the problem that prevents us from solving the system.
3862-
Type contextualType = CS->getContextualType();
38633868
if (!contextualType) {
38643869
// This contextual conversion constraint doesn't install an actual type.
38653870
if (CS->getContextualTypePurpose() == CTP_CalleeResult)
@@ -6068,7 +6073,7 @@ bool FailureDiagnosis::diagnoseTrailingClosureErrors(ApplyExpr *callExpr) {
60686073
// related to function type, let's try to diagnose it.
60696074
if (possibleTypes.empty() && contextualType &&
60706075
!contextualType->hasUnresolvedType())
6071-
return diagnoseContextualConversionError();
6076+
return diagnoseContextualConversionError(callExpr, contextualType);
60726077
} else {
60736078
possibleTypes.push_back(currentType);
60746079
}
@@ -6164,6 +6169,49 @@ bool FailureDiagnosis::diagnoseTrailingClosureErrors(ApplyExpr *callExpr) {
61646169
return false;
61656170
}
61666171

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

6223+
if (diagnoseCallContextualConversionErrors(callExpr, CS->getContextualType()))
6224+
return true;
6225+
61756226
// Type check the function subexpression to resolve a type for it if
61766227
// possible.
61776228
auto fnExpr = typeCheckChildIndependently(callExpr->getFn());
@@ -8516,7 +8567,7 @@ void ConstraintSystem::diagnoseFailureForExpr(Expr *expr) {
85168567
return;
85178568

85188569
// If this is a contextual conversion problem, dig out some information.
8519-
if (diagnosis.diagnoseContextualConversionError())
8570+
if (diagnosis.diagnoseContextualConversionError(expr, getContextualType()))
85208571
return;
85218572

85228573
// 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
@@ -1987,40 +1987,37 @@ void TypeChecker::getPossibleTypesOfExpressionWithoutApplying(
19871987
ExprTypeCheckListener *listener) {
19881988
PrettyStackTraceExpr stackTrace(Context, "type-checking", expr);
19891989

1990+
ExprCleaner cleaner(expr);
1991+
19901992
// Construct a constraint system from this expression.
19911993
ConstraintSystemOptions options;
19921994
options |= ConstraintSystemFlags::AllowFixes;
19931995
options |= ConstraintSystemFlags::ReturnAllDiscoveredSolutions;
19941996

19951997
ConstraintSystem cs(*this, dc, options);
1996-
CleanupIllFormedExpressionRAII cleanup(Context, expr);
19971998

19981999
// Attempt to solve the constraint system.
19992000
SmallVector<Solution, 4> viable;
2000-
const Type originalType = expr->getType();
2001-
const bool needClearType = originalType && originalType->hasError();
2002-
const auto recoverOriginalType = [&]() {
2003-
if (needClearType)
2004-
expr->setType(originalType);
2005-
};
20062001

20072002
// If the previous checking gives the expr error type,
20082003
// clear the result and re-check.
2009-
if (needClearType)
2010-
expr->setType(Type());
2004+
{
2005+
CleanupIllFormedExpressionRAII cleanup(Context, expr);
20112006

2012-
solveForExpression(expr, dc, /*convertType*/ Type(), allowFreeTypeVariables,
2013-
listener, cs, viable,
2014-
TypeCheckExprFlags::SuppressDiagnostics);
2007+
const Type originalType = expr->getType();
2008+
if (originalType && originalType->hasError())
2009+
expr->setType(Type());
2010+
2011+
solveForExpression(expr, dc, /*convertType*/ Type(), allowFreeTypeVariables,
2012+
listener, cs, viable,
2013+
TypeCheckExprFlags::SuppressDiagnostics);
2014+
}
20152015

20162016
for (auto &solution : viable) {
20172017
auto exprType = solution.simplifyType(cs.getType(expr));
20182018
assert(exprType && !exprType->hasTypeVariable());
20192019
types.push_back(exprType);
20202020
}
2021-
2022-
// Recover the original type if needed.
2023-
recoverOriginalType();
20242021
}
20252022

20262023
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: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,3 +1001,31 @@ func rdar32726044() -> Float {
10011001
let _: Float = Float(42) + 0 // Ok
10021002
return f
10031003
}
1004+
1005+
// SR-5045 - Attempting to return result of reduce(_:_:) in a method with no return produces ambiguous error
1006+
func sr5045() {
1007+
let doubles: [Double] = [1, 2, 3]
1008+
return doubles.reduce(0, +)
1009+
// expected-error@-1 {{unexpected non-void return value in void function}}
1010+
}
1011+
1012+
// rdar://problem/32934129 - QoI: misleading diagnostic
1013+
class L_32934129<T : Comparable> {
1014+
init(_ value: T) { self.value = value }
1015+
init(_ value: T, _ next: L_32934129<T>?) {
1016+
self.value = value
1017+
self.next = next
1018+
}
1019+
1020+
var value: T
1021+
var next: L_32934129<T>? = nil
1022+
1023+
func length() -> Int {
1024+
func inner(_ list: L_32934129<T>?, _ count: Int) {
1025+
guard let list = list else { return count } // expected-error {{unexpected non-void return value in void function}}
1026+
return inner(list.next, count + 1)
1027+
}
1028+
1029+
return inner(self, 0) // expected-error {{cannot convert return expression of type '()' to return type 'Int'}}
1030+
}
1031+
}

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)