tint/resolver: Move texture validation to the right place

Validating sampled and multisampled texture types does not belong in Validator::Variable().

Change-Id: Ie0f2502508c28af6fb6d3f4d7803171d946c511b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/93783
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@chromium.org>

src/tint/resolver/resolver.cc

@@ -240,13 +240,21 @@ sem::Type* Resolver::Type(const ast::Type* ty) {
         [&](const ast::Sampler* t) { return builder_->create<sem::Sampler>(t->kind); },
         [&](const ast::SampledTexture* t) -> sem::SampledTexture* {
             if (auto* el = Type(t->type)) {
-                return builder_->create<sem::SampledTexture>(t->dim, el);
+                auto* sem = builder_->create<sem::SampledTexture>(t->dim, el);
+                if (!validator_.SampledTexture(sem, t->source)) {
+                    return nullptr;
+                }
+                return sem;
             }
             return nullptr;
         },
         [&](const ast::MultisampledTexture* t) -> sem::MultisampledTexture* {
             if (auto* el = Type(t->type)) {
-                return builder_->create<sem::MultisampledTexture>(t->dim, el);
+                auto* sem = builder_->create<sem::MultisampledTexture>(t->dim, el);
+                if (!validator_.MultisampledTexture(sem, t->source)) {
+                    return nullptr;
+                }
+                return sem;
             }
             return nullptr;
         },

src/tint/resolver/type_validation_test.cc

@@ -767,7 +767,7 @@ INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
 using MultisampledTextureDimensionTest = ResolverTestWithParam<DimensionParams>;
 TEST_P(MultisampledTextureDimensionTest, All) {
     auto& params = GetParam();
-    Global(Source{{12, 34}}, "a", ty.multisampled_texture(params.dim, ty.i32()),
+    Global("a", ty.multisampled_texture(Source{{12, 34}}, params.dim, ty.i32()),
            ast::StorageClass::kNone, nullptr, ast::AttributeList{GroupAndBinding(0, 0)});
 
     if (params.is_valid) {
@@ -818,17 +818,16 @@ static constexpr TypeParams type_cases[] = {
 using SampledTextureTypeTest = ResolverTestWithParam<TypeParams>;
 TEST_P(SampledTextureTypeTest, All) {
     auto& params = GetParam();
-    Global(Source{{12, 34}}, "a",
+    Global(
-           ty.sampled_texture(ast::TextureDimension::k2d, params.type_func(*this)),
+        "a",
-           ast::StorageClass::kNone, nullptr, ast::AttributeList{GroupAndBinding(0, 0)});
+        ty.sampled_texture(Source{{12, 34}}, ast::TextureDimension::k2d, params.type_func(*this)),
+        ast::StorageClass::kNone, nullptr, ast::AttributeList{GroupAndBinding(0, 0)});
 
     if (params.is_valid) {
         EXPECT_TRUE(r()->Resolve()) << r()->error();
     } else {
         EXPECT_FALSE(r()->Resolve());
-        EXPECT_EQ(r()->error(),
+        EXPECT_EQ(r()->error(), "12:34 error: texture_2d<type>: type must be f32, i32 or u32");
-                  "12:34 error: texture_2d<type>: type must be f32, "
-                  "i32 or u32");
     }
 }
 INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@@ -838,8 +837,9 @@ INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
 using MultisampledTextureTypeTest = ResolverTestWithParam<TypeParams>;
 TEST_P(MultisampledTextureTypeTest, All) {
     auto& params = GetParam();
-    Global(Source{{12, 34}}, "a",
+    Global("a",
-           ty.multisampled_texture(ast::TextureDimension::k2d, params.type_func(*this)),
+           ty.multisampled_texture(Source{{12, 34}}, ast::TextureDimension::k2d,
+                                   params.type_func(*this)),
            ast::StorageClass::kNone, nullptr, ast::AttributeList{GroupAndBinding(0, 0)});
 
     if (params.is_valid) {
@@ -847,8 +847,7 @@ TEST_P(MultisampledTextureTypeTest, All) {
     } else {
         EXPECT_FALSE(r()->Resolve());
         EXPECT_EQ(r()->error(),
-                  "12:34 error: texture_multisampled_2d<type>: type must be f32, "
+                  "12:34 error: texture_multisampled_2d<type>: type must be f32, i32 or u32");
-                  "i32 or u32");
     }
 }
 INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@@ -887,8 +886,7 @@ TEST_P(StorageTextureDimensionTest, All) {
     } else {
         EXPECT_FALSE(r()->Resolve());
         EXPECT_EQ(r()->error(),
-                  "12:34 error: cube dimensions for storage textures are not "
+                  "12:34 error: cube dimensions for storage textures are not supported");
-                  "supported");
     }
 }
 INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
@@ -948,9 +946,8 @@ TEST_P(StorageTextureFormatTest, All) {
     } else {
         EXPECT_FALSE(r()->Resolve());
         EXPECT_EQ(r()->error(),
-                  "12:34 error: image format must be one of the texel formats "
+                  "12:34 error: image format must be one of the texel formats specified for "
-                  "specified for storage textues in "
+                  "storage textues in https://gpuweb.github.io/gpuweb/wgsl/#texel-formats");
-                  "https://gpuweb.github.io/gpuweb/wgsl/#texel-formats");
     }
 }
 INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,

src/tint/resolver/validator.cc

@@ -284,6 +284,29 @@ bool Validator::StorageTexture(const ast::StorageTexture* t) const {
     return true;
 }
 
+bool Validator::SampledTexture(const sem::SampledTexture* t, const Source& source) const {
+    if (!t->type()->UnwrapRef()->is_numeric_scalar()) {
+        AddError("texture_2d<type>: type must be f32, i32 or u32", source);
+        return false;
+    }
+
+    return true;
+}
+
+bool Validator::MultisampledTexture(const sem::MultisampledTexture* t, const Source& source) const {
+    if (t->dim() != ast::TextureDimension::k2d) {
+        AddError("only 2d multisampled textures are supported", source);
+        return false;
+    }
+
+    if (!t->type()->UnwrapRef()->is_numeric_scalar()) {
+        AddError("texture_multisampled_2d<type>: type must be f32, i32 or u32", source);
+        return false;
+    }
+
+    return true;
+}
+
 bool Validator::Materialize(const sem::Materialize* m) const {
     auto* from = m->Expr()->Type();
     auto* to = m->Type();
@@ -685,25 +708,6 @@ bool Validator::Variable(const sem::Variable* v) const {
         return false;
     }
 
-    if (auto* r = storage_ty->As<sem::SampledTexture>()) {
-        if (!r->type()->UnwrapRef()->is_numeric_scalar()) {
-            AddError("texture_2d<type>: type must be f32, i32 or u32", decl->source);
-            return false;
-        }
-    }
-
-    if (auto* r = storage_ty->As<sem::MultisampledTexture>()) {
-        if (r->dim() != ast::TextureDimension::k2d) {
-            AddError("only 2d multisampled textures are supported", decl->source);
-            return false;
-        }
-
-        if (!r->type()->UnwrapRef()->is_numeric_scalar()) {
-            AddError("texture_multisampled_2d<type>: type must be f32, i32 or u32", decl->source);
-            return false;
-        }
-    }
-
     if (v->Is<sem::LocalVariable>() && as_var &&
         IsValidationEnabled(decl->attributes, ast::DisabledValidation::kIgnoreStorageClass)) {
         if (!v->Type()->UnwrapRef()->IsConstructible()) {

src/tint/resolver/validator.h

@@ -321,6 +321,18 @@ class Validator {
     /// @returns true on success, false otherwise
     bool StorageTexture(const ast::StorageTexture* t) const;
 
+    /// Validates a sampled texture
+    /// @param t the texture to validate
+    /// @param source the source of the texture
+    /// @returns true on success, false otherwise
+    bool SampledTexture(const sem::SampledTexture* t, const Source& source) const;
+
+    /// Validates a multisampled texture
+    /// @param t the texture to validate
+    /// @param source the source of the texture
+    /// @returns true on success, false otherwise
+    bool MultisampledTexture(const sem::MultisampledTexture* t, const Source& source) const;
+
     /// Validates a structure
     /// @param str the structure to validate
     /// @param stage the current pipeline stage