From f90313396b63da549e8c535aaa577d9fe1dea5a1 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Sun, 26 Jun 2022 11:36:10 +0000 Subject: [PATCH] tint/reader/wgsl: Drop const_expr parsing The WGSL specification changed so that the enforcing of initializers being only const-expr is no longer done by the grammar. This is change is required to allow for 'const' expressions to reference each other. Bug: tint:1580 Change-Id: Ia95b6a0bc86ce391a38f4248f20898dd9a6cf27f Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/94683 Commit-Queue: Ben Clayton Kokoro: Kokoro Reviewed-by: David Neto --- src/tint/BUILD.gn | 1 - src/tint/CMakeLists.txt | 1 - src/tint/reader/wgsl/parser_impl.cc | 72 ++------ src/tint/reader/wgsl/parser_impl.h | 3 - .../wgsl/parser_impl_const_expr_test.cc | 173 ------------------ .../reader/wgsl/parser_impl_error_msg_test.cc | 50 +---- .../parser_impl_global_constant_decl_test.cc | 8 +- .../parser_impl_global_variable_decl_test.cc | 2 +- src/tint/resolver/resolver.cc | 31 ++-- src/tint/resolver/type_validation_test.cc | 3 +- src/tint/resolver/validator.cc | 13 +- src/tint/resolver/validator.h | 12 +- src/tint/resolver/variable_validation_test.cc | 17 +- ...rator_impl_variable_decl_statement_test.cc | 13 -- ...rator_impl_variable_decl_statement_test.cc | 13 -- ...rator_impl_variable_decl_statement_test.cc | 16 -- 16 files changed, 70 insertions(+), 358 deletions(-) delete mode 100644 src/tint/reader/wgsl/parser_impl_const_expr_test.cc diff --git a/src/tint/BUILD.gn b/src/tint/BUILD.gn index 8108e545d0..461556dac1 100644 --- a/src/tint/BUILD.gn +++ b/src/tint/BUILD.gn @@ -1333,7 +1333,6 @@ if (tint_build_unittests) { "reader/wgsl/parser_impl_bug_cases_test.cc", "reader/wgsl/parser_impl_call_stmt_test.cc", "reader/wgsl/parser_impl_case_body_test.cc", - "reader/wgsl/parser_impl_const_expr_test.cc", "reader/wgsl/parser_impl_const_literal_test.cc", "reader/wgsl/parser_impl_continue_stmt_test.cc", "reader/wgsl/parser_impl_continuing_stmt_test.cc", diff --git a/src/tint/CMakeLists.txt b/src/tint/CMakeLists.txt index 3475b1bd15..32359b5236 100644 --- a/src/tint/CMakeLists.txt +++ b/src/tint/CMakeLists.txt @@ -944,7 +944,6 @@ if(TINT_BUILD_TESTS) reader/wgsl/parser_impl_bug_cases_test.cc reader/wgsl/parser_impl_call_stmt_test.cc reader/wgsl/parser_impl_case_body_test.cc - reader/wgsl/parser_impl_const_expr_test.cc reader/wgsl/parser_impl_const_literal_test.cc reader/wgsl/parser_impl_continue_stmt_test.cc reader/wgsl/parser_impl_continuing_stmt_test.cc diff --git a/src/tint/reader/wgsl/parser_impl.cc b/src/tint/reader/wgsl/parser_impl.cc index f6ab166bab..c39df85ebe 100644 --- a/src/tint/reader/wgsl/parser_impl.cc +++ b/src/tint/reader/wgsl/parser_impl.cc @@ -549,13 +549,16 @@ Maybe ParserImpl::global_variable_decl(ast::AttributeList& return Failure::kNoMatch; } - const ast::Expression* constructor = nullptr; + const ast::Expression* initalizer = nullptr; if (match(Token::Type::kEqual)) { - auto expr = expect_const_expr(); + auto expr = logical_or_expression(); if (expr.errored) { return Failure::kErrored; } - constructor = expr.value; + if (!expr.matched) { + return add_error(peek(), "missing initalizer for 'var' declaration"); + } + initalizer = expr.value; } return create(decl->source, // source @@ -563,7 +566,7 @@ Maybe ParserImpl::global_variable_decl(ast::AttributeList& decl->type, // type decl->storage_class, // storage class decl->access, // access control - constructor, // constructor + initalizer, // constructor std::move(attrs)); // attributes } @@ -605,11 +608,14 @@ Maybe ParserImpl::global_constant_decl(ast::AttributeList& const ast::Expression* initializer = nullptr; if (has_initializer) { - auto init = expect_const_expr(); - if (init.errored) { + auto expr = logical_or_expression(); + if (expr.errored) { return Failure::kErrored; } - initializer = std::move(init.value); + if (!expr.matched) { + return add_error(peek(), "missing initializer for " + std::string(use)); + } + initializer = std::move(expr.value); } if (is_const) { @@ -3116,58 +3122,6 @@ Maybe ParserImpl::const_literal() { return Failure::kNoMatch; } -// const_expr -// : type_decl PAREN_LEFT ((const_expr COMMA)? const_expr COMMA?)? PAREN_RIGHT -// | const_literal -Expect ParserImpl::expect_const_expr() { - auto t = peek(); - auto source = t.source(); - if (t.IsLiteral()) { - auto lit = const_literal(); - if (lit.errored) { - return Failure::kErrored; - } - if (!lit.matched) { - return add_error(peek(), "unable to parse constant literal"); - } - return lit.value; - } - - if (peek_is(Token::Type::kParenLeft, 1) || peek_is(Token::Type::kLessThan, 1)) { - auto type = expect_type("const_expr"); - if (type.errored) { - return Failure::kErrored; - } - - auto params = expect_paren_block("type constructor", [&]() -> Expect { - ast::ExpressionList list; - while (continue_parsing()) { - if (peek_is(Token::Type::kParenRight)) { - break; - } - - auto arg = expect_const_expr(); - if (arg.errored) { - return Failure::kErrored; - } - list.emplace_back(arg.value); - - if (!match(Token::Type::kComma)) { - break; - } - } - return list; - }); - - if (params.errored) { - return Failure::kErrored; - } - - return builder_.Construct(source, type.value, params.value); - } - return add_error(peek(), "unable to parse const_expr"); -} - Maybe ParserImpl::attribute_list() { bool errored = false; ast::AttributeList attrs; diff --git a/src/tint/reader/wgsl/parser_impl.h b/src/tint/reader/wgsl/parser_impl.h index 89d5163d60..73c58296c7 100644 --- a/src/tint/reader/wgsl/parser_impl.h +++ b/src/tint/reader/wgsl/parser_impl.h @@ -541,9 +541,6 @@ class ParserImpl { /// Parses a `const_literal` grammar element /// @returns the const literal parsed or nullptr if none found Maybe const_literal(); - /// Parses a `const_expr` grammar element, erroring on parse failure. - /// @returns the parsed constructor expression or nullptr on error - Expect expect_const_expr(); /// Parses a `primary_expression` grammar element /// @returns the parsed expression or nullptr Maybe primary_expression(); diff --git a/src/tint/reader/wgsl/parser_impl_const_expr_test.cc b/src/tint/reader/wgsl/parser_impl_const_expr_test.cc deleted file mode 100644 index 80536bd6e6..0000000000 --- a/src/tint/reader/wgsl/parser_impl_const_expr_test.cc +++ /dev/null @@ -1,173 +0,0 @@ -// 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. - -#include "src/tint/reader/wgsl/parser_impl_test_helper.h" - -namespace tint::reader::wgsl { -namespace { - -TEST_F(ParserImplTest, ConstExpr_TypeDecl) { - auto p = parser("vec2(1., 2.)"); - auto e = p->expect_const_expr(); - ASSERT_FALSE(p->has_error()) << p->error(); - ASSERT_FALSE(e.errored); - ASSERT_TRUE(e->Is()); - - auto* t = e->As(); - ASSERT_TRUE(t->target.type->Is()); - EXPECT_EQ(t->target.type->As()->width, 2u); - - ASSERT_EQ(t->args.size(), 2u); - - ASSERT_TRUE(t->args[0]->Is()); - EXPECT_DOUBLE_EQ(t->args[0]->As()->value, 1.); - EXPECT_EQ(t->args[0]->As()->suffix, - ast::FloatLiteralExpression::Suffix::kNone); - - ASSERT_TRUE(t->args[1]->Is()); - EXPECT_DOUBLE_EQ(t->args[1]->As()->value, 2.); - EXPECT_EQ(t->args[1]->As()->suffix, - ast::FloatLiteralExpression::Suffix::kNone); -} - -TEST_F(ParserImplTest, ConstExpr_TypeDecl_Empty) { - auto p = parser("vec2()"); - auto e = p->expect_const_expr(); - ASSERT_FALSE(p->has_error()) << p->error(); - ASSERT_FALSE(e.errored); - ASSERT_TRUE(e->Is()); - - auto* t = e->As(); - ASSERT_TRUE(t->target.type->Is()); - EXPECT_EQ(t->target.type->As()->width, 2u); - - ASSERT_EQ(t->args.size(), 0u); -} - -TEST_F(ParserImplTest, ConstExpr_TypeDecl_TrailingComma) { - auto p = parser("vec2(1., 2.,)"); - auto e = p->expect_const_expr(); - ASSERT_FALSE(p->has_error()) << p->error(); - ASSERT_FALSE(e.errored); - ASSERT_TRUE(e->Is()); - - auto* t = e->As(); - ASSERT_TRUE(t->target.type->Is()); - EXPECT_EQ(t->target.type->As()->width, 2u); - - ASSERT_EQ(t->args.size(), 2u); - ASSERT_TRUE(t->args[0]->Is()); - ASSERT_TRUE(t->args[1]->Is()); -} - -TEST_F(ParserImplTest, ConstExpr_TypeDecl_MissingRightParen) { - auto p = parser("vec2(1., 2."); - auto e = p->expect_const_expr(); - ASSERT_TRUE(p->has_error()); - ASSERT_TRUE(e.errored); - ASSERT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:17: expected ')' for type constructor"); -} - -TEST_F(ParserImplTest, ConstExpr_TypeDecl_MissingLeftParen) { - auto p = parser("vec2 1., 2.)"); - auto e = p->expect_const_expr(); - ASSERT_TRUE(p->has_error()); - ASSERT_TRUE(e.errored); - ASSERT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:11: expected '(' for type constructor"); -} - -TEST_F(ParserImplTest, ConstExpr_TypeDecl_MissingComma) { - auto p = parser("vec2(1. 2."); - auto e = p->expect_const_expr(); - ASSERT_TRUE(p->has_error()); - ASSERT_TRUE(e.errored); - ASSERT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:14: expected ')' for type constructor"); -} - -TEST_F(ParserImplTest, ConstExpr_InvalidExpr) { - auto p = parser("vec2(1., if(a) {})"); - auto e = p->expect_const_expr(); - ASSERT_TRUE(p->has_error()); - ASSERT_TRUE(e.errored); - ASSERT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:15: invalid type for const_expr"); -} - -TEST_F(ParserImplTest, ConstExpr_ConstLiteral) { - auto p = parser("true"); - auto e = p->expect_const_expr(); - ASSERT_FALSE(p->has_error()) << p->error(); - ASSERT_FALSE(e.errored); - ASSERT_NE(e.value, nullptr); - ASSERT_TRUE(e.value->Is()); - EXPECT_TRUE(e.value->As()->value); -} - -TEST_F(ParserImplTest, ConstExpr_ConstLiteral_Invalid) { - auto p = parser("invalid"); - auto e = p->expect_const_expr(); - ASSERT_TRUE(p->has_error()); - ASSERT_TRUE(e.errored); - ASSERT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:1: unable to parse const_expr"); -} - -TEST_F(ParserImplTest, ConstExpr_TypeConstructor) { - auto p = parser("S(0)"); - - auto e = p->expect_const_expr(); - ASSERT_FALSE(e.errored); - ASSERT_TRUE(e->Is()); - ASSERT_NE(e->As()->target.type, nullptr); - ASSERT_TRUE(e->As()->target.type->Is()); - EXPECT_EQ(e->As()->target.type->As()->name, - p->builder().Symbols().Get("S")); -} - -TEST_F(ParserImplTest, ConstExpr_Recursion) { - std::stringstream out; - for (size_t i = 0; i < 200; i++) { - out << "f32("; - } - out << "1.0"; - for (size_t i = 0; i < 200; i++) { - out << ")"; - } - auto p = parser(out.str()); - auto e = p->expect_const_expr(); - ASSERT_TRUE(p->has_error()); - ASSERT_TRUE(e.errored); - ASSERT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:517: maximum parser recursive depth reached"); -} - -TEST_F(ParserImplTest, UnaryOp_Recursion) { - std::stringstream out; - for (size_t i = 0; i < 200; i++) { - out << "!"; - } - out << "1.0"; - auto p = parser(out.str()); - auto e = p->unary_expression(); - ASSERT_TRUE(p->has_error()); - ASSERT_TRUE(e.errored); - ASSERT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:130: maximum parser recursive depth reached"); -} - -} // namespace -} // namespace tint::reader::wgsl diff --git a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc b/src/tint/reader/wgsl/parser_impl_error_msg_test.cc index 5e99d34948..3273413b92 100644 --- a/src/tint/reader/wgsl/parser_impl_error_msg_test.cc +++ b/src/tint/reader/wgsl/parser_impl_error_msg_test.cc @@ -460,11 +460,7 @@ TEST_F(ParserImplErrorTest, FunctionMissingOpenLine) { var a : f32 = bar[0]; return; })", - R"(test.wgsl:2:17 error: unable to parse const_expr - var a : f32 = bar[0]; - ^^^ - -test.wgsl:3:3 error: statement found outside of function body + R"(test.wgsl:3:3 error: statement found outside of function body return; ^^^^^^ )"); @@ -504,27 +500,9 @@ const i : vec2 = vec2(1., 2.; TEST_F(ParserImplErrorTest, GlobalDeclConstBadConstLiteral) { EXPECT("const i : vec2 = vec2(!);", - R"(test.wgsl:1:33 error: unable to parse const_expr + R"(test.wgsl:1:34 error: unable to parse right side of ! expression const i : vec2 = vec2(!); - ^ -)"); -} - -TEST_F(ParserImplErrorTest, GlobalDeclConstBadConstLiteralSpaceLessThan) { - EXPECT("const i = 1 < 2;", - R"(test.wgsl:1:13 error: expected ';' for 'const' declaration -const i = 1 < 2; - ^ -)"); -} - -TEST_F(ParserImplErrorTest, GlobalDeclConstNotConstExpr) { - EXPECT( - "const a = 1;\n" - "const b = a;", - R"(test.wgsl:2:11 error: unable to parse const_expr -const b = a; - ^ + ^ )"); } @@ -605,27 +583,9 @@ let i : vec2 = vec2(1., 2.; TEST_F(ParserImplErrorTest, GlobalDeclLetBadConstLiteral) { EXPECT("let i : vec2 = vec2(!);", - R"(test.wgsl:1:31 error: unable to parse const_expr + R"(test.wgsl:1:32 error: unable to parse right side of ! expression let i : vec2 = vec2(!); - ^ -)"); -} - -TEST_F(ParserImplErrorTest, GlobalDeclLetBadConstLiteralSpaceLessThan) { - EXPECT("let i = 1 < 2;", - R"(test.wgsl:1:11 error: expected ';' for 'let' declaration -let i = 1 < 2; - ^ -)"); -} - -TEST_F(ParserImplErrorTest, GlobalDeclLetNotConstExpr) { - EXPECT( - "let a = 1;\n" - "let b = a;", - R"(test.wgsl:2:9 error: unable to parse const_expr -let b = a; - ^ + ^ )"); } diff --git a/src/tint/reader/wgsl/parser_impl_global_constant_decl_test.cc b/src/tint/reader/wgsl/parser_impl_global_constant_decl_test.cc index 70caa3f8b3..21c0078ac4 100644 --- a/src/tint/reader/wgsl/parser_impl_global_constant_decl_test.cc +++ b/src/tint/reader/wgsl/parser_impl_global_constant_decl_test.cc @@ -77,7 +77,7 @@ TEST_F(ParserImplTest, GlobalLetDecl_InvalidExpression) { EXPECT_TRUE(e.errored); EXPECT_FALSE(e.matched); EXPECT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:15: invalid type for const_expr"); + EXPECT_EQ(p->error(), "1:15: missing initializer for 'let' declaration"); } TEST_F(ParserImplTest, GlobalLetDecl_MissingExpression) { @@ -90,7 +90,7 @@ TEST_F(ParserImplTest, GlobalLetDecl_MissingExpression) { EXPECT_TRUE(e.errored); EXPECT_FALSE(e.matched); EXPECT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:14: unable to parse const_expr"); + EXPECT_EQ(p->error(), "1:14: missing initializer for 'let' declaration"); } TEST_F(ParserImplTest, GlobalConstDecl) { @@ -152,7 +152,7 @@ TEST_F(ParserImplTest, GlobalConstDecl_InvalidExpression) { EXPECT_TRUE(e.errored); EXPECT_FALSE(e.matched); EXPECT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:17: invalid type for const_expr"); + EXPECT_EQ(p->error(), "1:17: missing initializer for 'const' declaration"); } TEST_F(ParserImplTest, GlobalConstDecl_MissingExpression) { @@ -165,7 +165,7 @@ TEST_F(ParserImplTest, GlobalConstDecl_MissingExpression) { EXPECT_TRUE(e.errored); EXPECT_FALSE(e.matched); EXPECT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:16: unable to parse const_expr"); + EXPECT_EQ(p->error(), "1:16: missing initializer for 'const' declaration"); } TEST_F(ParserImplTest, GlobalOverrideDecl_WithId) { diff --git a/src/tint/reader/wgsl/parser_impl_global_variable_decl_test.cc b/src/tint/reader/wgsl/parser_impl_global_variable_decl_test.cc index 11371e6997..f6710423a9 100644 --- a/src/tint/reader/wgsl/parser_impl_global_variable_decl_test.cc +++ b/src/tint/reader/wgsl/parser_impl_global_variable_decl_test.cc @@ -152,7 +152,7 @@ TEST_F(ParserImplTest, GlobalVariableDecl_InvalidConstExpr) { EXPECT_TRUE(e.errored); EXPECT_FALSE(e.matched); EXPECT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:24: invalid type for const_expr"); + EXPECT_EQ(p->error(), "1:24: missing initalizer for 'var' declaration"); } TEST_F(ParserImplTest, GlobalVariableDecl_InvalidVariableDecl) { diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index 76aaea285b..235c57546d 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc @@ -353,8 +353,7 @@ sem::Variable* Resolver::Let(const ast::Let* v, bool is_global) { ty = rhs->Type()->UnwrapRef(); // Implicit load of RHS } - if (rhs && - !validator_.VariableConstructorOrCast(v, ast::StorageClass::kNone, ty, rhs->Type())) { + if (rhs && !validator_.VariableInitializer(v, ast::StorageClass::kNone, ty, rhs)) { return nullptr; } @@ -405,12 +404,11 @@ sem::Variable* Resolver::Override(const ast::Override* v) { ty = rhs->Type()->UnwrapRef(); // Implicit load of RHS } } else if (!ty) { - AddError("'override' declaration requires a type or initializer", v->source); + AddError("override declaration requires a type or initializer", v->source); return nullptr; } - if (rhs && - !validator_.VariableConstructorOrCast(v, ast::StorageClass::kNone, ty, rhs->Type())) { + if (rhs && !validator_.VariableInitializer(v, ast::StorageClass::kNone, ty, rhs)) { return nullptr; } @@ -479,8 +477,7 @@ sem::Variable* Resolver::Const(const ast::Const* c, bool is_global) { return nullptr; } - if (rhs && - !validator_.VariableConstructorOrCast(c, ast::StorageClass::kNone, ty, rhs->Type())) { + if (!validator_.VariableInitializer(c, ast::StorageClass::kNone, ty, rhs)) { return nullptr; } @@ -528,7 +525,7 @@ sem::Variable* Resolver::Var(const ast::Var* var, bool is_global) { } if (!storage_ty) { - AddError("'var' declaration requires a type or initializer", var->source); + AddError("var declaration requires a type or initializer", var->source); return nullptr; } @@ -558,7 +555,7 @@ sem::Variable* Resolver::Var(const ast::Var* var, bool is_global) { access = DefaultAccessForStorageClass(storage_class); } - if (rhs && !validator_.VariableConstructorOrCast(var, storage_class, storage_ty, rhs->Type())) { + if (rhs && !validator_.VariableInitializer(var, storage_class, storage_ty, rhs)) { return nullptr; } @@ -1864,8 +1861,8 @@ sem::Expression* Resolver::Literal(const ast::LiteralExpression* literal) { sem::Expression* Resolver::Identifier(const ast::IdentifierExpression* expr) { auto symbol = expr->symbol; auto* resolved = sem_.ResolvedSymbol(expr); - if (auto* var = As(resolved)) { - auto* user = builder_->create(expr, current_statement_, var); + if (auto* variable = As(resolved)) { + auto* user = builder_->create(expr, current_statement_, variable); if (current_statement_) { // If identifier is part of a loop continuing block, make sure it @@ -1901,12 +1898,20 @@ sem::Expression* Resolver::Identifier(const ast::IdentifierExpression* expr) { } if (current_function_) { - if (auto* global = var->As()) { + if (auto* global = variable->As()) { current_function_->AddDirectlyReferencedGlobal(global); } + } else if (variable->Declaration()->Is()) { + // Use of a module-scope 'var' outside of a function. + // Note: The spec is currently vague around the rules here. See + // https://github.com/gpuweb/gpuweb/issues/3081. Remove this comment when resolved. + std::string desc = "var '" + builder_->Symbols().NameFor(symbol) + "' "; + AddError(desc + "cannot not be referenced at module-scope", expr->source); + AddNote(desc + "declared here", variable->Declaration()->source); + return nullptr; } - var->AddUser(user); + variable->AddUser(user); return user; } diff --git a/src/tint/resolver/type_validation_test.cc b/src/tint/resolver/type_validation_test.cc index aaf3b31609..451659ef9f 100644 --- a/src/tint/resolver/type_validation_test.cc +++ b/src/tint/resolver/type_validation_test.cc @@ -421,7 +421,8 @@ TEST_F(ResolverTypeValidationTest, ArraySize_ModuleVar) { GlobalVar("a", ty.array(ty.f32(), Expr(Source{{12, 34}}, "size")), ast::StorageClass::kPrivate); EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: array size must evaluate to a constant integer expression"); + R"(12:34 error: var 'size' cannot not be referenced at module-scope +note: var 'size' declared here)"); } TEST_F(ResolverTypeValidationTest, ArraySize_FunctionConst) { diff --git a/src/tint/resolver/validator.cc b/src/tint/resolver/validator.cc index 6ffbd62725..e8f28bb4e9 100644 --- a/src/tint/resolver/validator.cc +++ b/src/tint/resolver/validator.cc @@ -319,17 +319,18 @@ bool Validator::Materialize(const sem::Type* to, return true; } -bool Validator::VariableConstructorOrCast(const ast::Variable* v, - ast::StorageClass storage_class, - const sem::Type* storage_ty, - const sem::Type* rhs_ty) const { - auto* value_type = rhs_ty->UnwrapRef(); // Implicit load of RHS +bool Validator::VariableInitializer(const ast::Variable* v, + ast::StorageClass storage_class, + const sem::Type* storage_ty, + const sem::Expression* initializer) const { + auto* initializer_ty = initializer->Type(); + auto* value_type = initializer_ty->UnwrapRef(); // Implicit load of RHS // Value type has to match storage type if (storage_ty != value_type) { std::stringstream s; s << "cannot initialize " << v->Kind() << " of type '" << sem_.TypeNameOf(storage_ty) - << "' with value of type '" << sem_.TypeNameOf(rhs_ty) << "'"; + << "' with value of type '" << sem_.TypeNameOf(initializer_ty) << "'"; AddError(s.str(), v->source); return false; } diff --git a/src/tint/resolver/validator.h b/src/tint/resolver/validator.h index 9610e3391e..4fdac53b90 100644 --- a/src/tint/resolver/validator.h +++ b/src/tint/resolver/validator.h @@ -378,16 +378,16 @@ class Validator { /// @returns true on success, false otherwise. bool Const(const sem::Variable* v) const; - /// Validates a variable constructor or cast + /// Validates a variable initializer /// @param v the variable to validate /// @param storage_class the storage class of the variable /// @param storage_type the type of the storage - /// @param rhs_type the right hand side of the expression + /// @param initializer the RHS initializer expression /// @returns true on succes, false otherwise - bool VariableConstructorOrCast(const ast::Variable* v, - ast::StorageClass storage_class, - const sem::Type* storage_type, - const sem::Type* rhs_type) const; + bool VariableInitializer(const ast::Variable* v, + ast::StorageClass storage_class, + const sem::Type* storage_type, + const sem::Expression* initializer) const; /// Validates a vector /// @param ty the vector to validate diff --git a/src/tint/resolver/variable_validation_test.cc b/src/tint/resolver/variable_validation_test.cc index 435111b0c0..51b430f66e 100644 --- a/src/tint/resolver/variable_validation_test.cc +++ b/src/tint/resolver/variable_validation_test.cc @@ -29,7 +29,7 @@ TEST_F(ResolverVariableValidationTest, VarNoInitializerNoType) { WrapInFunction(Var(Source{{12, 34}}, "a", nullptr)); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), "12:34 error: 'var' declaration requires a type or initializer"); + EXPECT_EQ(r()->error(), "12:34 error: var declaration requires a type or initializer"); } TEST_F(ResolverVariableValidationTest, GlobalVarNoInitializerNoType) { @@ -37,7 +37,18 @@ TEST_F(ResolverVariableValidationTest, GlobalVarNoInitializerNoType) { GlobalVar(Source{{12, 34}}, "a", nullptr); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), "12:34 error: 'var' declaration requires a type or initializer"); + EXPECT_EQ(r()->error(), "12:34 error: var declaration requires a type or initializer"); +} + +TEST_F(ResolverVariableValidationTest, GlobalVarUsedAtModuleScope) { + // var a : i32; + // var b : i32 = a; + GlobalVar(Source{{12, 34}}, "a", ty.i32(), ast::StorageClass::kPrivate, nullptr); + GlobalVar("b", ty.i32(), ast::StorageClass::kPrivate, Expr(Source{{56, 78}}, "a")); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), R"(56:78 error: var 'a' cannot not be referenced at module-scope +12:34 note: var 'a' declared here)"); } TEST_F(ResolverVariableValidationTest, OverrideNoInitializerNoType) { @@ -45,7 +56,7 @@ TEST_F(ResolverVariableValidationTest, OverrideNoInitializerNoType) { Override(Source{{12, 34}}, "a", nullptr, nullptr); EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), "12:34 error: 'override' declaration requires a type or initializer"); + EXPECT_EQ(r()->error(), "12:34 error: override declaration requires a type or initializer"); } TEST_F(ResolverVariableValidationTest, VarTypeNotStorable) { diff --git a/src/tint/writer/glsl/generator_impl_variable_decl_statement_test.cc b/src/tint/writer/glsl/generator_impl_variable_decl_statement_test.cc index f8983cbd93..c3a02b44b9 100644 --- a/src/tint/writer/glsl/generator_impl_variable_decl_statement_test.cc +++ b/src/tint/writer/glsl/generator_impl_variable_decl_statement_test.cc @@ -76,19 +76,6 @@ TEST_F(GlslGeneratorImplTest_VariableDecl, Emit_VariableDeclStatement_Private) { EXPECT_THAT(gen.result(), HasSubstr(" float a = 0.0f;\n")); } -TEST_F(GlslGeneratorImplTest_VariableDecl, Emit_VariableDeclStatement_Initializer_Private) { - GlobalVar("initializer", ty.f32(), ast::StorageClass::kPrivate); - GlobalVar("a", ty.f32(), ast::StorageClass::kPrivate, Expr("initializer")); - - WrapInFunction(Expr("a")); - - GeneratorImpl& gen = Build(); - - ASSERT_TRUE(gen.Generate()) << gen.error(); - EXPECT_THAT(gen.result(), HasSubstr(R"(float a = initializer; -)")); -} - TEST_F(GlslGeneratorImplTest_VariableDecl, Emit_VariableDeclStatement_Initializer_ZeroVec) { auto* var = Var("a", ty.vec3(), ast::StorageClass::kNone, vec3()); diff --git a/src/tint/writer/hlsl/generator_impl_variable_decl_statement_test.cc b/src/tint/writer/hlsl/generator_impl_variable_decl_statement_test.cc index 3a857b7961..1109014847 100644 --- a/src/tint/writer/hlsl/generator_impl_variable_decl_statement_test.cc +++ b/src/tint/writer/hlsl/generator_impl_variable_decl_statement_test.cc @@ -75,19 +75,6 @@ TEST_F(HlslGeneratorImplTest_VariableDecl, Emit_VariableDeclStatement_Private) { EXPECT_THAT(gen.result(), HasSubstr(" static float a = 0.0f;\n")); } -TEST_F(HlslGeneratorImplTest_VariableDecl, Emit_VariableDeclStatement_Initializer_Private) { - GlobalVar("initializer", ty.f32(), ast::StorageClass::kPrivate); - GlobalVar("a", ty.f32(), ast::StorageClass::kPrivate, Expr("initializer")); - - WrapInFunction(Expr("a")); - - GeneratorImpl& gen = Build(); - - ASSERT_TRUE(gen.Generate()) << gen.error(); - EXPECT_THAT(gen.result(), HasSubstr(R"(float a = initializer; -)")); -} - TEST_F(HlslGeneratorImplTest_VariableDecl, Emit_VariableDeclStatement_Initializer_ZeroVec) { auto* var = Var("a", ty.vec3(), ast::StorageClass::kNone, vec3()); diff --git a/src/tint/writer/msl/generator_impl_variable_decl_statement_test.cc b/src/tint/writer/msl/generator_impl_variable_decl_statement_test.cc index 311d33c984..7a0a37913b 100644 --- a/src/tint/writer/msl/generator_impl_variable_decl_statement_test.cc +++ b/src/tint/writer/msl/generator_impl_variable_decl_statement_test.cc @@ -123,22 +123,6 @@ TEST_F(MslGeneratorImplTest, Emit_VariableDeclStatement_Private) { EXPECT_THAT(gen.result(), HasSubstr("thread float tint_symbol_1 = 0.0f;\n")); } -TEST_F(MslGeneratorImplTest, Emit_VariableDeclStatement_Initializer_Private) { - GlobalLet("initializer", ty.f32(), Expr(0_f)); - GlobalVar("a", ty.f32(), ast::StorageClass::kPrivate, Expr("initializer")); - - WrapInFunction(Expr("a")); - - GeneratorImpl& gen = SanitizeAndBuild(); - - ASSERT_TRUE(gen.Generate()) << gen.error(); - EXPECT_THAT(gen.result(), HasSubstr(R"( - thread float tint_symbol_1 = initializer; - float const tint_symbol = tint_symbol_1; - return; -)")); -} - TEST_F(MslGeneratorImplTest, Emit_VariableDeclStatement_Workgroup) { GlobalVar("a", ty.f32(), ast::StorageClass::kWorkgroup);