From 6ce2edfa1fef02da3607a622abc1f483c85062b6 Mon Sep 17 00:00:00 2001 From: Antonio Maiorano Date: Fri, 26 Feb 2021 18:25:56 +0000 Subject: [PATCH] Validate sign of RHS matches LHS for scalar variable declarations Statements like `var u : u32 = 0;` should fail because '0' is a signed integer being used to initialize an unsigned variable. Added test. Bug: tint:79 Change-Id: I34f6d21b4355167f49f9a8b5d4ed34a808823185 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42702 Reviewed-by: Ben Clayton Reviewed-by: Antonio Maiorano Commit-Queue: Antonio Maiorano --- src/ast/module_clone_test.cc | 2 +- src/type_determiner.cc | 28 ++++++++++++++++++++ src/type_determiner.h | 2 ++ src/type_determiner_test.cc | 46 +++++++++++++++++++++++++++++++++ src/validator/validator_test.cc | 2 +- 5 files changed, 78 insertions(+), 2 deletions(-) diff --git a/src/ast/module_clone_test.cc b/src/ast/module_clone_test.cc index 7ed6be0e1b..22ffbb9867 100644 --- a/src/ast/module_clone_test.cc +++ b/src/ast/module_clone_test.cc @@ -68,7 +68,7 @@ fn f0(p0 : bool) -> f32 { fn f1(p0 : f32, p1 : i32) -> f32 { var l0 : i32 = 3; - var l1 : f32 = 8; + var l1 : f32 = 8.0; var l2 : u32 = bitcast(4); var l3 : vec2 = vec2(l0, l1); var l4 : S; diff --git a/src/type_determiner.cc b/src/type_determiner.cc index 9f8427a703..8861cdb901 100644 --- a/src/type_determiner.cc +++ b/src/type_determiner.cc @@ -194,6 +194,12 @@ bool TypeDeterminer::DetermineFunction(ast::Function* func) { bool TypeDeterminer::DetermineStatements(const ast::BlockStatement* stmts) { for (auto* stmt : *stmts) { + if (auto* decl = stmt->As()) { + if (!ValidateVariableDeclStatement(decl)) { + return false; + } + } + if (!DetermineVariableStorageClass(stmt)) { return false; } @@ -937,6 +943,28 @@ bool TypeDeterminer::DetermineUnaryOp(ast::UnaryOpExpression* expr) { return true; } +bool TypeDeterminer::ValidateVariableDeclStatement( + const ast::VariableDeclStatement* stmt) { + auto* ctor = stmt->variable()->constructor(); + if (!ctor) { + return true; + } + + if (auto* sce = ctor->As()) { + auto* lhs_type = stmt->variable()->type()->UnwrapAliasIfNeeded(); + auto* rhs_type = sce->literal()->type()->UnwrapAliasIfNeeded(); + + if (lhs_type != rhs_type) { + diagnostics_.add_error( + "constructor expression type does not match variable type", + stmt->source()); + return false; + } + } + + return true; +} + TypeDeterminer::VariableInfo* TypeDeterminer::CreateVariableInfo( ast::Variable* var) { auto* info = variable_infos_.Create(var); diff --git a/src/type_determiner.h b/src/type_determiner.h index 8f0802ad08..fb1b7a9d24 100644 --- a/src/type_determiner.h +++ b/src/type_determiner.h @@ -175,6 +175,8 @@ class TypeDeterminer { bool DetermineMemberAccessor(ast::MemberAccessorExpression* expr); bool DetermineUnaryOp(ast::UnaryOpExpression* expr); + bool ValidateVariableDeclStatement(const ast::VariableDeclStatement* stmt); + VariableInfo* CreateVariableInfo(ast::Variable*); /// @returns the resolved type of the ast::Expression `expr` diff --git a/src/type_determiner_test.cc b/src/type_determiner_test.cc index 9ae97b5aae..0c118a2cf8 100644 --- a/src/type_determiner_test.cc +++ b/src/type_determiner_test.cc @@ -437,6 +437,52 @@ TEST_F(TypeDeterminerTest, Stmt_VariableDecl) { EXPECT_TRUE(TypeOf(init)->Is()); } +TEST_F(TypeDeterminerTest, Stmt_VariableDecl_Alias) { + auto* my_int = ty.alias("MyInt", ty.i32()); + auto* var = Var("my_var", my_int, ast::StorageClass::kNone, Expr(2)); + auto* init = var->constructor(); + + auto* decl = create(var); + WrapInFunction(decl); + + EXPECT_TRUE(td()->Determine()) << td()->error(); + + ASSERT_NE(TypeOf(init), nullptr); + EXPECT_TRUE(TypeOf(init)->Is()); +} + +TEST_F(TypeDeterminerTest, Stmt_VariableDecl_MismatchedTypeScalarConstructor) { + u32 unsigned_value = 2u; // Type does not match variable type + auto* var = + Var("my_var", ty.i32(), ast::StorageClass::kNone, Expr(unsigned_value)); + + auto* decl = + create(Source{{{3, 3}, {3, 22}}}, var); + WrapInFunction(decl); + + EXPECT_FALSE(td()->Determine()); + EXPECT_EQ( + td()->error(), + R"(3:3 error: constructor expression type does not match variable type)"); +} + +TEST_F(TypeDeterminerTest, + Stmt_VariableDecl_MismatchedTypeScalarConstructor_Alias) { + auto* my_int = ty.alias("MyInt", ty.i32()); + u32 unsigned_value = 2u; // Type does not match variable type + auto* var = + Var("my_var", my_int, ast::StorageClass::kNone, Expr(unsigned_value)); + + auto* decl = + create(Source{{{3, 3}, {3, 22}}}, var); + WrapInFunction(decl); + + EXPECT_FALSE(td()->Determine()); + EXPECT_EQ( + td()->error(), + R"(3:3 error: constructor expression type does not match variable type)"); +} + TEST_F(TypeDeterminerTest, Stmt_VariableDecl_ModuleScope) { auto* init = Expr(2); Global("my_var", ty.i32(), ast::StorageClass::kNone, init); diff --git a/src/validator/validator_test.cc b/src/validator/validator_test.cc index e4865439e3..8b38d07ab3 100644 --- a/src/validator/validator_test.cc +++ b/src/validator/validator_test.cc @@ -792,7 +792,7 @@ TEST_F(ValidatorTest, RedeclaredIdentifierDifferentFunctions_Pass) { // func1 { var a : f32 = 3.0; return; } auto* var0 = Var("a", ty.f32(), ast::StorageClass::kNone, Expr(2.0f)); - auto* var1 = Var("a", ty.void_(), ast::StorageClass::kNone, Expr(1.0f)); + auto* var1 = Var("a", ty.f32(), ast::StorageClass::kNone, Expr(1.0f)); Func("func0", ast::VariableList{}, ty.void_(), ast::StatementList{