tint/resolver: Validate @size only on creation-fixed footprint member types.

Fixed: tint:1623
Change-Id: Id553dfe57effb4084a16fd6e6b02ad0cef85f836
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/106224
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>

src/tint/resolver/attribute_validation_test.cc

@@ -809,11 +809,25 @@ TEST_F(StructMemberAttributeTest, Size_Attribute_Var) {
 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")})});
+    Structure("mystruct",
+              utils::Vector{
+                  Member("a", ty.f32(), utils::Vector{MemberSize(Expr(Source{{12, 34}}, "val"))}),
+              });
     EXPECT_FALSE(r()->Resolve());
     EXPECT_EQ(
         r()->error(),
-        R"(error: @size requires a const-expression, but expression is an override-expression)");
+        R"(12:34 error: @size requires a const-expression, but expression is an override-expression)");
+}
+
+TEST_F(StructMemberAttributeTest, Size_On_RuntimeSizedArray) {
+    Structure("mystruct",
+              utils::Vector{
+                  Member("a", ty.array<i32>(), utils::Vector{MemberSize(Source{{12, 34}}, 8_a)}),
+              });
+    EXPECT_FALSE(r()->Resolve());
+    EXPECT_EQ(
+        r()->error(),
+        R"(12:34 error: @size can only be applied to members where the member's type size can be fully determined at shader creation time)");
 }
 
 }  // namespace StructAndStructMemberTests

src/tint/resolver/validator.cc

