From 5b3b7aa3ed3c06116ce67ca613db7c9bec2a4175 Mon Sep 17 00:00:00 2001 From: josh11b Date: Fri, 20 Oct 2023 13:40:48 -0700 Subject: [PATCH] Test for id overflow and string deduplication in `SharedValueStores` (#3319) Follow-on to #3311 --- toolchain/base/value_store.h | 4 +++- toolchain/base/value_store_test.cpp | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/toolchain/base/value_store.h b/toolchain/base/value_store.h index 01ceddb8276a..5bba06526216 100644 --- a/toolchain/base/value_store.h +++ b/toolchain/base/value_store.h @@ -84,7 +84,8 @@ class ValueStore { public: // Stores the value and returns an ID to reference it. auto Add(typename IdT::IndexedType value) -> IdT { - auto id = IdT(values_.size()); + IdT id = IdT(values_.size()); + CARBON_CHECK(id.index >= 0) << "Id overflow"; values_.push_back(std::move(value)); return id; } @@ -109,6 +110,7 @@ class ValueStore { auto Add(llvm::StringRef value) -> StringId { auto [it, inserted] = map_.insert({value, StringId(values_.size())}); if (inserted) { + CARBON_CHECK(it->second.index >= 0) << "Too many unique strings"; values_.push_back(value); } return it->second; diff --git a/toolchain/base/value_store_test.cpp b/toolchain/base/value_store_test.cpp index d1170d75dfc1..9f4285972b00 100644 --- a/toolchain/base/value_store_test.cpp +++ b/toolchain/base/value_store_test.cpp @@ -66,6 +66,10 @@ TEST(ValueStore, String) { EXPECT_THAT(a_id, Not(Eq(b_id))); EXPECT_THAT(value_stores.strings().Get(a_id), Eq(a)); EXPECT_THAT(value_stores.strings().Get(b_id), Eq(b)); + + // Adding the same string again should return the same id. + EXPECT_THAT(value_stores.strings().Add(a), Eq(a_id)); + EXPECT_THAT(value_stores.strings().Add(b), Eq(b_id)); } } // namespace