Detect missing position with invariant in nested struct.
Currently the IR validator will check for a missing `position` when
applying `invariant` inside a struct but misses of the `invariant` is
inside a nested struct.
Fixed: 441821182
Change-Id: I2db1a912c17f5e397306862607767cb6d8324cc3
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/261276
Auto-Submit: dan sinclair <dsinclair@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Commit-Queue: James Price <jrprice@google.com>
diff --git a/src/tint/lang/core/ir/validator.cc b/src/tint/lang/core/ir/validator.cc
index 47ee27d..eaa4759 100644
--- a/src/tint/lang/core/ir/validator.cc
+++ b/src/tint/lang/core/ir/validator.cc
@@ -208,6 +208,11 @@
if (auto* ty_struct = ty->As<core::type::Struct>()) {
for (const auto* mem : ty_struct->Members()) {
is_struct_impl(msg_anchor, mem->Attributes());
+
+ if (mem->Type()->Is<core::type::Struct>()) {
+ CheckIOAttributes(msg_anchor, ty_attr, mem->Type(), is_not_struct_impl,
+ is_struct_impl);
+ }
}
} else {
is_not_struct_impl(msg_anchor, ty_attr);
@@ -1093,7 +1098,7 @@
};
}
- /// @returns a function that validates that type is bool iff decorated with
+ /// @returns a function that validates that type is bool only if it is decorated with
/// @builtin(front_facing)
/// @param err error message to log when check fails
template <typename MSG_ANCHOR>
@@ -2291,9 +2296,9 @@
CheckFunctionParamAttributes(
param,
CheckInvariantFunc<FunctionParam>(
- "invariant can only decorate a param iff it is also decorated with position"),
+ "invariant can only decorate a param if it is also decorated with position"),
CheckInvariantFunc<FunctionParam>(
- "invariant can only decorate a param member iff it is also "
+ "invariant can only decorate a param member if it is also "
"decorated with position"));
if (func->IsFragment()) {
@@ -2368,9 +2373,9 @@
CheckFunctionReturnAttributes(
func,
CheckInvariantFunc<Function>(
- "invariant can only decorate outputs iff they are also position builtins"),
+ "invariant can only decorate outputs if they are also position builtins"),
CheckInvariantFunc<Function>(
- "invariant can only decorate output members iff they are also position builtins"));
+ "invariant can only decorate output members if they are also position builtins"));
// void needs to be filtered out, since it isn't constructible, but used in the IR when no
// return is specified.
if (DAWN_UNLIKELY(!func->ReturnType()->Is<core::type::Void>() &&
@@ -2552,9 +2557,9 @@
CheckIOAttributes(
ep, attr, ty,
CheckInvariantFunc<Function>(
- "invariant can only decorate vars iff they are also position builtins"),
+ "invariant can only decorate vars if they are also position builtins"),
CheckInvariantFunc<Function>(
- "invariant can only decorate members iff they are also position builtins"));
+ "invariant can only decorate members if they are also position builtins"));
// Builtin rules are not checked on module-scope variables, because they are often generated
// as part of the backend transforms, and have different rules for correctness.
diff --git a/src/tint/lang/core/ir/validator_function_test.cc b/src/tint/lang/core/ir/validator_function_test.cc
index a082032..131d765 100644
--- a/src/tint/lang/core/ir/validator_function_test.cc
+++ b/src/tint/lang/core/ir/validator_function_test.cc
@@ -482,7 +482,7 @@
EXPECT_THAT(
res.Failure().reason,
testing::HasSubstr(
- R"(:1:17 error: invariant can only decorate a param iff it is also decorated with position
+ R"(:1:17 error: invariant can only decorate a param if it is also decorated with position
%my_func = func(%my_param:vec4<f32> [@invariant]):void {
^^^^^^^^^^^^^^^^^^^
)")) << res.Failure();
@@ -527,7 +527,33 @@
EXPECT_THAT(
res.Failure().reason,
testing::HasSubstr(
- R"(:5:17 error: invariant can only decorate a param member iff it is also decorated with position
+ R"(:5:17 error: invariant can only decorate a param member if it is also decorated with position
+%my_func = func(%my_param:MyStruct):void {
+ ^^^^^^^^^^^^^^^^^^
+)")) << res.Failure();
+}
+
+TEST_F(IR_ValidatorTest, Function_Param_StructNested_InvariantWithoutPosition) {
+ IOAttributes attr;
+ attr.invariant = true;
+
+ auto* inner_ty =
+ ty.Struct(mod.symbols.New("Inner"), {{mod.symbols.New("pos"), ty.vec4<f32>(), attr}});
+
+ auto* str_ty = ty.Struct(mod.symbols.New("MyStruct"), {{mod.symbols.New("i"), inner_ty}});
+
+ auto* f = b.Function("my_func", ty.void_());
+ auto* p = b.FunctionParam("my_param", str_ty);
+ f->SetParams({p});
+
+ b.Append(f->Block(), [&] { b.Return(f); });
+
+ auto res = ir::Validate(mod);
+ ASSERT_NE(res, Success);
+ EXPECT_THAT(
+ res.Failure().reason,
+ testing::HasSubstr(
+ R"(:9:17 error: invariant can only decorate a param member if it is also decorated with position
%my_func = func(%my_param:MyStruct):void {
^^^^^^^^^^^^^^^^^^
)")) << res.Failure();
@@ -662,7 +688,7 @@
EXPECT_THAT(
res.Failure().reason,
testing::HasSubstr(
- R"(:1:1 error: invariant can only decorate outputs iff they are also position builtins
+ R"(:1:1 error: invariant can only decorate outputs if they are also position builtins
%my_func = func():vec4<f32> [@invariant] {
^^^^^^^^
)")) << res.Failure();
@@ -701,7 +727,7 @@
EXPECT_THAT(
res.Failure().reason,
testing::HasSubstr(
- R"(:5:1 error: invariant can only decorate output members iff they are also position builtins
+ R"(:5:1 error: invariant can only decorate output members if they are also position builtins
%my_func = func():MyStruct {
^^^^^^^^
)")) << res.Failure();