Skip to content

[QoI] Improve diagnostics for assignment expression #11042

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
Aug 3, 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
72 changes: 48 additions & 24 deletions lib/Sema/CSDiag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2134,7 +2134,8 @@ 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(Expr *expr, Type contextualType);
bool diagnoseContextualConversionError(Expr *expr, Type contextualType,
ContextualTypePurpose CTP);

/// For an expression being type checked with a CTP_CalleeResult contextual
/// type, try to diagnose a problem.
Expand All @@ -2143,7 +2144,8 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
/// Attempt to produce a diagnostic for a mismatch between a call's
/// type and its assumed contextual type.
bool diagnoseCallContextualConversionErrors(ApplyExpr *callEpxr,
Type contextualType);
Type contextualType,
ContextualTypePurpose CTP);

private:
/// Validate potential contextual type for type-checking one of the
Expand Down Expand Up @@ -3899,9 +3901,9 @@ static bool addTypeCoerceFixit(InFlightDiagnostic &diag, ConstraintSystem &CS,
/// of function type, giving more specific and simpler diagnostics, attaching
/// notes on the parameter, and offering fixits to insert @escaping. Returns
/// true if it detects and issues an error, false if it does nothing.
static bool tryDiagnoseNonEscapingParameterToEscaping(Expr *expr, Type srcType,
Type dstType,
ConstraintSystem &CS) {
static bool tryDiagnoseNonEscapingParameterToEscaping(
Expr *expr, Type srcType, Type dstType, ContextualTypePurpose dstPurpose,
ConstraintSystem &CS) {
assert(expr);
// Need to be referencing a parameter of function type
auto declRef = dyn_cast<DeclRefExpr>(expr);
Expand All @@ -3921,7 +3923,7 @@ static bool tryDiagnoseNonEscapingParameterToEscaping(Expr *expr, Type srcType,

// Pick a specific diagnostic for the specific use
auto paramDecl = cast<ParamDecl>(declRef->getDecl());
switch (CS.getContextualTypePurpose()) {
switch (dstPurpose) {
case CTP_CallArgument:
CS.TC.diagnose(declRef->getLoc(), diag::passing_noescape_to_escaping,
paramDecl->getName());
Expand Down Expand Up @@ -3949,13 +3951,13 @@ static bool tryDiagnoseNonEscapingParameterToEscaping(Expr *expr, Type srcType,
return true;
}

bool FailureDiagnosis::diagnoseContextualConversionError(Expr *expr,
Type contextualType) {
bool FailureDiagnosis::diagnoseContextualConversionError(
Expr *expr, Type contextualType, ContextualTypePurpose CTP) {
// 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.
if (!contextualType) {
// This contextual conversion constraint doesn't install an actual type.
if (CS.getContextualTypePurpose() == CTP_CalleeResult)
if (CTP == CTP_CalleeResult)
return diagnoseCalleeResultContextualConversionError();

return false;
Expand Down Expand Up @@ -3995,7 +3997,7 @@ bool FailureDiagnosis::diagnoseContextualConversionError(Expr *expr,
// If this is conversion failure due to a return statement with an argument
// that cannot be coerced to the result type of the function, emit a
// specific error.
switch (CS.getContextualTypePurpose()) {
switch (CTP) {
case CTP_Unused:
case CTP_CannotFail:
llvm_unreachable("These contextual type purposes cannot fail with a "
Expand Down Expand Up @@ -4156,8 +4158,7 @@ bool FailureDiagnosis::diagnoseContextualConversionError(Expr *expr,

// If this is a conversion from T to () in a call argument context, it is
// almost certainly an extra argument being passed in.
if (CS.getContextualTypePurpose() == CTP_CallArgument &&
contextualType->isVoid()) {
if (CTP == CTP_CallArgument && contextualType->isVoid()) {
diagnose(expr->getLoc(), diag::extra_argument_to_nullary_call)
.highlight(expr->getSourceRange());
return true;
Expand Down Expand Up @@ -4212,7 +4213,7 @@ bool FailureDiagnosis::diagnoseContextualConversionError(Expr *expr,

// Try for better/more specific diagnostics for non-escaping to @escaping
if (tryDiagnoseNonEscapingParameterToEscaping(expr, exprType, contextualType,
CS))
CTP, CS))
return true;

// Don't attempt fixits if we have an unsolved type variable, since
Expand Down Expand Up @@ -4261,7 +4262,7 @@ bool FailureDiagnosis::diagnoseContextualConversionError(Expr *expr,
}

// Attempt to add a fixit for the error.
switch (CS.getContextualTypePurpose()) {
switch (CTP) {
case CTP_CallArgument:
case CTP_ArrayElement:
case CTP_DictionaryKey:
Expand Down Expand Up @@ -6303,7 +6304,8 @@ bool FailureDiagnosis::diagnoseTrailingClosureErrors(ApplyExpr *callExpr) {
// related to function type, let's try to diagnose it.
if (possibleTypes.empty() && contextualType &&
!contextualType->hasUnresolvedType())
return diagnoseContextualConversionError(callExpr, contextualType);
return diagnoseContextualConversionError(callExpr, contextualType,
CS.getContextualTypePurpose());
} else {
possibleTypes.push_back(currentType);
}
Expand Down Expand Up @@ -6404,7 +6406,7 @@ bool FailureDiagnosis::diagnoseTrailingClosureErrors(ApplyExpr *callExpr) {
/// Check if there failure associated with expression is related
/// to given contextual type.
bool FailureDiagnosis::diagnoseCallContextualConversionErrors(
ApplyExpr *callExpr, Type contextualType) {
ApplyExpr *callExpr, Type contextualType, ContextualTypePurpose CTP) {
if (!contextualType || contextualType->hasUnresolvedType())
return false;

Expand Down Expand Up @@ -6436,7 +6438,7 @@ bool FailureDiagnosis::diagnoseCallContextualConversionErrors(
// 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);
return diagnoseContextualConversionError(callExpr, contextualType, CTP);

// If call produces a single type when type-checked with contextual
// expression, it means that the problem is elsewhere, any other
Expand Down Expand Up @@ -6482,7 +6484,8 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
if (diagnoseTrailingClosureErrors(callExpr))
return true;

if (diagnoseCallContextualConversionErrors(callExpr, CS.getContextualType()))
if (diagnoseCallContextualConversionErrors(callExpr, CS.getContextualType(),
CS.getContextualTypePurpose()))
return true;

auto *fnExpr = callExpr->getFn();
Expand Down Expand Up @@ -7004,11 +7007,31 @@ bool FailureDiagnosis::visitAssignExpr(AssignExpr *assignExpr) {
return true;
}

// If the source type is already an error type, we've already posted an error.
auto srcExpr = typeCheckChildIndependently(assignExpr->getSrc(),
destType->getRValueType(),
CTP_AssignSource);
if (!srcExpr) return true;
auto *srcExpr = assignExpr->getSrc();
auto contextualType = destType->getRValueType();

// Let's try to type-check assignment source expression without using
// destination as a contextual type, that allows us to diagnose
// contextual problems related to source much easier.
//
// If source expression requires contextual type to be present,
// let's avoid this step because it's always going to fail.
{
auto *srcExpr = assignExpr->getSrc();
ExprTypeSaverAndEraser eraser(srcExpr);

ConcreteDeclRef ref = nullptr;
auto type = CS.TC.getTypeOfExpressionWithoutApplying(srcExpr, CS.DC, ref);

if (type && !type->isEqual(contextualType))
return diagnoseContextualConversionError(
assignExpr->getSrc(), contextualType, CTP_AssignSource);
}

srcExpr = typeCheckChildIndependently(assignExpr->getSrc(), contextualType,
CTP_AssignSource);
if (!srcExpr)
return true;

// If we are assigning to _ and have unresolved types on the RHS, then we have
// an ambiguity problem.
Expand Down Expand Up @@ -8860,7 +8883,8 @@ void ConstraintSystem::diagnoseFailureForExpr(Expr *expr) {
return;

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

// If we can diagnose a problem based on the constraints left laying around in
Expand Down
2 changes: 1 addition & 1 deletion test/ClangImporter/objc_parse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func keyedSubscripting(_ b: B, idx: A, a: A) {
dict[NSString()] = a
let value = dict[NSString()]

dict[nil] = a // expected-error {{ambiguous reference}}
dict[nil] = a // expected-error {{cannot assign value of type 'A' to type 'Any?'}}
let q = dict[nil] // expected-error {{ambiguous subscript}}
_ = q
}
Expand Down
6 changes: 6 additions & 0 deletions test/Constraints/diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1085,3 +1085,9 @@ fun_31849281(a: { !$0 }, c: [nil, 42], b: { "\($0)" })

func f_31849281(x: Int, y: Int, z: Int) {}
f_31849281(42, y: 10, x: 20) // expected-error {{argument 'x' must precede unnamed argument #1}} {{12-12=x: 20, }} {{21-28=}}

func sr5081() {
var a = ["1", "2", "3", "4", "5"]
var b = [String]()
b = a[2...4] // expected-error {{cannot assign value of type 'ArraySlice<String>' to type '[String]'}}
}
2 changes: 1 addition & 1 deletion test/Constraints/generics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ class GenericClass<A> {}
func genericFunc<T>(t: T) {
_ = [T: GenericClass] // expected-error {{generic parameter 'A' could not be inferred}}
// expected-note@-1 {{explicitly specify the generic arguments to fix this issue}}
// expected-error@-2 2 {{type 'T' does not conform to protocol 'Hashable'}}
// expected-error@-2 3 {{type 'T' does not conform to protocol 'Hashable'}}
}

struct SR_3525<T> {}
Expand Down
4 changes: 2 additions & 2 deletions test/Generics/inheritance.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ func f0<T : A>(_ obji: T, _ ai: A, _ bi: B) {
a = obj

// Invalid assignments
obj = a // expected-error{{'A' is not convertible to 'T'}}
obj = b // expected-error{{'B' is not convertible to 'T'}}
obj = a // expected-error{{cannot assign value of type 'A' to type 'T'}}
obj = b // expected-error{{cannot assign value of type 'B' to type 'T'}}

// Downcast that is actually a coercion
a = (obj as? A)! // expected-warning{{conditional cast from 'T' to 'A' always succeeds}}
Expand Down
2 changes: 1 addition & 1 deletion test/Parse/pointer_conversion.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func mutablePointerArguments(_ p: UnsafeMutablePointer<Int>,

// We don't allow these conversions outside of function arguments.
var x: UnsafeMutablePointer<Int> = &i // expected-error{{cannot pass immutable value as inout argument: 'i' is immutable}}
x = &ii // expected-error{{cannot assign value of type '[Int]' to type 'Int'}}
x = &ii // expected-error{{cannot assign value of type 'inout [Int]' to type 'UnsafeMutablePointer<Int>'}}
_ = x
}

Expand Down
2 changes: 1 addition & 1 deletion test/TypeCoercion/overload_noncall.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func test_inout() {
x = accept_XY(&xy);

x = xy
x = &xy; // expected-error{{'&' used with non-inout argument of type 'X'}}
x = &xy; // expected-error{{cannot assign value of type 'inout X' to type 'X'}}
accept_Z(&xy); // expected-error{{cannot convert value of type 'X' to expected argument type 'Z'}}
}

Expand Down
8 changes: 4 additions & 4 deletions test/stdlib/UnsafePointerDiagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,17 @@ func unsafePointerConversionAvailability(
_ = UnsafeRawPointer(oumps)
_ = UnsafeRawPointer(oups)

_ = UnsafeMutablePointer<Void>(rp) // expected-warning 3 {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}} expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Void>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{overloads for 'UnsafeMutablePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}} expected-note{{Pointer conversion restricted: use '.assumingMemoryBound(to:)' or '.bindMemory(to:capacity:)' to view memory as a type}}
_ = UnsafeMutablePointer<Void>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Void>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{}} expected-warning 3 {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}} expected-note{{overloads for 'UnsafeMutablePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}}
_ = UnsafeMutablePointer<Void>(rp) // expected-warning 4 {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}} expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Void>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{overloads for 'UnsafeMutablePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}} expected-note{{Pointer conversion restricted: use '.assumingMemoryBound(to:)' or '.bindMemory(to:capacity:)' to view memory as a type}}
_ = UnsafeMutablePointer<Void>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafeMutablePointer<Void>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{}} expected-warning 4 {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}} expected-note{{overloads for 'UnsafeMutablePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}}
_ = UnsafeMutablePointer<Void>(umpv) // expected-warning {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}}
_ = UnsafeMutablePointer<Void>(upv) // expected-error {{'init' has been renamed to 'init(mutating:)'}} expected-warning {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}}
_ = UnsafeMutablePointer<Void>(umpi) // expected-warning {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}}
_ = UnsafeMutablePointer<Void>(upi) // expected-error {{'init' has been renamed to 'init(mutating:)'}} expected-warning {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}}
_ = UnsafeMutablePointer<Void>(umps) // expected-warning {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}}
_ = UnsafeMutablePointer<Void>(ups) // expected-error {{'init' has been renamed to 'init(mutating:)'}} expected-warning {{UnsafeMutablePointer<Void> has been replaced by UnsafeMutableRawPointer}}

_ = UnsafePointer<Void>(rp) // expected-error {{cannot invoke initializer for type 'UnsafePointer<Void>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{}} expected-warning 3 {{UnsafePointer<Void> has been replaced by UnsafeRawPointer}} expected-note{{overloads for 'UnsafePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafePointer<Pointee>), (UnsafePointer<Pointee>?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}}
_ = UnsafePointer<Void>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafePointer<Void>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{}} expected-warning 3 {{UnsafePointer<Void> has been replaced by UnsafeRawPointer}} expected-note{{overloads for 'UnsafePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafePointer<Pointee>), (UnsafePointer<Pointee>?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}}
_ = UnsafePointer<Void>(rp) // expected-error {{cannot invoke initializer for type 'UnsafePointer<Void>' with an argument list of type '(UnsafeRawPointer)'}} expected-note {{}} expected-warning 4 {{UnsafePointer<Void> has been replaced by UnsafeRawPointer}} expected-note{{overloads for 'UnsafePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafePointer<Pointee>), (UnsafePointer<Pointee>?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}}
_ = UnsafePointer<Void>(mrp) // expected-error {{cannot invoke initializer for type 'UnsafePointer<Void>' with an argument list of type '(UnsafeMutableRawPointer)'}} expected-note {{}} expected-warning 4 {{UnsafePointer<Void> has been replaced by UnsafeRawPointer}} expected-note{{overloads for 'UnsafePointer<Void>' exist with these partially matching parameter lists: (RawPointer), (OpaquePointer), (OpaquePointer?), (UnsafePointer<Pointee>), (UnsafePointer<Pointee>?), (UnsafeMutablePointer<Pointee>), (UnsafeMutablePointer<Pointee>?)}}
_ = UnsafePointer<Void>(umpv) // expected-warning {{UnsafePointer<Void> has been replaced by UnsafeRawPointer}}
_ = UnsafePointer<Void>(upv) // expected-warning {{UnsafePointer<Void> has been replaced by UnsafeRawPointer}}
_ = UnsafePointer<Void>(umpi) // expected-warning {{UnsafePointer<Void> has been replaced by UnsafeRawPointer}}
Expand Down