Add reader::spv::ResolveMemberNamesForStruct

Bug: tint:3
Change-Id: If778e98416c19c518261c68d4afdfdc99724967f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/17440
Reviewed-by: dan sinclair <dsinclair@google.com>

src/reader/spirv/namer.cc

@@ -16,6 +16,7 @@
 
 #include <algorithm>
 #include <sstream>
+#include <unordered_set>
 
 namespace tint {
 namespace reader {
@@ -113,6 +114,64 @@ bool Namer::SuggestSanitizedMemberName(uint32_t struct_id,
   return false;
 }
 
+void Namer::ResolveMemberNamesForStruct(uint32_t struct_id,
+                                        uint32_t num_members) {
+  auto& name_vector = struct_member_names_[struct_id];
+  // Resizing will set new entries to the empty string.
+  // It would have been an error if the client had registered a name for
+  // an out-of-bounds member index, so toss those away.
+  name_vector.resize(num_members);
+
+  std::unordered_set<std::string> used_names;
+
+  // Returns a name, based on the suggestion, which does not equal
+  // any name in the used_names set.
+  auto disambiguate_name =
+      [&used_names](const std::string& suggestion) -> std::string {
+    if (used_names.find(suggestion) == used_names.end()) {
+      // There is no collision.
+      return suggestion;
+    }
+
+    uint32_t i = 1;
+    std::string new_name;
+    do {
+      std::stringstream new_name_stream;
+      new_name_stream << suggestion << "_" << i;
+      new_name = new_name_stream.str();
+      ++i;
+    } while (used_names.find(new_name) != used_names.end());
+    return new_name;
+  };
+
+  // First ensure uniqueness among names for which we have already taken
+  // suggestions.
+  for (auto& name : name_vector) {
+    if (!name.empty()) {
+      // This modifies the names in-place, i.e. update the name_vector
+      // entries.
+      name = disambiguate_name(name);
+      used_names.insert(name);
+    }
+  }
+
+  // Now ensure uniqueness among the rest.  Doing this in a second pass
+  // allows us to preserve suggestions as much as possible.  Otherwise
+  // a generated name such as 'field1' might collide with a user-suggested
+  // name of 'field1' attached to a later member.
+  uint32_t index = 0;
+  for (auto& name : name_vector) {
+    if (name.empty()) {
+      std::stringstream suggestion;
+      suggestion << "field" << index;
+      // Again, modify the name-vector in-place.
+      name = disambiguate_name(suggestion.str());
+      used_names.insert(name);
+    }
+    index++;
+  }
+}
+
 }  // namespace spirv
 }  // namespace reader
 }  // namespace tint

src/reader/spirv/namer.h

@@ -95,14 +95,22 @@ class Namer {
 
   /// Saves a sanitized name for a member of a struct, if that member
   /// does not yet have a registered name.
-  /// @param id the SPIR-V ID for the struct
+  /// @param struct_id the SPIR-V ID for the struct
   /// @param member_index the index of the member inside the struct
   /// @param suggested_name the suggested name
   /// @returns true if a name was newly registered
-  bool SuggestSanitizedMemberName(uint32_t id,
+  bool SuggestSanitizedMemberName(uint32_t struct_id,
                                   uint32_t member_index,
                                   const std::string& suggested_name);
 
+  /// Ensure there are member names registered for members of the given struct
+  /// such that:
+  /// - Each member has a non-empty sanitized name.
+  /// - No two members in the struct have the same name.
+  /// @param struct_id the SPIR-V ID for the struct
+  /// @param num_members the number of members in the struct
+  void ResolveMemberNamesForStruct(uint32_t struct_id, uint32_t num_members);
+
  private:
   FailStream fail_stream_;
 

src/reader/spirv/namer_test.cc

@@ -216,6 +216,57 @@ TEST_F(SpvNamerTest,
   EXPECT_THAT(namer.GetMemberName(1, 2), Eq("mother"));
 }
 
+TEST_F(SpvNamerTest,
+       ResolveMemberNamesForStruct_GeneratesRegularNamesOnItsOwn) {
+  Namer namer(fail_stream_);
+  namer.ResolveMemberNamesForStruct(2, 4);
+  EXPECT_THAT(namer.GetMemberName(2, 0), Eq("field0"));
+  EXPECT_THAT(namer.GetMemberName(2, 1), Eq("field1"));
+  EXPECT_THAT(namer.GetMemberName(2, 2), Eq("field2"));
+  EXPECT_THAT(namer.GetMemberName(2, 3), Eq("field3"));
+}
+
+TEST_F(SpvNamerTest,
+       ResolveMemberNamesForStruct_ResolvesConflictBetweenSuggestedNames) {
+  Namer namer(fail_stream_);
+  namer.SuggestSanitizedMemberName(2, 0, "apple");
+  namer.SuggestSanitizedMemberName(2, 1, "apple");
+  namer.ResolveMemberNamesForStruct(2, 2);
+  EXPECT_THAT(namer.GetMemberName(2, 0), Eq("apple"));
+  EXPECT_THAT(namer.GetMemberName(2, 1), Eq("apple_1"));
+}
+
+TEST_F(SpvNamerTest, ResolveMemberNamesForStruct_FillsUnsuggestedGaps) {
+  Namer namer(fail_stream_);
+  namer.SuggestSanitizedMemberName(2, 1, "apple");
+  namer.SuggestSanitizedMemberName(2, 2, "core");
+  namer.ResolveMemberNamesForStruct(2, 4);
+  EXPECT_THAT(namer.GetMemberName(2, 0), Eq("field0"));
+  EXPECT_THAT(namer.GetMemberName(2, 1), Eq("apple"));
+  EXPECT_THAT(namer.GetMemberName(2, 2), Eq("core"));
+  EXPECT_THAT(namer.GetMemberName(2, 3), Eq("field3"));
+}
+
+TEST_F(SpvNamerTest,
+       ResolveMemberNamesForStruct_GeneratedNameAvoidsConflictWithSuggestion) {
+  Namer namer(fail_stream_);
+  namer.SuggestSanitizedMemberName(2, 0, "field1");
+  namer.ResolveMemberNamesForStruct(2, 2);
+  EXPECT_THAT(namer.GetMemberName(2, 0), Eq("field1"));
+  EXPECT_THAT(namer.GetMemberName(2, 1), Eq("field1_1"));
+}
+
+TEST_F(SpvNamerTest,
+       ResolveMemberNamesForStruct_TruncatesOutOfBoundsSuggestion) {
+  Namer namer(fail_stream_);
+  namer.SuggestSanitizedMemberName(2, 3, "sitar");
+  EXPECT_THAT(namer.GetMemberName(2, 3), Eq("sitar"));
+  namer.ResolveMemberNamesForStruct(2, 2);
+  EXPECT_THAT(namer.GetMemberName(2, 0), Eq("field0"));
+  EXPECT_THAT(namer.GetMemberName(2, 1), Eq("field1"));
+  EXPECT_THAT(namer.GetMemberName(2, 3), Eq(""));
+}
+
 }  // namespace
 }  // namespace spirv
 }  // namespace reader