Update namer to use symbol table.

This Cl updates the various namer objects to work off the symbol table
instead of names.

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

src/ast/module.cc

@@ -81,6 +81,10 @@ Symbol Module::RegisterSymbol(const std::string& name) {
   return symbol_table_.Register(name);
 }
 
+std::string Module::SymbolToName(const Symbol sym) const {
+  return symbol_table_.NameFor(sym);
+}
+
 bool Module::IsValid() const {
   for (auto* var : global_variables_) {
     if (var == nullptr || !var->IsValid()) {

src/ast/module.h

@@ -169,6 +169,11 @@ class Module {
   /// previously generated symbol will be returned.
   Symbol RegisterSymbol(const std::string& name);
 
+  /// Returns the `name` for `sym`
+  /// @param sym the symbol to retrieve the name for
+  /// @returns the use provided `name` for the symbol or "" if not found
+  std::string SymbolToName(const Symbol sym) const;
+
  private:
   Module(const Module&) = delete;
 

src/namer.cc

@@ -18,46 +18,44 @@
 #include <iomanip>
 #include <sstream>
 
+#include "src/symbol.h"
+
 namespace tint {
 
-Namer::Namer() = default;
+Namer::Namer(ast::Module* mod) : module_(mod) {}
 
 Namer::~Namer() = default;
 
-bool Namer::IsMapped(const std::string& name) {
-  auto it = name_map_.find(name);
-  return it != name_map_.end();
+bool Namer::IsUsed(const std::string& name) {
+  auto it = used_.find(name);
+  return it != used_.end();
 }
 
-HashingNamer::HashingNamer() = default;
-
-HashingNamer::~HashingNamer() = default;
-
-std::string HashingNamer::NameFor(const std::string& name) {
-  auto it = name_map_.find(name);
-  if (it != name_map_.end()) {
-    return it->second;
+std::string Namer::GenerateName(const std::string& prefix) {
+  std::string name = prefix;
+  uint32_t i = 0;
+  while (IsUsed(name)) {
+    name = prefix + "_" + std::to_string(i);
+    ++i;
   }
-
-  std::stringstream ret_name;
-  ret_name << "tint_";
-
-  ret_name << std::hex << std::setfill('0') << std::setw(2);
-  for (size_t i = 0; i < name.size(); ++i) {
-    ret_name << static_cast<uint32_t>(name[i]);
-  }
-
-  name_map_[name] = ret_name.str();
-  return ret_name.str();
-}
-
-NoopNamer::NoopNamer() = default;
-
-NoopNamer::~NoopNamer() = default;
-
-std::string NoopNamer::NameFor(const std::string& name) {
-  name_map_[name] = name;
+  used_.insert(name);
   return name;
 }
 
+MangleNamer::MangleNamer(ast::Module* mod) : Namer(mod) {}
+
+MangleNamer::~MangleNamer() = default;
+
+std::string MangleNamer::NameFor(const Symbol& sym) {
+  return sym.to_str();
+}
+
+UnsafeNamer::UnsafeNamer(ast::Module* mod) : Namer(mod) {}
+
+UnsafeNamer::~UnsafeNamer() = default;
+
+std::string UnsafeNamer::NameFor(const Symbol& sym) {
+  return module_->SymbolToName(sym);
+}
+
 }  // namespace tint

src/namer.h

@@ -19,52 +19,73 @@
 #include <unordered_map>
 #include <unordered_set>
 
+#include "src/ast/module.h"
+
 namespace tint {
 
 /// Base class for the namers.
 class Namer {
  public:
   /// Constructor
-  Namer();
+  /// @param mod the module this namer works with
+  explicit Namer(ast::Module* mod);
+  /// Destructor
   virtual ~Namer();
 
-  /// Returns a sanitized version of `name`
-  /// @param name the name to sanitize
-  /// @returns the sanitized version of `name`
-  virtual std::string NameFor(const std::string& name) = 0;
+  /// Returns the name for `sym`
+  /// @param sym the symbol to retrieve the name for
+  /// @returns the sanitized version of `name` or "" if not found
+  virtual std::string NameFor(const Symbol& sym) = 0;
 
-  /// Returns if the given name has been mapped already
-  /// @param name the name to check
-  /// @returns true if the name has been mapped
-  bool IsMapped(const std::string& name);
+  /// Generates a unique name for `prefix`
+  /// @param prefix the prefix name
+  /// @returns the unique name string
+  std::string GenerateName(const std::string& prefix);
 
  protected:
-  /// Map of original name to new name.
-  std::unordered_map<std::string, std::string> name_map_;
+  /// Checks if `name` has been used
+  /// @param name the name to check
+  /// @returns true if `name` has already been used
+  bool IsUsed(const std::string& name);
+
+  /// The module storing the symbol table
+  ast::Module* module_ = nullptr;
+
+ private:
+  // The list of names taken by the remapper
+  std::unordered_set<std::string> used_;
 };
 
-/// A namer class which hashes the name
-class HashingNamer : public Namer {
+/// A namer class which mangles the name
+class MangleNamer : public Namer {
  public:
-  HashingNamer();
-  ~HashingNamer() override;
+  /// Constructor
+  /// @param mod the module to retrieve names from
+  explicit MangleNamer(ast::Module* mod);
+  /// Destructor
+  ~MangleNamer() override;
 
-  /// Returns a sanitized version of `name`
-  /// @param name the name to sanitize
-  /// @returns the sanitized version of `name`
-  std::string NameFor(const std::string& name) override;
+  /// Returns a mangled name for `sym`
+  /// @param sym the symbol to name
+  /// @returns the name for `sym` or "" if not found
+  std::string NameFor(const Symbol& sym) override;
 };
 
-/// A namer which just returns the provided string
-class NoopNamer : public Namer {
+/// A namer which returns the user provided name. This is unsafe in general as
+/// it passes user provided data through to the backend compiler. It is useful
+/// for development and debugging.
+class UnsafeNamer : public Namer {
  public:
-  NoopNamer();
-  ~NoopNamer() override;
+  /// Constructor
+  /// @param mod the module to retrieve names from
+  explicit UnsafeNamer(ast::Module* mod);
+  /// Destructor
+  ~UnsafeNamer() override;
 
   /// Returns `name`
-  /// @param name the name
-  /// @returns `name`
-  std::string NameFor(const std::string& name) override;
+  /// @param sym the symbol
+  /// @returns `name` or "" if not found
+  std::string NameFor(const Symbol& sym) override;
 };
 
 }  // namespace tint

src/namer_test.cc

@@ -15,50 +15,49 @@
 #include "src/namer.h"
 
 #include "gtest/gtest.h"
+#include "src/ast/module.h"
 
 namespace tint {
 namespace {
 
-using Namer_HashingNamer_Test = testing::Test;
+using NamerTest = testing::Test;
 
-TEST_F(Namer_HashingNamer_Test, ReturnsName) {
-  HashingNamer n;
-  EXPECT_EQ("tint_6d795f6e616d65", n.NameFor("my_name"));
+TEST_F(NamerTest, GenerateName) {
+  ast::Module m;
+  MangleNamer n(&m);
+  EXPECT_EQ("name", n.GenerateName("name"));
+  EXPECT_EQ("name_0", n.GenerateName("name"));
+  EXPECT_EQ("name_1", n.GenerateName("name"));
 }
 
-TEST_F(Namer_HashingNamer_Test, ReturnsSameValueForSameName) {
-  HashingNamer n;
-  EXPECT_EQ("tint_6e616d6531", n.NameFor("name1"));
-  EXPECT_EQ("tint_6e616d6532", n.NameFor("name2"));
-  EXPECT_EQ("tint_6e616d6531", n.NameFor("name1"));
+using MangleNamerTest = testing::Test;
+
+TEST_F(MangleNamerTest, ReturnsName) {
+  ast::Module m;
+  auto s = m.RegisterSymbol("my_sym");
+
+  MangleNamer n(&m);
+  EXPECT_EQ("tint_symbol_1", n.NameFor(s));
 }
 
-TEST_F(Namer_HashingNamer_Test, IsMapped) {
-  HashingNamer n;
-  EXPECT_FALSE(n.IsMapped("my_name"));
-  EXPECT_EQ("tint_6d795f6e616d65", n.NameFor("my_name"));
-  EXPECT_TRUE(n.IsMapped("my_name"));
+TEST_F(MangleNamerTest, ReturnsSameValueForSameName) {
+  ast::Module m;
+  auto s1 = m.RegisterSymbol("my_sym");
+  auto s2 = m.RegisterSymbol("my_sym2");
+
+  MangleNamer n(&m);
+  EXPECT_EQ("tint_symbol_1", n.NameFor(s1));
+  EXPECT_EQ("tint_symbol_2", n.NameFor(s2));
+  EXPECT_EQ("tint_symbol_1", n.NameFor(s1));
 }
 
-using Namer_NoopNamer_Test = testing::Test;
+using UnsafeNamerTest = testing::Test;
+TEST_F(UnsafeNamerTest, ReturnsName) {
+  ast::Module m;
+  auto s = m.RegisterSymbol("my_sym");
 
-TEST_F(Namer_NoopNamer_Test, ReturnsName) {
-  NoopNamer n;
-  EXPECT_EQ("my_name", n.NameFor("my_name"));
-}
-
-TEST_F(Namer_NoopNamer_Test, ReturnsSameValueForSameName) {
-  NoopNamer n;
-  EXPECT_EQ("name1", n.NameFor("name1"));
-  EXPECT_EQ("name2", n.NameFor("name2"));
-  EXPECT_EQ("name1", n.NameFor("name1"));
-}
-
-TEST_F(Namer_NoopNamer_Test, IsMapped) {
-  NoopNamer n;
-  EXPECT_FALSE(n.IsMapped("my_name"));
-  EXPECT_EQ("my_name", n.NameFor("my_name"));
-  EXPECT_TRUE(n.IsMapped("my_name"));
+  UnsafeNamer n(&m);
+  EXPECT_EQ("my_sym", n.NameFor(s));
 }
 
 }  // namespace