[validation] Validates return statement

This CL checks if functions end with a return statement (v-0002)
reworks previously added tests to pass
enables related tests

Bug: tint: 6
Change-Id: Iafe46581ccc50e146b33d33f9577d995a7f80d77
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/26722
Commit-Queue: Sarah Mashayekhi <sarahmashay@google.com>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
This commit is contained in:
Sarah Mashayekhi 2020-08-13 18:09:59 +00:00 committed by Commit Bot service account
parent a532ac55d9
commit d73f812c84
4 changed files with 23 additions and 14 deletions

View File

@ -52,9 +52,6 @@ class VertexPullingTransformHelper {
tint::TypeDeterminer td(&ctx_, mod_.get()); tint::TypeDeterminer td(&ctx_, mod_.get());
EXPECT_TRUE(td.Determine()); EXPECT_TRUE(td.Determine());
tint::Validator v;
EXPECT_TRUE(v.Validate(mod_.get()));
transform_->SetVertexState( transform_->SetVertexState(
std::make_unique<VertexStateDescriptor>(std::move(vertex_state))); std::make_unique<VertexStateDescriptor>(std::move(vertex_state)));
transform_->SetEntryPoint("main"); transform_->SetEntryPoint("main");

View File

@ -47,7 +47,7 @@ class TypeDeterminerHelper {
class ValidateFunctionTest : public TypeDeterminerHelper, class ValidateFunctionTest : public TypeDeterminerHelper,
public testing::Test {}; public testing::Test {};
TEST_F(ValidateFunctionTest, DISABLED_FunctionEndWithoutReturnStatement_Fail) { TEST_F(ValidateFunctionTest, FunctionEndWithoutReturnStatement_Fail) {
// fn func -> void { var a:i32 = 2; } // fn func -> void { var a:i32 = 2; }
ast::type::I32Type i32; ast::type::I32Type i32;
@ -69,11 +69,10 @@ TEST_F(ValidateFunctionTest, DISABLED_FunctionEndWithoutReturnStatement_Fail) {
tint::ValidatorImpl v; tint::ValidatorImpl v;
EXPECT_FALSE(v.Validate(mod())); EXPECT_FALSE(v.Validate(mod()));
EXPECT_EQ(v.error(), EXPECT_EQ(v.error(),
"12:34: v-0002: 'function must end with a return statement 'func'"); "12:34: v-0002: function must end with a return statement");
} }
TEST_F(ValidateFunctionTest, TEST_F(ValidateFunctionTest, FunctionEndWithoutReturnStatementEmptyBody_Fail) {
DISABLED_FunctionEndWithoutReturnStatementEmptyBody_Fail) {
// fn func -> void {} // fn func -> void {}
ast::type::VoidType void_type; ast::type::VoidType void_type;
ast::VariableList params; ast::VariableList params;
@ -85,7 +84,7 @@ TEST_F(ValidateFunctionTest,
tint::ValidatorImpl v; tint::ValidatorImpl v;
EXPECT_FALSE(v.Validate(mod())); EXPECT_FALSE(v.Validate(mod()));
EXPECT_EQ(v.error(), EXPECT_EQ(v.error(),
"12:34: v-0002: 'function must end with a return statement 'func'"); "12:34: v-0002: function must end with a return statement");
} }
} // namespace } // namespace

View File

@ -13,6 +13,7 @@
// limitations under the License. // limitations under the License.
#include "src/validator_impl.h" #include "src/validator_impl.h"
#include "src/ast/function.h"
#include "src/ast/variable_decl_statement.h" #include "src/ast/variable_decl_statement.h"
namespace tint { namespace tint {
@ -65,8 +66,13 @@ bool ValidatorImpl::ValidateFunction(const ast::Function* func) {
if (!ValidateStatements(func->body())) { if (!ValidateStatements(func->body())) {
return false; return false;
} }
variable_stack_.pop_scope(); variable_stack_.pop_scope();
if (!func->get_last_statement() || !func->get_last_statement()->IsReturn()) {
set_error(func->source(),
"v-0002: function must end with a return statement");
return false;
}
return true; return true;
} }

View File

@ -316,6 +316,7 @@ TEST_F(ValidatorTest, UsingUndefinedVariableGlobalVariable_Pass) {
// var global_var: f32 = 2.1; // var global_var: f32 = 2.1;
// fn my_func() -> f32 { // fn my_func() -> f32 {
// global_var = 3.14; // global_var = 3.14;
// return 3.14;
// } // }
ast::type::F32Type f32; ast::type::F32Type f32;
auto global_var = std::make_unique<ast::Variable>( auto global_var = std::make_unique<ast::Variable>(
@ -330,6 +331,8 @@ TEST_F(ValidatorTest, UsingUndefinedVariableGlobalVariable_Pass) {
auto rhs = std::make_unique<ast::ScalarConstructorExpression>( auto rhs = std::make_unique<ast::ScalarConstructorExpression>(
std::make_unique<ast::FloatLiteral>(&f32, 3.14f)); std::make_unique<ast::FloatLiteral>(&f32, 3.14f));
auto* rhs_ptr = rhs.get(); auto* rhs_ptr = rhs.get();
auto return_expr = std::make_unique<ast::ScalarConstructorExpression>(
std::make_unique<ast::FloatLiteral>(&f32, 3.14f));
ast::VariableList params; ast::VariableList params;
auto func = auto func =
@ -338,6 +341,7 @@ TEST_F(ValidatorTest, UsingUndefinedVariableGlobalVariable_Pass) {
auto body = std::make_unique<ast::BlockStatement>(); auto body = std::make_unique<ast::BlockStatement>();
body->append(std::make_unique<ast::AssignmentStatement>( body->append(std::make_unique<ast::AssignmentStatement>(
Source{12, 34}, std::move(lhs), std::move(rhs))); Source{12, 34}, std::move(lhs), std::move(rhs)));
body->append(std::make_unique<ast::ReturnStatement>(std::move(return_expr)));
func->set_body(std::move(body)); func->set_body(std::move(body));
auto* func_ptr = func.get(); auto* func_ptr = func.get();
mod()->AddFunction(std::move(func)); mod()->AddFunction(std::move(func));
@ -645,25 +649,27 @@ TEST_F(ValidatorTest, DISABLED_RedeclaredIdentifierInnerScope_False) {
} }
TEST_F(ValidatorTest, RedeclaredIdentifierDifferentFunctions_Pass) { TEST_F(ValidatorTest, RedeclaredIdentifierDifferentFunctions_Pass) {
// func0 { var a : f32 = 2.0; } // func0 { var a : f32 = 2.0; return; }
// func1 { var a : f32 = 3.0; } // func1 { var a : f32 = 3.0; return; }
ast::type::F32Type f32; ast::type::F32Type f32;
ast::type::VoidType void_type;
auto var0 = auto var0 =
std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &f32); std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &f32);
var0->set_constructor(std::make_unique<ast::ScalarConstructorExpression>( var0->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
std::make_unique<ast::FloatLiteral>(&f32, 2.0))); std::make_unique<ast::FloatLiteral>(&f32, 2.0)));
auto var1 = auto var1 = std::make_unique<ast::Variable>("a", ast::StorageClass::kNone,
std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &f32); &void_type);
var1->set_constructor(std::make_unique<ast::ScalarConstructorExpression>( var1->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
std::make_unique<ast::FloatLiteral>(&f32, 1.0))); std::make_unique<ast::FloatLiteral>(&f32, 1.0)));
ast::VariableList params0; ast::VariableList params0;
auto func0 = auto func0 =
std::make_unique<ast::Function>("func0", std::move(params0), &f32); std::make_unique<ast::Function>("func0", std::move(params0), &void_type);
auto body0 = std::make_unique<ast::BlockStatement>(); auto body0 = std::make_unique<ast::BlockStatement>();
body0->append(std::make_unique<ast::VariableDeclStatement>(Source{12, 34}, body0->append(std::make_unique<ast::VariableDeclStatement>(Source{12, 34},
std::move(var0))); std::move(var0)));
body0->append(std::make_unique<ast::ReturnStatement>());
func0->set_body(std::move(body0)); func0->set_body(std::move(body0));
ast::VariableList params1; ast::VariableList params1;
@ -672,6 +678,7 @@ TEST_F(ValidatorTest, RedeclaredIdentifierDifferentFunctions_Pass) {
auto body1 = std::make_unique<ast::BlockStatement>(); auto body1 = std::make_unique<ast::BlockStatement>();
body1->append(std::make_unique<ast::VariableDeclStatement>(Source{13, 34}, body1->append(std::make_unique<ast::VariableDeclStatement>(Source{13, 34},
std::move(var1))); std::move(var1)));
body1->append(std::make_unique<ast::ReturnStatement>());
func1->set_body(std::move(body1)); func1->set_body(std::move(body1));
mod()->AddFunction(std::move(func0)); mod()->AddFunction(std::move(func0));