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

tint/validator: Hint 'var' instead of 'const' 

Involves expanding the source range of a variable declaration so we can point at something that can include the 'const'.

Fixed: tint:1740
Change-Id: Ie8f784de34a1792002aaa708c1b77053be54f1b5
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/108120
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
This commit is contained in:
Ben Clayton 2022-11-02 18:14:59 +00:00 committed by Dawn LUCI CQ
parent fd9c3fe4bc
commit 9535f72209
16 changed files with 106 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -154,21 +154,26 @@ class ParserImpl::MultiTokenSource {
/// Constructor that starts with the input `start` Source /// Constructor that starts with the input `start` Source
/// @param parser the parser /// @param parser the parser
/// @param start the start source of the range /// @param start the start source of the range
MultiTokenSource(ParserImpl* parser, const Source& start) : parser_(parser), start_(start) {} MultiTokenSource(ParserImpl* parser, const tint::Source& start)
: parser_(parser), start_(start) {}
/// Implicit conversion to Source that returns the combined source from start /// @returns the Source that returns the combined source from start to the current last token's
/// to the current last token's source. /// source.
operator Source() const { tint::Source Source() const {
Source end = parser_->last_source().End(); auto end = parser_->last_source().End();
if (end < start_) { if (end < start_) {
end = start_; end = start_;
} }
return Source::Combine(start_, end); return Source::Combine(start_, end);
} }
/// Implicit conversion to Source that returns the combined source from start to the current
/// last token's source.
operator tint::Source() const { return Source(); }
private: private:
ParserImpl* parser_; ParserImpl* parser_;
Source start_; tint::Source start_;
}; };
ParserImpl::TypedIdentifier::TypedIdentifier() = default; ParserImpl::TypedIdentifier::TypedIdentifier() = default;
@ -1901,12 +1906,15 @@ Maybe<const ast::ReturnStatement*> ParserImpl::return_statement() {
// | LET optionally_typed_ident EQUAL expression // | LET optionally_typed_ident EQUAL expression
// | CONST optionally_typed_ident EQUAL expression // | CONST optionally_typed_ident EQUAL expression
Maybe<const ast::VariableDeclStatement*> ParserImpl::variable_statement() { Maybe<const ast::VariableDeclStatement*> ParserImpl::variable_statement() {
auto decl_source_range = make_source_range();
if (match(Token::Type::kConst)) { if (match(Token::Type::kConst)) {
auto decl = expect_optionally_typed_ident("'const' declaration"); auto typed_ident = expect_optionally_typed_ident("'const' declaration");
if (decl.errored) { if (typed_ident.errored) {
return Failure::kErrored; return Failure::kErrored;
} }
auto decl_source = decl_source_range.Source();
if (!expect("'const' declaration", Token::Type::kEqual)) { if (!expect("'const' declaration", Token::Type::kEqual)) {
return Failure::kErrored; return Failure::kErrored;
} }
@ -1919,21 +1927,23 @@ Maybe<const ast::VariableDeclStatement*> ParserImpl::variable_statement() {
return add_error(peek(), "missing initializer for 'const' declaration"); return add_error(peek(), "missing initializer for 'const' declaration");
} }
auto* const_ = create<ast::Const>(decl->source, // source auto* const_ = create<ast::Const>(typed_ident->source, // source
builder_.Symbols().Register(decl->name), // symbol builder_.Symbols().Register(typed_ident->name), // symbol
decl->type, // type typed_ident->type, // type
initializer.value, // initializer initializer.value, // initializer
utils::Empty); // attributes utils::Empty); // attributes
return create<ast::VariableDeclStatement>(decl->source, const_); return create<ast::VariableDeclStatement>(decl_source, const_);
} }
if (match(Token::Type::kLet)) { if (match(Token::Type::kLet)) {
auto decl = expect_optionally_typed_ident("'let' declaration"); auto typed_ident = expect_optionally_typed_ident("'let' declaration");
if (decl.errored) { if (typed_ident.errored) {
return Failure::kErrored; return Failure::kErrored;
} }
auto decl_source = decl_source_range.Source();
if (!expect("'let' declaration", Token::Type::kEqual)) { if (!expect("'let' declaration", Token::Type::kEqual)) {
return Failure::kErrored; return Failure::kErrored;
} }
@ -1946,13 +1956,13 @@ Maybe<const ast::VariableDeclStatement*> ParserImpl::variable_statement() {
return add_error(peek(), "missing initializer for 'let' declaration"); return add_error(peek(), "missing initializer for 'let' declaration");
} }
auto* let = create<ast::Let>(decl->source, // source auto* let = create<ast::Let>(typed_ident->source, // source
builder_.Symbols().Register(decl->name), // symbol builder_.Symbols().Register(typed_ident->name), // symbol
decl->type, // type typed_ident->type, // type
initializer.value, // initializer initializer.value, // initializer
utils::Empty); // attributes utils::Empty); // attributes
return create<ast::VariableDeclStatement>(decl->source, let); return create<ast::VariableDeclStatement>(decl_source, let);
} }
auto decl = variable_decl(); auto decl = variable_decl();
@ -1963,6 +1973,8 @@ Maybe<const ast::VariableDeclStatement*> ParserImpl::variable_statement() {
return Failure::kNoMatch; return Failure::kNoMatch;
} }
auto decl_source = decl_source_range.Source();
const ast::Expression* initializer = nullptr; const ast::Expression* initializer = nullptr;
if (match(Token::Type::kEqual)) { if (match(Token::Type::kEqual)) {
auto initializer_expr = expression(); auto initializer_expr = expression();
@ -1976,7 +1988,7 @@ Maybe<const ast::VariableDeclStatement*> ParserImpl::variable_statement() {
initializer = initializer_expr.value; initializer = initializer_expr.value;
} }
auto* var = create<ast::Var>(decl->source, // source auto* var = create<ast::Var>(decl_source, // source
builder_.Symbols().Register(decl->name), // symbol builder_.Symbols().Register(decl->name), // symbol
decl->type, // type decl->type, // type
decl->address_space, // address space decl->address_space, // address space

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

@ -28,10 +28,10 @@ TEST_F(ParserImplTest, VariableStmt_VariableDecl) {
ASSERT_NE(e->variable, nullptr); ASSERT_NE(e->variable, nullptr);
EXPECT_EQ(e->variable->symbol, p->builder().Symbols().Get("a")); EXPECT_EQ(e->variable->symbol, p->builder().Symbols().Get("a"));
ASSERT_EQ(e->source.range.begin.line, 1u); EXPECT_EQ(e->source.range.begin.line, 1u);
ASSERT_EQ(e->source.range.begin.column, 5u); EXPECT_EQ(e->source.range.begin.column, 1u);
ASSERT_EQ(e->source.range.end.line, 1u); EXPECT_EQ(e->source.range.end.line, 1u);
ASSERT_EQ(e->source.range.end.column, 6u); EXPECT_EQ(e->source.range.end.column, 12u);
EXPECT_EQ(e->variable->initializer, nullptr); EXPECT_EQ(e->variable->initializer, nullptr);
} }
@ -47,10 +47,10 @@ TEST_F(ParserImplTest, VariableStmt_VariableDecl_WithInit) {
ASSERT_NE(e->variable, nullptr); ASSERT_NE(e->variable, nullptr);
EXPECT_EQ(e->variable->symbol, p->builder().Symbols().Get("a")); EXPECT_EQ(e->variable->symbol, p->builder().Symbols().Get("a"));
ASSERT_EQ(e->source.range.begin.line, 1u); EXPECT_EQ(e->source.range.begin.line, 1u);
ASSERT_EQ(e->source.range.begin.column, 5u); EXPECT_EQ(e->source.range.begin.column, 1u);
ASSERT_EQ(e->source.range.end.line, 1u); EXPECT_EQ(e->source.range.end.line, 1u);
ASSERT_EQ(e->source.range.end.column, 6u); EXPECT_EQ(e->source.range.end.column, 12u);
ASSERT_NE(e->variable->initializer, nullptr); ASSERT_NE(e->variable->initializer, nullptr);
EXPECT_TRUE(e->variable->initializer->Is<ast::LiteralExpression>()); EXPECT_TRUE(e->variable->initializer->Is<ast::LiteralExpression>());
@ -147,10 +147,10 @@ TEST_F(ParserImplTest, VariableStmt_Let) {
ASSERT_NE(e.value, nullptr); ASSERT_NE(e.value, nullptr);
ASSERT_TRUE(e->Is<ast::VariableDeclStatement>()); ASSERT_TRUE(e->Is<ast::VariableDeclStatement>());
ASSERT_EQ(e->source.range.begin.line, 1u); EXPECT_EQ(e->source.range.begin.line, 1u);
ASSERT_EQ(e->source.range.begin.column, 5u); EXPECT_EQ(e->source.range.begin.column, 1u);
ASSERT_EQ(e->source.range.end.line, 1u); EXPECT_EQ(e->source.range.end.line, 1u);
ASSERT_EQ(e->source.range.end.column, 6u); EXPECT_EQ(e->source.range.end.column, 12u);
} }
TEST_F(ParserImplTest, VariableStmt_Let_ComplexExpression) { TEST_F(ParserImplTest, VariableStmt_Let_ComplexExpression) {

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

@ -1382,6 +1382,14 @@ bool Validator::EvaluationStage(const sem::Expression* expr,
AddError(std::string(constraint) + " requires " + stage_name(latest_stage) + AddError(std::string(constraint) + " requires " + stage_name(latest_stage) +
", but expression is " + stage_name(expr->Stage()), ", but expression is " + stage_name(expr->Stage()),
expr->Declaration()->source); expr->Declaration()->source);
if (auto* stmt = expr->Stmt()) {
if (auto* decl = As<ast::VariableDeclStatement>(stmt->Declaration())) {
if (decl->variable->Is<ast::Const>()) {
AddNote("consider changing 'const' to 'let'", decl->source);
}
}
}
return false; return false;
} }
return true; return true;

39
src/tint/resolver/variable_validation_test.cc
View File

@ -417,10 +417,8 @@ TEST_F(ResolverVariableValidationTest, MatrixVarNoType) {
EXPECT_EQ(r()->error(), "12:34 error: missing matrix element type"); EXPECT_EQ(r()->error(), "12:34 error: missing matrix element type");
} }
TEST_F(ResolverVariableValidationTest, ConstInitWithVar) { TEST_F(ResolverVariableValidationTest, GlobalConstWithRuntimeExpression) {
auto* v = Var("v", Expr(1_i)); GlobalConst("c", Call(Source{{12, 34}}, "dpdx", 1._a));
auto* c = Const("c", Expr(Source{{12, 34}}, v));
WrapInFunction(v, c);
EXPECT_FALSE(r()->Resolve()); EXPECT_FALSE(r()->Resolve());
EXPECT_EQ( EXPECT_EQ(
@ -428,47 +426,64 @@ TEST_F(ResolverVariableValidationTest, ConstInitWithVar) {
R"(12:34 error: const initializer requires a const-expression, but expression is a runtime-expression)"); R"(12:34 error: const initializer requires a const-expression, but expression is a runtime-expression)");
} }
TEST_F(ResolverVariableValidationTest, ConstInitWithVar) {
auto* v = Var("v", Expr(1_i));
auto* c = Const("c", Expr(Source{{12, 34}}, v));
WrapInFunction(v, Decl(Source{{56, 78}}, c));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(
r()->error(),
R"(12:34 error: const initializer requires a const-expression, but expression is a runtime-expression
56:78 note: consider changing 'const' to 'let')");
}
TEST_F(ResolverVariableValidationTest, ConstInitWithOverride) { TEST_F(ResolverVariableValidationTest, ConstInitWithOverride) {
auto* o = Override("v", Expr(1_i)); auto* o = Override("v", Expr(1_i));
auto* c = Const("c", Expr(Source{{12, 34}}, o)); auto* c = Const("c", Expr(Source{{12, 34}}, o));
WrapInFunction(c); WrapInFunction(Decl(Source{{56, 78}}, c));
EXPECT_FALSE(r()->Resolve()); EXPECT_FALSE(r()->Resolve());
EXPECT_EQ( EXPECT_EQ(
r()->error(), r()->error(),
R"(12:34 error: const initializer requires a const-expression, but expression is an override-expression)"); R"(12:34 error: const initializer requires a const-expression, but expression is an override-expression
56:78 note: consider changing 'const' to 'let')");
} }
TEST_F(ResolverVariableValidationTest, ConstInitWithLet) { TEST_F(ResolverVariableValidationTest, ConstInitWithLet) {
auto* l = Let("v", Expr(1_i)); auto* l = Let("v", Expr(1_i));
auto* c = Const("c", Expr(Source{{12, 34}}, l)); auto* c = Const("c", Expr(Source{{12, 34}}, l));
WrapInFunction(l, c); WrapInFunction(l, Decl(Source{{56, 78}}, c));
EXPECT_FALSE(r()->Resolve()); EXPECT_FALSE(r()->Resolve());
EXPECT_EQ( EXPECT_EQ(
r()->error(), r()->error(),
R"(12:34 error: const initializer requires a const-expression, but expression is a runtime-expression)"); R"(12:34 error: const initializer requires a const-expression, but expression is a runtime-expression
56:78 note: consider changing 'const' to 'let')");
} }
TEST_F(ResolverVariableValidationTest, ConstInitWithRuntimeExpr) { TEST_F(ResolverVariableValidationTest, ConstInitWithRuntimeExpr) {
// const c = clamp(2, dpdx(0.5), 3); // const c = clamp(2, dpdx(0.5), 3);
WrapInFunction(Const("c", Call("clamp", 2_a, Call(Source{{12, 34}}, "dpdx", 0.5_a), 3_a))); auto* c = Const("c", Call("clamp", 2_a, Call(Source{{12, 34}}, "dpdx", 0.5_a), 3_a));
WrapInFunction(Decl(Source{{56, 78}}, c));
EXPECT_FALSE(r()->Resolve()); EXPECT_FALSE(r()->Resolve());
EXPECT_EQ( EXPECT_EQ(
r()->error(), r()->error(),
R"(12:34 error: const initializer requires a const-expression, but expression is a runtime-expression)"); R"(12:34 error: const initializer requires a const-expression, but expression is a runtime-expression
56:78 note: consider changing 'const' to 'let')");
} }
TEST_F(ResolverVariableValidationTest, ConstInitWithOverrideExpr) { TEST_F(ResolverVariableValidationTest, ConstInitWithOverrideExpr) {
auto* o = Override("v", Expr(1_i)); auto* o = Override("v", Expr(1_i));
auto* c = Const("c", Add(10_a, Expr(Source{{12, 34}}, o))); auto* c = Const("c", Add(10_a, Expr(Source{{12, 34}}, o)));
WrapInFunction(c); WrapInFunction(Decl(Source{{56, 78}}, c));
EXPECT_FALSE(r()->Resolve()); EXPECT_FALSE(r()->Resolve());
EXPECT_EQ( EXPECT_EQ(
r()->error(), r()->error(),
R"(12:34 error: const initializer requires a const-expression, but expression is an override-expression)"); R"(12:34 error: const initializer requires a const-expression, but expression is an override-expression
56:78 note: consider changing 'const' to 'let')");
} }
} // namespace } // namespace

4
test/tint/bug/tint/1369.wgsl.expected.dxc.hlsl
View File

@ -2,9 +2,9 @@ bug/tint/1369.wgsl:3:3 warning: code is unreachable
return true; return true;
^^^^^^ ^^^^^^
bug/tint/1369.wgsl:9:9 warning: code is unreachable bug/tint/1369.wgsl:9:5 warning: code is unreachable
var also_unreachable : bool; var also_unreachable : bool;
^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
static bool tint_discard = false; static bool tint_discard = false;

4
test/tint/bug/tint/1369.wgsl.expected.fxc.hlsl
View File

@ -2,9 +2,9 @@ bug/tint/1369.wgsl:3:3 warning: code is unreachable
return true; return true;
^^^^^^ ^^^^^^
bug/tint/1369.wgsl:9:9 warning: code is unreachable bug/tint/1369.wgsl:9:5 warning: code is unreachable
var also_unreachable : bool; var also_unreachable : bool;
^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
static bool tint_discard = false; static bool tint_discard = false;

4
test/tint/bug/tint/1369.wgsl.expected.glsl
View File

@ -2,9 +2,9 @@ bug/tint/1369.wgsl:3:3 warning: code is unreachable
return true; return true;
^^^^^^ ^^^^^^
bug/tint/1369.wgsl:9:9 warning: code is unreachable bug/tint/1369.wgsl:9:5 warning: code is unreachable
var also_unreachable : bool; var also_unreachable : bool;
^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
#version 310 es #version 310 es
precision mediump float; precision mediump float;

4
test/tint/bug/tint/1369.wgsl.expected.msl
View File

@ -2,9 +2,9 @@ bug/tint/1369.wgsl:3:3 warning: code is unreachable
return true; return true;
^^^^^^ ^^^^^^
bug/tint/1369.wgsl:9:9 warning: code is unreachable bug/tint/1369.wgsl:9:5 warning: code is unreachable
var also_unreachable : bool; var also_unreachable : bool;
^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
#include <metal_stdlib> #include <metal_stdlib>

4
test/tint/bug/tint/1369.wgsl.expected.spvasm
View File

@ -2,9 +2,9 @@ bug/tint/1369.wgsl:3:3 warning: code is unreachable
return true; return true;
^^^^^^ ^^^^^^
bug/tint/1369.wgsl:9:9 warning: code is unreachable bug/tint/1369.wgsl:9:5 warning: code is unreachable
var also_unreachable : bool; var also_unreachable : bool;
^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
; SPIR-V ; SPIR-V
; Version: 1.3 ; Version: 1.3

4
test/tint/bug/tint/1369.wgsl.expected.wgsl
View File

@ -2,9 +2,9 @@ bug/tint/1369.wgsl:3:3 warning: code is unreachable
return true; return true;
^^^^^^ ^^^^^^
bug/tint/1369.wgsl:9:9 warning: code is unreachable bug/tint/1369.wgsl:9:5 warning: code is unreachable
var also_unreachable : bool; var also_unreachable : bool;
^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^
fn call_discard() -> bool { fn call_discard() -> bool {
discard; discard;

4
test/tint/bug/tint/1474-b.wgsl.expected.dxc.hlsl
View File

@ -1,6 +1,6 @@
bug/tint/1474-b.wgsl:7:9 warning: code is unreachable bug/tint/1474-b.wgsl:7:5 warning: code is unreachable
let non_uniform_cond = non_uniform_value == 0; let non_uniform_cond = non_uniform_value == 0;
^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^
RWByteAddressBuffer non_uniform_value : register(u0, space0); RWByteAddressBuffer non_uniform_value : register(u0, space0);

4
test/tint/bug/tint/1474-b.wgsl.expected.fxc.hlsl
View File

@ -1,6 +1,6 @@
bug/tint/1474-b.wgsl:7:9 warning: code is unreachable bug/tint/1474-b.wgsl:7:5 warning: code is unreachable
let non_uniform_cond = non_uniform_value == 0; let non_uniform_cond = non_uniform_value == 0;
^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^
RWByteAddressBuffer non_uniform_value : register(u0, space0); RWByteAddressBuffer non_uniform_value : register(u0, space0);

4
test/tint/bug/tint/1474-b.wgsl.expected.glsl
View File

@ -1,6 +1,6 @@
bug/tint/1474-b.wgsl:7:9 warning: code is unreachable bug/tint/1474-b.wgsl:7:5 warning: code is unreachable
let non_uniform_cond = non_uniform_value == 0; let non_uniform_cond = non_uniform_value == 0;
^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^
#version 310 es #version 310 es

4
test/tint/bug/tint/1474-b.wgsl.expected.msl
View File

@ -1,6 +1,6 @@
bug/tint/1474-b.wgsl:7:9 warning: code is unreachable bug/tint/1474-b.wgsl:7:5 warning: code is unreachable
let non_uniform_cond = non_uniform_value == 0; let non_uniform_cond = non_uniform_value == 0;
^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^
#include <metal_stdlib> #include <metal_stdlib>

4
test/tint/bug/tint/1474-b.wgsl.expected.spvasm
View File

@ -1,6 +1,6 @@
bug/tint/1474-b.wgsl:7:9 warning: code is unreachable bug/tint/1474-b.wgsl:7:5 warning: code is unreachable
let non_uniform_cond = non_uniform_value == 0; let non_uniform_cond = non_uniform_value == 0;
^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^
; SPIR-V ; SPIR-V
; Version: 1.3 ; Version: 1.3

4
test/tint/bug/tint/1474-b.wgsl.expected.wgsl
View File

@ -1,6 +1,6 @@
bug/tint/1474-b.wgsl:7:9 warning: code is unreachable bug/tint/1474-b.wgsl:7:5 warning: code is unreachable
let non_uniform_cond = non_uniform_value == 0; let non_uniform_cond = non_uniform_value == 0;
^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^
@group(0) @binding(0) var<storage, read_write> non_uniform_value : i32; @group(0) @binding(0) var<storage, read_write> non_uniform_value : i32;