TypeResolver: Fix TypeOf() for CallExpression

While traversing and resolving the AST, TypeOf() was called to look up
the semantic node for the given AST expression in order to fetch the
resolved type. However, for CallExpression semantic nodes are
constructed at the end of the AST traversal, and GetType() for these
would unexpectedly return nullptr, causing a crash.

To fix, have TypeDeterminer maintain an internal map of ast::Expression
to resolved type. Always populate this internal map whenever SetType() is
called. At the end of the AST traversal, have CreateSemanticNodes()
construct the semantic nodes for any ast::Expression nodes that do not
already have a semantic node assigned.

With this, GetType() will always return the type set with SetType().

Fixes tint -> dawn autoroller.

Fixed: tint:488
Change-Id: I2830c496d9b2e4807ec01ed69aeafb3912f4a890
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40606
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>

src/type_determiner.cc

@@ -452,6 +452,7 @@ bool TypeDeterminer::DetermineCall(ast::CallExpression* call) {
 
     auto* function = iter->second;
     function_calls_.emplace(call, function);
+    SetType(call, function->declaration->return_type());
   }
 
   return true;
@@ -501,10 +502,12 @@ bool TypeDeterminer::DetermineIntrinsicCall(
     auto* intrinsic = builder_->create<semantic::Intrinsic>(intrinsic_type,
                                                             ret_ty, parameters);
     builder_->Sem().Add(call, builder_->create<semantic::Call>(intrinsic));
+    SetType(call, ret_ty);
     return false;
   }
 
   builder_->Sem().Add(call, builder_->create<semantic::Call>(result.intrinsic));
+  SetType(call, result.intrinsic->ReturnType());
   return true;
 }
 
@@ -781,6 +784,7 @@ bool TypeDeterminer::DetermineMemberAccessor(
   builder_->Sem().Add(
       expr,
       builder_->create<semantic::MemberAccessorExpression>(ret, is_swizzle));
+  SetType(expr, ret);
 
   return true;
 }
@@ -875,14 +879,23 @@ TypeDeterminer::VariableInfo* TypeDeterminer::CreateVariableInfo(
   return info;
 }
 
