Register function symbol in TypeDeterminer after processing body.

This CL removes the registration of functions to after the function body
has been processed. This allows us to catch recursive calls where the
function calls itself. Prior to this we'd find the called function and,
if the function was an entry point, end up in a loop trying to walk all
the callers. With this change, the function is not found when we do
the lookup and we exit as expected.

Bug: tint:258
Change-Id: Ie0173207b788e87de39867a5aa41e8cc13ec33de
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42520
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>

src/type_determiner.cc

@@ -170,8 +170,6 @@ bool TypeDeterminer::DetermineFunctions(const ast::FunctionList& funcs) {
 
 bool TypeDeterminer::DetermineFunction(ast::Function* func) {
   auto* func_info = function_infos_.Create<FunctionInfo>(func);
-  symbol_to_function_[func->symbol()] = func_info;
-  function_to_info_.emplace(func, func_info);
 
   ScopedAssignment<FunctionInfo*> sa(current_function_, func_info);
 
@@ -185,6 +183,12 @@ bool TypeDeterminer::DetermineFunction(ast::Function* func) {
   }
   variable_stack_.pop_scope();
 
+  // Register the function information _after_ processing the statements. This
+  // allows us to catch a function calling itself when determining the call
+  // information as this function doesn't exist until it's finished.
+  symbol_to_function_[func->symbol()] = func_info;
+  function_to_info_.emplace(func, func_info);
+
   return true;
 }
 
@@ -433,8 +437,15 @@ 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(
+        if (current_function_->declaration->symbol() == ident->symbol()) {
-            "v-0006: unable to find called function: " + name, call->source());
+          diagnostics_.add_error("recursion is not permitted. '" + name +
+                                     "' attempted to call itself.",
+                                 call->source());
+        } else {
+          diagnostics_.add_error(
+              "v-0006: unable to find called function: " + name,
+              call->source());
+        }
         return false;
       }
       auto* callee_func = callee_func_it->second;

src/type_determiner_test.cc

@@ -402,6 +402,28 @@ TEST_F(TypeDeterminerTest, Stmt_Call_undeclared) {
             "12:34 error: v-0006: unable to find called function: func");
 }
 
+TEST_F(TypeDeterminerTest, Stmt_Call_recursive) {
+  // fn main() -> void {main(); }
+
+  SetSource(Source::Location{12, 34});
+  auto* call_expr = Call("main");
+  ast::VariableList params0;
+
+  Func("main", params0, ty.f32(),
+       ast::StatementList{
+           create<ast::CallStatement>(call_expr),
+       },
+       ast::FunctionDecorationList{
+           create<ast::StageDecoration>(ast::PipelineStage::kVertex),
+       });
+
+  EXPECT_FALSE(td()->Determine());
+
+  EXPECT_EQ(td()->error(),
+            "12:34 error: recursion is not permitted. 'main' attempted to call "
+            "itself.");
+}
+
 TEST_F(TypeDeterminerTest, Stmt_VariableDecl) {
   auto* var = Var("my_var", ty.i32(), ast::StorageClass::kNone, Expr(2));
   auto* init = var->constructor();

src/validator/validator_function_test.cc

@@ -175,45 +175,6 @@ TEST_F(ValidateFunctionTest, FunctionNamesMustBeUnique_fail) {
   EXPECT_EQ(v.error(), "12:34 v-0016: function names must be unique 'func'");
 }
 
-TEST_F(ValidateFunctionTest, RecursionIsNotAllowed_Fail) {
-  // fn func() -> void {func(); return; }
-  ast::ExpressionList call_params;
-  auto* call_expr = create<ast::CallExpression>(
-      Source{Source::Location{12, 34}}, Expr("func"), call_params);
-
-  Func("func", ast::VariableList{}, ty.f32(),
-       ast::StatementList{
-           create<ast::CallStatement>(call_expr),
-           create<ast::ReturnStatement>(),
-       },
-       ast::FunctionDecorationList{});
-
-  ValidatorImpl& v = Build();
-
-  EXPECT_FALSE(v.Validate()) << v.error();
-  EXPECT_EQ(v.error(), "12:34 v-0004: recursion is not allowed: 'func'");
-}
-
-TEST_F(ValidateFunctionTest, RecursionIsNotAllowedExpr_Fail) {
-  // fn func() -> i32 {var a: i32 = func(); return 2; }
-  ast::ExpressionList call_params;
-  auto* call_expr = create<ast::CallExpression>(
-      Source{Source::Location{12, 34}}, Expr("func"), call_params);
-  auto* var = Var("a", ty.i32(), ast::StorageClass::kNone, call_expr);
-
-  Func("func", ast::VariableList{}, ty.i32(),
-       ast::StatementList{
-           create<ast::VariableDeclStatement>(var),
-           create<ast::ReturnStatement>(Expr(2)),
-       },
-       ast::FunctionDecorationList{});
-
-  ValidatorImpl& v = Build();
-
-  EXPECT_FALSE(v.Validate()) << v.error();
-  EXPECT_EQ(v.error(), "12:34 v-0004: recursion is not allowed: 'func'");
-}
-
 TEST_F(ValidateFunctionTest, Function_WithPipelineStage_NotVoid_Fail) {
   // [[stage(vertex)]]
   // fn vtx_main() -> i32 { return 0; }