@@ -2186,46 +2186,69 @@ bool Validator::Structure(const sem::Struct* str, ast::PipelineStage stage) cons
         const ast::InvariantAttribute* invariant_attribute = nullptr;
         const ast::InterpolateAttribute* interpolate_attribute = nullptr;
         for (auto* attr : member->Declaration()->attributes) {
-            if (!attr->IsAnyOf<ast::BuiltinAttribute,             //
+            bool ok = Switch(
-                               ast::InternalAttribute,            //
+                attr,  //
-                               ast::InterpolateAttribute,         //
+                [&](const ast::InvariantAttribute* invariant) {
-                               ast::InvariantAttribute,           //
+                    invariant_attribute = invariant;
-                               ast::LocationAttribute,            //
+                    return true;
-                               ast::StructMemberOffsetAttribute,  //
+                },
-                               ast::StructMemberSizeAttribute,    //
+                [&](const ast::LocationAttribute* location) {
-                               ast::StructMemberAlignAttribute>()) {
+                    has_location = true;
-                if (attr->Is<ast::StrideAttribute>() &&
+                    TINT_ASSERT(Resolver, member->Location().has_value());
-                    IsValidationDisabled(member->Declaration()->attributes,
+                    if (!LocationAttribute(location, member->Location().value(), member->Type(),
-                                         ast::DisabledValidation::kIgnoreStrideAttribute)) {
+                                           locations, stage, member->Declaration()->source)) {
-                    continue;
+                        return false;
-                }
+                    }
-                AddError("attribute is not valid for structure members", attr->source);
+                    return true;
+                },
+                [&](const ast::BuiltinAttribute* builtin) {
+                    if (!BuiltinAttribute(builtin, member->Type(), stage,
+                                          /* is_input */ false)) {
+                        return false;
+                    }
+                    if (builtin->builtin == ast::BuiltinValue::kPosition) {
+                        has_position = true;
+                    }
+                    return true;
+                },
+                [&](const ast::InterpolateAttribute* interpolate) {
+                    interpolate_attribute = interpolate;
+                    if (!InterpolateAttribute(interpolate, member->Type())) {
+                        return false;
+                    }
+                    return true;
+                },
+                [&](const ast::StructMemberSizeAttribute*) {
+                    if (!member->Type()->HasCreationFixedFootprint()) {
+                        AddError(
+                            "@size can only be applied to members where the member's type size "
+                            "can be fully determined at shader creation time",
+                            attr->source);
+                        return false;
+                    }
+                    return true;
+                },
+                [&](Default) {
+                    if (!attr->IsAnyOf<ast::BuiltinAttribute,             //
+                                       ast::InternalAttribute,            //
+                                       ast::InterpolateAttribute,         //
+                                       ast::InvariantAttribute,           //
+                                       ast::LocationAttribute,            //
+                                       ast::StructMemberOffsetAttribute,  //
+                                       ast::StructMemberAlignAttribute>()) {
+                        if (attr->Is<ast::StrideAttribute>() &&
+                            IsValidationDisabled(member->Declaration()->attributes,
+                                                 ast::DisabledValidation::kIgnoreStrideAttribute)) {
+                            return true;
+                        }
+                        AddError("attribute is not valid for structure members", attr->source);
+                        return false;
+                    }
+                    return true;
+                });
+            if (!ok) {
                 return false;
             }
-
-            if (auto* invariant = attr->As<ast::InvariantAttribute>()) {
-                invariant_attribute = invariant;
-            } else if (auto* location = attr->As<ast::LocationAttribute>()) {
-                has_location = true;
-                TINT_ASSERT(Resolver, member->Location().has_value());
-                if (!LocationAttribute(location, member->Location().value(), member->Type(),
-                                       locations, stage, member->Declaration()->source)) {
-                    return false;
-                }
-            } else if (auto* builtin = attr->As<ast::BuiltinAttribute>()) {
-                if (!BuiltinAttribute(builtin, member->Type(), stage,
-                                      /* is_input */ false)) {
-                    return false;
-                }
-                if (builtin->builtin == ast::BuiltinValue::kPosition) {
-                    has_position = true;
-                }
-            } else if (auto* interpolate = attr->As<ast::InterpolateAttribute>()) {
-                interpolate_attribute = interpolate;
-                if (!InterpolateAttribute(interpolate, member->Type())) {
-                    return false;
-                }
-            }
         }
 
         if (invariant_attribute && !has_position) {

src/tint/sem/abstract_numeric.cc

@@ -18,7 +18,12 @@ TINT_INSTANTIATE_TYPEINFO(tint::sem::AbstractNumeric);
 
 namespace tint::sem {
 
-AbstractNumeric::AbstractNumeric() : Base(TypeFlags{Flag::kConstructable}) {}
+AbstractNumeric::AbstractNumeric()
+    : Base(TypeFlags{
+          Flag::kConstructable,
+          Flag::kCreationFixedFootprint,
+          Flag::kFixedFootprint,
+      }) {}
 AbstractNumeric::AbstractNumeric(AbstractNumeric&&) = default;
 AbstractNumeric::~AbstractNumeric() = default;
 

src/tint/sem/array.cc

@@ -35,6 +35,15 @@ TypeFlags FlagsFrom(const Type* element, ArrayCount count) {
         if (element->IsConstructible()) {
             flags.Add(TypeFlag::kConstructable);
         }
+        if (element->HasCreationFixedFootprint()) {
+            flags.Add(TypeFlag::kCreationFixedFootprint);
+        }
+    }
+    if (std::holds_alternative<ConstantArrayCount>(count) ||
+        std::holds_alternative<OverrideArrayCount>(count)) {
+        if (element->HasFixedFootprint()) {
+            flags.Add(TypeFlag::kFixedFootprint);
+        }
     }
     return flags;
 }

src/tint/sem/array_test.cc

@@ -135,5 +135,25 @@ TEST_F(ArrayTest, IsConstructable) {
     EXPECT_FALSE(runtime_sized->IsConstructible());
 }
 
+TEST_F(ArrayTest, HasCreationFixedFootprint) {
+    auto* fixed_sized = create<Array>(create<U32>(), ConstantArrayCount{2u}, 4u, 8u, 32u, 16u);
+    auto* override_sized = create<Array>(create<U32>(), OverrideArrayCount{}, 4u, 8u, 32u, 16u);
+    auto* runtime_sized = create<Array>(create<U32>(), RuntimeArrayCount{}, 4u, 8u, 32u, 16u);
+
+    EXPECT_TRUE(fixed_sized->HasCreationFixedFootprint());
+    EXPECT_FALSE(override_sized->HasCreationFixedFootprint());
+    EXPECT_FALSE(runtime_sized->HasCreationFixedFootprint());
+}
+
+TEST_F(ArrayTest, HasFixedFootprint) {
+    auto* fixed_sized = create<Array>(create<U32>(), ConstantArrayCount{2u}, 4u, 8u, 32u, 16u);
+    auto* override_sized = create<Array>(create<U32>(), OverrideArrayCount{}, 4u, 8u, 32u, 16u);
+    auto* runtime_sized = create<Array>(create<U32>(), RuntimeArrayCount{}, 4u, 8u, 32u, 16u);
+
+    EXPECT_TRUE(fixed_sized->HasFixedFootprint());
+    EXPECT_TRUE(override_sized->HasFixedFootprint());
+    EXPECT_FALSE(runtime_sized->HasFixedFootprint());
+}
+
 }  // namespace
 }  // namespace tint::sem

