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

tint: Improve error for assignment to immutable variable 

Fixed: tint:1835
Change-Id: I48d1380fd6d9d5fdaae4a210e3c8695e26894905
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/121320
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This commit is contained in:
Ben Clayton 2023-02-23 20:50:36 +00:00 committed by Dawn LUCI CQ
parent 9cdc3343ff
commit 107aa81c89
7 changed files with 175 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -143,17 +143,6 @@ TEST_F(ResolverAssignmentValidationTest, AssignIncompatibleTypesInNestedBlockSta
EXPECT_EQ(r()->error(), "12:34 error: cannot assign 'f32' to 'i32'");
}
TEST_F(ResolverAssignmentValidationTest, AssignToScalar_Fail) {
// var my_var : i32 = 2i;
// 1 = my_var;
WrapInFunction(Var("my_var", ty.i32(), Expr(2_i)), //
Assign(Expr(Source{{12, 34}}, 1_i), "my_var"));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: cannot assign to value of type 'i32'");
}
TEST_F(ResolverAssignmentValidationTest, AssignCompatibleTypes_Pass) {
// var a : i32 = 1i;
// a = 2i;
@ -213,16 +202,87 @@ TEST_F(ResolverAssignmentValidationTest, AssignMaterializedThroughPointer_Pass)
EXPECT_TRUE(r()->Resolve()) << r()->error();
}
TEST_F(ResolverAssignmentValidationTest, AssignToScalar_Fail) {
// var my_var : i32 = 2i;
// 1 = my_var;
WrapInFunction(Var("my_var", ty.i32(), Expr(2_i)), //
Assign(Expr(Source{{12, 34}}, 1_i), "my_var"));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), R"(12:34 error: cannot assign to value expression of type 'i32')");
}
TEST_F(ResolverAssignmentValidationTest, AssignToOverride_Fail) {
// override a : i32 = 2i;
// {
// a = 2i
// }
Override(Source{{56, 78}}, "a", ty.i32(), Expr(2_i));
WrapInFunction(Assign(Expr(Source{{12, 34}}, "a"), 2_i));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), R"(12:34 error: cannot assign to override 'a'
12:34 note: 'override' variables are immutable
56:78 note: override 'a' declared here)");
}
TEST_F(ResolverAssignmentValidationTest, AssignToLet_Fail) {
// {
// let a : i32 = 2i;
// a = 2i
// }
auto* var = Let("a", ty.i32(), Expr(2_i));
WrapInFunction(var, Assign(Expr(Source{{12, 34}}, "a"), 2_i));
WrapInFunction(Let(Source{{56, 78}}, "a", ty.i32(), Expr(2_i)), //
Assign(Expr(Source{{12, 34}}, "a"), 2_i));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: cannot assign to 'let'\nnote: 'a' is declared here:");
EXPECT_EQ(r()->error(), R"(12:34 error: cannot assign to let 'a'
12:34 note: 'let' variables are immutable
56:78 note: let 'a' declared here)");
}
TEST_F(ResolverAssignmentValidationTest, AssignToConst_Fail) {
// {
// const a : i32 = 2i;
// a = 2i
// }
WrapInFunction(Const(Source{{56, 78}}, "a", ty.i32(), Expr(2_i)), //
Assign(Expr(Source{{12, 34}}, "a"), 2_i));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), R"(12:34 error: cannot assign to const 'a'
12:34 note: 'const' variables are immutable
56:78 note: const 'a' declared here)");
}
TEST_F(ResolverAssignmentValidationTest, AssignToParam_Fail) {
Func("foo", utils::Vector{Param(Source{{56, 78}}, "arg", ty.i32())}, ty.void_(),
utils::Vector{
Assign(Expr(Source{{12, 34}}, "arg"), Expr(1_i)),
Return(),
});
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
R"(12:34 error: cannot assign to parameter 'arg'
12:34 note: parameters are immutable
56:78 note: parameter 'arg' declared here)");
}
TEST_F(ResolverAssignmentValidationTest, AssignToLetMember_Fail) {
// struct S { i : i32 }
// {
// let a : S;
// a.i = 2i
// }
Structure("S", utils::Vector{Member("i", ty.i32())});
WrapInFunction(Let(Source{{98, 76}}, "a", ty("S"), Call("S")), //
Assign(MemberAccessor(Source{{12, 34}}, Expr(Source{{56, 78}}, "a"), "i"), 2_i));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), R"(12:34 error: cannot assign to value expression of type 'i32'
56:78 note: 'let' variables are immutable
98:76 note: let 'a' declared here)");
}
TEST_F(ResolverAssignmentValidationTest, AssignNonConstructible_Handle) {

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

@ -242,15 +242,16 @@ TEST_F(ResolverCompoundAssignmentValidationTest, ReadOnlyBuffer) {
"56:78 error: cannot store into a read-only type 'ref<storage, i32, read>'");
}
TEST_F(ResolverCompoundAssignmentValidationTest, LhsConstant) {
TEST_F(ResolverCompoundAssignmentValidationTest, LhsLet) {
// let a = 1i;
// a += 1i;
auto* a = Let(Source{{12, 34}}, "a", Expr(1_i));
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 'let'
12:34 note: 'a' is declared here:)");
EXPECT_EQ(r()->error(), R"(56:78 error: cannot assign to let 'a'
56:78 note: 'let' variables are immutable
12:34 note: let 'a' declared here)");
}
TEST_F(ResolverCompoundAssignmentValidationTest, LhsLiteral) {
@ -258,7 +259,7 @@ TEST_F(ResolverCompoundAssignmentValidationTest, LhsLiteral) {
WrapInFunction(CompoundAssign(Expr(Source{{56, 78}}, 1_i), 1_i, ast::BinaryOp::kAdd));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "56:78 error: cannot assign to value of type 'i32'");
EXPECT_EQ(r()->error(), R"(56:78 error: cannot assign to value expression of type 'i32')");
}
TEST_F(ResolverCompoundAssignmentValidationTest, LhsAtomic) {

32
src/tint/resolver/expression_kind_test.cc
View File

@ -598,30 +598,36 @@ INSTANTIATE_TEST_SUITE_P(
1:2 note: parameter 'PARAMETER' declared here)"},
{Def::kParameter, Use::kUnaryOp, kPass},
{Def::kStruct, Use::kAccess, R"(5:6 error: cannot use type 'STRUCT' as access)"},
{Def::kStruct, Use::kAddressSpace,
R"(5:6 error: cannot use type 'STRUCT' as address space)"},
{Def::kStruct, Use::kBinaryOp, R"(5:6 error: cannot use type 'STRUCT' as value
7:8 note: are you missing '()' for value constructor?
{Def::kStruct, Use::kAccess, R"(5:6 error: cannot use type 'STRUCT' as access
1:2 note: struct 'STRUCT' declared here)"},
{Def::kStruct, Use::kAddressSpace,
R"(5:6 error: cannot use type 'STRUCT' as address space
1:2 note: struct 'STRUCT' declared here)"},
{Def::kStruct, Use::kBinaryOp, R"(5:6 error: cannot use type 'STRUCT' as value
1:2 note: struct 'STRUCT' declared here
7:8 note: are you missing '()' for value constructor?)"},
{Def::kStruct, Use::kBuiltinValue,
R"(5:6 error: cannot use type 'STRUCT' as builtin value)"},
R"(5:6 error: cannot use type 'STRUCT' as builtin value
1:2 note: struct 'STRUCT' declared here)"},
{Def::kStruct, Use::kFunctionReturnType, kPass},
{Def::kStruct, Use::kInterpolationSampling,
R"(5:6 error: cannot use type 'STRUCT' as interpolation sampling)"},
R"(5:6 error: cannot use type 'STRUCT' as interpolation sampling
1:2 note: struct 'STRUCT' declared here)"},
{Def::kStruct, Use::kInterpolationType,
R"(5:6 error: cannot use type 'STRUCT' as interpolation type)"},
R"(5:6 error: cannot use type 'STRUCT' as interpolation type
1:2 note: struct 'STRUCT' declared here)"},
{Def::kStruct, Use::kMemberType, kPass},
{Def::kStruct, Use::kTexelFormat, R"(5:6 error: cannot use type 'STRUCT' as texel format)"},
{Def::kStruct, Use::kTexelFormat, R"(5:6 error: cannot use type 'STRUCT' as texel format
1:2 note: struct 'STRUCT' declared here)"},
{Def::kStruct, Use::kValueExpression,
R"(5:6 error: cannot use type 'STRUCT' as value
7:8 note: are you missing '()' for value constructor?
1:2 note: struct 'STRUCT' declared here)"},
1:2 note: struct 'STRUCT' declared here
7:8 note: are you missing '()' for value constructor?)"},
{Def::kStruct, Use::kVariableType, kPass},
{Def::kStruct, Use::kUnaryOp,
R"(5:6 error: cannot use type 'STRUCT' as value
7:8 note: are you missing '()' for value constructor?
1:2 note: struct 'STRUCT' declared here)"},
1:2 note: struct 'STRUCT' declared here
7:8 note: are you missing '()' for value constructor?)"},
{Def::kTexelFormat, Use::kAccess,
R"(5:6 error: cannot use texel format 'rgba8unorm' as access)"},

