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>
diff --git a/src/transform/emit_vertex_point_size.cc b/src/transform/emit_vertex_point_size.cc
index 944659f..c4e0464 100644
--- a/src/transform/emit_vertex_point_size.cc
+++ b/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 @@
}
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>(
- Source{}, // source
- out.Symbols().Register(kPointSizeVar), // symbol
- 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);
+ out.Global(pointsize, out.ty.f32(), ast::StorageClass::kOutput, nullptr,
+ ast::DecorationList{
+ out.create<ast::BuiltinDecoration>(ast::Builtin::kPointSize),
+ });
// 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.
- auto* one = out.create<ast::ScalarConstructorExpression>(
- Source{}, out.create<ast::FloatLiteral>(Source{}, f32, 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});
+ return CloneWithStatementsAtStart(&ctx, func,
+ {
+ out.Assign(pointsize, 1.0f),
+ });
});
ctx.Clone();
diff --git a/src/transform/emit_vertex_point_size_test.cc b/src/transform/emit_vertex_point_size_test.cc
index 7fb1b59..53b4d52 100644
--- a/src/transform/emit_vertex_point_size_test.cc
+++ b/src/transform/emit_vertex_point_size_test.cc
@@ -116,6 +116,29 @@
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
