Skip to content

[Diagnostics] Tailored diagnostic for "condition" expression #27107

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 3 commits into from
Sep 20, 2019
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
6 changes: 6 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,12 @@ ERROR(cannot_convert_partial_argument_value_protocol,none,
ERROR(cannot_convert_argument_value_nil,none,
"'nil' is not compatible with expected argument type %0", (Type))

ERROR(cannot_convert_condition_value,none,
"cannot convert value of type %0 to expected condition type %1",
(Type, Type))
ERROR(cannot_convert_condition_value_nil,none,
"'nil' is not compatible with expected condition type %0", (Type))

ERROR(cannot_yield_rvalue_by_reference_same_type,none,
"cannot yield immutable value of type %0 as an inout yield", (Type))
ERROR(cannot_yield_rvalue_by_reference,none,
Expand Down
13 changes: 13 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,8 @@ Optional<Diag<Type, Type>> GenericArgumentsMismatchFailure::getDiagnosticFor(
return diag::cannot_convert_coerce;
case CTP_SubscriptAssignSource:
return diag::cannot_convert_subscript_assign;
case CTP_Condition:
return diag::cannot_convert_condition_value;

case CTP_ThrowStmt:
case CTP_Unused:
Expand Down Expand Up @@ -1292,6 +1294,13 @@ bool RValueTreatedAsLValueFailure::diagnoseAsError() {
Expr *diagExpr = getRawAnchor();
SourceLoc loc = diagExpr->getLoc();

auto &cs = getConstraintSystem();
// Assignment is not allowed inside of a condition,
// so let's not diagnose immutability, because
// most likely the problem is related to use of `=` itself.
if (cs.getContextualTypePurpose() == CTP_Condition)
return false;

if (auto assignExpr = dyn_cast<AssignExpr>(diagExpr)) {
diagExpr = assignExpr->getDest();
}
Expand Down Expand Up @@ -1954,6 +1963,8 @@ getContextualNilDiagnostic(ContextualTypePurpose CTP) {
return diag::cannot_convert_assign_nil;
case CTP_SubscriptAssignSource:
return diag::cannot_convert_subscript_assign_nil;
case CTP_Condition:
return diag::cannot_convert_condition_value_nil;
}
llvm_unreachable("Unhandled ContextualTypePurpose in switch");
}
Expand Down Expand Up @@ -2691,6 +2702,8 @@ ContextualFailure::getDiagnosticFor(ContextualTypePurpose context,
case CTP_SubscriptAssignSource:
return forProtocol ? diag::cannot_convert_subscript_assign_protocol
: diag::cannot_convert_subscript_assign;
case CTP_Condition:
return diag::cannot_convert_condition_value;

case CTP_ThrowStmt:
case CTP_Unused:
Expand Down
57 changes: 31 additions & 26 deletions lib/Sema/TypeCheckConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3009,31 +3009,13 @@ bool TypeChecker::typeCheckCondition(Expr *&expr, DeclContext *dc) {
return !resultTy;
}

/// Expression type checking listener for conditions.
class ConditionListener : public ExprTypeCheckListener {
Expr *OrigExpr = nullptr;

public:
// Add the appropriate Boolean constraint.
bool builtConstraints(ConstraintSystem &cs, Expr *expr) override {
// Save the original expression.
OrigExpr = expr;

// Otherwise, the result must be convertible to Bool.
auto boolDecl = cs.getASTContext().getBoolDecl();
if (!boolDecl)
return true;

// Condition must convert to Bool.
cs.addConstraint(ConstraintKind::Conversion, cs.getType(expr),
boolDecl->getDeclaredType(),
cs.getConstraintLocator(expr));
return false;
}
};
auto *boolDecl = Context.getBoolDecl();
if (!boolDecl)
return true;

ConditionListener listener;
auto resultTy = typeCheckExpression(expr, dc, &listener);
auto resultTy = typeCheckExpression(
expr, dc, TypeLoc::withoutLoc(boolDecl->getDeclaredType()),
CTP_Condition);
return !resultTy;
}

Expand Down Expand Up @@ -3165,9 +3147,32 @@ bool TypeChecker::typeCheckExprPattern(ExprPattern *EP, DeclContext *DC,

Expr *matchCall = new (Context) BinaryExpr(matchOp, matchArgs,
/*Implicit=*/true);

// Check the expression as a condition.
bool hadError = typeCheckCondition(matchCall, DC);
//
// TODO: Type-check of `~=` operator can't (yet) use `typeCheckCondition`
// because that utilizes contextual type which interferes with diagnostics.
// We don't yet have a full access to pattern-matching context in
// constraint system, which is required to enable these situations
// to be properly diagnosed.
struct ConditionListener : public ExprTypeCheckListener {
// Add the appropriate Boolean constraint.
bool builtConstraints(ConstraintSystem &cs, Expr *expr) override {
// Otherwise, the result must be convertible to Bool.
auto boolDecl = cs.getASTContext().getBoolDecl();
if (!boolDecl)
return true;

// Condition must convert to Bool.
cs.addConstraint(ConstraintKind::Conversion, cs.getType(expr),
boolDecl->getDeclaredType(),
cs.getConstraintLocator(expr));
return false;
}
};

ConditionListener listener;
bool hadError = !typeCheckExpression(matchCall, DC, &listener);
// Save the type-checked expression in the pattern.
EP->setMatchExpr(matchCall);
// Set the type on the pattern.
Expand Down
2 changes: 2 additions & 0 deletions lib/Sema/TypeChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ enum ContextualTypePurpose {
CTP_AssignSource, ///< AssignExpr source operand coerced to result type.
CTP_SubscriptAssignSource, ///< AssignExpr source operand coerced to subscript
///< result type.
CTP_Condition, ///< Condition expression of various statements e.g.
///< `if`, `for`, `while` etc.

CTP_CannotFail, ///< Conversion can never fail. abort() if it does.
};
Expand Down
4 changes: 2 additions & 2 deletions test/Constraints/diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,10 @@ func r17224804(_ monthNumber : Int) {

// <rdar://problem/17020197> QoI: Operand of postfix '!' should have optional type; type is 'Int?'
func r17020197(_ x : Int?, y : Int) {
if x! { } // expected-error {{'Int' is not convertible to 'Bool'}}
if x! { } // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}

// <rdar://problem/12939553> QoI: diagnostic for using an integer in a condition is utterly terrible
if y {} // expected-error {{'Int' is not convertible to 'Bool'}}
if y {} // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
}

// <rdar://problem/20714480> QoI: Boolean expr not treated as Bool type when function return type is different
Expand Down
4 changes: 2 additions & 2 deletions test/Constraints/invalid_logicvalue_coercion.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

class C {}
var c = C()
if c as C { // expected-error{{'C' is not convertible to 'Bool'}}
if c as C { // expected-error{{cannot convert value of type 'C' to expected condition type 'Bool'}}
}

if ({1} as () -> Int) { // expected-error{{'() -> Int' is not convertible to 'Bool'}}
if ({1} as () -> Int) { // expected-error{{cannot convert value of type '() -> Int' to expected condition type 'Bool'}}
}
4 changes: 2 additions & 2 deletions test/Misc/misc_diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ let total = 15.0
let count = 7
let median = total / count // expected-error {{binary operator '/' cannot be applied to operands of type 'Double' and 'Int'}} expected-note {{overloads for '/' exist with these partially matching parameter lists:}}

if (1) {} // expected-error{{'Int' is not convertible to 'Bool'}}
if 1 {} // expected-error {{'Int' is not convertible to 'Bool'}}
if (1) {} // expected-error{{cannot convert value of type 'Int' to expected condition type 'Bool'}}
if 1 {} // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}

var a: [String] = [1] // expected-error{{cannot convert value of type 'Int' to expected element type 'String'}}
var b: Int = [1, 2, 3] // expected-error{{cannot convert value of type '[Int]' to specified type 'Int'}}
Expand Down
4 changes: 2 additions & 2 deletions test/Parse/switch.swift
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ case 10,
case _ where x % 2 == 0,
20:
x = 1
case var y where y % 2 == 0: // expected-warning {{variable 'y' was never mutated; consider changing to 'let' constant}}
case var y where y % 2 == 0:
x = y + 1
case _ where 0: // expected-error {{'Int' is not convertible to 'Bool'}}
case _ where 0: // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
x = 0
default:
x = 1
Expand Down
4 changes: 2 additions & 2 deletions test/Sema/pound_assert.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@

#assert(false, "error message")

#assert(123) // expected-error{{'Int' is not convertible to 'Bool'}}
#assert(123) // expected-error{{cannot convert value of type 'Int' to expected condition type 'Bool'}}

#assert(123, "error message") // expected-error{{'Int' is not convertible to 'Bool'}}
#assert(123, "error message") // expected-error{{cannot convert value of type 'Int' to expected condition type 'Bool'}}
10 changes: 5 additions & 5 deletions test/expr/expressions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -799,14 +799,14 @@ func testOptionalTypeParsing(_ a : AnyObject) -> String {
func testParenExprInTheWay() {
let x = 42

if x & 4.0 {} // expected-error {{binary operator '&' cannot be applied to operands of type 'Int' and 'Double'}} expected-note {{expected an argument list of type '(Int, Int)'}}

if x & 4.0 {} // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
// expected-error@-1 {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
if (x & 4.0) {} // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}

// expected-error@-1 {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
if !(x & 4.0) {} // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
// expected-error@-1 {{cannot convert value of type 'Int' to expected argument type 'Bool'}}
if x & x {} // expected-error {{'Int' is not convertible to 'Bool'}}

if x & x {} // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
}

// <rdar://problem/21352576> Mixed method/property overload groups can cause a crash during constraint optimization
Expand Down
2 changes: 1 addition & 1 deletion test/stmt/if_while_var.swift
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ if 1 != 2, case let x? : Int? = 42 {} // expected-warning {{immutable value 'x'

// Test error recovery.
// <rdar://problem/19939746> Improve error recovery for malformed if statements
if 1 != 2, { // expected-error {{'() -> ()' is not convertible to 'Bool'}}
if 1 != 2, { // expected-error {{cannot convert value of type '() -> ()' to expected condition type 'Bool'}}
} // expected-error {{expected '{' after 'if' condition}}
if 1 != 2, 4 == 57 {}
if 1 != 2, 4 == 57, let x = opt {} // expected-warning {{immutable value 'x' was never used; consider replacing with '_' or removing it}}
Expand Down
11 changes: 6 additions & 5 deletions test/stmt/statements.swift
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func funcdecl5(_ a: Int, y: Int) {
}

// This diagnostic is terrible - rdar://12939553
if x {} // expected-error {{'Int' is not convertible to 'Bool'}}
if x {} // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}

if true {
if (B) {
Expand Down Expand Up @@ -99,7 +99,7 @@ struct infloopbool {
}

func infloopbooltest() {
if (infloopbool()) {} // expected-error {{'infloopbool' is not convertible to 'Bool'}}
if (infloopbool()) {} // expected-error {{cannot convert value of type 'infloopbool' to expected condition type 'Bool'}}
}

// test "builder" API style
Expand Down Expand Up @@ -566,9 +566,10 @@ func fn(x: Int) {
}

func bad_if() {
if 1 {} // expected-error {{'Int' is not convertible to 'Bool'}}
if (x: false) {} // expected-error {{'(x: Bool)' is not convertible to 'Bool'}}
if (x: 1) {} // expected-error {{'(x: Int)' is not convertible to 'Bool'}}
if 1 {} // expected-error {{cannot convert value of type 'Int' to expected condition type 'Bool'}}
if (x: false) {} // expected-error {{cannot convert value of type '(x: Bool)' to expected condition type 'Bool'}}
if (x: 1) {} // expected-error {{cannot convert value of type '(x: Int)' to expected condition type 'Bool'}}
if nil {} // expected-error {{'nil' is not compatible with expected condition type 'Bool'}}
}

// Typo correction for loop labels
Expand Down