From 2c953eca48d89d696c5dbe321bec02ced4d2911b Mon Sep 17 00:00:00 2001 From: David Neto Date: Tue, 27 Apr 2021 14:39:47 +0000 Subject: [PATCH] spirv-reader: SaveName -> Register, add RegisterWithoutId Change-Id: Iab2eaa3b2d13a3cbb8f6bca01ed79be14dedbfd1 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49141 Commit-Queue: David Neto Auto-Submit: David Neto Reviewed-by: Ben Clayton --- src/reader/spirv/namer.cc | 21 ++++++++-- src/reader/spirv/namer.h | 8 +++- src/reader/spirv/namer_test.cc | 77 ++++++++++++++++++++++++---------- 3 files changed, 80 insertions(+), 26 deletions(-) diff --git a/src/reader/spirv/namer.cc b/src/reader/spirv/namer.cc index 9c5385f754..67450d1c90 100644 --- a/src/reader/spirv/namer.cc +++ b/src/reader/spirv/namer.cc @@ -18,6 +18,8 @@ #include #include +#include "src/debug.h" + namespace tint { namespace reader { namespace spirv { @@ -120,28 +122,39 @@ std::string Namer::FindUnusedDerivedName(const std::string& base_name) const { std::string Namer::MakeDerivedName(const std::string& base_name) { auto result = FindUnusedDerivedName(base_name); - // Register it. - name_to_id_[result] = 0; + const bool registered = RegisterWithoutId(result); + TINT_ASSERT(registered); return result; } -bool Namer::SaveName(uint32_t id, const std::string& name) { +bool Namer::Register(uint32_t id, const std::string& name) { if (HasName(id)) { return Fail() << "internal error: ID " << id << " already has registered name: " << id_to_name_[id]; } + if (!RegisterWithoutId(name)) { + return false; + } id_to_name_[id] = name; name_to_id_[name] = id; return true; } +bool Namer::RegisterWithoutId(const std::string& name) { + if (IsRegistered(name)) { + return Fail() << "internal error: name already registered: " << name; + } + name_to_id_[name] = 0; + return true; +} + bool Namer::SuggestSanitizedName(uint32_t id, const std::string& suggested_name) { if (HasName(id)) { return false; } - return SaveName(id, FindUnusedDerivedName(Sanitize(suggested_name))); + return Register(id, FindUnusedDerivedName(Sanitize(suggested_name))); } bool Namer::SuggestSanitizedMemberName(uint32_t struct_id, diff --git a/src/reader/spirv/namer.h b/src/reader/spirv/namer.h index 5957ae543b..e4ee8c4606 100644 --- a/src/reader/spirv/namer.h +++ b/src/reader/spirv/namer.h @@ -106,7 +106,13 @@ class Namer { /// @param id the SPIR-V ID /// @param name the name to map to the ID /// @returns true if the ID did not have a previously registered name. - bool SaveName(uint32_t id, const std::string& name); + bool Register(uint32_t id, const std::string& name); + + /// Registers a name, but not associated to any ID. Fails if the name + /// was already registered. + /// @param name the name to register + /// @returns true if the name was not already reegistered. + bool RegisterWithoutId(const std::string& name); /// Saves a sanitized name for the given ID, if that ID does not yet /// have a registered name, and if the sanitized name has not already diff --git a/src/reader/spirv/namer_test.cc b/src/reader/spirv/namer_test.cc index 52e38e1205..79f271b4d4 100644 --- a/src/reader/spirv/namer_test.cc +++ b/src/reader/spirv/namer_test.cc @@ -87,15 +87,15 @@ TEST_F(SpvNamerTest, FindUnusedDerivedName_NoRecordedName) { TEST_F(SpvNamerTest, FindUnusedDerivedName_HasRecordedName) { Namer namer(fail_stream_); - namer.SaveName(12, "rigby"); + namer.Register(12, "rigby"); EXPECT_THAT(namer.FindUnusedDerivedName("rigby"), Eq("rigby_1")); } TEST_F(SpvNamerTest, FindUnusedDerivedName_HasMultipleConflicts) { Namer namer(fail_stream_); - namer.SaveName(12, "rigby"); - namer.SaveName(13, "rigby_1"); - namer.SaveName(14, "rigby_3"); + namer.Register(12, "rigby"); + namer.Register(13, "rigby_1"); + namer.Register(14, "rigby_3"); // It picks the first non-conflicting suffix. EXPECT_THAT(namer.FindUnusedDerivedName("rigby"), Eq("rigby_2")); } @@ -107,7 +107,7 @@ TEST_F(SpvNamerTest, IsRegistered_NoRecordedName) { TEST_F(SpvNamerTest, IsRegistered_RegisteredById) { Namer namer(fail_stream_); - namer.SaveName(1, "abbey"); + namer.Register(1, "abbey"); EXPECT_TRUE(namer.IsRegistered("abbey")); } @@ -127,25 +127,60 @@ TEST_F(SpvNamerTest, MakeDerivedName_NoRecordedName) { TEST_F(SpvNamerTest, MakeDerivedName_HasRecordedName) { Namer namer(fail_stream_); - namer.SaveName(12, "rigby"); + namer.Register(12, "rigby"); EXPECT_THAT(namer.MakeDerivedName("rigby"), Eq("rigby_1")); } TEST_F(SpvNamerTest, MakeDerivedName_HasMultipleConflicts) { Namer namer(fail_stream_); - namer.SaveName(12, "rigby"); - namer.SaveName(13, "rigby_1"); - namer.SaveName(14, "rigby_3"); + namer.Register(12, "rigby"); + namer.Register(13, "rigby_1"); + namer.Register(14, "rigby_3"); // It picks the first non-conflicting suffix. EXPECT_THAT(namer.MakeDerivedName("rigby"), Eq("rigby_2")); } -TEST_F(SpvNamerTest, SaveNameOnce) { +TEST_F(SpvNamerTest, RegisterWithoutId_Once) { + Namer namer(fail_stream_); + + const std::string n("abbey"); + EXPECT_FALSE(namer.IsRegistered(n)); + EXPECT_TRUE(namer.RegisterWithoutId(n)); + EXPECT_TRUE(namer.IsRegistered(n)); + EXPECT_TRUE(success_); + EXPECT_TRUE(error().empty()); +} + +TEST_F(SpvNamerTest, RegisterWithoutId_Twice) { + Namer namer(fail_stream_); + + const std::string n("abbey"); + EXPECT_FALSE(namer.IsRegistered(n)); + EXPECT_TRUE(namer.RegisterWithoutId(n)); + // Fails on second attempt. + EXPECT_FALSE(namer.RegisterWithoutId(n)); + EXPECT_FALSE(success_); + EXPECT_EQ(error(),"internal error: name already registered: abbey"); +} + +TEST_F(SpvNamerTest, RegisterWithoutId_ConflictsWithIdRegisteredName) { + Namer namer(fail_stream_); + + const std::string n("abbey"); + EXPECT_TRUE(namer.Register(1,n)); + EXPECT_TRUE(namer.IsRegistered(n)); + // Fails on attempt to register without ID. + EXPECT_FALSE(namer.RegisterWithoutId(n)); + EXPECT_FALSE(success_); + EXPECT_EQ(error(),"internal error: name already registered: abbey"); +} + +TEST_F(SpvNamerTest, Register_Once) { Namer namer(fail_stream_); const uint32_t id = 9; EXPECT_FALSE(namer.HasName(id)); - const bool save_result = namer.SaveName(id, "abbey road"); + const bool save_result = namer.Register(id, "abbey road"); EXPECT_TRUE(save_result); EXPECT_TRUE(namer.HasName(id)); EXPECT_EQ(namer.GetName(id), "abbey road"); @@ -153,13 +188,13 @@ TEST_F(SpvNamerTest, SaveNameOnce) { EXPECT_TRUE(error().empty()); } -TEST_F(SpvNamerTest, SaveNameTwoIds) { +TEST_F(SpvNamerTest, Register_TwoIds) { Namer namer(fail_stream_); EXPECT_FALSE(namer.HasName(8)); EXPECT_FALSE(namer.HasName(9)); - EXPECT_TRUE(namer.SaveName(8, "abbey road")); - EXPECT_TRUE(namer.SaveName(9, "rubber soul")); + EXPECT_TRUE(namer.Register(8, "abbey road")); + EXPECT_TRUE(namer.Register(9, "rubber soul")); EXPECT_TRUE(namer.HasName(8)); EXPECT_TRUE(namer.HasName(9)); EXPECT_EQ(namer.GetName(9), "rubber soul"); @@ -168,12 +203,12 @@ TEST_F(SpvNamerTest, SaveNameTwoIds) { EXPECT_TRUE(error().empty()); } -TEST_F(SpvNamerTest, SaveNameFailsDueToIdReuse) { +TEST_F(SpvNamerTest, Register_FailsDueToIdReuse) { Namer namer(fail_stream_); const uint32_t id = 9; - EXPECT_TRUE(namer.SaveName(id, "abbey road")); - EXPECT_FALSE(namer.SaveName(id, "rubber soul")); + EXPECT_TRUE(namer.Register(id, "abbey road")); + EXPECT_FALSE(namer.Register(id, "rubber soul")); EXPECT_TRUE(namer.HasName(id)); EXPECT_EQ(namer.GetName(id), "abbey road"); EXPECT_FALSE(success_); @@ -191,7 +226,7 @@ TEST_F(SpvNamerTest, SuggestSanitizedName_RejectSuggestionWhenConflictOnSameId) { Namer namer(fail_stream_); - namer.SaveName(1, "lennon"); + namer.Register(1, "lennon"); EXPECT_FALSE(namer.SuggestSanitizedName(1, "mccartney")); EXPECT_THAT(namer.GetName(1), Eq("lennon")); } @@ -207,7 +242,7 @@ TEST_F(SpvNamerTest, SuggestSanitizedName_GenerateNewNameWhenConflictOnDifferentId) { Namer namer(fail_stream_); - namer.SaveName(7, "rice"); + namer.Register(7, "rice"); EXPECT_TRUE(namer.SuggestSanitizedName(9, "rice")); EXPECT_THAT(namer.GetName(9), Eq("rice_1")); } @@ -260,13 +295,13 @@ TEST_F(SpvNamerTest, Name_GeneratesNameIfNoneRegistered) { TEST_F(SpvNamerTest, Name_GeneratesNameWithoutConflict) { Namer namer(fail_stream_); - namer.SaveName(42, "x_14"); + namer.Register(42, "x_14"); EXPECT_THAT(namer.Name(14), Eq("x_14_1")); } TEST_F(SpvNamerTest, Name_ReturnsRegisteredName) { Namer namer(fail_stream_); - namer.SaveName(14, "hello"); + namer.Register(14, "hello"); EXPECT_THAT(namer.Name(14), Eq("hello")); }