transform/EmitVertexPointSize: Use SymbolTable::New()

And clean up some code in the process.

Avoids potential symbol collisions. Simplifies the logic.

Bug: tint:711
Bug: tint:712
Change-Id: I7c41dff81e1fb2abfd3f5d3fecf625a27c42f14d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47635
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>

src/transform/emit_vertex_point_size.cc

@@ -21,11 +21,6 @@
 
 namespace tint {
 namespace transform {
-namespace {
-
-const char kPointSizeVar[] = "tint_pointsize";
-
-}  // namespace
 
 EmitVertexPointSize::EmitVertexPointSize() = default;
 EmitVertexPointSize::~EmitVertexPointSize() = default;
@@ -37,38 +32,31 @@ Transform::Output EmitVertexPointSize::Run(const Program* in, const DataMap&) {
   }
 
   ProgramBuilder out;
-  auto* f32 = out.create<type::F32>();
+
+  CloneContext ctx(&out, in);
+
+  // Start by cloning all the symbols. This ensures that the authored symbols
+  // won't get renamed if they collide with new symbols below.
+  ctx.CloneSymbols();
+
+  Symbol pointsize = out.Symbols().New("tint_pointsize");
 
   // Declare the pointsize builtin output variable.
-  auto* pointsize_var = out.create<ast::Variable>(
+  out.Global(pointsize, out.ty.f32(), ast::StorageClass::kOutput, nullptr,
-      Source{},                               // source
+             ast::DecorationList{
-      out.Symbols().Register(kPointSizeVar),  // symbol
+                 out.create<ast::BuiltinDecoration>(ast::Builtin::kPointSize),
-      ast::StorageClass::kOutput,             // storage_class
+             });
-      f32,                                    // type
-      false,                                  // is_const
-      nullptr,                                // constructor
-      ast::DecorationList{
-          out.create<ast::BuiltinDecoration>(Source{},
-                                             ast::Builtin::kPointSize),
-      });
-  out.AST().AddGlobalVariable(pointsize_var);
 
   // Add the pointsize assignment statement to the front of all vertex stages.
-  CloneContext ctx(&out, in);
   ctx.ReplaceAll([&](ast::Function* func) -> ast::Function* {
     if (func->pipeline_stage() != ast::PipelineStage::kVertex) {
       return nullptr;  // Just clone func
     }
 
-    // Build the AST expression & statement for assigning pointsize one.
+    return CloneWithStatementsAtStart(&ctx, func,
-    auto* one = out.create<ast::ScalarConstructorExpression>(
+                                      {
-        Source{}, out.create<ast::FloatLiteral>(Source{}, f32, 1.0f));
+                                          out.Assign(pointsize, 1.0f),
-    auto* pointsize_ident = out.create<ast::IdentifierExpression>(
+                                      });
-        Source{}, out.Symbols().Register(kPointSizeVar));
-    auto* pointsize_assign =
-        out.create<ast::AssignmentStatement>(Source{}, pointsize_ident, one);
-
-    return CloneWithStatementsAtStart(&ctx, func, {pointsize_assign});
   });
   ctx.Clone();
 

src/transform/emit_vertex_point_size_test.cc

@@ -116,6 +116,29 @@ fn compute_entry() {
   EXPECT_EQ(expect, str(got));
 }
 
+TEST_F(EmitVertexPointSizeTest, AttemptSymbolCollision) {
+  auto* src = R"(
+[[stage(vertex)]]
+fn entry() {
+  var tint_pointsize : f32;
+}
+)";
+
+  auto* expect = R"(
+[[builtin(pointsize)]] var<out> tint_pointsize_1 : f32;
+
+[[stage(vertex)]]
+fn entry() {
+  tint_pointsize_1 = 1.0;
+  var tint_pointsize : f32;
+}
+)";
+
+  auto got = Run<EmitVertexPointSize>(src);
+
+  EXPECT_EQ(expect, str(got));
+}
+
 }  // namespace
 }  // namespace transform
 }  // namespace tint