From a8d97550537161381b2be7e81d5e41c156697205 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Mon, 11 Jan 2021 16:24:32 +0000 Subject: [PATCH] Allow setting the namer into the inspector. This CL adds an extra constructor to the inspector to change the namer user. The inspector tests are then updated to use the test namer. Change-Id: Ibc91de89b52161dc125b38d65e445b5833ad6c18 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/36943 Reviewed-by: Ryan Harrison --- src/inspector/inspector.cc | 14 ++++- src/inspector/inspector.h | 7 ++- src/inspector/inspector_test.cc | 108 +++++++++++++------------------- 3 files changed, 61 insertions(+), 68 deletions(-) diff --git a/src/inspector/inspector.cc b/src/inspector/inspector.cc index 47995d15d3..ae98f19aa8 100644 --- a/src/inspector/inspector.cc +++ b/src/inspector/inspector.cc @@ -43,10 +43,18 @@ namespace tint { namespace inspector { -Inspector::Inspector(ast::Module& module) - : module_(module), namer_(std::make_unique(&module)) {} +Inspector::Inspector(ast::Module& module, Namer* namer) + : module_(module), namer_(namer), namer_is_owned_(false) {} -Inspector::~Inspector() = default; +Inspector::Inspector(ast::Module& module) + : module_(module), + namer_(new UnsafeNamer(&module)), + namer_is_owned_(true) {} + +Inspector::~Inspector() { + if (namer_is_owned_) + delete namer_; +} std::vector Inspector::GetEntryPoints() { std::vector result; diff --git a/src/inspector/inspector.h b/src/inspector/inspector.h index ea82e1baa6..4a17b8ddd0 100644 --- a/src/inspector/inspector.h +++ b/src/inspector/inspector.h @@ -71,6 +71,10 @@ struct ResourceBinding { /// Extracts information from a module class Inspector { public: + /// Constructor + /// @param module Shader module to extract information from. + /// @param namer the namer to use with the inspector + Inspector(ast::Module& module, Namer* namer); /// Constructor /// @param module Shader module to extract information from. explicit Inspector(ast::Module& module); @@ -129,7 +133,8 @@ class Inspector { private: const ast::Module& module_; - std::unique_ptr namer_; + Namer* namer_ = nullptr; + bool namer_is_owned_ = false; std::string error_; /// @param name name of the entry point to find diff --git a/src/inspector/inspector_test.cc b/src/inspector/inspector_test.cc index 4de2319d51..e9357b400a 100644 --- a/src/inspector/inspector_test.cc +++ b/src/inspector/inspector_test.cc @@ -61,6 +61,7 @@ #include "src/ast/variable_decoration.h" #include "src/ast/workgroup_decoration.h" #include "src/type_determiner.h" +#include "src/writer/test_namer.h" #include "tint/tint.h" namespace tint { @@ -71,7 +72,8 @@ class InspectorHelper : public ast::BuilderWithModule { public: InspectorHelper() : td_(std::make_unique(mod)), - inspector_(std::make_unique(*mod)), + namer_(mod), + inspector_(std::make_unique(*mod, &namer_)), sampler_type_(ast::type::SamplerKind::kSampler), comparison_sampler_type_(ast::type::SamplerKind::kComparisonSampler) {} @@ -639,6 +641,7 @@ class InspectorHelper : public ast::BuilderWithModule { private: std::unique_ptr td_; + writer::TestNamer namer_; std::unique_ptr inspector_; ast::type::Sampler sampler_type_; @@ -720,14 +723,12 @@ TEST_F(InspectorGetEntryPointTest, OneEntryPoint) { }); mod->AddFunction(foo); - // TODO(dsinclair): Update to run the namer transform when available. - auto result = inspector()->GetEntryPoints(); ASSERT_FALSE(inspector()->has_error()) << inspector()->error(); ASSERT_EQ(1u, result.size()); EXPECT_EQ("foo", result[0].name); - EXPECT_EQ("foo", result[0].remapped_name); + EXPECT_EQ("test_foo", result[0].remapped_name); EXPECT_EQ(ast::PipelineStage::kVertex, result[0].stage); } @@ -744,17 +745,15 @@ TEST_F(InspectorGetEntryPointTest, MultipleEntryPoints) { }); mod->AddFunction(bar); - // TODO(dsinclair): Update to run the namer transform when available. - auto result = inspector()->GetEntryPoints(); ASSERT_FALSE(inspector()->has_error()) << inspector()->error(); ASSERT_EQ(2u, result.size()); EXPECT_EQ("foo", result[0].name); - EXPECT_EQ("foo", result[0].remapped_name); + EXPECT_EQ("test_foo", result[0].remapped_name); EXPECT_EQ(ast::PipelineStage::kVertex, result[0].stage); EXPECT_EQ("bar", result[1].name); - EXPECT_EQ("bar", result[1].remapped_name); + EXPECT_EQ("test_bar", result[1].remapped_name); EXPECT_EQ(ast::PipelineStage::kCompute, result[1].stage); } @@ -776,17 +775,15 @@ TEST_F(InspectorGetEntryPointTest, MixFunctionsAndEntryPoints) { }); mod->AddFunction(bar); - // TODO(dsinclair): Update to run the namer transform when available. - auto result = inspector()->GetEntryPoints(); EXPECT_FALSE(inspector()->has_error()); ASSERT_EQ(2u, result.size()); EXPECT_EQ("foo", result[0].name); - EXPECT_EQ("foo", result[0].remapped_name); + EXPECT_EQ("test_foo", result[0].remapped_name); EXPECT_EQ(ast::PipelineStage::kVertex, result[0].stage); EXPECT_EQ("bar", result[1].name); - EXPECT_EQ("bar", result[1].remapped_name); + EXPECT_EQ("test_bar", result[1].remapped_name); EXPECT_EQ(ast::PipelineStage::kFragment, result[1].stage); } @@ -865,9 +862,9 @@ TEST_F(InspectorGetEntryPointTest, EntryPointInOutVariables) { ASSERT_EQ(1u, result.size()); ASSERT_EQ(1u, result[0].input_variables.size()); - EXPECT_EQ("in_var", result[0].input_variables[0]); + EXPECT_EQ("test_in_var", result[0].input_variables[0]); ASSERT_EQ(1u, result[0].output_variables.size()); - EXPECT_EQ("out_var", result[0].output_variables[0]); + EXPECT_EQ("test_out_var", result[0].output_variables[0]); } TEST_F(InspectorGetEntryPointTest, FunctionInOutVariables) { @@ -892,9 +889,9 @@ TEST_F(InspectorGetEntryPointTest, FunctionInOutVariables) { ASSERT_EQ(1u, result.size()); ASSERT_EQ(1u, result[0].input_variables.size()); - EXPECT_EQ("in_var", result[0].input_variables[0]); + EXPECT_EQ("test_in_var", result[0].input_variables[0]); ASSERT_EQ(1u, result[0].output_variables.size()); - EXPECT_EQ("out_var", result[0].output_variables[0]); + EXPECT_EQ("test_out_var", result[0].output_variables[0]); } TEST_F(InspectorGetEntryPointTest, RepeatedInOutVariables) { @@ -919,9 +916,9 @@ TEST_F(InspectorGetEntryPointTest, RepeatedInOutVariables) { ASSERT_EQ(1u, result.size()); ASSERT_EQ(1u, result[0].input_variables.size()); - EXPECT_EQ("in_var", result[0].input_variables[0]); + EXPECT_EQ("test_in_var", result[0].input_variables[0]); ASSERT_EQ(1u, result[0].output_variables.size()); - EXPECT_EQ("out_var", result[0].output_variables[0]); + EXPECT_EQ("test_out_var", result[0].output_variables[0]); } TEST_F(InspectorGetEntryPointTest, EntryPointMultipleInOutVariables) { @@ -942,11 +939,11 @@ TEST_F(InspectorGetEntryPointTest, EntryPointMultipleInOutVariables) { ASSERT_EQ(1u, result.size()); ASSERT_EQ(2u, result[0].input_variables.size()); - EXPECT_TRUE(ContainsString(result[0].input_variables, "in_var")); - EXPECT_TRUE(ContainsString(result[0].input_variables, "in2_var")); + EXPECT_TRUE(ContainsString(result[0].input_variables, "test_in_var")); + EXPECT_TRUE(ContainsString(result[0].input_variables, "test_in2_var")); ASSERT_EQ(2u, result[0].output_variables.size()); - EXPECT_TRUE(ContainsString(result[0].output_variables, "out_var")); - EXPECT_TRUE(ContainsString(result[0].output_variables, "out2_var")); + EXPECT_TRUE(ContainsString(result[0].output_variables, "test_out_var")); + EXPECT_TRUE(ContainsString(result[0].output_variables, "test_out2_var")); } TEST_F(InspectorGetEntryPointTest, FunctionMultipleInOutVariables) { @@ -971,11 +968,11 @@ TEST_F(InspectorGetEntryPointTest, FunctionMultipleInOutVariables) { ASSERT_EQ(1u, result.size()); ASSERT_EQ(2u, result[0].input_variables.size()); - EXPECT_TRUE(ContainsString(result[0].input_variables, "in_var")); - EXPECT_TRUE(ContainsString(result[0].input_variables, "in2_var")); + EXPECT_TRUE(ContainsString(result[0].input_variables, "test_in_var")); + EXPECT_TRUE(ContainsString(result[0].input_variables, "test_in2_var")); ASSERT_EQ(2u, result[0].output_variables.size()); - EXPECT_TRUE(ContainsString(result[0].output_variables, "out_var")); - EXPECT_TRUE(ContainsString(result[0].output_variables, "out2_var")); + EXPECT_TRUE(ContainsString(result[0].output_variables, "test_out_var")); + EXPECT_TRUE(ContainsString(result[0].output_variables, "test_out2_var")); } TEST_F(InspectorGetEntryPointTest, MultipleEntryPointsInOutVariables) { @@ -997,26 +994,24 @@ TEST_F(InspectorGetEntryPointTest, MultipleEntryPointsInOutVariables) { ASSERT_TRUE(td()->Determine()) << td()->error(); - // TODO(dsinclair): Update to run the namer transform when available. - auto result = inspector()->GetEntryPoints(); ASSERT_FALSE(inspector()->has_error()) << inspector()->error(); ASSERT_EQ(2u, result.size()); ASSERT_EQ("foo", result[0].name); - ASSERT_EQ("foo", result[0].remapped_name); + ASSERT_EQ("test_foo", result[0].remapped_name); ASSERT_EQ(1u, result[0].input_variables.size()); - EXPECT_EQ("in_var", result[0].input_variables[0]); + EXPECT_EQ("test_in_var", result[0].input_variables[0]); ASSERT_EQ(1u, result[0].output_variables.size()); - EXPECT_EQ("out2_var", result[0].output_variables[0]); + EXPECT_EQ("test_out2_var", result[0].output_variables[0]); ASSERT_EQ("bar", result[1].name); - ASSERT_EQ("bar", result[1].remapped_name); + ASSERT_EQ("test_bar", result[1].remapped_name); ASSERT_EQ(1u, result[1].input_variables.size()); - EXPECT_EQ("in2_var", result[1].input_variables[0]); + EXPECT_EQ("test_in2_var", result[1].input_variables[0]); ASSERT_EQ(1u, result[1].output_variables.size()); - EXPECT_EQ("out_var", result[1].output_variables[0]); + EXPECT_EQ("test_out_var", result[1].output_variables[0]); } TEST_F(InspectorGetEntryPointTest, MultipleEntryPointsSharedInOutVariables) { @@ -1042,42 +1037,36 @@ TEST_F(InspectorGetEntryPointTest, MultipleEntryPointsSharedInOutVariables) { ASSERT_TRUE(td()->Determine()) << td()->error(); - // TODO(dsinclair): Update to run the namer transform when available. - auto result = inspector()->GetEntryPoints(); ASSERT_FALSE(inspector()->has_error()) << inspector()->error(); ASSERT_EQ(2u, result.size()); ASSERT_EQ("foo", result[0].name); - ASSERT_EQ("foo", result[0].remapped_name); + ASSERT_EQ("test_foo", result[0].remapped_name); EXPECT_EQ(2u, result[0].input_variables.size()); - EXPECT_TRUE(ContainsString(result[0].input_variables, "in_var")); - EXPECT_TRUE(ContainsString(result[0].input_variables, "in2_var")); + EXPECT_TRUE(ContainsString(result[0].input_variables, "test_in_var")); + EXPECT_TRUE(ContainsString(result[0].input_variables, "test_in2_var")); EXPECT_EQ(2u, result[0].output_variables.size()); - EXPECT_TRUE(ContainsString(result[0].output_variables, "out_var")); - EXPECT_TRUE(ContainsString(result[0].output_variables, "out2_var")); + EXPECT_TRUE(ContainsString(result[0].output_variables, "test_out_var")); + EXPECT_TRUE(ContainsString(result[0].output_variables, "test_out2_var")); ASSERT_EQ("bar", result[1].name); - ASSERT_EQ("bar", result[1].remapped_name); + ASSERT_EQ("test_bar", result[1].remapped_name); EXPECT_EQ(1u, result[1].input_variables.size()); - EXPECT_EQ("in2_var", result[1].input_variables[0]); + EXPECT_EQ("test_in2_var", result[1].input_variables[0]); EXPECT_EQ(1u, result[1].output_variables.size()); - EXPECT_EQ("out2_var", result[1].output_variables[0]); + EXPECT_EQ("test_out2_var", result[1].output_variables[0]); } -// TODO(rharrison): Reenable once GetRemappedNameForEntryPoint isn't a pass -// through -TEST_F(InspectorGetRemappedNameForEntryPointTest, DISABLED_NoFunctions) { +TEST_F(InspectorGetRemappedNameForEntryPointTest, NoFunctions) { auto result = inspector()->GetRemappedNameForEntryPoint("foo"); ASSERT_TRUE(inspector()->has_error()); EXPECT_EQ("", result); } -// TODO(rharrison): Reenable once GetRemappedNameForEntryPoint isn't a pass -// through -TEST_F(InspectorGetRemappedNameForEntryPointTest, DISABLED_NoEntryPoints) { +TEST_F(InspectorGetRemappedNameForEntryPointTest, NoEntryPoints) { mod->AddFunction(MakeEmptyBodyFunction("foo", {})); auto result = inspector()->GetRemappedNameForEntryPoint("foo"); @@ -1086,35 +1075,26 @@ TEST_F(InspectorGetRemappedNameForEntryPointTest, DISABLED_NoEntryPoints) { EXPECT_EQ("", result); } -// TODO(rharrison): Reenable once GetRemappedNameForEntryPoint isn't a pass -// through -TEST_F(InspectorGetRemappedNameForEntryPointTest, DISABLED_OneEntryPoint) { +TEST_F(InspectorGetRemappedNameForEntryPointTest, OneEntryPoint) { auto* foo = MakeEmptyBodyFunction( "foo", ast::FunctionDecorationList{ create(ast::PipelineStage::kVertex), }); mod->AddFunction(foo); - // TODO(dsinclair): Update to run the namer transform when available. - auto result = inspector()->GetRemappedNameForEntryPoint("foo"); ASSERT_FALSE(inspector()->has_error()) << inspector()->error(); - EXPECT_EQ("foo", result); + EXPECT_EQ("test_foo", result); } -// TODO(rharrison): Reenable once GetRemappedNameForEntryPoint isn't a pass -// through -TEST_F(InspectorGetRemappedNameForEntryPointTest, - DISABLED_MultipleEntryPoints) { +TEST_F(InspectorGetRemappedNameForEntryPointTest, MultipleEntryPoints) { auto* foo = MakeEmptyBodyFunction( "foo", ast::FunctionDecorationList{ create(ast::PipelineStage::kVertex), }); mod->AddFunction(foo); - // TODO(dsinclair): Update to run the namer transform when available. - auto* bar = MakeEmptyBodyFunction( "bar", ast::FunctionDecorationList{ create(ast::PipelineStage::kCompute), @@ -1124,12 +1104,12 @@ TEST_F(InspectorGetRemappedNameForEntryPointTest, { auto result = inspector()->GetRemappedNameForEntryPoint("foo"); ASSERT_FALSE(inspector()->has_error()) << inspector()->error(); - EXPECT_EQ("foo", result); + EXPECT_EQ("test_foo", result); } { auto result = inspector()->GetRemappedNameForEntryPoint("bar"); ASSERT_FALSE(inspector()->has_error()) << inspector()->error(); - EXPECT_EQ("bar", result); + EXPECT_EQ("test_bar", result); } }