resolver: Validate that a call statement has no return value

Also split out validation tests from call_test.cc into call_validation_test.cc.

Bug: tint:886
Change-Id: I1e1dee9b7c348363e89080cdecd3119cc004658f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54063
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>

src/resolver/builtins_validation_test.cc

@@ -351,7 +351,7 @@ TEST_P(FloatAllMatching, Scalar) {
     params.push_back(Expr(1.0f));
   }
   auto* builtin = Call(name, params);
-  Func("func", {}, ty.void_(), {create<ast::CallStatement>(builtin)},
+  Func("func", {}, ty.void_(), {Ignore(builtin)},
        {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
 
   EXPECT_TRUE(r()->Resolve()) << r()->error();
@@ -367,7 +367,7 @@ TEST_P(FloatAllMatching, Vec2) {
     params.push_back(vec2<f32>(1.0f, 1.0f));
   }
   auto* builtin = Call(name, params);
-  Func("func", {}, ty.void_(), {create<ast::CallStatement>(builtin)},
+  Func("func", {}, ty.void_(), {Ignore(builtin)},
        {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
 
   EXPECT_TRUE(r()->Resolve()) << r()->error();
@@ -383,7 +383,7 @@ TEST_P(FloatAllMatching, Vec3) {
     params.push_back(vec3<f32>(1.0f, 1.0f, 1.0f));
   }
   auto* builtin = Call(name, params);
-  Func("func", {}, ty.void_(), {create<ast::CallStatement>(builtin)},
+  Func("func", {}, ty.void_(), {Ignore(builtin)},
        {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
 
   EXPECT_TRUE(r()->Resolve()) << r()->error();
@@ -399,7 +399,7 @@ TEST_P(FloatAllMatching, Vec4) {
     params.push_back(vec4<f32>(1.0f, 1.0f, 1.0f, 1.0f));
   }
   auto* builtin = Call(name, params);
-  Func("func", {}, ty.void_(), {create<ast::CallStatement>(builtin)},
+  Func("func", {}, ty.void_(), {Ignore(builtin)},
        {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
 
   EXPECT_TRUE(r()->Resolve()) << r()->error();

src/resolver/call_test.cc

@@ -58,56 +58,6 @@ using u32 = builder::u32;
 
 using ResolverCallTest = ResolverTest;
 
-TEST_F(ResolverCallTest, Recursive_Invalid) {
-  // fn main() {main(); }
-
-  SetSource(Source::Location{12, 34});
-  auto* call_expr = Call("main");
-  ast::VariableList params0;
-
-  Func("main", params0, ty.void_(),
-       ast::StatementList{
-           create<ast::CallStatement>(call_expr),
-       },
-       ast::DecorationList{
-           Stage(ast::PipelineStage::kVertex),
-       });
-
-  EXPECT_FALSE(r()->Resolve());
-
-  EXPECT_EQ(r()->error(),
-            "12:34 error v-0004: recursion is not permitted. 'main' attempted "
-            "to call "
-            "itself.");
-}
-
-TEST_F(ResolverCallTest, Undeclared_Invalid) {
-  // fn main() {func(); return; }
-  // fn func() { return; }
-
-  SetSource(Source::Location{12, 34});
-  auto* call_expr = Call("func");
-  ast::VariableList params0;
-
-  Func("main", params0, ty.f32(),
-       ast::StatementList{
-           create<ast::CallStatement>(call_expr),
-           Return(),
-       },
-       ast::DecorationList{});
-
-  Func("func", params0, ty.f32(),
-       ast::StatementList{
-           Return(),
-       },
-       ast::DecorationList{});
-
-  EXPECT_FALSE(r()->Resolve());
-
-  EXPECT_EQ(r()->error(),
-            "12:34 error: v-0006: unable to find called function: func");
-}
-
 struct Params {
   builder::ast_expr_func_ptr create_value;
   builder::ast_type_func_ptr create_type;
@@ -153,42 +103,6 @@ TEST_F(ResolverCallTest, Valid) {
   EXPECT_TRUE(r()->Resolve()) << r()->error();
 }
 
-TEST_F(ResolverCallTest, TooFewArgs) {
-  Func("foo", {Param(Sym(), ty.i32()), Param(Sym(), ty.f32())}, ty.void_(),
-       {Return()});
-  auto* call = Call(Source{{12, 34}}, "foo", 1);
-  WrapInFunction(call);
-
-  EXPECT_FALSE(r()->Resolve());
-  EXPECT_EQ(
-      r()->error(),
-      "12:34 error: too few arguments in call to 'foo', expected 2, got 1");
-}
-
-TEST_F(ResolverCallTest, TooManyArgs) {
-  Func("foo", {Param(Sym(), ty.i32()), Param(Sym(), ty.f32())}, ty.void_(),
-       {Return()});
-  auto* call = Call(Source{{12, 34}}, "foo", 1, 1.0f, 1.0f);
-  WrapInFunction(call);
-
-  EXPECT_FALSE(r()->Resolve());
-  EXPECT_EQ(
-      r()->error(),
-      "12:34 error: too many arguments in call to 'foo', expected 2, got 3");
-}
-
-TEST_F(ResolverCallTest, MismatchedArgs) {
-  Func("foo", {Param(Sym(), ty.i32()), Param(Sym(), ty.f32())}, ty.void_(),
-       {Return()});
-  auto* call = Call("foo", Expr(Source{{12, 34}}, true), 1.0f);
-  WrapInFunction(call);
-
-  EXPECT_FALSE(r()->Resolve());
-  EXPECT_EQ(r()->error(),
-            "12:34 error: type mismatch for argument 1 in call to 'foo', "
-            "expected 'i32', got 'bool'");
-}
-
 }  // namespace
 }  // namespace resolver
 }  // namespace tint

src/resolver/call_validation_test.cc

@@ -0,0 +1,135 @@
+// Copyright 2021 The Tint Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "src/resolver/resolver.h"
+
+#include "gmock/gmock.h"
+#include "src/ast/call_statement.h"
+#include "src/resolver/resolver_test_helper.h"
+
+namespace tint {
+namespace resolver {
+namespace {
+
+using ResolverCallValidationTest = ResolverTest;
+
+TEST_F(ResolverCallValidationTest, Recursive_Invalid) {
+  // fn main() {main(); }
+
+  SetSource(Source::Location{12, 34});
+  auto* call_expr = Call("main");
+  ast::VariableList params0;
+
+  Func("main", params0, ty.void_(),
+       ast::StatementList{
+           create<ast::CallStatement>(call_expr),
+       },
+       ast::DecorationList{
+           Stage(ast::PipelineStage::kVertex),
+       });
+
+  EXPECT_FALSE(r()->Resolve());
+
+  EXPECT_EQ(r()->error(),
+            "12:34 error v-0004: recursion is not permitted. 'main' attempted "
+            "to call "
+            "itself.");
+}
+
+TEST_F(ResolverCallValidationTest, Undeclared_Invalid) {
+  // fn main() {func(); return; }
+  // fn func() { return; }
+
+  SetSource(Source::Location{12, 34});
+  auto* call_expr = Call("func");
+  ast::VariableList params0;
+
+  Func("main", params0, ty.f32(),
+       ast::StatementList{
+           create<ast::CallStatement>(call_expr),
+           Return(),
+       },
+       ast::DecorationList{});
+
+  Func("func", params0, ty.f32(),
+       ast::StatementList{
+           Return(),
+       },
+       ast::DecorationList{});
+
+  EXPECT_FALSE(r()->Resolve());
+
+  EXPECT_EQ(r()->error(),
+            "12:34 error: v-0006: unable to find called function: func");
+}
+
+TEST_F(ResolverCallValidationTest, TooFewArgs) {
+  Func("foo", {Param(Sym(), ty.i32()), Param(Sym(), ty.f32())}, ty.void_(),
+       {Return()});
+  auto* call = Call(Source{{12, 34}}, "foo", 1);
+  WrapInFunction(call);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(
+      r()->error(),
+      "12:34 error: too few arguments in call to 'foo', expected 2, got 1");
+}
+
+TEST_F(ResolverCallValidationTest, TooManyArgs) {
+  Func("foo", {Param(Sym(), ty.i32()), Param(Sym(), ty.f32())}, ty.void_(),
+       {Return()});
+  auto* call = Call(Source{{12, 34}}, "foo", 1, 1.0f, 1.0f);
+  WrapInFunction(call);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(
+      r()->error(),
+      "12:34 error: too many arguments in call to 'foo', expected 2, got 3");
+}
+
+TEST_F(ResolverCallValidationTest, MismatchedArgs) {
+  Func("foo", {Param(Sym(), ty.i32()), Param(Sym(), ty.f32())}, ty.void_(),
+       {Return()});
+  auto* call = Call("foo", Expr(Source{{12, 34}}, true), 1.0f);
+  WrapInFunction(call);
+
+  EXPECT_FALSE(r()->Resolve());
+  EXPECT_EQ(r()->error(),
+            "12:34 error: type mismatch for argument 1 in call to 'foo', "
+            "expected 'i32', got 'bool'");
+}
+
+TEST_F(ResolverCallValidationTest, UnusedRetval) {
+  // fn main() {func(); return; }
+  // fn func() { return; }
+
+  Func("func", {}, ty.f32(), {Return(Expr(1.0f))}, {});
+
+  Func("main", {}, ty.f32(),
+       ast::StatementList{
+           create<ast::CallStatement>(Source{{12, 34}}, Call("func")),
+           Return(),
+       },
+       {});
+
+  EXPECT_FALSE(r()->Resolve());
+
+  EXPECT_EQ(r()->error(),
+            "12:34 error: result of called function was not used. If this was "
+            "intentional wrap the function call in ignore()");
+}
+
+}  // namespace
+}  // namespace resolver
+}  // namespace tint

src/resolver/intrinsic_test.cc

@@ -55,7 +55,7 @@ TEST_P(ResolverIntrinsicDerivativeTest, Scalar) {
   Global("ident", ty.f32(), ast::StorageClass::kInput);
 
   auto* expr = Call(name, "ident");
-  Func("func", {}, ty.void_(), {create<ast::CallStatement>(expr)},
+  Func("func", {}, ty.void_(), {Ignore(expr)},
        {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
 
   EXPECT_TRUE(r()->Resolve()) << r()->error();
@@ -69,7 +69,7 @@ TEST_P(ResolverIntrinsicDerivativeTest, Vector) {
   Global("ident", ty.vec4<f32>(), ast::StorageClass::kInput);
 
   auto* expr = Call(name, "ident");
-  Func("func", {}, ty.void_(), {create<ast::CallStatement>(expr)},
+  Func("func", {}, ty.void_(), {Ignore(expr)},
        {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
 
   EXPECT_TRUE(r()->Resolve()) << r()->error();
@@ -1938,7 +1938,7 @@ TEST_P(ResolverIntrinsicTest_Texture, Call) {
   param.buildSamplerVariable(this);
 
   auto* call = Call(param.function, param.args(this));
-  Func("func", {}, ty.void_(), {create<ast::CallStatement>(call)},
+  Func("func", {}, ty.void_(), {Ignore(call)},
        {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
 
   ASSERT_TRUE(r()->Resolve()) << r()->error();

src/resolver/intrinsic_validation_test.cc

@@ -53,8 +53,7 @@ TEST_F(ResolverIntrinsicValidationTest, InvalidPipelineStageDirect) {
 
   auto* dpdx = create<ast::CallExpression>(Source{{3, 4}}, Expr("dpdx"),
                                            ast::ExpressionList{Expr(1.0f)});
-  Func(Source{{1, 2}}, "func", ast::VariableList{}, ty.void_(),
-       {create<ast::CallStatement>(dpdx)},
+  Func(Source{{1, 2}}, "func", ast::VariableList{}, ty.void_(), {Ignore(dpdx)},
        {Stage(ast::PipelineStage::kCompute)});
 
   EXPECT_FALSE(r()->Resolve());
@@ -70,18 +69,16 @@ TEST_F(ResolverIntrinsicValidationTest, InvalidPipelineStageIndirect) {
 
   auto* dpdx = create<ast::CallExpression>(Source{{3, 4}}, Expr("dpdx"),
                                            ast::ExpressionList{Expr(1.0f)});
-  Func(Source{{1, 2}}, "f0", ast::VariableList{}, ty.void_(),
-       {create<ast::CallStatement>(dpdx)});
+  Func(Source{{1, 2}}, "f0", ast::VariableList{}, ty.void_(), {Ignore(dpdx)});
 
   Func(Source{{3, 4}}, "f1", ast::VariableList{}, ty.void_(),
-       {create<ast::CallStatement>(Call("f0"))});
+       {Ignore(Call("f0"))});
 
   Func(Source{{5, 6}}, "f2", ast::VariableList{}, ty.void_(),
-       {create<ast::CallStatement>(Call("f1"))});
+       {Ignore(Call("f1"))});
 
   Func(Source{{7, 8}}, "main", ast::VariableList{}, ty.void_(),
-       {create<ast::CallStatement>(Call("f2"))},
-       {Stage(ast::PipelineStage::kCompute)});
+       {Ignore(Call("f2"))}, {Stage(ast::PipelineStage::kCompute)});
 
   EXPECT_FALSE(r()->Resolve());
   EXPECT_EQ(r()->error(),

src/resolver/resolver.cc

@@ -1436,7 +1436,13 @@ bool Resolver::Statement(ast::Statement* stmt) {
   }
   if (auto* c = stmt->As<ast::CallStatement>()) {
     Mark(c->expr());
-    return Expression(c->expr());
+    if (!Expression(c->expr())) {
+      return false;
+    }
+    if (!ValidateCallStatement(c)) {
+      return false;
+    }
+    return true;
   }
   if (auto* c = stmt->As<ast::CaseStatement>()) {
     return CaseStatement(c);
@@ -1710,6 +1716,40 @@ bool Resolver::Call(ast::CallExpression* call) {
   return true;
 }
 
+bool Resolver::ValidateCallStatement(ast::CallStatement* stmt) {
+  const sem::Type* return_type = nullptr;
+  // A function call is made to either a user declared function or an intrinsic.
+  // function_calls_ only maps CallExpression to user declared functions
+  auto it = function_calls_.find(stmt->expr());
+  if (it != function_calls_.end()) {
+    return_type = it->second.function->return_type;
+  } else {
+    // Must be an intrinsic call
+    auto* target = builder_->Sem().Get(stmt->expr())->Target();
+    if (auto* intrinsic = target->As<sem::Intrinsic>()) {
+      return_type = intrinsic->ReturnType();
+    } else {
+      TINT_ICE(diagnostics_) << "call target was not an intrinsic, but a "
+                             << intrinsic->TypeInfo().name;
+    }
+  }
+
+  if (!return_type->Is<sem::Void>()) {
+    // https://gpuweb.github.io/gpuweb/wgsl/#function-call-statement
+    // A function call statement executes a function call where the called
+    // function does not return a value. If the called function returns a value,
+    // that value must be consumed either through assignment, evaluation in
+    // another expression or through use of the ignore built-in function (see
+    // § 16.13 Value-steering functions).
+    diagnostics_.add_error(
+        "result of called function was not used. If this was intentional wrap "
+        "the function call in ignore()",
+        stmt->source());
+    return false;
+  }
+  return true;
+}
+
 bool Resolver::IntrinsicCall(ast::CallExpression* call,
                              sem::IntrinsicType intrinsic_type) {
   std::vector<const sem::Type*> arg_tys;

src/resolver/resolver.h

@@ -39,6 +39,7 @@ class BinaryExpression;
 class BitcastExpression;
 class CallExpression;
 class CaseStatement;
+class CallStatement;
 class ConstructorExpression;
 class Function;
 class IdentifierExpression;
@@ -255,6 +256,7 @@ class Resolver {
                                      uint32_t el_align,
                                      const Source& source);
   bool ValidateAssignment(const ast::AssignmentStatement* a);
+  bool ValidateCallStatement(ast::CallStatement* stmt);
   bool ValidateEntryPoint(const ast::Function* func, const FunctionInfo* info);
   bool ValidateFunction(const ast::Function* func, const FunctionInfo* info);
   bool ValidateGlobalVariable(const VariableInfo* var);

src/resolver/resolver_test.cc

@@ -266,7 +266,7 @@ TEST_F(ResolverTest, Stmt_Switch) {
 
 TEST_F(ResolverTest, Stmt_Call) {
   ast::VariableList params;
-  Func("my_func", params, ty.f32(), {Return(0.0f)}, ast::DecorationList{});
+  Func("my_func", params, ty.void_(), {Return()}, ast::DecorationList{});
 
   auto* expr = Call("my_func");
 
@@ -276,7 +276,7 @@ TEST_F(ResolverTest, Stmt_Call) {
   EXPECT_TRUE(r()->Resolve()) << r()->error();
 
   ASSERT_NE(TypeOf(expr), nullptr);
-  EXPECT_TRUE(TypeOf(expr)->Is<sem::F32>());
+  EXPECT_TRUE(TypeOf(expr)->Is<sem::Void>());
   EXPECT_EQ(StmtOf(expr), call);
 }