encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

tint: Rework errors around variable declarations 

Standarize the messages, using 'let', 'override' or 'var'.

Module-scope 'let' needs to be replaced with 'const', but baby steps.

Bug: tint:1582
Change-Id: I290aede118a30ab0f4294c89ec43005371c87b45
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/93446
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
This commit is contained in:
Ben Clayton 2022-06-16 08:36:30 +00:00 committed by Dawn LUCI CQ
parent 418e873ad2
commit 2a8861d20e
17 changed files with 169 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
src/tint/reader/wgsl/parser_impl.cc
View File

@ -433,7 +433,7 @@ Maybe<bool> ParserImpl::global_decl() {
}
if (gc.matched) {
if (!expect("let declaration", Token::Type::kSemicolon)) {
if (!expect("'let' declaration", Token::Type::kSemicolon)) {
return Failure::kErrored;
}
@ -562,7 +562,7 @@ Maybe<const ast::Variable*> ParserImpl::global_constant_decl(ast::AttributeList&
bool is_overridable = false;
const char* use = nullptr;
if (match(Token::Type::kLet)) {
use = "let declaration";
use = "'let' declaration";
} else if (match(Token::Type::kOverride)) {
use = "override declaration";
is_overridable = true;
@ -575,8 +575,18 @@ Maybe<const ast::Variable*> ParserImpl::global_constant_decl(ast::AttributeList&
return Failure::kErrored;
}
bool has_initializer = false;
if (is_overridable) {
has_initializer = match(Token::Type::kEqual);
} else {
if (!expect(use, Token::Type::kEqual)) {
return Failure::kErrored;
}
has_initializer = true;
}
const ast::Expression* initializer = nullptr;
if (match(Token::Type::kEqual)) {
if (has_initializer) {
auto init = expect_const_expr();
if (init.errored) {
return Failure::kErrored;
@ -1757,13 +1767,13 @@ Maybe<const ast::ReturnStatement*> ParserImpl::return_stmt() {
// | CONST variable_ident_decl EQUAL logical_or_expression
Maybe<const ast::VariableDeclStatement*> ParserImpl::variable_stmt() {
if (match(Token::Type::kLet)) {
auto decl = expect_variable_ident_decl("let declaration",
auto decl = expect_variable_ident_decl("'let' declaration",
/*allow_inferred = */ true);
if (decl.errored) {
return Failure::kErrored;
}
if (!expect("let declaration", Token::Type::kEqual)) {
if (!expect("'let' declaration", Token::Type::kEqual)) {
return Failure::kErrored;
}
@ -1772,7 +1782,7 @@ Maybe<const ast::VariableDeclStatement*> ParserImpl::variable_stmt() {
return Failure::kErrored;
}
if (!constructor.matched) {
return add_error(peek(), "missing constructor for let declaration");
return add_error(peek(), "missing constructor for 'let' declaration");
}
auto* var = create<ast::Variable>(decl->source, // source
@ -1803,7 +1813,7 @@ Maybe<const ast::VariableDeclStatement*> ParserImpl::variable_stmt() {
return Failure::kErrored;
}
if (!constructor_expr.matched) {
return add_error(peek(), "missing constructor for variable declaration");
return add_error(peek(), "missing constructor for 'var' declaration");
}
constructor = constructor_expr.value;

14
src/tint/reader/wgsl/parser_impl_error_msg_test.cc
View File

@ -204,7 +204,7 @@ fn f() { x = vec2<u32>(1,2; }
TEST_F(ParserImplErrorTest, ConstVarStmtInvalid) {
EXPECT("fn f() { let >; }",
R"(test.wgsl:1:14 error: expected identifier for let declaration
R"(test.wgsl:1:14 error: expected identifier for 'let' declaration
fn f() { let >; }
^
)");
@ -212,7 +212,7 @@ fn f() { let >; }
TEST_F(ParserImplErrorTest, ConstVarStmtMissingAssignment) {
EXPECT("fn f() { let a : i32; }",
R"(test.wgsl:1:21 error: expected '=' for let declaration
R"(test.wgsl:1:21 error: expected '=' for 'let' declaration
fn f() { let a : i32; }
^
)");
@ -220,7 +220,7 @@ fn f() { let a : i32; }
TEST_F(ParserImplErrorTest, ConstVarStmtMissingConstructor) {
EXPECT("fn f() { let a : i32 = >; }",
R"(test.wgsl:1:24 error: missing constructor for let declaration
R"(test.wgsl:1:24 error: missing constructor for 'let' declaration
fn f() { let a : i32 = >; }
^
)");
@ -472,7 +472,7 @@ test.wgsl:3:3 error: statement found outside of function body
TEST_F(ParserImplErrorTest, GlobalDeclConstInvalidIdentifier) {
EXPECT("let ^ : i32 = 1;",
R"(test.wgsl:1:5 error: expected identifier for let declaration
R"(test.wgsl:1:5 error: expected identifier for 'let' declaration
let ^ : i32 = 1;
^
)");
@ -480,7 +480,7 @@ let ^ : i32 = 1;
TEST_F(ParserImplErrorTest, GlobalDeclConstMissingSemicolon) {
EXPECT("let i : i32 = 1",
R"(test.wgsl:1:16 error: expected ';' for let declaration
R"(test.wgsl:1:16 error: expected ';' for 'let' declaration
let i : i32 = 1
^
)");
@ -512,7 +512,7 @@ let i : vec2<i32> = vec2<i32>(!);
TEST_F(ParserImplErrorTest, GlobalDeclConstBadConstLiteralSpaceLessThan) {
EXPECT("let i = 1 < 2;",
R"(test.wgsl:1:11 error: expected ';' for let declaration
R"(test.wgsl:1:11 error: expected ';' for 'let' declaration
let i = 1 < 2;
^
)");
@ -1215,7 +1215,7 @@ fn f() { var a : u32 }
TEST_F(ParserImplErrorTest, VarStmtInvalidAssignment) {
EXPECT("fn f() { var a : u32 = >; }",
R"(test.wgsl:1:24 error: missing constructor for variable declaration
R"(test.wgsl:1:24 error: missing constructor for 'var' declaration
fn f() { var a : u32 = >; }
^
)");

4
src/tint/reader/wgsl/parser_impl_for_stmt_test.cc
View File

@ -253,7 +253,7 @@ TEST_F(ForStmtErrorTest, MissingRightBrace) {
// Test a for loop with an invalid initializer statement.
TEST_F(ForStmtErrorTest, InvalidInitializerAsConstDecl) {
std::string for_str = "for (let x: i32;;) { }";
std::string error_str = "1:16: expected '=' for let declaration";
std::string error_str = "1:16: expected '=' for 'let' declaration";
TestForWithError(for_str, error_str);
}
@ -304,7 +304,7 @@ TEST_F(ForStmtErrorTest, InvalidContinuingMatch) {
// Test a for loop with an invalid body.
TEST_F(ForStmtErrorTest, InvalidBody) {
std::string for_str = "for (;;) { let x: i32; }";
std::string error_str = "1:22: expected '=' for let declaration";
std::string error_str = "1:22: expected '=' for 'let' declaration";
TestForWithError(for_str, error_str);
}

11
src/tint/reader/wgsl/parser_impl_global_decl_test.cc
View File

@ -61,18 +61,25 @@ TEST_F(ParserImplTest, GlobalDecl_GlobalConstant) {
EXPECT_EQ(v->symbol, program.Symbols().Get("a"));
}
TEST_F(ParserImplTest, GlobalDecl_GlobalConstant_MissingInitializer) {
auto p = parser("let a : vec2<i32>;");
p->global_decl();
ASSERT_TRUE(p->has_error());
EXPECT_EQ(p->error(), "1:18: expected '=' for 'let' declaration");
}
TEST_F(ParserImplTest, GlobalDecl_GlobalConstant_Invalid) {
auto p = parser("let a : vec2<i32> 1.0;");
p->global_decl();
ASSERT_TRUE(p->has_error());
EXPECT_EQ(p->error(), "1:19: expected ';' for let declaration");
EXPECT_EQ(p->error(), "1:19: expected '=' for 'let' declaration");
}
TEST_F(ParserImplTest, GlobalDecl_GlobalConstant_MissingSemicolon) {
auto p = parser("let a : vec2<i32> = vec2<i32>(1, 2)");
p->global_decl();
ASSERT_TRUE(p->has_error());
EXPECT_EQ(p->error(), "1:36: expected ';' for let declaration");
EXPECT_EQ(p->error(), "1:36: expected ';' for 'let' declaration");
}
TEST_F(ParserImplTest, GlobalDecl_TypeAlias) {

2
src/tint/reader/wgsl/parser_impl_statement_test.cc
View File

@ -114,7 +114,7 @@ TEST_F(ParserImplTest, Statement_Variable_Invalid) {
EXPECT_TRUE(e.errored);
EXPECT_FALSE(e.matched);
EXPECT_EQ(e.value, nullptr);
EXPECT_EQ(p->error(), "1:14: missing constructor for variable declaration");
EXPECT_EQ(p->error(), "1:14: missing constructor for 'var' declaration");
}
TEST_F(ParserImplTest, Statement_Variable_MissingSemicolon) {

8
src/tint/reader/wgsl/parser_impl_variable_stmt_test.cc
View File

@ -63,7 +63,7 @@ TEST_F(ParserImplTest, VariableStmt_VariableDecl_ConstructorInvalid) {
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
EXPECT_EQ(p->error(), "1:15: missing constructor for variable declaration");
EXPECT_EQ(p->error(), "1:15: missing constructor for 'var' declaration");
}
TEST_F(ParserImplTest, VariableStmt_VariableDecl_ArrayInit) {
@ -160,7 +160,7 @@ TEST_F(ParserImplTest, VariableStmt_Let_MissingEqual) {
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
EXPECT_EQ(p->error(), "1:13: expected '=' for let declaration");
EXPECT_EQ(p->error(), "1:13: expected '=' for 'let' declaration");
}
TEST_F(ParserImplTest, VariableStmt_Let_MissingConstructor) {
@ -170,7 +170,7 @@ TEST_F(ParserImplTest, VariableStmt_Let_MissingConstructor) {
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
EXPECT_EQ(p->error(), "1:14: missing constructor for let declaration");
EXPECT_EQ(p->error(), "1:14: missing constructor for 'let' declaration");
}
TEST_F(ParserImplTest, VariableStmt_Let_InvalidConstructor) {
@ -180,7 +180,7 @@ TEST_F(ParserImplTest, VariableStmt_Let_InvalidConstructor) {
EXPECT_TRUE(e.errored);
EXPECT_EQ(e.value, nullptr);
EXPECT_TRUE(p->has_error());
EXPECT_EQ(p->error(), "1:15: missing constructor for let declaration");
EXPECT_EQ(p->error(), "1:15: missing constructor for 'let' declaration");
}
} // namespace

2
src/tint/resolver/assignment_validation_test.cc
View File

@ -222,7 +222,7 @@ TEST_F(ResolverAssignmentValidationTest, AssignToLet_Fail) {
WrapInFunction(var, Assign(Expr(Source{{12, 34}}, "a"), 2_i));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: cannot assign to const\nnote: 'a' is declared here:");
EXPECT_EQ(r()->error(), "12:34 error: cannot assign to 'let'\nnote: 'a' is declared here:");
}
TEST_F(ResolverAssignmentValidationTest, AssignNonConstructible_Handle) {

54
src/tint/resolver/attribute_validation_test.cc
View File

@ -721,7 +721,7 @@ TEST_P(VariableAttributeTest, IsValid) {
} else {
EXPECT_FALSE(r()->Resolve());
if (!IsBindingAttribute(params.kind)) {
EXPECT_EQ(r()->error(), "12:34 error: attribute is not valid for variables");
EXPECT_EQ(r()->error(), "12:34 error: attribute is not valid for module-scope 'var'");
}
}
}
@ -783,11 +783,59 @@ TEST_P(ConstantAttributeTest, IsValid) {
EXPECT_TRUE(r()->Resolve()) << r()->error();
} else {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: attribute is not valid for constants");
EXPECT_EQ(r()->error(),
"12:34 error: attribute is not valid for module-scope 'let' declaration");
}
}
INSTANTIATE_TEST_SUITE_P(ResolverAttributeValidationTest,
ConstantAttributeTest,
testing::Values(TestParams{AttributeKind::kAlign, false},
TestParams{AttributeKind::kBinding, false},
TestParams{AttributeKind::kBuiltin, false},
TestParams{AttributeKind::kGroup, false},
TestParams{AttributeKind::kId, false},
TestParams{AttributeKind::kInterpolate, false},
TestParams{AttributeKind::kInvariant, false},
TestParams{AttributeKind::kLocation, false},
TestParams{AttributeKind::kOffset, false},
TestParams{AttributeKind::kSize, false},
TestParams{AttributeKind::kStage, false},
TestParams{AttributeKind::kStride, false},
TestParams{AttributeKind::kWorkgroup, false},
TestParams{AttributeKind::kBindingAndGroup, false}));
TEST_F(ConstantAttributeTest, DuplicateAttribute) {
GlobalConst("a", ty.f32(), Expr(1.23_f),
ast::AttributeList{
create<ast::IdAttribute>(Source{{12, 34}}, 0),
create<ast::IdAttribute>(Source{{56, 78}}, 1),
});
WrapInFunction();
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
R"(56:78 error: duplicate id attribute
12:34 note: first attribute declared here)");
}
using OverrideAttributeTest = TestWithParams;
TEST_P(OverrideAttributeTest, IsValid) {
auto& params = GetParam();
Override("a", ty.f32(), Expr(1.23_f), createAttributes(Source{{12, 34}}, *this, params.kind));
WrapInFunction();
if (params.should_pass) {
EXPECT_TRUE(r()->Resolve()) << r()->error();
} else {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: attribute is not valid for 'override' declaration");
}
}
INSTANTIATE_TEST_SUITE_P(ResolverAttributeValidationTest,
OverrideAttributeTest,
testing::Values(TestParams{AttributeKind::kAlign, false},
TestParams{AttributeKind::kBinding, false},
TestParams{AttributeKind::kBuiltin, false},
@ -803,7 +851,7 @@ INSTANTIATE_TEST_SUITE_P(ResolverAttributeValidationTest,
TestParams{AttributeKind::kWorkgroup, false},
TestParams{AttributeKind::kBindingAndGroup, false}));
TEST_F(ConstantAttributeTest, DuplicateAttribute) {
TEST_F(OverrideAttributeTest, DuplicateAttribute) {
GlobalConst("a", ty.f32(), Expr(1.23_f),
ast::AttributeList{
create<ast::IdAttribute>(Source{{12, 34}}, 0),

2
src/tint/resolver/compound_assignment_validation_test.cc
View File

@ -249,7 +249,7 @@ TEST_F(ResolverCompoundAssignmentValidationTest, LhsConstant) {
WrapInFunction(a, CompoundAssign(Expr(Source{{56, 78}}, "a"), 1_i, ast::BinaryOp::kAdd));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), R"(56:78 error: cannot assign to const
EXPECT_EQ(r()->error(), R"(56:78 error: cannot assign to 'let'
12:34 note: 'a' is declared here:)");
}

2
src/tint/resolver/increment_decrement_validation_test.cc
View File

@ -151,7 +151,7 @@ TEST_F(ResolverIncrementDecrementValidationTest, Constant) {
WrapInFunction(a, Increment(Expr(Source{{56, 78}}, "a")));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), R"(56:78 error: cannot modify constant value
EXPECT_EQ(r()->error(), R"(56:78 error: cannot modify 'let'
12:34 note: 'a' is declared here:)");
}

2
src/tint/resolver/inferred_type_test.cc
View File

@ -98,7 +98,7 @@ TEST_P(ResolverInferredTypeParamTest, GlobalVar_Fail) {
WrapInFunction();
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: global var declaration must specify a type");
EXPECT_EQ(r()->error(), "12:34 error: module-scope 'var' declaration must specify a type");
}
TEST_P(ResolverInferredTypeParamTest, LocalLet_Pass) {

31
src/tint/resolver/resolver.cc
View File

@ -326,19 +326,19 @@ sem::Variable* Resolver::Variable(const ast::Variable* var,
// If the variable has no declared type, infer it from the RHS
if (!storage_ty) {
if (!var->is_const && kind == VariableKind::kGlobal) {
AddError("global var declaration must specify a type", var->source);
AddError("module-scope 'var' declaration must specify a type", var->source);
return nullptr;
}
storage_ty = rhs->Type()->UnwrapRef(); // Implicit load of RHS
}
} else if (var->is_const && !var->is_overridable && kind != VariableKind::kParameter) {
AddError("let declaration must have an initializer", var->source);
AddError("'let' declaration must have an initializer", var->source);
return nullptr;
} else if (!var->type) {
AddError((kind == VariableKind::kGlobal)
? "module scope var declaration requires a type and initializer"
: "function scope var declaration requires a type or initializer",
? "module-scope 'var' declaration requires a type or initializer"
: "function-scope 'var' declaration requires a type or initializer",
var->source);
return nullptr;
}
@ -368,7 +368,7 @@ sem::Variable* Resolver::Variable(const ast::Variable* var,
storage_class != ast::StorageClass::kFunction &&
validator_.IsValidationEnabled(var->attributes,
ast::DisabledValidation::kIgnoreStorageClass)) {
AddError("function variable has a non-function storage class", var->source);
AddError("function-scope 'var' declaration must use 'function' storage class", var->source);
return nullptr;
}
@ -519,11 +519,13 @@ sem::GlobalVariable* Resolver::GlobalVariable(const ast::Variable* var) {
auto storage_class = sem->StorageClass();
if (!var->is_const && storage_class == ast::StorageClass::kNone) {
AddError("global variables must have a storage class", var->source);
AddError("module-scope 'var' declaration must have a storage class", var->source);
return nullptr;
}
if (var->is_const && storage_class != ast::StorageClass::kNone) {
AddError("global constants shouldn't have a storage class", var->source);
AddError(var->is_overridable ? "'override' declaration must not have a storage class"
: "'let' declaration must not have a storage class",
var->source);
return nullptr;
}
@ -2069,21 +2071,16 @@ sem::Array* Resolver::Array(const ast::Array* arr) {
if (auto* ident = count_expr->As<ast::IdentifierExpression>()) {
// Make sure the identifier is a non-overridable module-scope constant.
auto* var = sem_.ResolvedSymbol<sem::GlobalVariable>(ident);
if (!var || !var->Declaration()->is_const) {
AddError("array size identifier must be a module-scope constant", size_source);
return nullptr;
}
if (var->IsOverridable()) {
AddError("array size expression must not be pipeline-overridable", size_source);
if (!var || !var->Declaration()->is_const || var->IsOverridable()) {
AddError("array size identifier must be a literal or a module-scope 'let'",
size_source);
return nullptr;
}
count_expr = var->Declaration()->constructor;
} else if (!count_expr->Is<ast::LiteralExpression>()) {
AddError(
"array size expression must be either a literal or a module-scope "
"constant",
size_source);
AddError("array size identifier must be a literal or a module-scope 'let'",
size_source);
return nullptr;
}

6
src/tint/resolver/storage_class_validation_test.cc
View File

@ -30,7 +30,8 @@ TEST_F(ResolverStorageClassValidationTest, GlobalVariableNoStorageClass_Fail) {
Global(Source{{12, 34}}, "g", ty.f32(), ast::StorageClass::kNone);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: global variables must have a storage class");
EXPECT_EQ(r()->error(),
"12:34 error: module-scope 'var' declaration must have a storage class");
}
TEST_F(ResolverStorageClassValidationTest, GlobalVariableFunctionStorageClass_Fail) {
@ -39,8 +40,7 @@ TEST_F(ResolverStorageClassValidationTest, GlobalVariableFunctionStorageClass_Fa
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: variables declared at module scope must not be in "
"the function storage class");
"12:34 error: module-scope 'var' must not use storage class 'function'");
}
TEST_F(ResolverStorageClassValidationTest, Private_RuntimeArray) {

26
src/tint/resolver/type_validation_test.cc
View File

@ -90,11 +90,21 @@ TEST_F(ResolverTypeValidationTest, GlobalVariableWithStorageClass_Pass) {
TEST_F(ResolverTypeValidationTest, GlobalLetWithStorageClass_Fail) {
// let<private> global_var: f32;
AST().AddGlobalVariable(create<ast::Variable>(
Source{{12, 34}}, Symbols().Register("global_var"), ast::StorageClass::kPrivate,
Source{{12, 34}}, Symbols().Register("global_let"), ast::StorageClass::kPrivate,
ast::Access::kUndefined, ty.f32(), true, false, Expr(1.23_f), ast::AttributeList{}));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: global constants shouldn't have a storage class");
EXPECT_EQ(r()->error(), "12:34 error: 'let' declaration must not have a storage class");
}
TEST_F(ResolverTypeValidationTest, OverrideWithStorageClass_Fail) {
// let<private> global_var: f32;
AST().AddGlobalVariable(create<ast::Variable>(
Source{{12, 34}}, Symbols().Register("global_override"), ast::StorageClass::kPrivate,
ast::Access::kUndefined, ty.f32(), true, true, Expr(1.23_f), ast::AttributeList{}));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: 'override' declaration must not have a storage class");
}
TEST_F(ResolverTypeValidationTest, GlobalConstNoStorageClass_Pass) {
@ -334,7 +344,8 @@ TEST_F(ResolverTypeValidationTest, ArraySize_Overridable) {
Override("size", nullptr, Expr(10_i));
Global("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 expression must not be pipeline-overridable");
EXPECT_EQ(r()->error(),
"12:34 error: array size identifier must be a literal or a module-scope 'let'");
}
TEST_F(ResolverTypeValidationTest, ArraySize_ModuleVar) {
@ -343,7 +354,8 @@ TEST_F(ResolverTypeValidationTest, ArraySize_ModuleVar) {
Global("size", ty.i32(), Expr(10_i), ast::StorageClass::kPrivate);
Global("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 identifier must be a module-scope constant");
EXPECT_EQ(r()->error(),
"12:34 error: array size identifier must be a literal or a module-scope 'let'");
}
TEST_F(ResolverTypeValidationTest, ArraySize_FunctionLet) {
@ -355,7 +367,8 @@ TEST_F(ResolverTypeValidationTest, ArraySize_FunctionLet) {
auto* a = Var("a", ty.array(ty.f32(), Expr(Source{{12, 34}}, "size")));
WrapInFunction(Block(Decl(size), Decl(a)));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: array size identifier must be a module-scope constant");
EXPECT_EQ(r()->error(),
"12:34 error: array size identifier must be a literal or a module-scope 'let'");
}
TEST_F(ResolverTypeValidationTest, ArraySize_InvalidExpr) {
@ -364,8 +377,7 @@ TEST_F(ResolverTypeValidationTest, ArraySize_InvalidExpr) {
WrapInFunction(Block(Decl(a)));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: array size expression must be either a literal or a "
"module-scope constant");
"12:34 error: array size identifier must be a literal or a module-scope 'let'");
}
TEST_F(ResolverTypeValidationTest, RuntimeArrayInFunction_Fail) {

6
src/tint/resolver/validation_test.cc
View File

@ -304,7 +304,8 @@ TEST_F(ResolverValidationTest, StorageClass_FunctionVariableWorkgroupClass) {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "error: function variable has a non-function storage class");
EXPECT_EQ(r()->error(),
"error: function-scope 'var' declaration must use 'function' storage class");
}
TEST_F(ResolverValidationTest, StorageClass_FunctionVariableI32) {
@ -317,7 +318,8 @@ TEST_F(ResolverValidationTest, StorageClass_FunctionVariableI32) {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "error: function variable has a non-function storage class");
EXPECT_EQ(r()->error(),
"error: function-scope 'var' declaration must use 'function' storage class");
}
TEST_F(ResolverValidationTest, StorageClass_SamplerExplicitStorageClass) {

61
src/tint/resolver/validator.cc
View File

@ -506,25 +506,29 @@ bool Validator::GlobalVariable(
for (auto* attr : decl->attributes) {
if (decl->is_const) {
if (auto* id_attr = attr->As<ast::IdAttribute>()) {
uint32_t id = id_attr->value;
auto it = constant_ids.find(id);
if (it != constant_ids.end() && it->second != var) {
AddError("pipeline constant IDs must be unique", attr->source);
AddNote(
"a pipeline constant with an ID of " + std::to_string(id) +
" was previously declared "
"here:",
ast::GetAttribute<ast::IdAttribute>(it->second->Declaration()->attributes)
->source);
return false;
}
if (id > 65535) {
AddError("pipeline constant IDs must be between 0 and 65535", attr->source);
if (decl->is_overridable) {
if (auto* id_attr = attr->As<ast::IdAttribute>()) {
uint32_t id = id_attr->value;
auto it = constant_ids.find(id);
if (it != constant_ids.end() && it->second != var) {
AddError("pipeline constant IDs must be unique", attr->source);
AddNote("a pipeline constant with an ID of " + std::to_string(id) +
" was previously declared here:",
ast::GetAttribute<ast::IdAttribute>(
it->second->Declaration()->attributes)
->source);
return false;
}
if (id > 65535) {
AddError("pipeline constant IDs must be between 0 and 65535", attr->source);
return false;
}
} else {
AddError("attribute is not valid for 'override' declaration", attr->source);
return false;
}
} else {
AddError("attribute is not valid for constants", attr->source);
AddError("attribute is not valid for module-scope 'let' declaration", attr->source);
return false;
}
} else {
@ -536,17 +540,14 @@ bool Validator::GlobalVariable(
if (!(attr->IsAnyOf<ast::BindingAttribute, ast::GroupAttribute,
ast::InternalAttribute>()) &&
(!is_shader_io_attribute || !has_io_storage_class)) {
AddError("attribute is not valid for variables", attr->source);
AddError("attribute is not valid for module-scope 'var'", attr->source);
return false;
}
}
}
if (var->StorageClass() == ast::StorageClass::kFunction) {
AddError(
"variables declared at module scope must not be in the function "
"storage class",
decl->source);
AddError("module-scope 'var' must not use storage class 'function'", decl->source);
return false;
}
@ -559,10 +560,7 @@ bool Validator::GlobalVariable(
// Each resource variable must be declared with both group and binding
// attributes.
if (!binding_point) {
AddError(
"resource variables require @group and @binding "
"attributes",
decl->source);
AddError("resource variables require @group and @binding attributes", decl->source);
return false;
}
break;
@ -571,10 +569,8 @@ bool Validator::GlobalVariable(
if (binding_point.binding || binding_point.group) {
// https://gpuweb.github.io/gpuweb/wgsl/#attribute-binding
// Must only be applied to a resource variable
AddError(
"non-resource variables must not have @group or @binding "
"attributes",
decl->source);
AddError("non-resource variables must not have @group or @binding attributes",
decl->source);
return false;
}
}
@ -2162,7 +2158,9 @@ bool Validator::Assignment(const ast::Statement* a, const sem::Type* rhs_ty) con
return false;
}
if (decl->is_const) {
AddError("cannot assign to const", lhs->source);
AddError(
decl->is_overridable ? "cannot assign to 'override'" : "cannot assign to 'let'",
lhs->source);
AddNote("'" + symbols_.NameFor(decl->symbol) + "' is declared here:", decl->source);
return false;
}
@ -2210,7 +2208,8 @@ bool Validator::IncrementDecrementStatement(const ast::IncrementDecrementStateme
return false;
}
if (decl->is_const) {
AddError("cannot modify constant value", lhs->source);
AddError(decl->is_overridable ? "cannot modify 'override'" : "cannot modify 'let'",
lhs->source);
AddNote("'" + symbols_.NameFor(decl->symbol) + "' is declared here:", decl->source);
return false;
}

10
src/tint/resolver/var_let_validation_test.cc
View File

@ -29,7 +29,7 @@ TEST_F(ResolverVarLetValidationTest, LetNoInitializer) {
WrapInFunction(Let(Source{{12, 34}}, "a", ty.i32(), nullptr));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: let declaration must have an initializer");
EXPECT_EQ(r()->error(), "12:34 error: 'let' declaration must have an initializer");
}
TEST_F(ResolverVarLetValidationTest, GlobalLetNoInitializer) {
@ -37,7 +37,7 @@ TEST_F(ResolverVarLetValidationTest, GlobalLetNoInitializer) {
GlobalConst(Source{{12, 34}}, "a", ty.i32(), nullptr);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: let declaration must have an initializer");
EXPECT_EQ(r()->error(), "12:34 error: 'let' declaration must have an initializer");
}
TEST_F(ResolverVarLetValidationTest, VarNoInitializerNoType) {
@ -46,8 +46,7 @@ TEST_F(ResolverVarLetValidationTest, VarNoInitializerNoType) {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: function scope var declaration requires a type or "
"initializer");
"12:34 error: function-scope 'var' declaration requires a type or initializer");
}
TEST_F(ResolverVarLetValidationTest, GlobalVarNoInitializerNoType) {
@ -56,8 +55,7 @@ TEST_F(ResolverVarLetValidationTest, GlobalVarNoInitializerNoType) {
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: module scope var declaration requires a type and "
"initializer");
"12:34 error: module-scope 'var' declaration requires a type or initializer");
}
TEST_F(ResolverVarLetValidationTest, VarTypeNotStorable) {