12
src/tint/resolver/function_validation_test.cc
View File

@ -487,18 +487,6 @@ TEST_F(ResolverFunctionValidationTest, FunctionConstInitWithParam) {
ASSERT_TRUE(r()->Resolve()) << r()->error();
}
TEST_F(ResolverFunctionValidationTest, FunctionParamsConst) {
Func("foo", utils::Vector{Param(Sym("arg"), ty.i32())}, ty.void_(),
utils::Vector{
Assign(Expr(Source{{12, 34}}, "arg"), Expr(1_i)),
Return(),
});
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
"12:34 error: cannot assign to function parameter\nnote: 'arg' is declared here:");
}
TEST_F(ResolverFunctionValidationTest, WorkgroupSize_GoodType_ConstU32) {
// const x = 4u;
// const y = 8u;

73
src/tint/resolver/sem_helper.cc
View File

@ -39,77 +39,67 @@ type::Type* SemHelper::TypeOf(const ast::Expression* expr) const {
return sem ? const_cast<type::Type*>(sem->Type()) : nullptr;
}
void SemHelper::ErrorUnexpectedExprKind(const sem::Expression* expr,
std::string_view wanted) const {
Switch(
std::string SemHelper::Describe(const sem::Expression* expr) const {
return Switch(
expr, //
[&](const sem::VariableUser* var_expr) {
auto* variable = var_expr->Variable()->Declaration();
auto name = builder_->Symbols().NameFor(variable->name->symbol);
std::string kind = Switch(
auto* kind = Switch(
variable, //
[&](const ast::Var*) { return "var"; }, //
[&](const ast::Let*) { return "let"; }, //
[&](const ast::Const*) { return "const"; }, //
[&](const ast::Parameter*) { return "parameter"; }, //
[&](const ast::Override*) { return "override"; }, //
[&](Default) { return "variable"; });
AddError("cannot use " + kind + " '" + name + "' as " + std::string(wanted),
var_expr->Declaration()->source);
NoteDeclarationSource(variable);
return std::string(kind) + " '" + name + "'";
},
[&](const sem::ValueExpression* val_expr) {
auto type = val_expr->Type()->FriendlyName(builder_->Symbols());
AddError("cannot use expression of type '" + type + "' as " + std::string(wanted),
val_expr->Declaration()->source);
return "value expression of type '" + type + "'";
},
[&](const sem::TypeExpression* ty_expr) {
auto name = ty_expr->Type()->FriendlyName(builder_->Symbols());
AddError("cannot use type '" + name + "' as " + std::string(wanted),
ty_expr->Declaration()->source);
return "type '" + name + "'";
},
[&](const sem::FunctionExpression* fn_expr) {
auto* fn = fn_expr->Function()->Declaration();
auto name = builder_->Symbols().NameFor(fn->name->symbol);
AddError("cannot use function '" + name + "' as " + std::string(wanted),
fn_expr->Declaration()->source);
NoteDeclarationSource(fn);
return "function '" + name + "'";
},
[&](const sem::BuiltinEnumExpression<builtin::Access>* access) {
AddError("cannot use access '" + utils::ToString(access->Value()) + "' as " +
std::string(wanted),
access->Declaration()->source);
return "access '" + utils::ToString(access->Value()) + "'";
},
[&](const sem::BuiltinEnumExpression<builtin::AddressSpace>* addr) {
AddError("cannot use address space '" + utils::ToString(addr->Value()) + "' as " +
std::string(wanted),
addr->Declaration()->source);
return "address space '" + utils::ToString(addr->Value()) + "'";
},
[&](const sem::BuiltinEnumExpression<builtin::BuiltinValue>* builtin) {
AddError("cannot use builtin value '" + utils::ToString(builtin->Value()) + "' as " +
std::string(wanted),
builtin->Declaration()->source);
return "builtin value '" + utils::ToString(builtin->Value()) + "'";
},
[&](const sem::BuiltinEnumExpression<builtin::InterpolationSampling>* fmt) {
AddError("cannot use interpolation sampling '" + utils::ToString(fmt->Value()) +
"' as " + std::string(wanted),
fmt->Declaration()->source);
return "interpolation sampling '" + utils::ToString(fmt->Value()) + "'";
},
[&](const sem::BuiltinEnumExpression<builtin::InterpolationType>* fmt) {
AddError("cannot use interpolation type '" + utils::ToString(fmt->Value()) + "' as " +
std::string(wanted),
fmt->Declaration()->source);
return "interpolation type '" + utils::ToString(fmt->Value()) + "'";
},
[&](const sem::BuiltinEnumExpression<builtin::TexelFormat>* fmt) {
AddError("cannot use texel format '" + utils::ToString(fmt->Value()) + "' as " +
std::string(wanted),
fmt->Declaration()->source);
return "texel format '" + utils::ToString(fmt->Value()) + "'";
},
[&](Default) {
[&](Default) -> std::string {
TINT_ICE(Resolver, builder_->Diagnostics())
<< "unhandled sem::Expression type: " << (expr ? expr->TypeInfo().name : "<null>");
return "<unknown>";
});
}
void SemHelper::ErrorUnexpectedExprKind(const sem::Expression* expr,
std::string_view wanted) const {
AddError("cannot use " + Describe(expr) + " as " + std::string(wanted),
expr->Declaration()->source);
NoteDeclarationSource(expr->Declaration());
}
void SemHelper::ErrorExpectedValueExpr(const sem::Expression* expr) const {
ErrorUnexpectedExprKind(expr, "value");
if (auto* ty_expr = expr->As<sem::TypeExpression>()) {
@ -117,14 +107,23 @@ void SemHelper::ErrorExpectedValueExpr(const sem::Expression* expr) const {
AddNote("are you missing '()' for value constructor?",
Source{{ident->source.range.end}});
}
if (auto* str = ty_expr->Type()->As<type::Struct>()) {
AddNote("struct '" + str->FriendlyName(builder_->Symbols()) + "' declared here",
str->Source());
}
}
}
void SemHelper::NoteDeclarationSource(const ast::Node* node) const {
if (!node) {
return;
}
Switch(
Get(node), //
[&](const sem::VariableUser* var_expr) { node = var_expr->Variable()->Declaration(); },
[&](const sem::TypeExpression* ty_expr) {
Switch(ty_expr->Type(), //
[&](const sem::Struct* s) { node = s->Declaration(); });
},
[&](const sem::FunctionExpression* fn_expr) { node = fn_expr->Function()->Declaration(); });
Switch(
node,
[&](const ast::Struct* n) {

4
src/tint/resolver/sem_helper.h
View File

@ -226,6 +226,10 @@ class SemHelper {
/// @param node the AST node.
void NoteDeclarationSource(const ast::Node* node) const;
/// @param expr the expression to describe
/// @return a string that describes @p expr. Useful for diagnostics.
std::string Describe(const sem::Expression* expr) const;
private:
/// Adds the given error message to the diagnostics
void AddError(const std::string& msg, const Source& source) const;

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

@ -2391,26 +2391,47 @@ bool Validator::Assignment(const ast::Statement* a, const type::Type* rhs_ty) co
}
// https://gpuweb.github.io/gpuweb/wgsl/#assignment-statement
auto const* lhs_ty = sem_.TypeOf(lhs);
if (auto* var_user = sem_.Get<sem::VariableUser>(lhs)) {
auto* v = var_user->Variable()->Declaration();
const char* err = Switch(
v, //
[&](const ast::Parameter*) { return "cannot assign to function parameter"; },
[&](const ast::Let*) { return "cannot assign to 'let'"; },
[&](const ast::Override*) { return "cannot assign to 'override'"; });
if (err) {
AddError(err, lhs->source);
AddNote("'" + symbols_.NameFor(v->name->symbol) + "' is declared here:", v->source);
return false;
}
}
auto const* lhs_sem = sem_.GetVal(lhs);
auto const* lhs_ty = lhs_sem->Type();
auto* lhs_ref = lhs_ty->As<type::Reference>();
if (!lhs_ref) {
// LHS is not a reference, so it has no storage.
AddError("cannot assign to value of type '" + sem_.TypeNameOf(lhs_ty) + "'", lhs->source);
AddError("cannot assign to " + sem_.Describe(lhs_sem), lhs->source);
auto* expr = lhs;
while (expr) {
expr = Switch(
expr, //
[&](const ast::AccessorExpression* e) { return e->object; },
[&](const ast::IdentifierExpression* i) {
if (auto user = sem_.Get<sem::VariableUser>(i)) {
Switch(
user->Variable()->Declaration(), //
[&](const ast::Let* v) {
AddNote("'let' variables are immutable",
user->Declaration()->source);
sem_.NoteDeclarationSource(v);
},
[&](const ast::Const* v) {
AddNote("'const' variables are immutable",
user->Declaration()->source);
sem_.NoteDeclarationSource(v);
},
[&](const ast::Override* v) {
AddNote("'override' variables are immutable",
user->Declaration()->source);
sem_.NoteDeclarationSource(v);
},
[&](const ast::Parameter* v) {
AddNote("parameters are immutable", user->Declaration()->source);
sem_.NoteDeclarationSource(v);
});
}
return nullptr;
});
}
return false;
}