tint::ProgramBuilder: Simplify variable constructors
Expand the Option argument paradigm to:
* Remove the requirement to always pass a 'type' parameter. Type inferencing is the easier, and increasingly common way to declare a variable, so this prevents a whole lot of `nullptr` smell which negatively impacts readability.
* Accept attributes directly as arguments, removing the `utils::Vector{ ... }` smell.
Rename `ProgramBuilder::VarOptionals` to `VarOptions`, and add equivalent `LetOptions`, `ConstOptions` and `OverrideOptions`.
Clean up all the calls to `Var()`, `Let()`, `Const()` and `Override()`:
* Use the `Group()` and `Binding()` helpers where possible
* Removing `nullptr` type arguments
* Replace attribute vectors with the list of attributes.
* Remove already-defaulted `ast::StorageClass::kNone` arguments.
* Remove already-defaulted `ast::Access::kUndefined` arguments.
Finally, remove the `GroupAndBinding()` helper, which only existed because you needed to pass attributes as a vector.
Change-Id: I8890e4eb0ffac9f9df2207b28a6f02a163e34d96
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/99580
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
diff --git a/src/tint/resolver/variable_validation_test.cc b/src/tint/resolver/variable_validation_test.cc
index 97b177e..0fb50a5 100644
--- a/src/tint/resolver/variable_validation_test.cc
+++ b/src/tint/resolver/variable_validation_test.cc
@@ -26,7 +26,7 @@
TEST_F(ResolverVariableValidationTest, VarNoInitializerNoType) {
// var a;
- WrapInFunction(Var(Source{{12, 34}}, "a", nullptr));
+ WrapInFunction(Var(Source{{12, 34}}, "a"));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: var declaration requires a type or initializer");
@@ -34,7 +34,7 @@
TEST_F(ResolverVariableValidationTest, GlobalVarNoInitializerNoType) {
// var a;
- GlobalVar(Source{{12, 34}}, "a", nullptr);
+ GlobalVar(Source{{12, 34}}, "a");
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: var declaration requires a type or initializer");
@@ -43,7 +43,7 @@
TEST_F(ResolverVariableValidationTest, VarInitializerNoReturnValueBuiltin) {
// fn f() { var a = storageBarrier(); }
auto* NoReturnValueBuiltin = Call(Source{{12, 34}}, "storageBarrier");
- WrapInFunction(Var("a", nullptr, ast::StorageClass::kNone, NoReturnValueBuiltin));
+ WrapInFunction(Var("a", NoReturnValueBuiltin));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: builtin 'storageBarrier' does not return a value");
@@ -52,7 +52,7 @@
TEST_F(ResolverVariableValidationTest, GlobalVarInitializerNoReturnValueBuiltin) {
// var a = storageBarrier();
auto* NoReturnValueBuiltin = Call(Source{{12, 34}}, "storageBarrier");
- GlobalVar("a", nullptr, ast::StorageClass::kNone, NoReturnValueBuiltin);
+ GlobalVar("a", NoReturnValueBuiltin);
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: builtin 'storageBarrier' does not return a value");
@@ -61,7 +61,7 @@
TEST_F(ResolverVariableValidationTest, GlobalVarUsedAtModuleScope) {
// var<private> a : i32;
// var<private> b : i32 = a;
- GlobalVar(Source{{12, 34}}, "a", ty.i32(), ast::StorageClass::kPrivate, nullptr);
+ GlobalVar(Source{{12, 34}}, "a", ty.i32(), ast::StorageClass::kPrivate);
GlobalVar("b", ty.i32(), ast::StorageClass::kPrivate, Expr(Source{{56, 78}}, "a"));
EXPECT_FALSE(r()->Resolve());
@@ -71,7 +71,7 @@
TEST_F(ResolverVariableValidationTest, OverrideNoInitializerNoType) {
// override a;
- Override(Source{{12, 34}}, "a", nullptr, nullptr);
+ Override(Source{{12, 34}}, "a");
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: override declaration requires a type or initializer");
@@ -84,9 +84,9 @@
// override bang : i32;
constexpr size_t kLimit = std::numeric_limits<decltype(OverrideId::value)>::max();
for (size_t i = 0; i <= kLimit; i++) {
- Override("o" + std::to_string(i), ty.i32(), nullptr);
+ Override("o" + std::to_string(i), ty.i32());
}
- Override(Source{{12, 34}}, "bang", ty.i32(), nullptr);
+ Override(Source{{12, 34}}, "bang", ty.i32());
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: number of 'override' variables exceeded limit of 65535");
@@ -98,14 +98,11 @@
// ...
// @id(N) override oN : i32;
constexpr size_t kLimit = std::numeric_limits<decltype(OverrideId::value)>::max();
- Override("reserved", ty.i32(), nullptr,
- utils::Vector{
- Id(kLimit),
- });
+ Override("reserved", ty.i32(), Id(kLimit));
for (size_t i = 0; i < kLimit; i++) {
- Override("o" + std::to_string(i), ty.i32(), nullptr);
+ Override("o" + std::to_string(i), ty.i32());
}
- Override(Source{{12, 34}}, "bang", ty.i32(), nullptr);
+ Override(Source{{12, 34}}, "bang", ty.i32());
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "12:34 error: number of 'override' variables exceeded limit of 65535");
@@ -114,7 +111,7 @@
TEST_F(ResolverVariableValidationTest, VarTypeNotConstructible) {
// var i : i32;
// var p : pointer<function, i32> = &v;
- auto* i = Var("i", ty.i32(), ast::StorageClass::kNone);
+ auto* i = Var("i", ty.i32());
auto* p = Var("a", ty.pointer<i32>(Source{{56, 78}}, ast::StorageClass::kFunction),
ast::StorageClass::kNone, AddressOf(Source{{12, 34}}, "i"));
WrapInFunction(i, p);
@@ -126,9 +123,9 @@
TEST_F(ResolverVariableValidationTest, LetTypeNotConstructible) {
// @group(0) @binding(0) var t1 : texture_2d<f32>;
// let t2 : t1;
- auto* t1 = GlobalVar("t1", ty.sampled_texture(ast::TextureDimension::k2d, ty.f32()),
- GroupAndBinding(0, 0));
- auto* t2 = Let(Source{{56, 78}}, "t2", nullptr, Expr(t1));
+ auto* t1 = GlobalVar("t1", ty.sampled_texture(ast::TextureDimension::k2d, ty.f32()), Group(0),
+ Binding(0));
+ auto* t2 = Let(Source{{56, 78}}, "t2", Expr(t1));
WrapInFunction(t2);
EXPECT_FALSE(r()->Resolve());
@@ -137,7 +134,7 @@
TEST_F(ResolverVariableValidationTest, OverrideExplicitTypeNotScalar) {
// override o : vec3<f32>;
- Override(Source{{56, 78}}, "o", ty.vec3<f32>(), nullptr);
+ Override(Source{{56, 78}}, "o", ty.vec3<f32>());
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "56:78 error: vec3<f32> cannot be used as the type of a 'override'");
@@ -145,7 +142,7 @@
TEST_F(ResolverVariableValidationTest, OverrideInferedTypeNotScalar) {
// override o = vec3(1.0f);
- Override(Source{{56, 78}}, "o", nullptr, vec3<f32>(1.0_f));
+ Override(Source{{56, 78}}, "o", vec3<f32>(1.0_f));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(), "56:78 error: vec3<f32> cannot be used as the type of a 'override'");
@@ -171,7 +168,7 @@
TEST_F(ResolverVariableValidationTest, VarConstructorWrongType) {
// var v : i32 = 2u
- WrapInFunction(Var(Source{{3, 3}}, "v", ty.i32(), ast::StorageClass::kNone, Expr(2_u)));
+ WrapInFunction(Var(Source{{3, 3}}, "v", ty.i32(), Expr(2_u)));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
@@ -198,7 +195,7 @@
TEST_F(ResolverVariableValidationTest, VarConstructorWrongTypeViaAlias) {
auto* a = Alias("I32", ty.i32());
- WrapInFunction(Var(Source{{3, 3}}, "v", ty.Of(a), ast::StorageClass::kNone, Expr(2_u)));
+ WrapInFunction(Var(Source{{3, 3}}, "v", ty.Of(a), Expr(2_u)));
EXPECT_FALSE(r()->Resolve());
EXPECT_EQ(r()->error(),
@@ -210,7 +207,7 @@
// let b : ptr<function,f32> = a;
const auto priv = ast::StorageClass::kFunction;
auto* var_a = Var("a", ty.f32(), priv);
- auto* var_b = Let(Source{{12, 34}}, "b", ty.pointer<f32>(priv), Expr("a"), {});
+ auto* var_b = Let(Source{{12, 34}}, "b", ty.pointer<f32>(priv), Expr("a"));
WrapInFunction(var_a, var_b);
ASSERT_FALSE(r()->Resolve());
@@ -241,7 +238,7 @@
GlobalVar("v", ty.f32(), ast::StorageClass::kPrivate, Expr(2.1_f));
- WrapInFunction(Var(Source{{12, 34}}, "v", ty.f32(), ast::StorageClass::kNone, Expr(2_f)));
+ WrapInFunction(Var(Source{{12, 34}}, "v", ty.f32(), Expr(2_f)));
EXPECT_TRUE(r()->Resolve()) << r()->error();
}
@@ -251,8 +248,8 @@
// var v : f32;
// { var v : f32; }
// }
- auto* var_outer = Var("v", ty.f32(), ast::StorageClass::kNone);
- auto* var_inner = Var(Source{{12, 34}}, "v", ty.f32(), ast::StorageClass::kNone);
+ auto* var_outer = Var("v", ty.f32());
+ auto* var_inner = Var(Source{{12, 34}}, "v", ty.f32());
auto* inner = Block(Decl(var_inner));
auto* outer_body = Block(Decl(var_outer), inner);
@@ -266,9 +263,9 @@
// var v : f32 = 3.14;
// if (true) { var v : f32 = 2.0; }
// }
- auto* var_a_float = Var("v", ty.f32(), ast::StorageClass::kNone, Expr(3.1_f));
+ auto* var_a_float = Var("v", ty.f32(), Expr(3.1_f));
- auto* var = Var(Source{{12, 34}}, "v", ty.f32(), ast::StorageClass::kNone, Expr(2_f));
+ auto* var = Var(Source{{12, 34}}, "v", ty.f32(), Expr(2_f));
auto* cond = Expr(true);
auto* body = Block(Decl(var));
@@ -297,11 +294,7 @@
auto* buf = Structure("S", utils::Vector{
Member("inner", ty.Of(inner)),
});
- auto* storage = GlobalVar("s", ty.Of(buf), ast::StorageClass::kStorage,
- utils::Vector{
- create<ast::BindingAttribute>(0u),
- create<ast::GroupAttribute>(0u),
- });
+ auto* storage = GlobalVar("s", ty.Of(buf), ast::StorageClass::kStorage, Binding(0), Group(0));
auto* expr = IndexAccessor(MemberAccessor(MemberAccessor(storage, "inner"), "arr"), 2_i);
auto* ptr =
@@ -355,8 +348,8 @@
// fn foo() {
// var v = s;
// }
- GlobalVar("s", ty.sampler(ast::SamplerKind::kSampler), GroupAndBinding(0, 0));
- auto* v = Var(Source{{12, 34}}, "v", nullptr, Expr("s"));
+ GlobalVar("s", ty.sampler(ast::SamplerKind::kSampler), Group(0), Binding(0));
+ auto* v = Var(Source{{12, 34}}, "v", Expr("s"));
WrapInFunction(v);
EXPECT_FALSE(r()->Resolve());
@@ -424,8 +417,8 @@
}
TEST_F(ResolverVariableValidationTest, ConstInitWithVar) {
- auto* v = Var("v", nullptr, Expr(1_i));
- auto* c = Const("c", nullptr, Expr(Source{{12, 34}}, v));
+ auto* v = Var("v", Expr(1_i));
+ auto* c = Const("c", Expr(Source{{12, 34}}, v));
WrapInFunction(v, c);
EXPECT_FALSE(r()->Resolve());
@@ -433,8 +426,8 @@
}
TEST_F(ResolverVariableValidationTest, ConstInitWithOverride) {
- auto* o = Override("v", nullptr, Expr(1_i));
- auto* c = Const("c", nullptr, Expr(Source{{12, 34}}, o));
+ auto* o = Override("v", Expr(1_i));
+ auto* c = Const("c", Expr(Source{{12, 34}}, o));
WrapInFunction(c);
EXPECT_FALSE(r()->Resolve());
@@ -442,8 +435,8 @@
}
TEST_F(ResolverVariableValidationTest, ConstInitWithLet) {
- auto* l = Let("v", nullptr, Expr(1_i));
- auto* c = Const("c", nullptr, Expr(Source{{12, 34}}, l));
+ auto* l = Let("v", Expr(1_i));
+ auto* c = Const("c", Expr(Source{{12, 34}}, l));
WrapInFunction(l, c);
EXPECT_FALSE(r()->Resolve());
