Merge pull request #27107 from xedin/if-conditional-diag

[Diagnostics] Tailored diagnostic for "condition" expression

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -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,

lib/Sema/CSDiagnostics.cpp

@@ -747,6 +747,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:
@@ -1347,6 +1349,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();
   }
@@ -2009,6 +2018,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");
 }
@@ -2746,6 +2757,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:

lib/Sema/TypeCheckConstraints.cpp

@@ -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;
 }
 
@@ -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.

lib/Sema/TypeChecker.h

@@ -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.
 };

test/Constraints/diagnostics.swift

@@ -194,10 +194,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

test/Constraints/invalid_logicvalue_coercion.swift

@@ -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'}}
 }

test/Misc/misc_diagnostics.swift

@@ -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'}}

test/Parse/switch.swift

@@ -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

test/Sema/pound_assert.swift

@@ -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'}}

test/expr/expressions.swift

@@ -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

test/stmt/if_while_var.swift

@@ -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}}

test/stmt/statements.swift

@@ -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) {
@@ -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
@@ -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