commit f1b643ee7040065a5d7052341cf3928cefd1efd5
author Antonio Maiorano <amaiorano@google.com> Thu Apr 01 19:40:37 2021 +0000
committer Commit Bot service account <commit-bot@chromium.org> Thu Apr 01 19:40:37 2021 +0000
tree cda0a8fedf2107b34056eef6963b23ca92b9cd88
parent 15c6ed048c2477a8b9ad2a7ab751f8d6ed9cd9e6

Improve binary expression validation error message

Example:
```
  var a : i32;
  var b : f32;
  if (a == b) {
    return vec4<f32>(0.4, 0.4, 0.8, 1.0);
  }
```

Outputs:
```
error: test7.wgsl:6:9 error: Binary expression operand types are invalid for this operation: i32 equal f32
  if (a == b) {
        ^^
```

Bug: tint:663
Change-Id: Idd2bb5a248b3c7d652483931d7dd58d5123e9ee8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46640
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>

src/ast/binary_expression.h

diff --git a/src/ast/binary_expression.h b/src/ast/binary_expression.h
index b9b8c1c..9c9d840 100644
--- a/src/ast/binary_expression.h
+++ b/src/ast/binary_expression.h
@@ -182,66 +182,52 @@
   }
 }
 
-inline std::ostream& operator<<(std::ostream& out, BinaryOp op) {
+constexpr const char* FriendlyName(BinaryOp op) {
   switch (op) {
     case BinaryOp::kNone:
-      out << "none";
-      break;
+      return "none";
     case BinaryOp::kAnd:
-      out << "and";
-      break;
+      return "and";
     case BinaryOp::kOr:
-      out << "or";
-      break;
+      return "or";
     case BinaryOp::kXor:
-      out << "xor";
-      break;
+      return "xor";
     case BinaryOp::kLogicalAnd:
-      out << "logical_and";
-      break;
+      return "logical_and";
     case BinaryOp::kLogicalOr:
-      out << "logical_or";
-      break;
+      return "logical_or";
     case BinaryOp::kEqual:
-      out << "equal";
-      break;
+      return "equal";
     case BinaryOp::kNotEqual:
-      out << "not_equal";
-      break;
+      return "not_equal";
     case BinaryOp::kLessThan:
-      out << "less_than";
-      break;
+      return "less_than";
     case BinaryOp::kGreaterThan:
-      out << "greater_than";
-      break;
+      return "greater_than";
     case BinaryOp::kLessThanEqual:
-      out << "less_than_equal";
-      break;
+      return "less_than_equal";
     case BinaryOp::kGreaterThanEqual:
-      out << "greater_than_equal";
-      break;
+      return "greater_than_equal";
     case BinaryOp::kShiftLeft:
-      out << "shift_left";
-      break;
+      return "shift_left";
     case BinaryOp::kShiftRight:
-      out << "shift_right";
-      break;
+      return "shift_right";
     case BinaryOp::kAdd:
-      out << "add";
-      break;
+      return "add";
     case BinaryOp::kSubtract:
-      out << "subtract";
-      break;
+      return "subtract";
     case BinaryOp::kMultiply:
-      out << "multiply";
-      break;
+      return "multiply";
     case BinaryOp::kDivide:
-      out << "divide";
-      break;
+      return "divide";
     case BinaryOp::kModulo:
-      out << "modulo";
-      break;
+      return "modulo";
   }
+  return "INVALID";
+}
+
+inline std::ostream& operator<<(std::ostream& out, BinaryOp op) {
+  out << FriendlyName(op);
   return out;
 }
 

src/resolver/resolver.cc

diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 900e169..1158ad6 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -1174,7 +1174,10 @@
   }
 
   diagnostics_.add_error(
-      "Binary expression operand types are invalid for this operation",
+      "Binary expression operand types are invalid for this operation: " +
+          lhs_type->FriendlyName(builder_->Symbols()) + " " +
+          FriendlyName(expr->op()) + " " +
+          rhs_type->FriendlyName(builder_->Symbols()),
       expr->source());
   return false;
 }

src/resolver/resolver_test.cc

diff --git a/src/resolver/resolver_test.cc b/src/resolver/resolver_test.cc
index 1c785d9..12d019e 100644
--- a/src/resolver/resolver_test.cc
+++ b/src/resolver/resolver_test.cc
@@ -1270,7 +1270,10 @@
   ASSERT_FALSE(r()->Resolve()) << r()->error();
   ASSERT_EQ(r()->error(),
             "12:34 error: Binary expression operand types are invalid for "
-            "this operation");
+            "this operation: " +
+                lhs_type->FriendlyName(Symbols()) + " " +
+                FriendlyName(expr->op()) + " " +
+                rhs_type->FriendlyName(Symbols()));
 }
 INSTANTIATE_TEST_SUITE_P(
     ResolverTest,