Remove deprecated transform APIs

Change-Id: I330b52ed6b485690a64c74fe34bebfe02fb52598
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42265
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>

src/transform/first_index_offset.cc

@@ -115,15 +115,17 @@ Transform::Output FirstIndexOffset::Run(const Program* in) {
     }
   }
 
+  State state{&out, binding_, group_};
+
   Symbol vertex_index_sym;
   Symbol instance_index_sym;
 
   // Lazily construct the UniformBuffer on first call to
   // maybe_create_buffer_var()
   ast::Variable* buffer_var = nullptr;
-  auto maybe_create_buffer_var = [&](ProgramBuilder* dst) {
+  auto maybe_create_buffer_var = [&]() {
     if (buffer_var == nullptr) {
-      buffer_var = AddUniformBuffer(dst);
+      buffer_var = state.AddUniformBuffer();
     }
   };
 
@@ -138,13 +140,13 @@ Transform::Output FirstIndexOffset::Run(const Program* in) {
         ast::Builtin blt_type = blt_dec->value();
         if (blt_type == ast::Builtin::kVertexIndex) {
           vertex_index_sym = var->symbol();
-          has_vertex_index_ = true;
+          state.has_vertex_index = true;
           return clone_variable_with_new_name(
               &ctx, var,
               kIndexOffsetPrefix + in->Symbols().NameFor(var->symbol()));
         } else if (blt_type == ast::Builtin::kInstanceIndex) {
           instance_index_sym = var->symbol();
-          has_instance_index_ = true;
+          state.has_instance_index = true;
           return clone_variable_with_new_name(
               &ctx, var,
               kIndexOffsetPrefix + in->Symbols().NameFor(var->symbol()));
@@ -157,7 +159,7 @@ Transform::Output FirstIndexOffset::Run(const Program* in) {
                    // which determines the original builtin variable names,
                    // but this should be fine, as variables are cloned first.
       [&](ast::Function* func) -> ast::Function* {
-        maybe_create_buffer_var(ctx.dst);
+        maybe_create_buffer_var();
         if (buffer_var == nullptr) {
           return nullptr;  // no transform need, just clone func
         }
@@ -165,66 +167,48 @@ Transform::Output FirstIndexOffset::Run(const Program* in) {
         ast::StatementList statements;
         for (const auto& data : func_sem->LocalReferencedBuiltinVariables()) {
           if (data.second->value() == ast::Builtin::kVertexIndex) {
-            statements.emplace_back(
+            statements.emplace_back(state.CreateFirstIndexOffset(
-                CreateFirstIndexOffset(in->Symbols().NameFor(vertex_index_sym),
+                in->Symbols().NameFor(vertex_index_sym), kFirstVertexName,
-                                       kFirstVertexName, buffer_var, ctx.dst));
+                buffer_var));
           } else if (data.second->value() == ast::Builtin::kInstanceIndex) {
-            statements.emplace_back(CreateFirstIndexOffset(
+            statements.emplace_back(state.CreateFirstIndexOffset(
                 in->Symbols().NameFor(instance_index_sym), kFirstInstanceName,
-                buffer_var, ctx.dst));
+                buffer_var));
           }
         }
         return CloneWithStatementsAtStart(&ctx, func, statements);
       });
   ctx.Clone();
 
-  return Output(
+  return Output(Program(std::move(out)),
-      Program(std::move(out)),
+                std::make_unique<Data>(
-      std::make_unique<Data>(has_vertex_index_, has_instance_index_,
+                    state.has_vertex_index, state.has_instance_index,
-                             vertex_index_offset_, instance_index_offset_));
+                    state.vertex_index_offset, state.instance_index_offset));
 }
 
-bool FirstIndexOffset::HasVertexIndex() {
+ast::Variable* FirstIndexOffset::State::AddUniformBuffer() {
-  return has_vertex_index_;
-}
-
-bool FirstIndexOffset::HasInstanceIndex() {
-  return has_instance_index_;
-}
-
-uint32_t FirstIndexOffset::GetFirstVertexOffset() {
-  assert(has_vertex_index_);
-  return vertex_index_offset_;
-}
-
-uint32_t FirstIndexOffset::GetFirstInstanceOffset() {
-  assert(has_instance_index_);
-  return instance_index_offset_;
-}
-
-ast::Variable* FirstIndexOffset::AddUniformBuffer(ProgramBuilder* dst) {
   auto* u32_type = dst->create<type::U32>();
   ast::StructMemberList members;
   uint32_t offset = 0;
-  if (has_vertex_index_) {
+  if (has_vertex_index) {
     ast::StructMemberDecorationList member_dec;
     member_dec.push_back(
         dst->create<ast::StructMemberOffsetDecoration>(Source{}, offset));
     members.push_back(dst->create<ast::StructMember>(
         Source{}, dst->Symbols().Register(kFirstVertexName), u32_type,
         std::move(member_dec)));
-    vertex_index_offset_ = offset;
+    vertex_index_offset = offset;
     offset += 4;
   }
 
-  if (has_instance_index_) {
+  if (has_instance_index) {
     ast::StructMemberDecorationList member_dec;
     member_dec.push_back(
         dst->create<ast::StructMemberOffsetDecoration>(Source{}, offset));
     members.push_back(dst->create<ast::StructMember>(
         Source{}, dst->Symbols().Register(kFirstInstanceName), u32_type,
         std::move(member_dec)));
-    instance_index_offset_ = offset;
+    instance_index_offset = offset;
     offset += 4;
   }
 
@@ -243,8 +227,8 @@ ast::Variable* FirstIndexOffset::AddUniformBuffer(ProgramBuilder* dst) {
       false,                                 // is_const
       nullptr,                               // constructor
       ast::VariableDecorationList{
-          dst->create<ast::BindingDecoration>(Source{}, binding_),
+          dst->create<ast::BindingDecoration>(Source{}, binding),
-          dst->create<ast::GroupDecoration>(Source{}, group_),
+          dst->create<ast::GroupDecoration>(Source{}, group),
       });
 
   dst->AST().AddGlobalVariable(idx_var);
@@ -254,11 +238,10 @@ ast::Variable* FirstIndexOffset::AddUniformBuffer(ProgramBuilder* dst) {
   return idx_var;
 }
 
-ast::VariableDeclStatement* FirstIndexOffset::CreateFirstIndexOffset(
+ast::VariableDeclStatement* FirstIndexOffset::State::CreateFirstIndexOffset(
     const std::string& original_name,
     const std::string& field_name,
-    ast::Variable* buffer_var,
+    ast::Variable* buffer_var) {
-    ProgramBuilder* dst) {
   auto* buffer =
       dst->create<ast::IdentifierExpression>(Source{}, buffer_var->symbol());
 

src/transform/first_index_offset.h

@@ -101,45 +101,32 @@ class FirstIndexOffset : public Transform {
   /// @returns the transformation result
   Output Run(const Program* program) override;
 
-  /// [DEPRECATED] - Use Data
-  /// @returns whether shader uses vertex_index
-  bool HasVertexIndex();
-
-  /// [DEPRECATED] - Use Data
-  /// @returns whether shader uses instance_index
-  bool HasInstanceIndex();
-
-  /// [DEPRECATED] - Use Data
-  /// @returns offset of firstVertex into constant buffer
-  uint32_t GetFirstVertexOffset();
-
-  /// [DEPRECATED] - Use Data
-  /// @returns offset of firstInstance into constant buffer
-  uint32_t GetFirstInstanceOffset();
-
  private:
-  /// Adds uniform buffer with firstVertex/Instance to the program builder
+  struct State {
-  /// @returns variable of new uniform buffer
+    /// Adds uniform buffer with firstVertex/Instance to the program builder
-  ast::Variable* AddUniformBuffer(ProgramBuilder* builder);
+    /// @returns variable of new uniform buffer
-  /// Adds constant with modified original_name builtin to func
+    ast::Variable* AddUniformBuffer();
-  /// @param original_name the name of the original builtin used in function
+    /// Adds constant with modified original_name builtin to func
-  /// @param field_name name of field in firstVertex/Instance buffer
+    /// @param original_name the name of the original builtin used in function
-  /// @param buffer_var variable of firstVertex/Instance buffer
+    /// @param field_name name of field in firstVertex/Instance buffer
-  /// @param builder the target to contain the new ast nodes
+    /// @param buffer_var variable of firstVertex/Instance buffer
-  ast::VariableDeclStatement* CreateFirstIndexOffset(
+    ast::VariableDeclStatement* CreateFirstIndexOffset(
-      const std::string& original_name,
+        const std::string& original_name,
-      const std::string& field_name,
+        const std::string& field_name,
-      ast::Variable* buffer_var,
+        ast::Variable* buffer_var);
-      ProgramBuilder* builder);
+
+    ProgramBuilder* const dst;
+    uint32_t const binding;
+    uint32_t const group;
+
+    bool has_vertex_index = false;
+    bool has_instance_index = false;
+    uint32_t vertex_index_offset = 0;
+    uint32_t instance_index_offset = 0;
+  };
 
   uint32_t binding_;
   uint32_t group_;
-
-  bool has_vertex_index_ = false;
-  bool has_instance_index_ = false;
-
-  uint32_t vertex_index_offset_ = 0;
-  uint32_t instance_index_offset_ = 0;
 };
 }  // namespace transform
 }  // namespace tint

src/transform/transform.h

@@ -136,10 +136,6 @@ class Transform {
 
     /// Extra output generated by the transforms.
     DataMap data;
-
-    /// Diagnostics raised while running the Transform.
-    /// [DEPRECATED] Use `program.Diagnostics()`
-    diag::List diagnostics;
   };
 
   /// Runs the transform on `program`, returning the transformation result.

src/transform/vertex_pulling.cc

@@ -56,39 +56,13 @@ static const char kDefaultInstanceIndexName[] = "_tint_pulling_instance_index";
 
 }  // namespace
 
-VertexPulling::VertexPulling() = default;
+VertexPulling::VertexPulling(const Config& config) : cfg(config) {}
-
-VertexPulling::VertexPulling(const Config& config)
-    : cfg(config), vertex_state_set(true) {}
 
 VertexPulling::~VertexPulling() = default;
 
-void VertexPulling::SetVertexState(const VertexStateDescriptor& vertex_state) {
-  cfg.vertex_state = vertex_state;
-  vertex_state_set = true;
-}
-
-void VertexPulling::SetEntryPoint(std::string entry_point) {
-  cfg.entry_point_name = std::move(entry_point);
-}
-
-void VertexPulling::SetPullingBufferBindingGroup(uint32_t number) {
-  cfg.pulling_group = number;
-}
-
-void VertexPulling::SetPullingBufferBindingSet(uint32_t number) {
-  cfg.pulling_group = number;
-}
-
 Transform::Output VertexPulling::Run(const Program* in) {
   ProgramBuilder out;
 
-  // Check SetVertexState was called
-  if (!vertex_state_set) {
-    out.Diagnostics().add_error("SetVertexState not called");
-    return Output(Program(std::move(out)));
-  }
-
   // Find entry point
   auto* func = in->AST().Functions().Find(
       in->Symbols().Get(cfg.entry_point_name), ast::PipelineStage::kVertex);

src/transform/vertex_pulling.h

@@ -158,9 +158,6 @@ class VertexPulling : public Transform {
     uint32_t pulling_group = 4u;
   };
 
-  /// Constructor
-  VertexPulling();
-
   /// Constructor
   /// @param config the configuration options for the transform
   explicit VertexPulling(const Config& config);
@@ -168,25 +165,6 @@ class VertexPulling : public Transform {
   /// Destructor
   ~VertexPulling() override;
 
-  /// Sets the vertex state descriptor, containing info about attributes
-  /// [DEPRECATED] Use the VertexPulling(const Config&)
-  /// @param vertex_state the vertex state descriptor
-  void SetVertexState(const VertexStateDescriptor& vertex_state);
-  /// Sets the entry point to add assignments into
-  /// [DEPRECATED] Use the VertexPulling(const Config&)
-  /// @param entry_point the vertex stage entry point
-  void SetEntryPoint(std::string entry_point);
-  /// Sets the "set" we will put all our vertex buffers into (as storage
-  /// buffers)
-  /// [DEPRECATED] Use the VertexPulling(const Config&)
-  /// @param number the set number we will use
-  void SetPullingBufferBindingSet(uint32_t number);
-  /// Sets the "group" we will put all our vertex buffers into (as storage
-  /// buffers)
-  /// [DEPRECATED] Use the VertexPulling(const Config&)
-  /// @param number the group number we will use
-  void SetPullingBufferBindingGroup(uint32_t number);
-
   /// Runs the transform on `program`, returning the transformation result.
   /// @param program the source program to transform
   /// @returns the transformation result
@@ -194,7 +172,6 @@ class VertexPulling : public Transform {
 
  private:
   Config cfg;
-  bool vertex_state_set = false;
 
   struct State {
     State(CloneContext& ctx, const Config& c);

src/transform/vertex_pulling_test.cc

@@ -24,19 +24,6 @@ namespace {
 
 using VertexPullingTest = TransformTest;
 
-TEST_F(VertexPullingTest, Error_NoVertexState) {
-  auto* src = R"(
-[[stage(vertex)]]
-fn main() -> void {}
-)";
-
-  auto* expect = "error: SetVertexState not called";
-
-  auto got = Transform<VertexPulling>(src);
-
-  EXPECT_EQ(expect, str(got));
-}
-
 TEST_F(VertexPullingTest, Error_NoEntryPoint) {
   auto* src = "";