commit 0e66b403a05c2d58988dcf24760400589caee354
author Ben Clayton <bclayton@google.com> Thu Jun 03 08:17:44 2021 +0000
committer Tint LUCI CQ <tint-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Jun 03 08:17:44 2021 +0000
tree dddb3647a9d48ff888a4dc795c8b32731f2afb0f
parent 8324d227a1c46f24cad2fcdee21eca01c8ada116

Resolver: Remove SetType() that takes ast::Type

This was calling Type() without actually checking that the resolve succeeded.
Have the caller resolve the AST type to the semantic type, so that there's a sensible place to handle errors.

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

src/resolver/resolver.cc

diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index 956d091..d94008a 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -1617,7 +1617,11 @@
   if (!Expression(expr->expr())) {
     return false;
   }
-  SetType(expr, expr->type());
+  auto* ty = Type(expr->type());
+  if (!ty) {
+    return false;
+  }
+  SetType(expr, ty, expr->type()->FriendlyName(builder_->Symbols()));
   return true;
 }
 
@@ -1715,9 +1719,12 @@
       }
     }
 
-    SetType(expr, type_ctor->type());
+    auto* type = Type(type_ctor->type());
+    if (!type) {
+      return false;
+    }
 
-    const sem::Type* type = TypeOf(expr);
+    SetType(expr, type, type_ctor->type()->FriendlyName(builder_->Symbols()));
 
     // Now that the argument types have been determined, make sure that they
     // obey the constructor type rules laid out in
@@ -1731,7 +1738,11 @@
     // TODO(crbug.com/tint/634): Validate array constructor
   } else if (auto* scalar_ctor = expr->As<ast::ScalarConstructorExpression>()) {
     Mark(scalar_ctor->literal());
-    SetType(expr, TypeOf(scalar_ctor->literal()));
+    auto* type = TypeOf(scalar_ctor->literal());
+    if (!type) {
+      return false;
+    }
+    SetType(expr, type);
   } else {
     TINT_ICE(diagnostics_) << "unexpected constructor expression type";
   }
@@ -2436,10 +2447,6 @@
   return nullptr;
 }
 
-void Resolver::SetType(ast::Expression* expr, const ast::Type* type) {
-  SetType(expr, Type(type), type->FriendlyName(builder_->Symbols()));
-}
-
 void Resolver::SetType(ast::Expression* expr, const sem::Type* type) {
   SetType(expr, type, type->FriendlyName(builder_->Symbols()));
 }
@@ -2634,7 +2641,7 @@
     size = a->SizeInBytes();
     return true;
   }
-  TINT_UNREACHABLE(diagnostics_) << "Invalid type " << ty->TypeInfo().name;
+  TINT_UNREACHABLE(diagnostics_) << "invalid type " << ty->TypeInfo().name;
   return false;
 }
 

src/resolver/resolver.h

diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h
index 8bf89ac..252f8bc 100644
--- a/src/resolver/resolver.h
+++ b/src/resolver/resolver.h
@@ -318,12 +318,6 @@
   /// @param lit the literal
   sem::Type* TypeOf(const ast::Literal* lit);
 
-  /// Creates a sem::Expression node with the pre-resolved AST type `type`, and
-  /// assigns this semantic node to the expression `expr`.
-  /// @param expr the expression
-  /// @param type the AST type
-  void SetType(ast::Expression* expr, const ast::Type* type);
-
   /// Creates a sem::Expression node with the resolved type `type`, and
   /// assigns this semantic node to the expression `expr`.
   /// @param expr the expression