src/tint/sem/atomic.cc

@@ -22,7 +22,12 @@ TINT_INSTANTIATE_TYPEINFO(tint::sem::Atomic);
 
 namespace tint::sem {
 
-Atomic::Atomic(const sem::Type* subtype) : Base(TypeFlags{}), subtype_(subtype) {
+Atomic::Atomic(const sem::Type* subtype)
+    : Base(TypeFlags{
+          Flag::kCreationFixedFootprint,
+          Flag::kFixedFootprint,
+      }),
+      subtype_(subtype) {
     TINT_ASSERT(AST, !subtype->Is<Reference>());
 }
 

src/tint/sem/bool.cc

@@ -20,7 +20,12 @@ TINT_INSTANTIATE_TYPEINFO(tint::sem::Bool);
 
 namespace tint::sem {
 
-Bool::Bool() : Base(TypeFlags{Flag::kConstructable}) {}
+Bool::Bool()
+    : Base(TypeFlags{
+          Flag::kConstructable,
+          Flag::kCreationFixedFootprint,
+          Flag::kFixedFootprint,
+      }) {}
 
 Bool::Bool(Bool&&) = default;
 

src/tint/sem/f16.cc

@@ -21,7 +21,12 @@ TINT_INSTANTIATE_TYPEINFO(tint::sem::F16);
 namespace tint {
 namespace sem {
 
-F16::F16() : Base(TypeFlags{Flag::kConstructable}) {}
+F16::F16()
+    : Base(TypeFlags{
+          Flag::kConstructable,
+          Flag::kCreationFixedFootprint,
+          Flag::kFixedFootprint,
+      }) {}
 
 F16::F16(F16&&) = default;
 

src/tint/sem/f32.cc

@@ -20,7 +20,12 @@ TINT_INSTANTIATE_TYPEINFO(tint::sem::F32);
 
 namespace tint::sem {
 
-F32::F32() : Base(TypeFlags{Flag::kConstructable}) {}
+F32::F32()
+    : Base(TypeFlags{
+          Flag::kConstructable,
+          Flag::kCreationFixedFootprint,
+          Flag::kFixedFootprint,
+      }) {}
 
 F32::F32(F32&&) = default;
 

src/tint/sem/i32.cc

@@ -20,7 +20,12 @@ TINT_INSTANTIATE_TYPEINFO(tint::sem::I32);
 
 namespace tint::sem {
 
-I32::I32() : Base(TypeFlags{Flag::kConstructable}) {}
+I32::I32()
+    : Base(TypeFlags{
+          Flag::kConstructable,
+          Flag::kCreationFixedFootprint,
+          Flag::kFixedFootprint,
+      }) {}
 
 I32::I32(I32&&) = default;
 

src/tint/sem/matrix.cc

@@ -23,7 +23,11 @@ TINT_INSTANTIATE_TYPEINFO(tint::sem::Matrix);
 namespace tint::sem {
 
 Matrix::Matrix(const Vector* column_type, uint32_t columns)
-    : Base(TypeFlags{Flag::kConstructable}),
+    : Base(TypeFlags{
+          Flag::kConstructable,
+          Flag::kCreationFixedFootprint,
+          Flag::kFixedFootprint,
+      }),
       subtype_(column_type->type()),
       column_type_(column_type),
       rows_(column_type->Width()),

src/tint/sem/struct.cc

@@ -30,11 +30,20 @@ namespace tint::sem {
 namespace {
 
 TypeFlags FlagsFrom(const StructMemberList& members) {
-    TypeFlags flags{TypeFlag::kConstructable};
+    TypeFlags flags{
+        TypeFlag::kConstructable,
+        TypeFlag::kCreationFixedFootprint,
+        TypeFlag::kFixedFootprint,
+    };
     for (auto* member : members) {
         if (!member->Type()->IsConstructible()) {
             flags.Remove(TypeFlag::kConstructable);
-            break;
+        }
+        if (!member->Type()->HasFixedFootprint()) {
+            flags.Remove(TypeFlag::kFixedFootprint);
+        }
+        if (!member->Type()->HasCreationFixedFootprint()) {
+            flags.Remove(TypeFlag::kCreationFixedFootprint);
         }
     }
     return flags;

src/tint/sem/struct_test.cc

@@ -157,5 +157,71 @@ TEST_F(StructTest, IsConstructable) {
     EXPECT_FALSE(sem_outer_runtime_sized_array->IsConstructible());
 }
 
+TEST_F(StructTest, HasCreationFixedFootprint) {
+    auto* inner =  //
+        Structure("Inner", utils::Vector{
+                               Member("a", ty.i32()),
+                               Member("b", ty.u32()),
+                               Member("c", ty.f32()),
+                               Member("d", ty.vec3<f32>()),
+                               Member("e", ty.mat4x2<f32>()),
+                               Member("f", ty.array<f32, 32>()),
+                           });
+
+    auto* outer = Structure("Outer", utils::Vector{
+                                         Member("inner", ty.type_name("Inner")),
+                                     });
+
+    auto* outer_with_runtime_sized_array =
+        Structure("OuterRuntimeSizedArray", utils::Vector{
+                                                Member("inner", ty.type_name("Inner")),
+                                                Member("runtime_sized_array", ty.array<i32>()),
+                                            });
+
+    auto p = Build();
+    ASSERT_TRUE(p.IsValid()) << p.Diagnostics().str();
+
+    auto* sem_inner = p.Sem().Get(inner);
+    auto* sem_outer = p.Sem().Get(outer);
+    auto* sem_outer_with_runtime_sized_array = p.Sem().Get(outer_with_runtime_sized_array);
+
+    EXPECT_TRUE(sem_inner->HasCreationFixedFootprint());
+    EXPECT_TRUE(sem_outer->HasCreationFixedFootprint());
+    EXPECT_FALSE(sem_outer_with_runtime_sized_array->HasCreationFixedFootprint());
+}
+
+TEST_F(StructTest, HasFixedFootprint) {
+    auto* inner =  //
+        Structure("Inner", utils::Vector{
+                               Member("a", ty.i32()),
+                               Member("b", ty.u32()),
+                               Member("c", ty.f32()),
+                               Member("d", ty.vec3<f32>()),
+                               Member("e", ty.mat4x2<f32>()),
+                               Member("f", ty.array<f32, 32>()),
+                           });
+
+    auto* outer = Structure("Outer", utils::Vector{
+                                         Member("inner", ty.type_name("Inner")),
+                                     });
+
+    auto* outer_with_runtime_sized_array =
+        Structure("OuterRuntimeSizedArray", utils::Vector{
+                                                Member("inner", ty.type_name("Inner")),
+                                                Member("runtime_sized_array", ty.array<i32>()),
+                                            });
+
+    auto p = Build();
+    ASSERT_TRUE(p.IsValid()) << p.Diagnostics().str();
+
+    auto* sem_inner = p.Sem().Get(inner);
+    auto* sem_outer = p.Sem().Get(outer);
+    auto* sem_outer_with_runtime_sized_array = p.Sem().Get(outer_with_runtime_sized_array);
+
+    EXPECT_TRUE(sem_inner->HasFixedFootprint());
+    EXPECT_TRUE(sem_outer->HasFixedFootprint());
+    EXPECT_FALSE(sem_outer_with_runtime_sized_array->HasFixedFootprint());
+}
+
 }  // namespace
 }  // namespace tint::sem

src/tint/sem/type.cc

@@ -34,7 +34,11 @@ TINT_INSTANTIATE_TYPEINFO(tint::sem::Type);
 
 namespace tint::sem {
 
-Type::Type(TypeFlags flags) : flags_(flags) {}
+Type::Type(TypeFlags flags) : flags_(flags) {
+    if (IsConstructible()) {
+        TINT_ASSERT(Semantic, HasCreationFixedFootprint());
+    }
+}
 
 Type::Type(Type&&) = default;
 

src/tint/sem/type.h

@@ -34,6 +34,12 @@ enum TypeFlag {
     /// Type is constructable.
     /// @see https://gpuweb.github.io/gpuweb/wgsl/#constructible-types
     kConstructable,
+    /// Type has a creation-fixed footprint.
+    /// @see https://www.w3.org/TR/WGSL/#fixed-footprint-types
+    kCreationFixedFootprint,
+    /// Type has a fixed footprint.
+    /// @see https://www.w3.org/TR/WGSL/#fixed-footprint-types
+    kFixedFootprint,
 };
 
 /// An alias to utils::EnumSet<TypeFlag>
@@ -83,6 +89,16 @@ class Type : public Castable<Type, Node> {
     /// https://gpuweb.github.io/gpuweb/wgsl/#constructible-types
     inline bool IsConstructible() const { return flags_.Contains(Flag::kConstructable); }
 
+    /// @returns true has a creation-fixed footprint.
+    /// @see https://www.w3.org/TR/WGSL/#fixed-footprint-types
+    inline bool HasCreationFixedFootprint() const {
+        return flags_.Contains(Flag::kCreationFixedFootprint);
+    }
+
+    /// @returns true has a fixed footprint.
+    /// @see https://www.w3.org/TR/WGSL/#fixed-footprint-types
+    inline bool HasFixedFootprint() const { return flags_.Contains(Flag::kFixedFootprint); }
+
     /// @returns true if this type is a scalar
     bool is_scalar() const;
     /// @returns true if this type is a numeric scalar

src/tint/sem/u32.cc

@@ -20,7 +20,12 @@ TINT_INSTANTIATE_TYPEINFO(tint::sem::U32);
 
 namespace tint::sem {
 
-U32::U32() : Base(TypeFlags{Flag::kConstructable}) {}
+U32::U32()
+    : Base(TypeFlags{
+          Flag::kConstructable,
+          Flag::kCreationFixedFootprint,
+          Flag::kFixedFootprint,
+      }) {}
 
 U32::~U32() = default;
 

src/tint/sem/vector.cc

@@ -22,7 +22,13 @@ TINT_INSTANTIATE_TYPEINFO(tint::sem::Vector);
 namespace tint::sem {
 
 Vector::Vector(Type const* subtype, uint32_t width)
-    : Base(TypeFlags{Flag::kConstructable}), subtype_(subtype), width_(width) {
+    : Base(TypeFlags{
+          Flag::kConstructable,
+          Flag::kCreationFixedFootprint,
+          Flag::kFixedFootprint,
+      }),
+      subtype_(subtype),
+      width_(width) {
     TINT_ASSERT(Semantic, width_ > 1);
     TINT_ASSERT(Semantic, width_ < 5);
 }