spirv-reader: SaveName -> Register, add RegisterWithoutId
Change-Id: Iab2eaa3b2d13a3cbb8f6bca01ed79be14dedbfd1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49141
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/reader/spirv/namer.cc b/src/reader/spirv/namer.cc
index 9c5385f..67450d1 100644
--- a/src/reader/spirv/namer.cc
+++ b/src/reader/spirv/namer.cc
@@ -18,6 +18,8 @@
#include <sstream>
#include <unordered_set>
+#include "src/debug.h"
+
namespace tint {
namespace reader {
namespace spirv {
@@ -120,28 +122,39 @@
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 5957ae5..e4ee8c4 100644
--- a/src/reader/spirv/namer.h
+++ b/src/reader/spirv/namer.h
@@ -106,7 +106,13 @@
/// @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 52e38e1..79f271b 100644
--- a/src/reader/spirv/namer_test.cc
+++ b/src/reader/spirv/namer_test.cc
@@ -87,15 +87,15 @@
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_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_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 @@
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 @@
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 @@
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 @@
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_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"));
}
