Convert `size` attribute to expressions.

This CL updates the size attribute to parse expressions.

Bug: tint:1633
Change-Id: Ia12650848e7041faa53013d195f4313b8d3e9969
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/103320
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Auto-Submit: Dan Sinclair <dsinclair@chromium.org>

src/tint/reader/wgsl/parser_impl.cc

@@ -3551,13 +3551,16 @@ Maybe<const ast::Attribute*> ParserImpl::attribute() {
     if (t == "size") {
         const char* use = "size attribute";
         return expect_paren_block(use, [&]() -> Result {
-            auto val = expect_positive_sint(use);
+            auto expr = expression();
-            if (val.errored) {
+            if (expr.errored) {
                 return Failure::kErrored;
             }
+            if (!expr.matched) {
+                return add_error(peek(), "expected size expression");
+            }
             match(Token::Type::kComma);
 
-            return builder_.MemberSize(t.source(), AInt(val.value));
+            return builder_.MemberSize(t.source(), expr.value);
         });
     }
 

src/tint/reader/wgsl/parser_impl_error_msg_test.cc

@@ -862,17 +862,9 @@ struct S { @align(fn) i : i32, };
 }
 
 TEST_F(ParserImplErrorTest, GlobalDeclStructMemberSizeInvaldValue) {
-    EXPECT("struct S { @size(x) i : i32, };",
+    EXPECT("struct S { @size(if) i : i32, };",
-           R"(test.wgsl:1:18 error: expected signed integer literal for size attribute
+           R"(test.wgsl:1:18 error: expected size expression
-struct S { @size(x) i : i32, };
+struct S { @size(if) i : i32, };
-                 ^
-)");
-}
-
-TEST_F(ParserImplErrorTest, GlobalDeclStructMemberSizeNegativeValue) {
-    EXPECT("struct S { @size(-2) i : i32, };",
-           R"(test.wgsl:1:18 error: size attribute must be positive
-struct S { @size(-2) i : i32, };
                  ^^
 )");
 }

src/tint/reader/wgsl/parser_impl_struct_body_decl_test.cc

@@ -71,12 +71,12 @@ TEST_F(ParserImplTest, StructBodyDecl_InvalidAlign) {
 TEST_F(ParserImplTest, StructBodyDecl_InvalidSize) {
     auto p = parser(R"(
 {
-  @size(nan) a : i32,
+  @size(if) a : i32,
 })");
     auto m = p->expect_struct_body_decl();
     ASSERT_TRUE(p->has_error());
     ASSERT_TRUE(m.errored);
-    EXPECT_EQ(p->error(), "3:9: expected signed integer literal for size attribute");
+    EXPECT_EQ(p->error(), "3:9: expected size expression");
 }
 
 TEST_F(ParserImplTest, StructBodyDecl_MissingClosingBracket) {

src/tint/reader/wgsl/parser_impl_struct_member_attribute_decl_test.cc

@@ -39,12 +39,12 @@ TEST_F(ParserImplTest, AttributeDecl_Single) {
 }
 
 TEST_F(ParserImplTest, AttributeDecl_InvalidAttribute) {
-    auto p = parser("@size(nan)");
+    auto p = parser("@size(if)");
     auto attrs = p->attribute_list();
     EXPECT_TRUE(p->has_error()) << p->error();
     EXPECT_TRUE(attrs.errored);
     EXPECT_FALSE(attrs.matched);
-    EXPECT_EQ(p->error(), "1:7: expected signed integer literal for size attribute");
+    EXPECT_EQ(p->error(), "1:7: expected size expression");
 }
 
 }  // namespace

src/tint/reader/wgsl/parser_impl_struct_member_attribute_test.cc

@@ -23,7 +23,7 @@ TEST_F(ParserImplTest, Attribute_Size) {
     EXPECT_TRUE(attr.matched);
     EXPECT_FALSE(attr.errored);
     ASSERT_NE(attr.value, nullptr);
-    ASSERT_FALSE(p->has_error());
+    ASSERT_FALSE(p->has_error()) << p->error();
 
     auto* member_attr = attr.value->As<ast::Attribute>();
     ASSERT_NE(member_attr, nullptr);
@@ -34,13 +34,39 @@ TEST_F(ParserImplTest, Attribute_Size) {
     EXPECT_EQ(o->expr->As<ast::IntLiteralExpression>()->value, 4u);
 }
 
+TEST_F(ParserImplTest, Attribute_Size_Expression) {
+    auto p = parser("size(4 + 5)");
+    auto attr = p->attribute();
+    EXPECT_TRUE(attr.matched);
+    EXPECT_FALSE(attr.errored);
+    ASSERT_NE(attr.value, nullptr);
+    ASSERT_FALSE(p->has_error()) << p->error();
+
+    auto* member_attr = attr.value->As<ast::Attribute>();
+    ASSERT_NE(member_attr, nullptr);
+    ASSERT_TRUE(member_attr->Is<ast::StructMemberSizeAttribute>());
+
+    auto* o = member_attr->As<ast::StructMemberSizeAttribute>();
+    ASSERT_TRUE(o->expr->Is<ast::BinaryExpression>());
+    auto* expr = o->expr->As<ast::BinaryExpression>();
+
+    EXPECT_EQ(ast::BinaryOp::kAdd, expr->op);
+    auto* v = expr->lhs->As<ast::IntLiteralExpression>();
+    ASSERT_NE(nullptr, v);
+    EXPECT_EQ(v->value, 4u);
+
+    v = expr->rhs->As<ast::IntLiteralExpression>();
+    ASSERT_NE(nullptr, v);
+    EXPECT_EQ(v->value, 5u);
+}
+
 TEST_F(ParserImplTest, Attribute_Size_TrailingComma) {
     auto p = parser("size(4,)");
     auto attr = p->attribute();
     EXPECT_TRUE(attr.matched);
     EXPECT_FALSE(attr.errored);
     ASSERT_NE(attr.value, nullptr);
-    ASSERT_FALSE(p->has_error());
+    ASSERT_FALSE(p->has_error()) << p->error();
 
     auto* member_attr = attr.value->As<ast::Attribute>();
     ASSERT_NE(member_attr, nullptr);
@@ -78,17 +104,17 @@ TEST_F(ParserImplTest, Attribute_Size_MissingValue) {
     EXPECT_TRUE(attr.errored);
     EXPECT_EQ(attr.value, nullptr);
     EXPECT_TRUE(p->has_error());
-    EXPECT_EQ(p->error(), "1:6: expected signed integer literal for size attribute");
+    EXPECT_EQ(p->error(), "1:6: expected size expression");
 }
 
 TEST_F(ParserImplTest, Attribute_Size_MissingInvalid) {
-    auto p = parser("size(nan)");
+    auto p = parser("size(if)");
     auto attr = p->attribute();
     EXPECT_FALSE(attr.matched);
     EXPECT_TRUE(attr.errored);
     EXPECT_EQ(attr.value, nullptr);
     EXPECT_TRUE(p->has_error());
-    EXPECT_EQ(p->error(), "1:6: expected signed integer literal for size attribute");
+    EXPECT_EQ(p->error(), "1:6: expected size expression");
 }
 
 TEST_F(ParserImplTest, Attribute_Align) {

src/tint/reader/wgsl/parser_impl_struct_member_test.cc

@@ -115,14 +115,14 @@ TEST_F(ParserImplTest, StructMember_ParsesWithMultipleattributes) {
 }
 
 TEST_F(ParserImplTest, StructMember_InvalidAttribute) {
-    auto p = parser("@size(nan) a : i32,");
+    auto p = parser("@size(if) a : i32,");
 
     auto m = p->expect_struct_member();
     ASSERT_TRUE(m.errored);
     ASSERT_EQ(m.value, nullptr);
 
     ASSERT_TRUE(p->has_error());
-    EXPECT_EQ(p->error(), "1:7: expected signed integer literal for size attribute");
+    EXPECT_EQ(p->error(), "1:7: expected size expression");
 }
 
 }  // namespace

src/tint/resolver/attribute_validation_test.cc

@@ -743,6 +743,78 @@ TEST_F(StructMemberAttributeTest, Align_Attribute_Override) {
         R"(error: @align requires a const-expression, but expression is an override-expression)");
 }
 
+TEST_F(StructMemberAttributeTest, Size_Attribute_Const) {
+    GlobalConst("val", ty.i32(), Expr(4_i));
+
+    Structure("mystruct", utils::Vector{Member("a", ty.f32(), utils::Vector{MemberSize("val")})});
+    EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
+TEST_F(StructMemberAttributeTest, Size_Attribute_ConstNegative) {
+    GlobalConst("val", ty.i32(), Expr(-2_i));
+
+    Structure("mystruct", utils::Vector{Member(
+                              "a", ty.f32(), utils::Vector{MemberSize(Source{{12, 34}}, "val")})});
+    EXPECT_FALSE(r()->Resolve());
+    EXPECT_EQ(r()->error(), R"(12:34 error: 'size' attribute must be positive)");
+}
+
+TEST_F(StructMemberAttributeTest, Size_Attribute_ConstF32) {
+    GlobalConst("val", ty.f32(), Expr(1.23_f));
+
+    Structure("mystruct", utils::Vector{Member(
+                              "a", ty.f32(), utils::Vector{MemberSize(Source{{12, 34}}, "val")})});
+    EXPECT_FALSE(r()->Resolve());
+    EXPECT_EQ(r()->error(), R"(12:34 error: 'size' must be an i32 or u32 value)");
+}
+
+TEST_F(StructMemberAttributeTest, Size_Attribute_ConstU32) {
+    GlobalConst("val", ty.u32(), Expr(4_u));
+
+    Structure("mystruct", utils::Vector{Member(
+                              "a", ty.f32(), utils::Vector{MemberSize(Source{{12, 34}}, "val")})});
+    EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
+TEST_F(StructMemberAttributeTest, Size_Attribute_ConstAInt) {
+    GlobalConst("val", Expr(4_a));
+
+    Structure("mystruct", utils::Vector{Member(
+                              "a", ty.f32(), utils::Vector{MemberSize(Source{{12, 34}}, "val")})});
+    EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
+TEST_F(StructMemberAttributeTest, Size_Attribute_ConstAFloat) {
+    GlobalConst("val", Expr(2.0_a));
+
+    Structure("mystruct", utils::Vector{Member(
+                              "a", ty.f32(), utils::Vector{MemberSize(Source{{12, 34}}, "val")})});
+    EXPECT_FALSE(r()->Resolve());
+    EXPECT_EQ(r()->error(), R"(12:34 error: 'size' must be an i32 or u32 value)");
+}
+
+TEST_F(StructMemberAttributeTest, Size_Attribute_Var) {
+    GlobalVar(Source{{1, 2}}, "val", ty.f32(), ast::AddressSpace::kPrivate, ast::Access::kUndefined,
+              Expr(1.23_f));
+
+    Structure(Source{{6, 4}}, "mystruct",
+              utils::Vector{Member(Source{{12, 5}}, "a", ty.f32(),
+                                   utils::Vector{MemberSize(Expr(Source{{12, 35}}, "val"))})});
+    EXPECT_FALSE(r()->Resolve());
+    EXPECT_EQ(r()->error(), R"(12:35 error: var 'val' cannot be referenced at module-scope
+1:2 note: var 'val' declared here)");
+}
+
+TEST_F(StructMemberAttributeTest, Size_Attribute_Override) {
+    Override("val", ty.f32(), Expr(1.23_f));
+
+    Structure("mystruct", utils::Vector{Member("a", ty.f32(), utils::Vector{MemberSize("val")})});
+    EXPECT_FALSE(r()->Resolve());
+    EXPECT_EQ(
+        r()->error(),
+        R"(error: @size requires a const-expression, but expression is an override-expression)");
+}
+
 }  // namespace StructAndStructMemberTests
 
 using ArrayAttributeTest = TestWithParams;

src/tint/resolver/dependency_graph.cc

@@ -426,12 +426,15 @@ class DependencyScanner {
             TraverseExpression(align->expr);
             return;
         }
+        if (auto* size = attr->As<ast::StructMemberSizeAttribute>()) {
+            TraverseExpression(size->expr);
+            return;
+        }
 
         if (attr->IsAnyOf<ast::BindingAttribute, ast::BuiltinAttribute, ast::GroupAttribute,
                           ast::IdAttribute, ast::InternalAttribute, ast::InterpolateAttribute,
                           ast::InvariantAttribute, ast::LocationAttribute, ast::StageAttribute,
-                          ast::StrideAttribute, ast::StructMemberOffsetAttribute,
+                          ast::StrideAttribute, ast::StructMemberOffsetAttribute>()) {
-                          ast::StructMemberSizeAttribute>()) {
             return;
         }
 

src/tint/resolver/dependency_graph_test.cc

@@ -1230,9 +1230,11 @@ TEST_F(ResolverDependencyGraphTraversalTest, SymbolsReached) {
 
     Alias(Sym(), T);
     Structure(Sym(),  //
-              utils::Vector{
+              utils::Vector{Member(Sym(), T,
-                  Member(Sym(), T, utils::Vector{MemberAlign(V)})  //
+                                   utils::Vector{
-              });
+                                       //
+                                       MemberAlign(V), MemberSize(V)  //
+                                   })});
     GlobalVar(Sym(), T, V);
     GlobalConst(Sym(), T, V);
     Func(Sym(),                           //

src/tint/resolver/resolver.cc

@@ -2885,15 +2885,26 @@ sem::Struct* Resolver::Structure(const ast::Struct* str) {
                 if (!materialized) {
                     return nullptr;
                 }
+                if (!materialized->Type()->IsAnyOf<sem::U32, sem::I32>()) {
+                    AddError("'size' must be an i32 or u32 value", s->source);
+                    return nullptr;
+                }
+
                 auto const_value = materialized->ConstantValue();
                 if (!const_value) {
                     AddError("'size' must be constant expression", s->expr->source);
                     return nullptr;
                 }
+                {
+                    auto value = const_value->As<AInt>();
+                    if (value <= 0) {
+                        AddError("'size' attribute must be positive", s->source);
+                        return nullptr;
+                    }
+                }
                 auto value = const_value->As<uint64_t>();
-
                 if (value < size) {
-                    AddError("size must be at least as big as the type's size (" +
+                    AddError("'size' must be at least as big as the type's size (" +
                                  std::to_string(size) + ")",
                              s->source);
                     return nullptr;

src/tint/resolver/validation_test.cc

@@ -1266,11 +1266,11 @@ TEST_F(ResolverValidationTest, ZeroStructMemberAlignAttribute) {
 
 TEST_F(ResolverValidationTest, ZeroStructMemberSizeAttribute) {
     Structure("S", utils::Vector{
-                       Member("a", ty.f32(), utils::Vector{MemberSize(Source{{12, 34}}, 0_a)}),
+                       Member("a", ty.f32(), utils::Vector{MemberSize(Source{{12, 34}}, 1_a)}),
                    });
 
     EXPECT_FALSE(r()->Resolve());
-    EXPECT_EQ(r()->error(), "12:34 error: size must be at least as big as the type's size (4)");
+    EXPECT_EQ(r()->error(), "12:34 error: 'size' must be at least as big as the type's size (4)");
 }
 
 TEST_F(ResolverValidationTest, OffsetAndSizeAttribute) {