Skip to content

[4.0] [QoI] Improve contextual error diagnostics related to calls #10616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 56 additions & 5 deletions lib/Sema/CSDiag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2066,12 +2066,17 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{

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

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

/// Attempt to produce a diagnostic for a mismatch between a call's
/// type and its assumed contextual type.
bool diagnoseCallContextualConversionErrors(ApplyExpr *callEpxr,
Type contextualType);

private:
/// Validate potential contextual type for type-checking one of the
/// sub-expressions, usually correct/valid types are the ones which
Expand Down Expand Up @@ -3863,10 +3868,10 @@ static bool tryDiagnoseNonEscapingParameterToEscaping(Expr *expr, Type srcType,
return true;
}

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

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

/// Check if there failure associated with expresssion is related
/// to given contextual type.
bool FailureDiagnosis::diagnoseCallContextualConversionErrors(
ApplyExpr *callExpr, Type contextualType) {
if (!contextualType || contextualType->hasUnresolvedType())
return false;

auto &TC = CS->TC;
auto *DC = CS->DC;

auto typeCheckExpr = [](TypeChecker &TC, Expr *expr, DeclContext *DC,
SmallVectorImpl<Type> &types,
Type contextualType = Type()) {
CalleeListener listener(contextualType);
TC.getPossibleTypesOfExpressionWithoutApplying(
expr, DC, types, FreeTypeVariableBinding::Disallow, &listener);
};

// First let's type-check expression without contextual type, and
// see if that's going to produce a type, if so, let's type-check
// again, this time using given contextual type.
SmallVector<Type, 4> withoutContextual;
typeCheckExpr(TC, callExpr, DC, withoutContextual);

// If there are no types returned, it means that problem was
// nothing to do with contextual information, probably parameter/argument
// mismatch.
if (withoutContextual.empty())
return false;

SmallVector<Type, 4> withContextual;
typeCheckExpr(TC, callExpr, DC, withContextual, contextualType);
// If type-checking with contextual type didn't produce any results
// it means that we have a contextual mismatch.
if (withContextual.empty())
return diagnoseContextualConversionError(callExpr, contextualType);

// If call produces a single type when type-checked with contextual
// expression, it means that the problem is elsewhere, any other
// outcome is ambiguous.
return false;
}

bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
// If this call involves trailing closure as an argument,
// let's treat it specially, because re-typecheck of the
Expand All @@ -6174,6 +6222,9 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
if (diagnoseTrailingClosureErrors(callExpr))
return true;

if (diagnoseCallContextualConversionErrors(callExpr, CS->getContextualType()))
return true;

// Type check the function subexpression to resolve a type for it if
// possible.
auto fnExpr = typeCheckChildIndependently(callExpr->getFn());
Expand Down Expand Up @@ -8518,7 +8569,7 @@ void ConstraintSystem::diagnoseFailureForExpr(Expr *expr) {
return;

// If this is a contextual conversion problem, dig out some information.
if (diagnosis.diagnoseContextualConversionError())
if (diagnosis.diagnoseContextualConversionError(expr, getContextualType()))
return;

// If we can diagnose a problem based on the constraints left laying around in
Expand Down
27 changes: 12 additions & 15 deletions lib/Sema/TypeCheckConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1980,40 +1980,37 @@ void TypeChecker::getPossibleTypesOfExpressionWithoutApplying(
ExprTypeCheckListener *listener) {
PrettyStackTraceExpr stackTrace(Context, "type-checking", expr);

ExprCleaner cleaner(expr);

// Construct a constraint system from this expression.
ConstraintSystemOptions options;
options |= ConstraintSystemFlags::AllowFixes;
options |= ConstraintSystemFlags::ReturnAllDiscoveredSolutions;

ConstraintSystem cs(*this, dc, options);
CleanupIllFormedExpressionRAII cleanup(Context, expr);

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

// If the previous checking gives the expr error type,
// clear the result and re-check.
if (needClearType)
expr->setType(Type());
{
CleanupIllFormedExpressionRAII cleanup(Context, expr);

solveForExpression(expr, dc, /*convertType*/ Type(), allowFreeTypeVariables,
listener, cs, viable,
TypeCheckExprFlags::SuppressDiagnostics);
const Type originalType = expr->getType();
if (originalType && originalType->hasError())
expr->setType(Type());

solveForExpression(expr, dc, /*convertType*/ Type(), allowFreeTypeVariables,
listener, cs, viable,
TypeCheckExprFlags::SuppressDiagnostics);
}

for (auto &solution : viable) {
auto exprType = solution.simplifyType(cs.getType(expr));
assert(exprType && !exprType->hasTypeVariable());
types.push_back(exprType);
}

// Recover the original type if needed.
recoverOriginalType();
}

bool TypeChecker::typeCheckCompletionSequence(Expr *&expr, DeclContext *DC) {
Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/array_literal.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ useDoubleList([1.0,2,3])
useDoubleList([1.0,2.0,3.0])

useIntDict(["Niners" => 31, "Ravens" => 34])
useIntDict(["Niners" => 31, "Ravens" => 34.0]) // expected-error{{cannot convert value of type 'Double' to expected argument type 'Int'}}
useIntDict(["Niners" => 31, "Ravens" => 34.0]) // expected-error{{cannot convert value of type '(String, Double)' to expected element type '(String, Int)'}}
// <rdar://problem/22333090> QoI: Propagate contextual information in a call to operands
useDoubleDict(["Niners" => 31, "Ravens" => 34.0])
useDoubleDict(["Niners" => 31.0, "Ravens" => 34])
Expand Down
27 changes: 27 additions & 0 deletions test/Constraints/diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -997,3 +997,30 @@ func rdar32726044() -> Float {
return f
}

// SR-5045 - Attempting to return result of reduce(_:_:) in a method with no return produces ambiguous error
func sr5045() {
let doubles: [Double] = [1, 2, 3]
return doubles.reduce(0, +)
// expected-error@-1 {{unexpected non-void return value in void function}}
}

// rdar://problem/32934129 - QoI: misleading diagnostic
class L_32934129<T : Comparable> {
init(_ value: T) { self.value = value }
init(_ value: T, _ next: L_32934129<T>?) {
self.value = value
self.next = next
}

var value: T
var next: L_32934129<T>? = nil

func length() -> Int {
func inner(_ list: L_32934129<T>?, _ count: Int) {
guard let list = list else { return count } // expected-error {{unexpected non-void return value in void function}}
return inner(list.next, count + 1)
}

return inner(self, 0) // expected-error {{cannot convert return expression of type '()' to return type 'Int'}}
}
}
2 changes: 1 addition & 1 deletion test/Constraints/fixes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -143,4 +143,4 @@ struct S1116 {
}

let a1116: [S1116] = []
var s1116 = Set(1...10).subtracting(a1116.map({ $0.s })) // expected-error {{'map' produces '[T]', not the expected contextual result type 'Set<Int>'}}
var s1116 = Set(1...10).subtracting(a1116.map({ $0.s })) // expected-error {{cannot convert value of type '[Int?]' to expected argument type 'Set<Int>'}}
2 changes: 1 addition & 1 deletion test/Generics/deduction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func useIdentity(_ x: Int, y: Float, i32: Int32) {

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

Expand Down
4 changes: 2 additions & 2 deletions test/Serialization/Recovery/typedefs.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ public func testVTableBuilding(user: User) {
} // CHECK-IR: ret void

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

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

Expand Down