From 6834a132c23366c49a3d62945a9786a63df415eb Mon Sep 17 00:00:00 2001
From: James Price <jrprice@google.com>
Date: Thu, 5 Dec 2024 17:21:38 +0000
Subject: [PATCH] [tint] Always perform requested renames in Renamer

The Renamer was not applying `requested_names` when only renaming
keywords. Fix the `should_rename` check to consider `requested_names`.

This is required in order for Dawn to work with symbol naming disabled
when it is using `requested_names` to set the entry point name.

Bug: 375192379
Change-Id: I188608872dfedfda02a807137c694289e1e78a04
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/218138
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
---
 src/tint/lang/wgsl/ast/transform/renamer.cc   |  3 +
 .../lang/wgsl/ast/transform/renamer_test.cc   | 76 ++++++++++++++++++-
 2 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/src/tint/lang/wgsl/ast/transform/renamer.cc b/src/tint/lang/wgsl/ast/transform/renamer.cc
index 19e6072870..414721c773 100644
--- a/src/tint/lang/wgsl/ast/transform/renamer.cc
+++ b/src/tint/lang/wgsl/ast/transform/renamer.cc
@@ -1364,6 +1364,9 @@ Transform::ApplyResult Renamer::Apply(const Program& src,
         if (target == Target::kAll) {
             return true;
         }
+        if (requested_names->count(symbol.Name())) {
+            return true;
+        }
         auto name = symbol.Name();
         if (!tint::utf8::IsASCII(name)) {
             // name is non-ascii. All of the backend keywords are ascii, so rename.
diff --git a/src/tint/lang/wgsl/ast/transform/renamer_test.cc b/src/tint/lang/wgsl/ast/transform/renamer_test.cc
index 7fc648f25f..7c3f13d276 100644
--- a/src/tint/lang/wgsl/ast/transform/renamer_test.cc
+++ b/src/tint/lang/wgsl/ast/transform/renamer_test.cc
@@ -103,7 +103,7 @@ fn tint_symbol_2(@builtin(vertex_index) tint_symbol_1 : u32) -> @builtin(positio
     EXPECT_THAT(data->remappings, ContainerEq(expected_remappings));
 }
 
-TEST_F(RenamerTest, RequestedNames) {
+TEST_F(RenamerTest, RequestedNamesWithRenameAll) {
     auto* src = R"(
 struct ShaderIO {
     @location(1) var1: f32,
@@ -170,6 +170,80 @@ fn tint_symbol_2(@builtin(vertex_index) tint_symbol_3 : u32) -> tint_symbol {
     EXPECT_THAT(data->remappings, ContainerEq(expected_remappings));
 }
 
+using RenamerTestRequestedNamesWithoutRenameAll = TransformTestWithParam<Renamer::Target>;
+
+TEST_P(RenamerTestRequestedNamesWithoutRenameAll, RequestedNames) {
+    auto* src = R"(
+struct ShaderIO {
+    @location(1) var1: f32,
+    @location(3) @interpolate(flat) var3: u32,
+    @builtin(position) pos: vec4f,
+}
+
+@vertex fn entry_point(@builtin(vertex_index) vert_idx : u32)
+     -> ShaderIO {
+  var pos = array(
+      vec2f(-1.0, 3.0),
+      vec2f(-1.0, -3.0),
+      vec2f(3.0, 0.0));
+
+  var shaderIO: ShaderIO;
+  shaderIO.var1 = 0.0;
+  shaderIO.var3 = 1u;
+  shaderIO.pos = vec4f(pos[vert_idx], 0.0, 1.0);
+
+  return shaderIO;
+}
+)";
+
+    auto* expect = R"(
+struct ShaderIO {
+  @location(1)
+  user_var1 : f32,
+  @location(3) @interpolate(flat)
+  user_var3 : u32,
+  @builtin(position)
+  pos : vec4f,
+}
+
+@vertex
+fn entry_point(@builtin(vertex_index) vert_idx : u32) -> ShaderIO {
+  var pos = array(vec2f(-(1.0), 3.0), vec2f(-(1.0), -(3.0)), vec2f(3.0, 0.0));
+  var shaderIO : ShaderIO;
+  shaderIO.user_var1 = 0.0;
+  shaderIO.user_var3 = 1u;
+  shaderIO.pos = vec4f(pos[vert_idx], 0.0, 1.0);
+  return shaderIO;
+}
+)";
+
+    DataMap inputs;
+    inputs.Add<Renamer::Config>(GetParam(),
+                                /* remappings */
+                                Renamer::Remappings{
+                                    {"var1", "user_var1"},
+                                    {"var3", "user_var3"},
+                                });
+    auto got = Run<Renamer>(src, inputs);
+
+    EXPECT_EQ(expect, str(got));
+
+    auto* data = got.data.Get<Renamer::Data>();
+
+    ASSERT_NE(data, nullptr);
+    Renamer::Remappings expected_remappings = {
+        {"var1", "user_var1"},
+        {"var3", "user_var3"},
+    };
+    EXPECT_THAT(data->remappings, ContainerEq(expected_remappings));
+}
+
+INSTANTIATE_TEST_SUITE_P(RenamerTestRequestedNamesWithoutRenameAll,
+                         RenamerTestRequestedNamesWithoutRenameAll,
+                         testing::Values(Renamer::Target::kGlslKeywords,
+                                         Renamer::Target::kHlslKeywords,
+                                         Renamer::Target::kMslKeywords));
+
 TEST_F(RenamerTest, PreserveSwizzles) {
     auto* src = R"(
 @vertex