validator: Support assignment through pointer
Also enable a test to check assigning to scalar literal.
Fixed: tint:419
Change-Id: Ic565af22c4ef6b60c41faaf9fabe3bd55fe48d2d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/37961
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
diff --git a/src/validator/validator_impl.cc b/src/validator/validator_impl.cc
index 3461e51..15dfbbd 100644
--- a/src/validator/validator_impl.cc
+++ b/src/validator/validator_impl.cc
@@ -30,6 +30,7 @@
#include "src/ast/type/array_type.h"
#include "src/ast/type/i32_type.h"
#include "src/ast/type/matrix_type.h"
+#include "src/ast/type/pointer_type.h"
#include "src/ast/type/struct_type.h"
#include "src/ast/type/u32_type.h"
#include "src/ast/type/vector_type.h"
@@ -277,6 +278,10 @@
"redeclared identifier '" + module_.SymbolToName(symbol) + "'");
return false;
}
+ // TODO(dneto): Check type compatibility of the initializer.
+ // - if it's non-constant, then is storable or can be dereferenced to be
+ // storable.
+ // - types match or the RHS can be dereferenced to equal the LHS type.
variable_stack_.set(symbol, decl->variable());
if (auto* arr =
decl->variable()->type()->UnwrapAll()->As<ast::type::Array>()) {
@@ -429,57 +434,78 @@
return true;
}
-bool ValidatorImpl::ValidateAssign(const ast::AssignmentStatement* a) {
- if (!a) {
+bool ValidatorImpl::ValidateBadAssignmentToIdentifier(
+ const ast::AssignmentStatement* assign) {
+ auto* ident = assign->lhs()->As<ast::IdentifierExpression>();
+ if (!ident) {
+ // It wasn't an identifier in the first place.
+ return true;
+ }
+ ast::Variable* var;
+ if (variable_stack_.get(ident->symbol(), &var)) {
+ // Give a nicer message if the LHS of the assignment is a const identifier.
+ // It's likely to be a common programmer error.
+ if (var->is_const()) {
+ add_error(assign->source(), "v-0021",
+ "cannot re-assign a constant: '" +
+ module_.SymbolToName(ident->symbol()) + "'");
+ return false;
+ }
+ } else {
+ // The identifier is not defined. This should already have been caught
+ // when validating the subexpression.
+ add_error(
+ ident->source(), "v-0006",
+ "'" + module_.SymbolToName(ident->symbol()) + "' is not declared");
return false;
}
- if (!(ValidateConstant(a))) {
- return false;
- }
- if (!(ValidateExpression(a->lhs()) && ValidateExpression(a->rhs()))) {
- return false;
- }
- if (!ValidateResultTypes(a)) {
- return false;
- }
-
return true;
}
-bool ValidatorImpl::ValidateConstant(const ast::AssignmentStatement* assign) {
+bool ValidatorImpl::ValidateAssign(const ast::AssignmentStatement* assign) {
if (!assign) {
return false;
}
-
- if (auto* ident = assign->lhs()->As<ast::IdentifierExpression>()) {
- ast::Variable* var;
- if (variable_stack_.get(ident->symbol(), &var)) {
- if (var->is_const()) {
- add_error(assign->source(), "v-0021",
- "cannot re-assign a constant: '" +
- module_.SymbolToName(ident->symbol()) + "'");
- return false;
- }
+ auto* lhs = assign->lhs();
+ auto* rhs = assign->rhs();
+ if (!ValidateExpression(lhs)) {
+ return false;
+ }
+ if (!ValidateExpression(rhs)) {
+ return false;
+ }
+ // Pointers are not storable in WGSL, but the right-hand side must be
+ // storable. The raw right-hand side might be a pointer value which must be
+ // loaded (dereferenced) to provide the value to be stored.
+ auto* rhs_result_type = rhs->result_type()->UnwrapAll();
+ if (!IsStorable(rhs_result_type)) {
+ add_error(assign->source(), "v-000x",
+ "invalid assignment: right-hand-side is not storable: " +
+ rhs->result_type()->type_name());
+ return false;
+ }
+ auto* lhs_result_type = lhs->result_type()->UnwrapIfNeeded();
+ if (auto* lhs_reference_type = As<ast::type::Pointer>(lhs_result_type)) {
+ auto* lhs_store_type = lhs_reference_type->type()->UnwrapIfNeeded();
+ if (lhs_store_type != rhs_result_type) {
+ add_error(assign->source(), "v-000x",
+ "invalid assignment: can't assign value of type '" +
+ rhs_result_type->type_name() + "' to '" +
+ lhs_store_type->type_name() + "'");
+ return false;
}
- }
- return true;
-}
-
-bool ValidatorImpl::ValidateResultTypes(const ast::AssignmentStatement* a) {
- if (!a->lhs()->result_type() || !a->rhs()->result_type()) {
- add_error(a->source(), "result_type() is nullptr");
+ } else {
+ if (!ValidateBadAssignmentToIdentifier(assign)) {
+ return false;
+ }
+ // Issue a generic error.
+ add_error(
+ assign->source(), "v-000x",
+ "invalid assignment: left-hand-side does not reference storage: " +
+ lhs->result_type()->type_name());
return false;
}
- auto* lhs_result_type = a->lhs()->result_type()->UnwrapAll();
- auto* rhs_result_type = a->rhs()->result_type()->UnwrapAll();
- if (lhs_result_type != rhs_result_type) {
- // TODO(sarahM0): figur out what should be the error number.
- add_error(a->source(), "v-000x",
- "invalid assignment of '" + lhs_result_type->type_name() +
- "' to '" + rhs_result_type->type_name() + "'");
- return false;
- }
return true;
}
diff --git a/src/validator/validator_impl.h b/src/validator/validator_impl.h
index d619b82..f0a1b5d 100644
--- a/src/validator/validator_impl.h
+++ b/src/validator/validator_impl.h
@@ -100,6 +100,12 @@
/// @param assign the assignment to check
/// @returns true if the validation was successful
bool ValidateAssign(const ast::AssignmentStatement* assign);
+ /// Validates a bad assignment to an identifier. Issues an error
+ /// and returns false if the left hand side is an identifier.
+ /// @param assign the assignment to check
+ /// @returns true if the LHS of theassignment is not an identifier expression
+ bool ValidateBadAssignmentToIdentifier(
+ const ast::AssignmentStatement* assign);
/// Validates an expression
/// @param expr the expression to check
/// @return true if the expression is valid
@@ -108,14 +114,6 @@
/// @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 assign the assignment to check
- /// @returns ture if successful
- bool ValidateResultTypes(const ast::AssignmentStatement* assign);
- /// Validate v-0021: Cannot re-assign a constant
- /// @param assign is the assigment to check if its lhs is a const
- /// @returns false if lhs of assign is a constant identifier
- bool ValidateConstant(const ast::AssignmentStatement* assign);
/// Validates declaration name uniqueness
/// @param decl is the new declaration to be added
/// @returns true if no previous declaration with the `decl` 's name
@@ -154,6 +152,11 @@
/// @returns true if the given type is storable.
bool IsStorable(ast::type::Type* type);
+ /// Testing method to inserting a given variable into the current scope.
+ void RegisterVariableForTesting(ast::Variable* var) {
+ variable_stack_.set(var->symbol(), var);
+ }
+
private:
const ast::Module& module_;
diag::List diags_;
diff --git a/src/validator/validator_test.cc b/src/validator/validator_test.cc
index 1aa10f5..de38988 100644
--- a/src/validator/validator_test.cc
+++ b/src/validator/validator_test.cc
@@ -60,18 +60,27 @@
class ValidatorTest : public ValidatorTestHelper, public testing::Test {};
-TEST_F(ValidatorTest, DISABLED_AssignToScalar_Fail) {
+TEST_F(ValidatorTest, AssignToScalar_Fail) {
+ // var my_var : i32 = 2;
// 1 = my_var;
+ auto* var = Var("my_var", ast::StorageClass::kNone, ty.i32, Expr(2),
+ ast::VariableDecorationList{});
+
auto* lhs = Expr(1);
auto* rhs = Expr("my_var");
SetSource(Source{Source::Location{12, 34}});
- create<ast::AssignmentStatement>(lhs, rhs);
+ auto* assign = create<ast::AssignmentStatement>(lhs, rhs);
+ RegisterVariable(var);
+ EXPECT_TRUE(td()->DetermineResultType(assign));
// TODO(sarahM0): Invalidate assignment to scalar.
+ EXPECT_FALSE(v()->ValidateAssign(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");
+ EXPECT_EQ(v()->error(),
+ "12:34 v-000x: invalid assignment: left-hand-side does not "
+ "reference storage: __i32");
}
TEST_F(ValidatorTest, UsingUndefinedVariable_Fail) {
@@ -116,11 +125,75 @@
auto* assign = create<ast::AssignmentStatement>(
Source{Source::Location{12, 34}}, lhs, rhs);
- td()->RegisterVariableForTesting(var);
+ RegisterVariable(var);
EXPECT_TRUE(td()->DetermineResultType(assign)) << td()->error();
ASSERT_NE(lhs->result_type(), nullptr);
ASSERT_NE(rhs->result_type(), nullptr);
- EXPECT_TRUE(v()->ValidateResultTypes(assign));
+ EXPECT_TRUE(v()->ValidateAssign(assign)) << v()->error();
+}
+
+TEST_F(ValidatorTest, AssignCompatibleTypesThroughAlias_Pass) {
+ // alias myint = i32;
+ // var a :myint = 2;
+ // a = 2
+ auto* myint = ty.alias("myint", ty.i32);
+ auto* var = Var("a", ast::StorageClass::kNone, myint, Expr(2),
+ ast::VariableDecorationList{});
+
+ auto* lhs = Expr("a");
+ auto* rhs = Expr(2);
+
+ auto* assign = create<ast::AssignmentStatement>(
+ Source{Source::Location{12, 34}}, lhs, rhs);
+ RegisterVariable(var);
+ EXPECT_TRUE(td()->DetermineResultType(assign)) << td()->error();
+ ASSERT_NE(lhs->result_type(), nullptr);
+ ASSERT_NE(rhs->result_type(), nullptr);
+ EXPECT_TRUE(v()->ValidateAssign(assign)) << v()->error();
+}
+
+TEST_F(ValidatorTest, AssignCompatibleTypesInferRHSLoad_Pass) {
+ // var a :i32 = 2;
+ // var b :i32 = 3;
+ // a = b;
+ auto* var_a = Var("a", ast::StorageClass::kNone, ty.i32, Expr(2),
+ ast::VariableDecorationList{});
+ auto* var_b = Var("b", ast::StorageClass::kNone, ty.i32, Expr(3),
+ ast::VariableDecorationList{});
+
+ auto* lhs = Expr("a");
+ auto* rhs = Expr("b");
+
+ auto* assign = create<ast::AssignmentStatement>(
+ Source{Source::Location{12, 34}}, lhs, rhs);
+ RegisterVariable(var_a);
+ RegisterVariable(var_b);
+ EXPECT_TRUE(td()->DetermineResultType(assign)) << td()->error();
+ ASSERT_NE(lhs->result_type(), nullptr);
+ ASSERT_NE(rhs->result_type(), nullptr);
+ EXPECT_TRUE(v()->ValidateAssign(assign)) << v()->error();
+}
+
+TEST_F(ValidatorTest, AssignThroughPointer_Pass) {
+ // var a :i32;
+ // const b : ptr<function,i32> = a;
+ // b = 2;
+ const auto func = ast::StorageClass::kFunction;
+ auto* var_a = Var("a", func, ty.i32, Expr(2), {});
+ auto* var_b = Const("b", ast::StorageClass::kNone, ty.pointer<int>(func),
+ Expr("a"), {});
+
+ auto* lhs = Expr("b");
+ auto* rhs = Expr(2);
+
+ auto* assign = create<ast::AssignmentStatement>(
+ Source{Source::Location{12, 34}}, lhs, rhs);
+ RegisterVariable(var_a);
+ RegisterVariable(var_b);
+ EXPECT_TRUE(td()->DetermineResultType(assign)) << td()->error();
+ ASSERT_NE(lhs->result_type(), nullptr);
+ ASSERT_NE(rhs->result_type(), nullptr);
+ EXPECT_TRUE(v()->ValidateAssign(assign)) << v()->error();
}
TEST_F(ValidatorTest, AssignIncompatibleTypes_Fail) {
@@ -137,16 +210,42 @@
auto* assign = create<ast::AssignmentStatement>(
Source{Source::Location{12, 34}}, lhs, rhs);
- td()->RegisterVariableForTesting(var);
+ RegisterVariable(var);
EXPECT_TRUE(td()->DetermineResultType(assign)) << td()->error();
ASSERT_NE(lhs->result_type(), nullptr);
ASSERT_NE(rhs->result_type(), nullptr);
- EXPECT_FALSE(v()->ValidateResultTypes(assign));
+ EXPECT_FALSE(v()->ValidateAssign(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'");
+ "12:34 v-000x: invalid assignment: can't assign value of type "
+ "'__f32' to '__i32'");
+}
+
+TEST_F(ValidatorTest, AssignThroughPointerWrongeStoreType_Fail) {
+ // var a :f32;
+ // const b : ptr<function,f32> = a;
+ // b = 2;
+ const auto priv = ast::StorageClass::kFunction;
+ auto* var_a = Var("a", priv, ty.f32, Expr(2), {});
+ auto* var_b = Const("b", ast::StorageClass::kNone, ty.pointer<float>(priv),
+ Expr("a"), {});
+
+ auto* lhs = Expr("a");
+ auto* rhs = Expr(2);
+
+ auto* assign = create<ast::AssignmentStatement>(
+ Source{Source::Location{12, 34}}, lhs, rhs);
+ RegisterVariable(var_a);
+ RegisterVariable(var_b);
+ EXPECT_TRUE(td()->DetermineResultType(assign)) << td()->error();
+ ASSERT_NE(lhs->result_type(), nullptr);
+ ASSERT_NE(rhs->result_type(), nullptr);
+ EXPECT_FALSE(v()->ValidateAssign(assign));
+ EXPECT_EQ(v()->error(),
+ "12:34 v-000x: invalid assignment: can't assign value of type "
+ "'__i32' to '__f32'");
}
TEST_F(ValidatorTest, AssignCompatibleTypesInBlockStatement_Pass) {
@@ -199,7 +298,8 @@
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'");
+ "12:34 v-000x: invalid assignment: can't assign value of type "
+ "'__f32' to '__i32'");
}
TEST_F(ValidatorTest, GlobalVariableWithStorageClass_Pass) {
diff --git a/src/validator/validator_test_helper.h b/src/validator/validator_test_helper.h
index bc15b32..a7f4374 100644
--- a/src/validator/validator_test_helper.h
+++ b/src/validator/validator_test_helper.h
@@ -39,6 +39,13 @@
/// @returns a pointer to the type_determiner object
TypeDeterminer* td() const { return td_.get(); }
+ /// Inserts a variable into the current scope.
+ /// @param var the variable to register.
+ void RegisterVariable(ast::Variable* var) {
+ v_->RegisterVariableForTesting(var);
+ td_->RegisterVariableForTesting(var);
+ }
+
private:
std::unique_ptr<ValidatorImpl> v_;
std::unique_ptr<TypeDeterminer> td_;
