From 23dceb46fc289716cafb399ea897907dc507512e Mon Sep 17 00:00:00 2001 From: David Neto Date: Mon, 23 Mar 2020 16:48:03 +0000 Subject: [PATCH] Add reader::spv::ResolveMemberNamesForStruct Bug: tint:3 Change-Id: If778e98416c19c518261c68d4afdfdc99724967f Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/17440 Reviewed-by: dan sinclair --- src/reader/spirv/namer.cc | 59 ++++++++++++++++++++++++++++++++++ src/reader/spirv/namer.h | 12 +++++-- src/reader/spirv/namer_test.cc | 51 +++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 2 deletions(-) diff --git a/src/reader/spirv/namer.cc b/src/reader/spirv/namer.cc index 4353291671..c0957eb774 100644 --- a/src/reader/spirv/namer.cc +++ b/src/reader/spirv/namer.cc @@ -16,6 +16,7 @@ #include #include +#include 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 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 diff --git a/src/reader/spirv/namer.h b/src/reader/spirv/namer.h index d288216af3..bb9c6a6a96 100644 --- a/src/reader/spirv/namer.h +++ b/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_; diff --git a/src/reader/spirv/namer_test.cc b/src/reader/spirv/namer_test.cc index 764863d179..75c46c1201 100644 --- a/src/reader/spirv/namer_test.cc +++ b/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