[type-determiner] Fixup handling of unknown identifiers.
This Cl updates the identifier type determination check to fail if the
identifier is not found and is not an intrinsic method.
Bug: tint:139
Change-Id: I332dd7fb42dae62bdee459c4a8819bdb5685c903
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/30081
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>
diff --git a/src/type_determiner.cc b/src/type_determiner.cc
index 06e8a2a..acc5b53 100644
--- a/src/type_determiner.cc
+++ b/src/type_determiner.cc
@@ -369,7 +369,6 @@
if (!DetermineResultType(expr->expr())) {
return false;
}
-
expr->set_result_type(expr->type());
return true;
}
@@ -787,12 +786,15 @@
return true;
}
- SetIntrinsicIfNeeded(expr);
-
+ if (!SetIntrinsicIfNeeded(expr)) {
+ set_error(expr->source(),
+ "v-0006: identifier must be declared before use: " + name);
+ return false;
+ }
return true;
}
-void TypeDeterminer::SetIntrinsicIfNeeded(ast::IdentifierExpression* ident) {
+bool TypeDeterminer::SetIntrinsicIfNeeded(ast::IdentifierExpression* ident) {
if (ident->name() == "abs") {
ident->set_intrinsic(ast::Intrinsic::kAbs);
} else if (ident->name() == "acos") {
@@ -927,7 +929,10 @@
ident->set_intrinsic(ast::Intrinsic::kTextureSampleLevel);
} else if (ident->name() == "trunc") {
ident->set_intrinsic(ast::Intrinsic::kTrunc);
+ } else {
+ return false;
}
+ return true;
}
bool TypeDeterminer::DetermineMemberAccessor(
diff --git a/src/type_determiner.h b/src/type_determiner.h
index 482a439b..30fc548 100644
--- a/src/type_determiner.h
+++ b/src/type_determiner.h
@@ -108,7 +108,8 @@
/// Sets the intrinsic data information for the identifier if needed
/// @param ident the identifier expression
- void SetIntrinsicIfNeeded(ast::IdentifierExpression* ident);
+ /// @returns true if an intrinsic was set
+ bool SetIntrinsicIfNeeded(ast::IdentifierExpression* ident);
private:
void set_error(const Source& src, const std::string& msg);
diff --git a/src/type_determiner_test.cc b/src/type_determiner_test.cc
index e80c60f..df8b79c 100644
--- a/src/type_determiner_test.cc
+++ b/src/type_determiner_test.cc
@@ -400,7 +400,7 @@
ast::type::F32Type f32;
ast::ExpressionList call_params;
auto call_expr = std::make_unique<ast::CallExpression>(
- Source{12, 34}, std::make_unique<ast::IdentifierExpression>("func"),
+ std::make_unique<ast::IdentifierExpression>(Source{12, 34}, "func"),
std::move(call_params));
ast::VariableList params0;
auto func_main =
@@ -419,7 +419,7 @@
EXPECT_FALSE(td()->Determine()) << td()->error();
EXPECT_EQ(td()->error(),
- "12:34: v-0005: function must be declared before use: 'func'");
+ "12:34: v-0006: identifier must be declared before use: func");
}
TEST_F(TypeDeterminerTest, Stmt_VariableDecl) {
@@ -615,6 +615,9 @@
ast::BitcastExpression bitcast(
&f32, std::make_unique<ast::IdentifierExpression>("name"));
+ ast::Variable v("name", ast::StorageClass::kPrivate, &f32);
+ td()->RegisterVariableForTesting(&v);
+
EXPECT_TRUE(td()->DetermineResultType(&bitcast));
ASSERT_NE(bitcast.result_type(), nullptr);
EXPECT_TRUE(bitcast.result_type()->IsF32());
@@ -688,9 +691,11 @@
ast::ExpressionList params;
params.push_back(std::make_unique<ast::IdentifierExpression>("name"));
-
ast::TypeConstructorExpression cast(&f32, std::move(params));
+ ast::Variable v("name", ast::StorageClass::kPrivate, &f32);
+ td()->RegisterVariableForTesting(&v);
+
EXPECT_TRUE(td()->DetermineResultType(&cast));
ASSERT_NE(cast.result_type(), nullptr);
EXPECT_TRUE(cast.result_type()->IsF32());
@@ -852,6 +857,11 @@
EXPECT_TRUE(ident.result_type()->IsF32());
}
+TEST_F(TypeDeterminerTest, Expr_Identifier_Unknown) {
+ ast::IdentifierExpression a("a");
+ EXPECT_FALSE(td()->DetermineResultType(&a));
+}
+
TEST_F(TypeDeterminerTest, Function_RegisterInputOutputVariables) {
ast::type::F32Type f32;
@@ -1005,8 +1015,11 @@
mod()->AddFunction(std::move(func));
+ ast::Variable v("var", ast::StorageClass::kFunction, &f32);
+ td()->RegisterVariableForTesting(&v);
+
// Register the function
- EXPECT_TRUE(td()->Determine());
+ EXPECT_TRUE(td()->Determine()) << td()->error();
EXPECT_EQ(func_ptr->referenced_module_variables().size(), 0u);
}
@@ -2478,7 +2491,7 @@
auto param = GetParam();
ast::IdentifierExpression ident(param.name);
- td()->SetIntrinsicIfNeeded(&ident);
+ EXPECT_TRUE(td()->SetIntrinsicIfNeeded(&ident));
EXPECT_EQ(ident.intrinsic(), param.intrinsic);
EXPECT_TRUE(ident.IsIntrinsic());
}
@@ -2558,7 +2571,7 @@
TEST_F(TypeDeterminerTest, IntrinsicNotSetIfNotMatched) {
ast::IdentifierExpression ident("not_intrinsic");
- td()->SetIntrinsicIfNeeded(&ident);
+ EXPECT_FALSE(td()->SetIntrinsicIfNeeded(&ident));
EXPECT_EQ(ident.intrinsic(), ast::Intrinsic::kNone);
EXPECT_FALSE(ident.IsIntrinsic());
}
@@ -4726,6 +4739,17 @@
mod()->AddFunction(std::move(ep_1));
mod()->AddFunction(std::move(ep_2));
+ mod()->AddGlobalVariable(std::make_unique<ast::Variable>(
+ "first", ast::StorageClass::kPrivate, &f32));
+ mod()->AddGlobalVariable(std::make_unique<ast::Variable>(
+ "second", ast::StorageClass::kPrivate, &f32));
+ mod()->AddGlobalVariable(std::make_unique<ast::Variable>(
+ "call_a", ast::StorageClass::kPrivate, &f32));
+ mod()->AddGlobalVariable(std::make_unique<ast::Variable>(
+ "call_b", ast::StorageClass::kPrivate, &f32));
+ mod()->AddGlobalVariable(std::make_unique<ast::Variable>(
+ "call_c", ast::StorageClass::kPrivate, &f32));
+
// Register the functions and calculate the callers
ASSERT_TRUE(td()->Determine()) << td()->error();
diff --git a/src/validator_test.cc b/src/validator_test.cc
index 62ada22..4bcab97 100644
--- a/src/validator_test.cc
+++ b/src/validator_test.cc
@@ -87,9 +87,9 @@
auto assign = std::make_unique<ast::AssignmentStatement>(
Source{12, 34}, std::move(lhs), std::move(rhs));
- EXPECT_TRUE(td()->DetermineResultType(assign.get())) << td()->error();
- EXPECT_FALSE(v()->ValidateStatement(assign.get()));
- EXPECT_EQ(v()->error(), "12:34: v-0006: 'b' is not declared");
+ EXPECT_FALSE(td()->DetermineResultType(assign.get()));
+ EXPECT_EQ(td()->error(),
+ "12:34: v-0006: identifier must be declared before use: b");
}
TEST_F(ValidatorTest, UsingUndefinedVariableInBlockStatement_Fail) {
@@ -106,9 +106,9 @@
body->append(std::make_unique<ast::AssignmentStatement>(
Source{12, 34}, std::move(lhs), std::move(rhs)));
- EXPECT_TRUE(td()->DetermineStatements(body.get())) << td()->error();
- EXPECT_FALSE(v()->ValidateStatements(body.get()));
- EXPECT_EQ(v()->error(), "12:34: v-0006: 'b' is not declared");
+ EXPECT_FALSE(td()->DetermineStatements(body.get()));
+ EXPECT_EQ(td()->error(),
+ "12:34: v-0006: identifier must be declared before use: b");
}
TEST_F(ValidatorTest, AssignCompatibleTypes_Pass) {
diff --git a/src/writer/hlsl/generator_impl_intrinsic_test.cc b/src/writer/hlsl/generator_impl_intrinsic_test.cc
index 7860c75..78db9f9 100644
--- a/src/writer/hlsl/generator_impl_intrinsic_test.cc
+++ b/src/writer/hlsl/generator_impl_intrinsic_test.cc
@@ -108,6 +108,9 @@
}
TEST_F(HlslGeneratorImplTest_Intrinsic, Intrinsic_Call) {
+ ast::type::F32Type f32;
+ ast::type::VectorType vec(&f32, 3);
+
ast::ExpressionList params;
params.push_back(std::make_unique<ast::IdentifierExpression>("param1"));
params.push_back(std::make_unique<ast::IdentifierExpression>("param2"));
@@ -115,6 +118,12 @@
ast::CallExpression call(std::make_unique<ast::IdentifierExpression>("dot"),
std::move(params));
+ ast::Variable v1("param1", ast::StorageClass::kFunction, &vec);
+ ast::Variable v2("param2", ast::StorageClass::kFunction, &vec);
+
+ td().RegisterVariableForTesting(&v1);
+ td().RegisterVariableForTesting(&v2);
+
ASSERT_TRUE(td().DetermineResultType(&call)) << td().error();
gen().increment_indent();
diff --git a/src/writer/msl/generator_impl_intrinsic_test.cc b/src/writer/msl/generator_impl_intrinsic_test.cc
index 9378229..4f02220 100644
--- a/src/writer/msl/generator_impl_intrinsic_test.cc
+++ b/src/writer/msl/generator_impl_intrinsic_test.cc
@@ -112,6 +112,9 @@
}
TEST_F(MslGeneratorImplTest, Intrinsic_Call) {
+ ast::type::F32Type f32;
+ ast::type::VectorType vec(&f32, 3);
+
ast::ExpressionList params;
params.push_back(std::make_unique<ast::IdentifierExpression>("param1"));
params.push_back(std::make_unique<ast::IdentifierExpression>("param2"));
@@ -122,6 +125,13 @@
Context ctx;
ast::Module m;
TypeDeterminer td(&ctx, &m);
+
+ ast::Variable v1("param1", ast::StorageClass::kFunction, &vec);
+ ast::Variable v2("param2", ast::StorageClass::kFunction, &vec);
+
+ td.RegisterVariableForTesting(&v1);
+ td.RegisterVariableForTesting(&v2);
+
ASSERT_TRUE(td.DetermineResultType(&call)) << td.error();
GeneratorImpl g(&m);
diff --git a/src/writer/spirv/builder_constructor_expression_test.cc b/src/writer/spirv/builder_constructor_expression_test.cc
index 35e8730..62c1ab8 100644
--- a/src/writer/spirv/builder_constructor_expression_test.cc
+++ b/src/writer/spirv/builder_constructor_expression_test.cc
@@ -2465,6 +2465,14 @@
Context ctx;
ast::Module mod;
TypeDeterminer td(&ctx, &mod);
+
+ ast::Variable var_a("a", ast::StorageClass::kPrivate, &f32);
+ ast::Variable var_b("b", ast::StorageClass::kPrivate, &f32);
+ ast::Variable var_c("c", ast::StorageClass::kPrivate, &f32);
+ td.RegisterVariableForTesting(&var_a);
+ td.RegisterVariableForTesting(&var_b);
+ td.RegisterVariableForTesting(&var_c);
+
ASSERT_TRUE(td.DetermineResultType(&t)) << td.error();
Builder b(&mod);
@@ -2614,6 +2622,14 @@
Context ctx;
ast::Module mod;
TypeDeterminer td(&ctx, &mod);
+
+ ast::Variable var_a("a", ast::StorageClass::kPrivate, &f32);
+ ast::Variable var_b("b", ast::StorageClass::kPrivate, &f32);
+ ast::Variable var_c("c", ast::StorageClass::kPrivate, &f32);
+ td.RegisterVariableForTesting(&var_a);
+ td.RegisterVariableForTesting(&var_b);
+ td.RegisterVariableForTesting(&var_c);
+
ASSERT_TRUE(td.DetermineResultType(&t)) << td.error();
Builder b(&mod);
@@ -2821,6 +2837,12 @@
Context ctx;
ast::Module mod;
TypeDeterminer td(&ctx, &mod);
+
+ ast::Variable var_a("a", ast::StorageClass::kPrivate, &f32);
+ ast::Variable var_b("b", ast::StorageClass::kPrivate, &f32);
+ td.RegisterVariableForTesting(&var_a);
+ td.RegisterVariableForTesting(&var_b);
+
ASSERT_TRUE(td.DetermineResultType(&t)) << td.error();
Builder b(&mod);
