TypeDeterminer: Error if an IdentifierExpression to a callable is not called

IdentifierExpressions that resolve to functions and intrinsics were not being assigned a semantic::Expression as there is no function / intrinsic type to give them. This lead to NPEs in the writers when these identifiers were reached and TypeOf() is called.

Adding a new tint::type::Callable is an option, but until functions become a type in the language, this seems like a very large and debatable change.

Attempting to detect IdentifierExpressions with no semantic node in the validator is another option, but this is clunky as it has to detect incomplete semantic info from the TD, and cannot identify whether this actually resolved to a function or an intrinsic.

Instead we now error in the TD if encounter an IdentifierExpression that resolves to a function or intrinsic which is not called (ident is not followed with parenthesis).

Fixed: chromium:1180544
Fixed: chromium:1180814
Change-Id: I121dd194356419f94b09c7ee1ed544a350a114b3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42220
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>

src/source.h

@@ -114,9 +114,19 @@ class Source {
   /// @param content the source file content
   inline Source(const Range& rng,
                 const std::string& path,
-                FileContent* content = nullptr)
+                const FileContent* content = nullptr)
       : range(rng), file_path(path), file_content(content) {}
 
+  /// @returns a Source that points to the begin range of this Source.
+  inline Source Begin() const {
+    return Source(Range{range.begin}, file_path, file_content);
+  }
+
+  /// @returns a Source that points to the end range of this Source.
+  inline Source End() const {
+    return Source(Range{range.end}, file_path, file_content);
+  }
+
   /// range is the span of text this source refers to in #file_path
   Range range;
   /// file is the optional file path this source refers to

src/type_determiner.cc

@@ -406,9 +406,6 @@ bool TypeDeterminer::DetermineBitcast(ast::BitcastExpression* expr) {
 }
 
 bool TypeDeterminer::DetermineCall(ast::CallExpression* call) {
-  if (!DetermineResultType(call->func())) {
-    return false;
-  }
   if (!DetermineResultType(call->params())) {
     return false;
   }
@@ -436,8 +433,8 @@ bool TypeDeterminer::DetermineCall(ast::CallExpression* call) {
 
       auto callee_func_it = symbol_to_function_.find(ident->symbol());
       if (callee_func_it == symbol_to_function_.end()) {
-        diagnostics_.add_error("unable to find called function: " + name,
-                               call->source());
+        diagnostics_.add_error(
+            "v-0006: unable to find called function: " + name, call->source());
         return false;
       }
       auto* callee_func = callee_func_it->second;
@@ -558,14 +555,16 @@ bool TypeDeterminer::DetermineIdentifier(ast::IdentifierExpression* expr) {
 
   auto iter = symbol_to_function_.find(symbol);
   if (iter != symbol_to_function_.end()) {
-    // Identifier is to a function, which has no type (currently).
-    return true;
+    diagnostics_.add_error("missing '(' for function call",
+                           expr->source().End());
+    return false;
   }
 
   std::string name = builder_->Symbols().NameFor(symbol);
   if (MatchIntrinsicType(name) != IntrinsicType::kNone) {
-    // Identifier is to an intrinsic function, which has no type (currently).
-    return true;
+    diagnostics_.add_error("missing '(' for intrinsic call",
+                           expr->source().End());
+    return false;
   }
 
   diagnostics_.add_error(

src/type_determiner_test.cc

@@ -396,9 +396,8 @@ TEST_F(TypeDeterminerTest, Stmt_Call_undeclared) {
 
   EXPECT_FALSE(td()->Determine());
 
-  EXPECT_EQ(
-      td()->error(),
-      "12:34 error: v-0006: identifier must be declared before use: func");
+  EXPECT_EQ(td()->error(),
+            "12:34 error: v-0006: unable to find called function: func");
 }
 
 TEST_F(TypeDeterminerTest, Stmt_VariableDecl) {
@@ -692,6 +691,27 @@ TEST_F(TypeDeterminerTest, Expr_Call_Intrinsic) {
   EXPECT_TRUE(TypeOf(call)->Is<type::F32>());
 }
 
+TEST_F(TypeDeterminerTest, Expr_DontCall_Function) {
+  Func("func", {}, ty.void_(), {}, {});
+  auto* ident = create<ast::IdentifierExpression>(
+      Source{{Source::Location{3, 3}, Source::Location{3, 8}}},
+      Symbols().Register("func"));
+  WrapInFunction(ident);
+
+  EXPECT_FALSE(td()->Determine());
+  EXPECT_EQ(td()->error(), "3:8 error: missing '(' for function call");
+}
+
+TEST_F(TypeDeterminerTest, Expr_DontCall_Intrinsic) {
+  auto* ident = create<ast::IdentifierExpression>(
+      Source{{Source::Location{3, 3}, Source::Location{3, 8}}},
+      Symbols().Register("round"));
+  WrapInFunction(ident);
+
+  EXPECT_FALSE(td()->Determine());
+  EXPECT_EQ(td()->error(), "3:8 error: missing '(' for intrinsic call");
+}
+
 TEST_F(TypeDeterminerTest, Expr_Cast) {
   Global("name", ty.f32(), ast::StorageClass::kPrivate);