From a0cf62f41534c47bdd480a98b91e7911cbd6d981 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Sat, 17 Apr 2021 00:42:41 +0000 Subject: [PATCH] Fix all tests that shared AST nodes This is not valid. Will become an ICE. Bug: tint:469 Change-Id: I02c0eea16daf7d83f4d6c251e06d9cac0dfd7ce4 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47777 Kokoro: Kokoro Commit-Queue: Ben Clayton Reviewed-by: Antonio Maiorano Reviewed-by: James Price --- src/program_builder.cc | 13 ++++--- src/resolver/intrinsic_test.cc | 33 +++++----------- .../type_constructor_validation_test.cc | 9 ++--- src/transform/bound_array_accessors.cc | 6 +-- src/transform/canonicalize_entry_point_io.cc | 13 ++++--- .../wgsl/generator_impl_global_decl_test.cc | 28 +++++++++++++ ...rator_impl_variable_decl_statement_test.cc | 39 ++----------------- .../wgsl/generator_impl_variable_test.cc | 1 - 8 files changed, 61 insertions(+), 81 deletions(-) diff --git a/src/program_builder.cc b/src/program_builder.cc index 9bd988a8ff..ccd27c3e27 100644 --- a/src/program_builder.cc +++ b/src/program_builder.cc @@ -111,17 +111,18 @@ ast::ConstructorExpression* ProgramBuilder::ConstructValueFilledWith( type, static_cast(elem_value))); } if (auto* v = unwrapped_type->As()) { - auto* elem_default_value = ConstructValueFilledWith(v->type(), elem_value); ast::ExpressionList el(v->size()); - std::fill(el.begin(), el.end(), elem_default_value); + for (size_t i = 0; i < el.size(); i++) { + el[i] = ConstructValueFilledWith(v->type(), elem_value); + } return create(type, std::move(el)); } if (auto* m = unwrapped_type->As()) { auto* col_vec_type = create(m->type(), m->rows()); - auto* vec_default_value = - ConstructValueFilledWith(col_vec_type, elem_value); - ast::ExpressionList el(m->columns()); - std::fill(el.begin(), el.end(), vec_default_value); + ast::ExpressionList el(col_vec_type->size()); + for (size_t i = 0; i < el.size(); i++) { + el[i] = ConstructValueFilledWith(col_vec_type, elem_value); + } return create(type, std::move(el)); } TINT_ASSERT(false); diff --git a/src/resolver/intrinsic_test.cc b/src/resolver/intrinsic_test.cc index ace8df53ab..9d67ef129a 100644 --- a/src/resolver/intrinsic_test.cc +++ b/src/resolver/intrinsic_test.cc @@ -468,7 +468,7 @@ TEST_F(ResolverIntrinsicTest, Select_Error_NoParams) { } TEST_F(ResolverIntrinsicTest, Select_Error_SelectorInt) { - auto* expr = Call("select", Expr(1), Expr(1), Expr(1)); + auto* expr = Call("select", 1, 1, 1); WrapInFunction(expr); EXPECT_FALSE(r()->Resolve()); @@ -483,8 +483,9 @@ TEST_F(ResolverIntrinsicTest, Select_Error_SelectorInt) { } TEST_F(ResolverIntrinsicTest, Select_Error_Matrix) { - auto* mat = mat2x2(vec2(1.0f, 1.0f), vec2(1.0f, 1.0f)); - auto* expr = Call("select", mat, mat, Expr(true)); + auto* expr = Call( + "select", mat2x2(vec2(1.0f, 1.0f), vec2(1.0f, 1.0f)), + mat2x2(vec2(1.0f, 1.0f), vec2(1.0f, 1.0f)), Expr(true)); WrapInFunction(expr); EXPECT_FALSE(r()->Resolve()); @@ -499,7 +500,7 @@ TEST_F(ResolverIntrinsicTest, Select_Error_Matrix) { } TEST_F(ResolverIntrinsicTest, Select_Error_MismatchTypes) { - auto* expr = Call("select", 1.0f, vec2(2.0f, 3.0f), Expr(true)); + auto* expr = Call("select", 1.0f, vec2(2.0f, 3.0f), Expr(true)); WrapInFunction(expr); EXPECT_FALSE(r()->Resolve()); @@ -514,8 +515,8 @@ TEST_F(ResolverIntrinsicTest, Select_Error_MismatchTypes) { } TEST_F(ResolverIntrinsicTest, Select_Error_MismatchVectorSize) { - auto* expr = Call("select", vec2(1.0f, 2.0f), - vec3(3.0f, 4.0f, 5.0f), Expr(true)); + auto* expr = Call("select", vec2(1.0f, 2.0f), + vec3(3.0f, 4.0f, 5.0f), Expr(true)); WrapInFunction(expr); EXPECT_FALSE(r()->Resolve()); @@ -1007,15 +1008,7 @@ TEST_P(ResolverIntrinsicTest_SingleParam_FloatOrInt, Sint_Scalar) { TEST_P(ResolverIntrinsicTest_SingleParam_FloatOrInt, Sint_Vector) { auto param = GetParam(); - ast::ExpressionList vals; - vals.push_back(Expr(1)); - vals.push_back(Expr(1)); - vals.push_back(Expr(3)); - - ast::ExpressionList params; - params.push_back(vec3(vals)); - - auto* call = Call(param.name, params); + auto* call = Call(param.name, vec3(1, 1, 3)); WrapInFunction(call); EXPECT_TRUE(r()->Resolve()) << r()->error(); @@ -1028,10 +1021,7 @@ TEST_P(ResolverIntrinsicTest_SingleParam_FloatOrInt, Sint_Vector) { TEST_P(ResolverIntrinsicTest_SingleParam_FloatOrInt, Uint_Scalar) { auto param = GetParam(); - ast::ExpressionList params; - params.push_back(Expr(1u)); - - auto* call = Call(param.name, params); + auto* call = Call(param.name, 1u); WrapInFunction(call); EXPECT_TRUE(r()->Resolve()) << r()->error(); @@ -1087,10 +1077,7 @@ TEST_F(ResolverIntrinsicTest, Length_Scalar) { } TEST_F(ResolverIntrinsicTest, Length_FloatVector) { - ast::ExpressionList params; - params.push_back(vec3(1.0f, 1.0f, 3.0f)); - - auto* call = Call("length", params); + auto* call = Call("length", vec3(1.0f, 1.0f, 3.0f)); WrapInFunction(call); EXPECT_TRUE(r()->Resolve()) << r()->error(); diff --git a/src/resolver/type_constructor_validation_test.cc b/src/resolver/type_constructor_validation_test.cc index 15bb536060..81a44e167c 100644 --- a/src/resolver/type_constructor_validation_test.cc +++ b/src/resolver/type_constructor_validation_test.cc @@ -51,8 +51,7 @@ TEST_F(ResolverTypeConstructorValidationTest, InferTypeTest_Simple) { auto* a_ident = Expr("a"); auto* b_ident = Expr("b"); - WrapInFunction(Decl(a), Decl(b), Assign(a_ident, a_ident), - Assign(b_ident, b_ident)); + WrapInFunction(Decl(a), Decl(b), Assign(a_ident, "a"), Assign(b_ident, "b")); ASSERT_TRUE(r()->Resolve()) << r()->error(); ASSERT_EQ(TypeOf(a_ident), ty.pointer(ty.i32(), sc)); @@ -75,7 +74,7 @@ TEST_P(InferTypeTest_FromConstructorExpression, All) { // Self-assign 'a' to force the expression to be resolved so we can test its // type below auto* a_ident = Expr("a"); - WrapInFunction(Decl(a), Assign(a_ident, a_ident)); + WrapInFunction(Decl(a), Assign(a_ident, "a")); ASSERT_TRUE(r()->Resolve()) << r()->error(); ASSERT_EQ(TypeOf(a_ident), ty.pointer(rhs_type, sc)); @@ -126,7 +125,7 @@ TEST_P(InferTypeTest_FromArithmeticExpression, All) { // Self-assign 'a' to force the expression to be resolved so we can test its // type below auto* a_ident = Expr("a"); - WrapInFunction(Decl(a), Assign(a_ident, a_ident)); + WrapInFunction(Decl(a), Assign(a_ident, "a")); ASSERT_TRUE(r()->Resolve()) << r()->error(); ASSERT_EQ(TypeOf(a_ident), ty.pointer(rhs_type, sc)); @@ -171,7 +170,7 @@ TEST_P(InferTypeTest_FromCallExpression, All) { // Self-assign 'a' to force the expression to be resolved so we can test its // type below auto* a_ident = Expr("a"); - WrapInFunction(Decl(a), Assign(a_ident, a_ident)); + WrapInFunction(Decl(a), Assign(a_ident, "a")); ASSERT_TRUE(r()->Resolve()) << r()->error(); ASSERT_EQ(TypeOf(a_ident), ty.pointer(rhs_type, sc)); diff --git a/src/transform/bound_array_accessors.cc b/src/transform/bound_array_accessors.cc index 4f5692f8d4..55485a413b 100644 --- a/src/transform/bound_array_accessors.cc +++ b/src/transform/bound_array_accessors.cc @@ -69,11 +69,7 @@ ast::ArrayAccessorExpression* BoundArrayAccessors::Transform( if (size == 0) { if (is_arr) { - // Call Cloneable::Clone() instead of CloneContext::Clone() to ensure the - // AST node is duplicated. CloneContext::Clone() will ensure that repeated - // calls with the same pointer return the *same* cloned node - in this - // case we actually want two copies. - auto* arr = static_cast(expr->array()->Clone(ctx)); + auto* arr = ctx->Clone(expr->array()); auto* arr_len = b.Call("arrayLength", arr); auto* limit = b.Sub(arr_len, b.Expr(1u)); new_idx = b.Call("min", b.Construct(ctx->Clone(old_idx)), limit); diff --git a/src/transform/canonicalize_entry_point_io.cc b/src/transform/canonicalize_entry_point_io.cc index 27d5c8193b..9f7e8813a5 100644 --- a/src/transform/canonicalize_entry_point_io.cc +++ b/src/transform/canonicalize_entry_point_io.cc @@ -222,7 +222,10 @@ Output CanonicalizeEntryPointIO::Run(const Program* in, const DataMap&) { for (auto* ret : sem_func->ReturnStatements()) { auto* ret_sem = ctx.src->Sem().Get(ret); // Reconstruct the return value using the newly created struct. - auto* new_ret_value = ctx.Clone(ret->value()); + std::function new_ret_value = [&ctx, ret] { + return ctx.Clone(ret->value()); + }; + ast::ExpressionList ret_values; if (ret_type->Is()) { if (!ret->value()->Is()) { @@ -230,17 +233,17 @@ Output CanonicalizeEntryPointIO::Run(const Program* in, const DataMap&) { // re-evaluating it multiple times. auto temp = ctx.dst->Symbols().New(); auto* temp_var = ctx.dst->Decl( - ctx.dst->Const(temp, ctx.Clone(ret_type), new_ret_value)); + ctx.dst->Const(temp, ctx.Clone(ret_type), new_ret_value())); ctx.InsertBefore(ret_sem->Block()->statements(), ret, temp_var); - new_ret_value = ctx.dst->Expr(temp); + new_ret_value = [&ctx, temp] { return ctx.dst->Expr(temp); }; } for (auto* member : new_struct_members) { ret_values.push_back( - ctx.dst->MemberAccessor(new_ret_value, member->symbol())); + ctx.dst->MemberAccessor(new_ret_value(), member->symbol())); } } else { - ret_values.push_back(new_ret_value); + ret_values.push_back(new_ret_value()); } auto* new_ret = ctx.dst->create( diff --git a/src/writer/wgsl/generator_impl_global_decl_test.cc b/src/writer/wgsl/generator_impl_global_decl_test.cc index d853fb2f28..b7e492112a 100644 --- a/src/writer/wgsl/generator_impl_global_decl_test.cc +++ b/src/writer/wgsl/generator_impl_global_decl_test.cc @@ -14,6 +14,8 @@ #include "src/ast/stage_decoration.h" #include "src/ast/variable_decl_statement.h" +#include "src/type/access_control_type.h" +#include "src/type/sampled_texture_type.h" #include "src/writer/wgsl/test_helper.h" namespace tint { @@ -111,6 +113,32 @@ TEST_F(WgslGeneratorImplTest, Emit_GlobalsInterleaved) { )"); } +TEST_F(WgslGeneratorImplTest, Emit_Global_Sampler) { + Global("s", create(type::SamplerKind::kSampler), + ast::StorageClass::kUniformConstant); + + GeneratorImpl& gen = Build(); + + gen.increment_indent(); + + ASSERT_TRUE(gen.Generate(nullptr)) << gen.error(); + EXPECT_EQ(gen.result(), " var s : sampler;\n"); +} + +TEST_F(WgslGeneratorImplTest, Emit_Global_Texture) { + auto* st = + create(type::TextureDimension::k1d, ty.f32()); + Global("t", ty.access(ast::AccessControl::kReadOnly, st), + ast::StorageClass::kUniformConstant); + + GeneratorImpl& gen = Build(); + + gen.increment_indent(); + + ASSERT_TRUE(gen.Generate(nullptr)) << gen.error(); + EXPECT_EQ(gen.result(), " var t : [[access(read)]] texture_1d;\n"); +} + } // namespace } // namespace wgsl } // namespace writer diff --git a/src/writer/wgsl/generator_impl_variable_decl_statement_test.cc b/src/writer/wgsl/generator_impl_variable_decl_statement_test.cc index 2878e430cd..e27397f2bd 100644 --- a/src/writer/wgsl/generator_impl_variable_decl_statement_test.cc +++ b/src/writer/wgsl/generator_impl_variable_decl_statement_test.cc @@ -13,8 +13,6 @@ // limitations under the License. #include "src/ast/variable_decl_statement.h" -#include "src/type/access_control_type.h" -#include "src/type/sampled_texture_type.h" #include "src/writer/wgsl/test_helper.h" namespace tint { @@ -25,7 +23,7 @@ namespace { using WgslGeneratorImplTest = TestHelper; TEST_F(WgslGeneratorImplTest, Emit_VariableDeclStatement) { - auto* var = Global("a", ty.f32(), ast::StorageClass::kInput); + auto* var = Var("a", ty.f32(), ast::StorageClass::kInput); auto* stmt = create(var); WrapInFunction(stmt); @@ -43,7 +41,7 @@ TEST_F(WgslGeneratorImplTest, Emit_VariableDeclStatement_Function) { // storage class. Rely on defaulting. // https://github.com/gpuweb/gpuweb/issues/654 - auto* var = Global("a", ty.f32(), ast::StorageClass::kFunction); + auto* var = Var("a", ty.f32(), ast::StorageClass::kFunction); auto* stmt = create(var); WrapInFunction(stmt); @@ -57,7 +55,7 @@ TEST_F(WgslGeneratorImplTest, Emit_VariableDeclStatement_Function) { } TEST_F(WgslGeneratorImplTest, Emit_VariableDeclStatement_Private) { - auto* var = Global("a", ty.f32(), ast::StorageClass::kPrivate); + auto* var = Var("a", ty.f32(), ast::StorageClass::kPrivate); auto* stmt = create(var); WrapInFunction(stmt); @@ -70,37 +68,6 @@ TEST_F(WgslGeneratorImplTest, Emit_VariableDeclStatement_Private) { EXPECT_EQ(gen.result(), " var a : f32;\n"); } -TEST_F(WgslGeneratorImplTest, Emit_VariableDeclStatement_Sampler) { - auto* var = Global("s", create(type::SamplerKind::kSampler), - ast::StorageClass::kUniformConstant); - - auto* stmt = create(var); - - GeneratorImpl& gen = Build(); - - gen.increment_indent(); - - ASSERT_TRUE(gen.EmitStatement(stmt)) << gen.error(); - EXPECT_EQ(gen.result(), " var s : sampler;\n"); -} - -TEST_F(WgslGeneratorImplTest, Emit_VariableDeclStatement_Texture) { - auto* st = - create(type::TextureDimension::k1d, ty.f32()); - auto* var = Global( - "t", create(ast::AccessControl::kReadOnly, st), - ast::StorageClass::kUniformConstant); - - auto* stmt = create(var); - - GeneratorImpl& gen = Build(); - - gen.increment_indent(); - - ASSERT_TRUE(gen.EmitStatement(stmt)) << gen.error(); - EXPECT_EQ(gen.result(), " var t : [[access(read)]] texture_1d;\n"); -} - } // namespace } // namespace wgsl } // namespace writer diff --git a/src/writer/wgsl/generator_impl_variable_test.cc b/src/writer/wgsl/generator_impl_variable_test.cc index 878fd6b6ab..cf1bed0058 100644 --- a/src/writer/wgsl/generator_impl_variable_test.cc +++ b/src/writer/wgsl/generator_impl_variable_test.cc @@ -76,7 +76,6 @@ TEST_F(WgslGeneratorImplTest, EmitVariable_Decorated_Multiple) { TEST_F(WgslGeneratorImplTest, EmitVariable_Constructor) { auto* v = Global("a", ty.f32(), ast::StorageClass::kInput, Expr(1.0f)); - WrapInFunction(Decl(v)); GeneratorImpl& gen = Build();