From d1003645cb398b080ff65dd60c8340b488e60072 Mon Sep 17 00:00:00 2001 From: James Price Date: Fri, 4 Mar 2022 12:55:13 +0000 Subject: [PATCH] Cleaned up several DISABLED unit tests Many were redundant, some were now fixed. Change-Id: Iecc761fbed82764cd25224f843d754c5948422ad Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/82681 Kokoro: Kokoro Reviewed-by: Ben Clayton --- src/tint/transform/binding_remapper_test.cc | 60 ------------------- .../glsl/generator_impl_constructor_test.cc | 11 ++-- .../writer/glsl/generator_impl_type_test.cc | 30 ---------- .../hlsl/generator_impl_constructor_test.cc | 8 +-- .../writer/hlsl/generator_impl_type_test.cc | 30 ---------- .../writer/msl/generator_impl_type_test.cc | 13 ++-- src/tint/writer/spirv/builder_block_test.cc | 4 +- 7 files changed, 15 insertions(+), 141 deletions(-) diff --git a/src/tint/transform/binding_remapper_test.cc b/src/tint/transform/binding_remapper_test.cc index 7816db324f..4e0e5152d6 100644 --- a/src/tint/transform/binding_remapper_test.cc +++ b/src/tint/transform/binding_remapper_test.cc @@ -178,66 +178,6 @@ fn f() { EXPECT_EQ(expect, str(got)); } -// TODO(crbug.com/676): Possibly enable if the spec allows for access -// attributes in type aliases. If not, just remove. -TEST_F(BindingRemapperTest, DISABLED_RemapAccessControlsWithAliases) { - auto* src = R"( -struct S { - a : f32; -}; - -type, read ReadOnlyS = S; - -type, write WriteOnlyS = S; - -type A = S; - -@group(2) @binding(1) var a : ReadOnlyS; - -@group(3) @binding(2) var b : WriteOnlyS; - -@group(4) @binding(3) var c : A; - -@stage(compute) @workgroup_size(1) -fn f() { -} -)"; - - auto* expect = R"( -struct S { - a : f32; -}; - -type, read ReadOnlyS = S; - -type, write WriteOnlyS = S; - -type A = S; - -@group(2) @binding(1) var a : S; - -@group(3) @binding(2) var b : WriteOnlyS; - -@group(4) @binding(3) var c : S; - -@stage(compute) @workgroup_size(1) -fn f() { -} -)"; - - DataMap data; - data.Add( - BindingRemapper::BindingPoints{}, - BindingRemapper::AccessControls{ - {{2, 1}, ast::Access::kWrite}, // Modify access control - // Keep @group(3) @binding(2) as is - {{4, 3}, ast::Access::kRead}, // Add access control - }); - auto got = Run(src, data); - - EXPECT_EQ(expect, str(got)); -} - TEST_F(BindingRemapperTest, RemapAll) { auto* src = R"( struct S { diff --git a/src/tint/writer/glsl/generator_impl_constructor_test.cc b/src/tint/writer/glsl/generator_impl_constructor_test.cc index 4d477bb1c4..ff90549968 100644 --- a/src/tint/writer/glsl/generator_impl_constructor_test.cc +++ b/src/tint/writer/glsl/generator_impl_constructor_test.cc @@ -192,17 +192,16 @@ TEST_F(GlslGeneratorImplTest_Constructor, EmitConstructor_Type_Array) { "vec3(7.0f, 8.0f, 9.0f))")); } -// TODO(bclayton): Zero-init arrays -TEST_F(GlslGeneratorImplTest_Constructor, - DISABLED_EmitConstructor_Type_Array_Empty) { +TEST_F(GlslGeneratorImplTest_Constructor, EmitConstructor_Type_Array_Empty) { WrapInFunction(Construct(ty.array(ty.vec3(), 3))); GeneratorImpl& gen = Build(); ASSERT_TRUE(gen.Generate()) << gen.error(); - EXPECT_THAT(gen.result(), - HasSubstr("{vec3(0.0f, 0.0f, 0.0f), vec3(0.0f, 0.0f, 0.0f)," - " vec3(0.0f, 0.0f, 0.0f)}")); + EXPECT_THAT( + gen.result(), + HasSubstr("vec3[3](vec3(0.0f, 0.0f, 0.0f), vec3(0.0f, 0.0f, 0.0f)," + " vec3(0.0f, 0.0f, 0.0f))")); } TEST_F(GlslGeneratorImplTest_Constructor, EmitConstructor_Type_Struct) { diff --git a/src/tint/writer/glsl/generator_impl_type_test.cc b/src/tint/writer/glsl/generator_impl_type_test.cc index 183d76869f..64547e3da0 100644 --- a/src/tint/writer/glsl/generator_impl_type_test.cc +++ b/src/tint/writer/glsl/generator_impl_type_test.cc @@ -58,21 +58,6 @@ TEST_F(GlslGeneratorImplTest_Type, EmitType_ArrayOfArray) { EXPECT_EQ(out.str(), "bool ary[5][4]"); } -// TODO(dsinclair): Is this possible? What order should it output in? -TEST_F(GlslGeneratorImplTest_Type, - DISABLED_EmitType_ArrayOfArrayOfRuntimeArray) { - auto* arr = ty.array(ty.array(ty.array(), 5), 0); - Global("G", arr, ast::StorageClass::kPrivate); - - GeneratorImpl& gen = Build(); - - std::stringstream out; - ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone, - ast::Access::kReadWrite, "ary")) - << gen.error(); - EXPECT_EQ(out.str(), "bool ary[5][4][1]"); -} - TEST_F(GlslGeneratorImplTest_Type, EmitType_ArrayOfArrayOfArray) { auto* arr = ty.array(ty.array(ty.array(), 5), 6); Global("G", arr, ast::StorageClass::kPrivate); @@ -149,21 +134,6 @@ TEST_F(GlslGeneratorImplTest_Type, EmitType_Matrix) { EXPECT_EQ(out.str(), "mat2x3"); } -// TODO(dsinclair): How to annotate as workgroup? -TEST_F(GlslGeneratorImplTest_Type, DISABLED_EmitType_Pointer) { - auto* f32 = create(); - auto* p = create(f32, ast::StorageClass::kWorkgroup, - ast::Access::kReadWrite); - - GeneratorImpl& gen = Build(); - - std::stringstream out; - ASSERT_TRUE(gen.EmitType(out, p, ast::StorageClass::kNone, - ast::Access::kReadWrite, "")) - << gen.error(); - EXPECT_EQ(out.str(), "float*"); -} - TEST_F(GlslGeneratorImplTest_Type, EmitType_StructDecl) { auto* s = Structure("S", { Member("a", ty.i32()), diff --git a/src/tint/writer/hlsl/generator_impl_constructor_test.cc b/src/tint/writer/hlsl/generator_impl_constructor_test.cc index db79af99f6..4afcffe715 100644 --- a/src/tint/writer/hlsl/generator_impl_constructor_test.cc +++ b/src/tint/writer/hlsl/generator_impl_constructor_test.cc @@ -219,17 +219,13 @@ TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Type_Array) { " float3(7.0f, 8.0f, 9.0f)}")); } -// TODO(bclayton): Zero-init arrays -TEST_F(HlslGeneratorImplTest_Constructor, - DISABLED_EmitConstructor_Type_Array_Empty) { +TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Type_Array_Empty) { WrapInFunction(Construct(ty.array(ty.vec3(), 3))); GeneratorImpl& gen = Build(); ASSERT_TRUE(gen.Generate()) << gen.error(); - EXPECT_THAT(gen.result(), - HasSubstr("{float3(0.0f, 0.0f, 0.0f), float3(0.0f, 0.0f, 0.0f)," - " float3(0.0f, 0.0f, 0.0f)}")); + EXPECT_THAT(gen.result(), HasSubstr("(float3[3])0")); } TEST_F(HlslGeneratorImplTest_Constructor, EmitConstructor_Type_Struct) { diff --git a/src/tint/writer/hlsl/generator_impl_type_test.cc b/src/tint/writer/hlsl/generator_impl_type_test.cc index 6d7d9cd444..4e4ae3222e 100644 --- a/src/tint/writer/hlsl/generator_impl_type_test.cc +++ b/src/tint/writer/hlsl/generator_impl_type_test.cc @@ -58,21 +58,6 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_ArrayOfArray) { EXPECT_EQ(out.str(), "bool ary[5][4]"); } -// TODO(dsinclair): Is this possible? What order should it output in? -TEST_F(HlslGeneratorImplTest_Type, - DISABLED_EmitType_ArrayOfArrayOfRuntimeArray) { - auto* arr = ty.array(ty.array(ty.array(), 5)); - Global("G", arr, ast::StorageClass::kPrivate); - - GeneratorImpl& gen = Build(); - - std::stringstream out; - ASSERT_TRUE(gen.EmitType(out, program->TypeOf(arr), ast::StorageClass::kNone, - ast::Access::kReadWrite, "ary")) - << gen.error(); - EXPECT_EQ(out.str(), "bool ary[5][4][1]"); -} - TEST_F(HlslGeneratorImplTest_Type, EmitType_ArrayOfArrayOfArray) { auto* arr = ty.array(ty.array(ty.array(), 5), 6); Global("G", arr, ast::StorageClass::kPrivate); @@ -149,21 +134,6 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Matrix) { EXPECT_EQ(out.str(), "float2x3"); } -// TODO(dsinclair): How to annotate as workgroup? -TEST_F(HlslGeneratorImplTest_Type, DISABLED_EmitType_Pointer) { - auto* f32 = create(); - auto* p = create(f32, ast::StorageClass::kWorkgroup, - ast::Access::kReadWrite); - - GeneratorImpl& gen = Build(); - - std::stringstream out; - ASSERT_TRUE(gen.EmitType(out, p, ast::StorageClass::kNone, - ast::Access::kReadWrite, "")) - << gen.error(); - EXPECT_EQ(out.str(), "float*"); -} - TEST_F(HlslGeneratorImplTest_Type, EmitType_StructDecl) { auto* s = Structure("S", { Member("a", ty.i32()), diff --git a/src/tint/writer/msl/generator_impl_type_test.cc b/src/tint/writer/msl/generator_impl_type_test.cc index 5f2298b66c..d5de70ea76 100644 --- a/src/tint/writer/msl/generator_impl_type_test.cc +++ b/src/tint/writer/msl/generator_impl_type_test.cc @@ -670,8 +670,7 @@ TEST_F(MslGeneratorImplTest, AttemptTintPadSymbolCollision) { )"); } -// TODO(dsinclair): How to translate @block -TEST_F(MslGeneratorImplTest, DISABLED_EmitType_Struct_WithAttribute) { +TEST_F(MslGeneratorImplTest, EmitType_Struct_WithAttribute) { auto* s = Structure("S", { Member("a", ty.i32()), @@ -687,12 +686,14 @@ TEST_F(MslGeneratorImplTest, DISABLED_EmitType_Struct_WithAttribute) { GeneratorImpl& gen = Build(); - std::stringstream out; - ASSERT_TRUE(gen.EmitType(out, program->TypeOf(s), "")) << gen.error(); - EXPECT_EQ(out.str(), R"(struct { + TextGenerator::TextBuffer buf; + auto* sem_s = program->TypeOf(s)->As(); + ASSERT_TRUE(gen.EmitStructType(&buf, sem_s)) << gen.error(); + EXPECT_EQ(buf.String(), R"(struct S { /* 0x0000 */ int a; /* 0x0004 */ float b; -})"); +}; +)"); } TEST_F(MslGeneratorImplTest, EmitType_U32) { diff --git a/src/tint/writer/spirv/builder_block_test.cc b/src/tint/writer/spirv/builder_block_test.cc index 0d2b381aae..579d7bcf47 100644 --- a/src/tint/writer/spirv/builder_block_test.cc +++ b/src/tint/writer/spirv/builder_block_test.cc @@ -22,9 +22,7 @@ namespace { using BuilderTest = TestHelper; -// TODO(amaiorano): Disabled because Resolver now emits "redeclared identifier -// 'var'". -TEST_F(BuilderTest, DISABLED_Block) { +TEST_F(BuilderTest, Block) { // Note, this test uses shadow variables which aren't allowed in WGSL but // serves to prove the block code is pushing new scopes as needed. auto* inner = Block(Decl(Var("var", ty.f32(), ast::StorageClass::kNone)),