[Validation] v-0006: variables must be defined before use
Bug:tint 6
Change-Id: I22f3117a8d59eaba97166de1f188156a9e3cd7a0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/26381
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Sarah Mashayekhi <sarahmashay@google.com>
diff --git a/src/validator_impl.cc b/src/validator_impl.cc
index 923ffa8..ae7bbef 100644
--- a/src/validator_impl.cc
+++ b/src/validator_impl.cc
@@ -66,6 +66,9 @@
}
bool ValidatorImpl::ValidateStatements(const ast::BlockStatement* block) {
+ if (!block) {
+ return false;
+ }
for (const auto& stmt : *block) {
// TODO(sarahM0): move the folowing to a function
if (stmt->IsVariableDecl()) {
@@ -80,13 +83,35 @@
}
bool ValidatorImpl::ValidateStatement(const ast::Statement* stmt) {
- if (stmt->IsAssign() && !ValidateAssign(stmt->AsAssign()))
+ if (!stmt) {
return false;
-
+ }
+ if (stmt->IsAssign()) {
+ return ValidateAssign(stmt->AsAssign());
+ }
return true;
}
bool ValidatorImpl::ValidateAssign(const ast::AssignmentStatement* a) {
+ if (!a) {
+ return false;
+ }
+ if (!(ValidateExpression(a->lhs()) && ValidateExpression(a->rhs()))) {
+ return false;
+ }
+ if (!ValidateResultTypes(a)) {
+ return false;
+ }
+
+ return true;
+}
+
+bool ValidatorImpl::ValidateResultTypes(const ast::AssignmentStatement* a) {
+ if (!a->lhs()->result_type() || !a->rhs()->result_type()) {
+ set_error(a->source(), "result_type() is nullptr");
+ return false;
+ }
+
auto* lhs_result_type = a->lhs()->result_type()->UnwrapAliasPtrAlias();
auto* rhs_result_type = a->rhs()->result_type()->UnwrapAliasPtrAlias();
if (lhs_result_type != rhs_result_type) {
@@ -99,6 +124,27 @@
return true;
}
+bool ValidatorImpl::ValidateExpression(const ast::Expression* expr) {
+ if (!expr) {
+ return false;
+ }
+ if (expr->IsIdentifier()) {
+ return ValidateIdentifier(expr->AsIdentifier());
+ }
+
+ return true;
+}
+
+bool ValidatorImpl::ValidateIdentifier(const ast::IdentifierExpression* ident) {
+ ast::Variable* var;
+ if (!variable_stack_.get(ident->name(), &var)) {
+ set_error(ident->source(),
+ "v-0006: '" + ident->name() + "' is not declared");
+ return false;
+ }
+ return true;
+}
+
bool ValidatorImpl::CheckImports(const ast::Module* module) {
for (const auto& import : module->imports()) {
if (import->path() != "GLSL.std.450") {
diff --git a/src/validator_impl.h b/src/validator_impl.h
index d4f4534..1fa0205 100644
--- a/src/validator_impl.h
+++ b/src/validator_impl.h
@@ -19,6 +19,7 @@
#include "src/ast/assignment_statement.h"
#include "src/ast/expression.h"
+#include "src/ast/identifier_expression.h"
#include "src/ast/module.h"
#include "src/ast/statement.h"
#include "src/ast/variable.h"
@@ -72,6 +73,18 @@
/// @param module the modele to check imports
/// @returns ture if input complies with v-0001 rule
bool CheckImports(const ast::Module* module);
+ /// Validates an expression
+ /// @param expr the expression to check
+ /// @return true if the expresssion is valid
+ bool ValidateExpression(const ast::Expression* expr);
+ /// Validates v-0006:Variables must be defined before use
+ /// @param ident the identifer to check if its in the scope
+ /// @return true if idnet was defined
+ bool ValidateIdentifier(const ast::IdentifierExpression* ident);
+ /// Validates if the input follows type checking rules
+ /// @param 'a' the assignment to check
+ /// @returns ture if successful
+ bool ValidateResultTypes(const ast::AssignmentStatement* a);
private:
std::string error_;
diff --git a/src/validator_test.cc b/src/validator_test.cc
index 994db44..fe485fc 100644
--- a/src/validator_test.cc
+++ b/src/validator_test.cc
@@ -20,6 +20,7 @@
#include "src/ast/as_expression.h"
#include "src/ast/assignment_statement.h"
#include "src/ast/binary_expression.h"
+#include "src/ast/bool_literal.h"
#include "src/ast/break_statement.h"
#include "src/ast/call_expression.h"
#include "src/ast/case_statement.h"
@@ -48,6 +49,8 @@
#include "src/ast/type/struct_type.h"
#include "src/ast/type/vector_type.h"
#include "src/ast/type_constructor_expression.h"
+#include "src/ast/variable.h"
+#include "src/ast/variable_decl_statement.h"
#include "src/type_determiner.h"
namespace tint {
@@ -59,6 +62,7 @@
: td_(std::make_unique<TypeDeterminer>(&ctx_, &mod_)) {}
TypeDeterminer* td() const { return td_.get(); }
+ ast::Module* mod() { return &mod_; }
private:
Context ctx_;
@@ -105,7 +109,7 @@
auto lhs = std::make_unique<ast::ScalarConstructorExpression>(
std::make_unique<ast::SintLiteral>(&i32, 1));
auto rhs = std::make_unique<ast::IdentifierExpression>("my_var");
- ast::AssignmentStatement assign(Source{12, 32}, std::move(lhs),
+ ast::AssignmentStatement assign(Source{12, 34}, std::move(lhs),
std::move(rhs));
tint::ValidatorImpl v;
@@ -115,13 +119,79 @@
EXPECT_EQ(v.error(), "12:34: v-000x: invalid assignment");
}
+TEST_F(ValidatorTest, UsingUndefinedVariable_Fail) {
+ // b = 2;
+ ast::type::I32Type i32;
+
+ auto lhs = std::make_unique<ast::IdentifierExpression>(Source{12, 34}, "b");
+ auto rhs = std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::SintLiteral>(&i32, 2));
+ auto assign = std::make_unique<ast::AssignmentStatement>(
+ Source{12, 34}, std::move(lhs), std::move(rhs));
+
+ EXPECT_TRUE(td()->DetermineResultType(assign.get())) << td()->error();
+ tint::ValidatorImpl v;
+ EXPECT_FALSE(v.ValidateStatement(assign.get()));
+ EXPECT_EQ(v.error(), "12:34: v-0006: 'b' is not declared");
+}
+
+TEST_F(ValidatorTest, UsingUndefinedVariableInBlockStatement_Fail) {
+ // {
+ // b = 2;
+ // }
+ ast::type::I32Type i32;
+
+ auto lhs = std::make_unique<ast::IdentifierExpression>(Source{12, 34}, "b");
+ auto rhs = std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::SintLiteral>(&i32, 2));
+
+ auto body = std::make_unique<ast::BlockStatement>();
+ body->append(std::make_unique<ast::AssignmentStatement>(
+ Source{12, 34}, std::move(lhs), std::move(rhs)));
+
+ EXPECT_TRUE(td()->DetermineStatements(body.get())) << td()->error();
+ tint::ValidatorImpl v;
+ EXPECT_FALSE(v.ValidateStatements(body.get()));
+ EXPECT_EQ(v.error(), "12:34: v-0006: 'b' is not declared");
+}
+
+TEST_F(ValidatorTest, AssignCompatibleTypes_Pass) {
+ // var a :i32 = 2;
+ // a = 2
+ ast::type::I32Type i32;
+ auto var =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &i32);
+ var->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::SintLiteral>(&i32, 2)));
+
+ auto lhs = std::make_unique<ast::IdentifierExpression>("a");
+ auto* lhs_ptr = lhs.get();
+ auto rhs = std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::SintLiteral>(&i32, 2));
+ auto* rhs_ptr = rhs.get();
+
+ ast::AssignmentStatement assign(Source{12, 34}, std::move(lhs),
+ std::move(rhs));
+ td()->RegisterVariableForTesting(var.get());
+ EXPECT_TRUE(td()->DetermineResultType(&assign)) << td()->error();
+ ASSERT_NE(lhs_ptr->result_type(), nullptr);
+ ASSERT_NE(rhs_ptr->result_type(), nullptr);
+ tint::ValidatorImpl v;
+ EXPECT_TRUE(v.ValidateResultTypes(&assign));
+}
+
TEST_F(ValidatorTest, AssignIncompatibleTypes_Fail) {
- // var a :i32;
- // a = 2.3;
+ // {
+ // var a :i32 = 2;
+ // a = 2.3;
+ // }
ast::type::F32Type f32;
ast::type::I32Type i32;
- ast::Variable var("a", ast::StorageClass::kPrivate, &i32);
+ auto var =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &i32);
+ var->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::SintLiteral>(&i32, 2)));
auto lhs = std::make_unique<ast::IdentifierExpression>("a");
auto* lhs_ptr = lhs.get();
auto rhs = std::make_unique<ast::ScalarConstructorExpression>(
@@ -130,36 +200,217 @@
ast::AssignmentStatement assign(Source{12, 34}, std::move(lhs),
std::move(rhs));
- td()->RegisterVariableForTesting(&var);
+ td()->RegisterVariableForTesting(var.get());
EXPECT_TRUE(td()->DetermineResultType(&assign)) << td()->error();
ASSERT_NE(lhs_ptr->result_type(), nullptr);
ASSERT_NE(rhs_ptr->result_type(), nullptr);
tint::ValidatorImpl v;
- EXPECT_FALSE(v.ValidateAssign(&assign));
+ EXPECT_FALSE(v.ValidateResultTypes(&assign));
ASSERT_TRUE(v.has_error());
// TODO(sarahM0): figure out what should be the error number.
EXPECT_EQ(v.error(),
"12:34: v-000x: invalid assignment of '__i32' to '__f32'");
}
-TEST_F(ValidatorTest, AssignCompatibleTypes_Pass) {
- // var a :i32;
- // a = 2;
+TEST_F(ValidatorTest, AssignCompatibleTypesInBlockStatement_Pass) {
+ // {
+ // var a :i32 = 2;
+ // a = 2
+ // }
ast::type::I32Type i32;
+ auto var =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &i32);
+ var->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::SintLiteral>(&i32, 2)));
- ast::Variable var("a", ast::StorageClass::kPrivate, &i32);
auto lhs = std::make_unique<ast::IdentifierExpression>("a");
+ auto* lhs_ptr = lhs.get();
auto rhs = std::make_unique<ast::ScalarConstructorExpression>(
std::make_unique<ast::SintLiteral>(&i32, 2));
+ auto* rhs_ptr = rhs.get();
- ast::AssignmentStatement assign(Source{12, 34}, std::move(lhs),
- std::move(rhs));
+ auto body = std::make_unique<ast::BlockStatement>();
+ body->append(std::make_unique<ast::VariableDeclStatement>(std::move(var)));
+ body->append(std::make_unique<ast::AssignmentStatement>(
+ Source{12, 34}, std::move(lhs), std::move(rhs)));
- td()->RegisterVariableForTesting(&var);
- EXPECT_TRUE(td()->DetermineResultType(&assign)) << td()->error();
+ EXPECT_TRUE(td()->DetermineStatements(body.get())) << td()->error();
+ ASSERT_NE(lhs_ptr->result_type(), nullptr);
+ ASSERT_NE(rhs_ptr->result_type(), nullptr);
+
tint::ValidatorImpl v;
- EXPECT_TRUE(v.ValidateAssign(&assign));
+ EXPECT_TRUE(v.ValidateStatements(body.get()));
+}
+
+TEST_F(ValidatorTest, AssignIncompatibleTypesInBlockStatement_Fail) {
+ // {
+ // var a :i32 = 2;
+ // a = 2.3;
+ // }
+ ast::type::F32Type f32;
+ ast::type::I32Type i32;
+
+ auto var =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &i32);
+ var->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::SintLiteral>(&i32, 2)));
+ auto lhs = std::make_unique<ast::IdentifierExpression>("a");
+ auto* lhs_ptr = lhs.get();
+ auto rhs = std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 2.3f));
+ auto* rhs_ptr = rhs.get();
+
+ ast::BlockStatement block;
+ block.append(std::make_unique<ast::VariableDeclStatement>(std::move(var)));
+ block.append(std::make_unique<ast::AssignmentStatement>(
+ Source{12, 34}, std::move(lhs), std::move(rhs)));
+
+ EXPECT_TRUE(td()->DetermineStatements(&block)) << td()->error();
+ ASSERT_NE(lhs_ptr->result_type(), nullptr);
+ ASSERT_NE(rhs_ptr->result_type(), nullptr);
+
+ tint::ValidatorImpl v;
+ EXPECT_FALSE(v.ValidateStatements(&block));
+ ASSERT_TRUE(v.has_error());
+ // TODO(sarahM0): figure out what should be the error number.
+ EXPECT_EQ(v.error(),
+ "12:34: v-000x: invalid assignment of '__i32' to '__f32'");
+}
+
+TEST_F(ValidatorTest, UnsingUndefinedVariableGlobalVariable_Fail) {
+ ast::type::F32Type f32;
+ auto global_var = std::make_unique<ast::Variable>(
+ "global_var", ast::StorageClass::kPrivate, &f32);
+ global_var->set_constructor(
+ std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 2.1)));
+ mod()->AddGlobalVariable(std::move(global_var));
+
+ auto lhs = std::make_unique<ast::IdentifierExpression>(Source{12, 34},
+ "not_global_var");
+ auto rhs = std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 3.14f));
+
+ ast::VariableList params;
+ auto func =
+ std::make_unique<ast::Function>("my_func", std::move(params), &f32);
+
+ auto body = std::make_unique<ast::BlockStatement>();
+ body->append(std::make_unique<ast::AssignmentStatement>(
+ Source{12, 34}, std::move(lhs), std::move(rhs)));
+ func->set_body(std::move(body));
+ mod()->AddFunction(std::move(func));
+
+ tint::ValidatorImpl v;
+ EXPECT_FALSE(v.Validate(mod()));
+ EXPECT_EQ(v.error(), "12:34: v-0006: 'not_global_var' is not declared");
+}
+
+TEST_F(ValidatorTest, UnsingUndefinedVariableGlobalVariable_Pass) {
+ ast::type::F32Type f32;
+ auto global_var = std::make_unique<ast::Variable>(
+ "global_var", ast::StorageClass::kPrivate, &f32);
+ global_var->set_constructor(
+ std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 2.1)));
+ mod()->AddGlobalVariable(std::move(global_var));
+
+ auto lhs = std::make_unique<ast::IdentifierExpression>("global_var");
+ auto* lhs_ptr = lhs.get();
+ auto rhs = std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 3.14f));
+ auto* rhs_ptr = rhs.get();
+
+ ast::VariableList params;
+ auto func =
+ std::make_unique<ast::Function>("my_func", std::move(params), &f32);
+
+ auto body = std::make_unique<ast::BlockStatement>();
+ body->append(std::make_unique<ast::AssignmentStatement>(
+ Source{12, 34}, std::move(lhs), std::move(rhs)));
+ func->set_body(std::move(body));
+ auto* func_ptr = func.get();
+ mod()->AddFunction(std::move(func));
+
+ EXPECT_TRUE(td()->Determine()) << td()->error();
+ EXPECT_TRUE(td()->DetermineFunction(func_ptr)) << td()->error();
+ tint::ValidatorImpl v;
+ ASSERT_NE(lhs_ptr->result_type(), nullptr);
+ ASSERT_NE(rhs_ptr->result_type(), nullptr);
+ EXPECT_TRUE(v.Validate(mod())) << v.error();
+}
+
+TEST_F(ValidatorTest, UnsingUndefinedVariableInnerScope_Fail) {
+ // {
+ // if (true) { var a : f32 = 2.0; }
+ // a = 3.14;
+ // }
+ ast::type::F32Type f32;
+ auto var =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &f32);
+ var->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 2.0)));
+
+ ast::type::BoolType bool_type;
+ auto cond = std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::BoolLiteral>(&bool_type, true));
+ auto body = std::make_unique<ast::BlockStatement>();
+ body->append(std::make_unique<ast::VariableDeclStatement>(std::move(var)));
+
+ auto lhs = std::make_unique<ast::IdentifierExpression>(Source{12, 34}, "a");
+ auto* lhs_ptr = lhs.get();
+ auto rhs = std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 3.14f));
+ auto* rhs_ptr = rhs.get();
+
+ auto outer_body = std::make_unique<ast::BlockStatement>();
+ outer_body->append(
+ std::make_unique<ast::IfStatement>(std::move(cond), std::move(body)));
+ outer_body->append(std::make_unique<ast::AssignmentStatement>(
+ Source{12, 34}, std::move(lhs), std::move(rhs)));
+
+ EXPECT_TRUE(td()->DetermineStatements(outer_body.get())) << td()->error();
+ tint::ValidatorImpl v;
+ ASSERT_NE(lhs_ptr->result_type(), nullptr);
+ ASSERT_NE(rhs_ptr->result_type(), nullptr);
+ EXPECT_FALSE(v.ValidateStatements(outer_body.get()));
+ EXPECT_EQ(v.error(), "12:34: v-0006: 'a' is not declared");
+}
+
+TEST_F(ValidatorTest, UnsingUndefinedVariableOuterScope_Pass) {
+ // {
+ // var a : f32 = 2.0;
+ // if (true) { a = 3.14; }
+ // }
+ ast::type::F32Type f32;
+ auto var =
+ std::make_unique<ast::Variable>("a", ast::StorageClass::kNone, &f32);
+ var->set_constructor(std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 2.0)));
+
+ auto lhs = std::make_unique<ast::IdentifierExpression>(Source{12, 34}, "a");
+ auto* lhs_ptr = lhs.get();
+ auto rhs = std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::FloatLiteral>(&f32, 3.14f));
+ auto* rhs_ptr = rhs.get();
+ ast::type::BoolType bool_type;
+ auto cond = std::make_unique<ast::ScalarConstructorExpression>(
+ std::make_unique<ast::BoolLiteral>(&bool_type, true));
+ auto body = std::make_unique<ast::BlockStatement>();
+ body->append(std::make_unique<ast::AssignmentStatement>(
+ Source{12, 34}, std::move(lhs), std::move(rhs)));
+
+ auto outer_body = std::make_unique<ast::BlockStatement>();
+ outer_body->append(
+ std::make_unique<ast::VariableDeclStatement>(std::move(var)));
+ outer_body->append(
+ std::make_unique<ast::IfStatement>(std::move(cond), std::move(body)));
+ EXPECT_TRUE(td()->DetermineStatements(outer_body.get())) << td()->error();
+ tint::ValidatorImpl v;
+ ASSERT_NE(lhs_ptr->result_type(), nullptr);
+ ASSERT_NE(rhs_ptr->result_type(), nullptr);
+ EXPECT_TRUE(v.ValidateStatements(outer_body.get())) << v.error();
}
TEST_F(ValidatorTest, DISABLED_AssignToConstant_Fail) {
diff --git a/test/undefined-variable-global-scope.wgsl b/test/undefined-variable-global-scope.wgsl
new file mode 100644
index 0000000..5fa9fb2
--- /dev/null
+++ b/test/undefined-variable-global-scope.wgsl
@@ -0,0 +1,23 @@
+# Copyright 2020 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.
+
+# Vertex shader
+var<private> a : i32;
+
+fn main() -> void {
+ a = 2;
+ return;
+}
+entry_point vertex as "main" = main;
+
diff --git a/test/undefined-variable-inner-block.wgsl b/test/undefined-variable-inner-block.wgsl
new file mode 100644
index 0000000..06fc420
--- /dev/null
+++ b/test/undefined-variable-inner-block.wgsl
@@ -0,0 +1,20 @@
+# Copyright 2020 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.
+
+fn main() -> void {
+ var a : f32 = 2.0;
+ { a = 3.14;}
+ return;
+}
+entry_point fragment = main;