-void TypeDeterminer::SetType(ast::Expression* expr, type::Type* type) const {
-  return builder_->Sem().Add(expr,
-                             builder_->create<semantic::Expression>(type));
+type::Type* TypeDeterminer::TypeOf(ast::Expression* expr) {
+  auto it = expr_types_.find(expr);
+  if (it != expr_types_.end()) {
+    return it->second;
+  }
+  return nullptr;
+}
+
+void TypeDeterminer::SetType(ast::Expression* expr, type::Type* type) {
+  assert(expr_types_.count(expr) == 0);
+  expr_types_.emplace(expr, type);
 }
 
 void TypeDeterminer::CreateSemanticNodes() const {
   auto& sem = builder_->Sem();
 
+  // Create semantic nodes for all ast::Variables
   for (auto it : variable_to_info_) {
     auto* var = it.first;
     auto* info = it.second;
@@ -899,6 +912,7 @@ void TypeDeterminer::CreateSemanticNodes() const {
     return out;
   };
 
+  // Create semantic nodes for all ast::Functions
   std::unordered_map<FunctionInfo*, semantic::Function*> func_info_to_sem_func;
   for (auto it : function_to_info_) {
     auto* func = it.first;
@@ -911,11 +925,23 @@ void TypeDeterminer::CreateSemanticNodes() const {
     sem.Add(func, sem_func);
   }
 
+  // Create semantic nodes for all ast::CallExpressions
   for (auto it : function_calls_) {
     auto* call = it.first;
     auto* func_info = it.second;
     auto* sem_func = func_info_to_sem_func.at(func_info);
-    builder_->Sem().Add(call, builder_->create<semantic::Call>(sem_func));
+    sem.Add(call, builder_->create<semantic::Call>(sem_func));
+  }
+
+  // Create semantic nodes for all remaining expression types
+  for (auto it : expr_types_) {
+    auto* expr = it.first;
+    auto* type = it.second;
+    if (sem.Get(expr)) {
+      // Expression has already been assigned a semantic node
+      continue;
+    }
+    sem.Add(expr, builder_->create<semantic::Expression>(type));
   }
 }
 

src/type_determiner.h

@@ -188,15 +188,13 @@ class TypeDeterminer {
 
   /// @returns the resolved type of the ast::Expression `expr`
   /// @param expr the expression
-  type::Type* TypeOf(ast::Expression* expr) const {
-    return builder_->TypeOf(expr);
-  }
+  type::Type* TypeOf(ast::Expression* expr);
 
   /// Creates a semantic::Expression node with the resolved type `type`, and
   /// assigns this semantic node to the expression `expr`.
   /// @param expr the expression
   /// @param type the resolved type
-  void SetType(ast::Expression* expr, type::Type* type) const;
+  void SetType(ast::Expression* expr, type::Type* type);
 
   ProgramBuilder* const builder_;
   std::unique_ptr<IntrinsicTable> const intrinsic_table_;
@@ -206,6 +204,7 @@ class TypeDeterminer {
   std::unordered_map<ast::Function*, FunctionInfo*> function_to_info_;
   std::unordered_map<ast::Variable*, VariableInfo*> variable_to_info_;
   std::unordered_map<ast::CallExpression*, FunctionInfo*> function_calls_;
+  std::unordered_map<ast::Expression*, type::Type*> expr_types_;
   FunctionInfo* current_function_ = nullptr;
   BlockAllocator<VariableInfo> variable_infos_;
   BlockAllocator<FunctionInfo> function_infos_;

src/type_determiner_test.cc

@@ -604,6 +604,20 @@ TEST_F(TypeDeterminerTest, Expr_Call) {
   EXPECT_TRUE(TypeOf(call)->Is<type::F32>());
 }
 
+TEST_F(TypeDeterminerTest, Expr_Call_InBinaryOp) {
+  ast::VariableList params;
+  Func("func", params, ty.f32(), ast::StatementList{},
+       ast::FunctionDecorationList{});
+
+  auto* expr = Add(Call("func"), Call("func"));
+  WrapInFunction(expr);
+
+  EXPECT_TRUE(td()->Determine()) << td()->error();
+
+  ASSERT_NE(TypeOf(expr), nullptr);
+  EXPECT_TRUE(TypeOf(expr)->Is<type::F32>());
+}
+
 TEST_F(TypeDeterminerTest, Expr_Call_WithParams) {
   ast::VariableList params;
   Func("my_func", params, ty.f32(), ast::StatementList{},
@@ -977,6 +991,25 @@ TEST_F(TypeDeterminerTest, Expr_Accessor_MultiLevel) {
   EXPECT_EQ(TypeOf(mem)->As<type::Vector>()->size(), 2u);
 }
 
+TEST_F(TypeDeterminerTest, Expr_MemberAccessor_InBinaryOp) {
+  auto* strct = create<ast::Struct>(
+      ast::StructMemberList{Member("first_member", ty.f32()),
+                            Member("second_member", ty.f32())},
+      ast::StructDecorationList{});
+
+  auto* st = ty.struct_("S", strct);
+  Global("my_struct", ast::StorageClass::kNone, st);
+
+  auto* expr = Add(MemberAccessor("my_struct", "first_member"),
+                   MemberAccessor("my_struct", "second_member"));
+  WrapInFunction(expr);
+
+  EXPECT_TRUE(td()->Determine()) << td()->error();
+
+  ASSERT_NE(TypeOf(expr), nullptr);
+  EXPECT_TRUE(TypeOf(expr)->Is<type::F32>());
+}
+
 using Expr_Binary_BitwiseTest = TypeDeterminerTestWithParam<ast::BinaryOp>;
 TEST_P(Expr_Binary_BitwiseTest, Scalar) {
   auto op = GetParam();