diff --git a/python/protobuf.c b/python/protobuf.c index d3b6677728..5e039f1d48 100644 --- a/python/protobuf.c +++ b/python/protobuf.c @@ -173,7 +173,7 @@ bool PyUpb_WeakMap_Next(PyUpb_WeakMap* map, const void** key, PyObject** obj, intptr_t* iter) { uintptr_t u_key; upb_value val; - if (!upb_inttable_next2(&map->table, &u_key, &val, iter)) return false; + if (!upb_inttable_next(&map->table, &u_key, &val, iter)) return false; *key = (void*)(u_key << PyUpb_PtrShift); *obj = upb_value_getptr(val); return true; diff --git a/upb/hash/common.c b/upb/hash/common.c index a6a9bca238..efcbf1398e 100644 --- a/upb/hash/common.c +++ b/upb/hash/common.c @@ -40,9 +40,9 @@ // Must be last. #include "upb/port/def.inc" -#define UPB_MAXARRSIZE 16 /* 64k. */ +#define UPB_MAXARRSIZE 16 // 2**16 = 64k. -/* From Chromium. */ +// From Chromium. #define ARRAY_SIZE(x) \ ((sizeof(x) / sizeof(0 [x])) / ((size_t)(!(sizeof(x) % sizeof(0 [x]))))) @@ -65,7 +65,7 @@ static int log2ceil(uint64_t v) { int ret = 0; bool pow2 = is_pow2(v); while (v >>= 1) ret++; - ret = pow2 ? ret : ret + 1; /* Ceiling. */ + ret = pow2 ? ret : ret + 1; // Ceiling. return UPB_MIN(UPB_MAXARRSIZE, ret); } @@ -472,14 +472,13 @@ void upb_strtable_clear(upb_strtable* t) { bool upb_strtable_resize(upb_strtable* t, size_t size_lg2, upb_Arena* a) { upb_strtable new_table; - upb_strtable_iter i; - if (!init(&new_table.t, size_lg2, a)) return false; - upb_strtable_begin(&i, t); - for (; !upb_strtable_done(&i); upb_strtable_next(&i)) { - upb_StringView key = upb_strtable_iter_key(&i); - upb_strtable_insert(&new_table, key.data, key.size, - upb_strtable_iter_value(&i), a); + + intptr_t iter = UPB_STRTABLE_BEGIN; + upb_StringView key; + upb_value val; + while (upb_strtable_next2(t, &key, &val, &iter)) { + upb_strtable_insert(&new_table, key.data, key.size, val, a); } *t = new_table; return true; @@ -598,12 +597,13 @@ static void check(upb_inttable* t) { UPB_UNUSED(t); #if defined(UPB_DEBUG_TABLE) && !defined(NDEBUG) { - /* This check is very expensive (makes inserts/deletes O(N)). */ + // This check is very expensive (makes inserts/deletes O(N)). size_t count = 0; - upb_inttable_iter i; - upb_inttable_begin(&i, t); - for (; !upb_inttable_done(&i); upb_inttable_next(&i), count++) { - UPB_ASSERT(upb_inttable_lookup(t, upb_inttable_iter_key(&i), NULL)); + intptr_t iter = UPB_INTTABLE_BEGIN; + uintptr_t key; + upb_value val; + while (upb_inttable_next(t, &key, &val, &iter)) { + UPB_ASSERT(upb_inttable_lookup(t, key, NULL)); } UPB_ASSERT(count == upb_inttable_count(t)); } @@ -716,22 +716,22 @@ void upb_inttable_compact(upb_inttable* t, upb_Arena* a) { /* The max key in each bucket. */ uintptr_t max[UPB_MAXARRSIZE + 1] = {0}; - upb_inttable_iter i; - size_t arr_count; - int size_lg2; - upb_inttable new_t; - - upb_inttable_begin(&i, t); - for (; !upb_inttable_done(&i); upb_inttable_next(&i)) { - uintptr_t key = upb_inttable_iter_key(&i); - int bucket = log2ceil(key); - max[bucket] = UPB_MAX(max[bucket], key); - counts[bucket]++; + { + intptr_t iter = UPB_INTTABLE_BEGIN; + uintptr_t key; + upb_value val; + while (upb_inttable_next(t, &key, &val, &iter)) { + int bucket = log2ceil(key); + max[bucket] = UPB_MAX(max[bucket], key); + counts[bucket]++; + } } /* Find the largest power of two that satisfies the MIN_DENSITY * definition (while actually having some keys). */ - arr_count = upb_inttable_count(t); + size_t arr_count = upb_inttable_count(t); + int size_lg2; + upb_inttable new_t; for (size_lg2 = ARRAY_SIZE(counts) - 1; size_lg2 > 0; size_lg2--) { if (counts[size_lg2] == 0) { @@ -754,55 +754,28 @@ void upb_inttable_compact(upb_inttable* t, upb_Arena* a) { int hashsize_lg2 = log2ceil(hash_size); upb_inttable_sizedinit(&new_t, arr_size, hashsize_lg2, a); - upb_inttable_begin(&i, t); - for (; !upb_inttable_done(&i); upb_inttable_next(&i)) { - uintptr_t k = upb_inttable_iter_key(&i); - upb_inttable_insert(&new_t, k, upb_inttable_iter_value(&i), a); + + { + intptr_t iter = UPB_INTTABLE_BEGIN; + uintptr_t key; + upb_value val; + while (upb_inttable_next(t, &key, &val, &iter)) { + upb_inttable_insert(&new_t, key, val, a); + } } + UPB_ASSERT(new_t.array_size == arr_size); UPB_ASSERT(new_t.t.size_lg2 == hashsize_lg2); } *t = new_t; } -/* Iteration. */ - -static const upb_tabent* int_tabent(const upb_inttable_iter* i) { - UPB_ASSERT(!i->array_part); - return &i->t->t.entries[i->index]; -} - -static upb_tabval int_arrent(const upb_inttable_iter* i) { - UPB_ASSERT(i->array_part); - return i->t->array[i->index]; -} - -void upb_inttable_begin(upb_inttable_iter* i, const upb_inttable* t) { - i->t = t; - i->index = -1; - i->array_part = true; - upb_inttable_next(i); -} - -void upb_inttable_next(upb_inttable_iter* iter) { - const upb_inttable* t = iter->t; - if (iter->array_part) { - while (++iter->index < t->array_size) { - if (upb_arrhas(int_arrent(iter))) { - return; - } - } - iter->array_part = false; - iter->index = begin(&t->t); - } else { - iter->index = next(&t->t, iter->index); - } -} +// Iteration. -bool upb_inttable_next2(const upb_inttable* t, uintptr_t* key, upb_value* val, - intptr_t* iter) { +bool upb_inttable_next(const upb_inttable* t, uintptr_t* key, upb_value* val, + intptr_t* iter) { intptr_t i = *iter; - if (i < t->array_size) { + if ((size_t)(i + 1) <= t->array_size) { while (++i < t->array_size) { upb_tabval ent = t->array[i]; if (upb_arrhas(ent)) { @@ -812,9 +785,10 @@ bool upb_inttable_next2(const upb_inttable* t, uintptr_t* key, upb_value* val, return true; } } + i--; // Back up to exactly one position before the start of the table. } - size_t tab_idx = next(&t->t, i == -1 ? -1 : i - t->array_size); + size_t tab_idx = next(&t->t, i - t->array_size); if (tab_idx < upb_table_size(&t->t)) { upb_tabent* ent = &t->t.entries[tab_idx]; *key = ent->key; @@ -892,37 +866,3 @@ void upb_strtable_removeiter(upb_strtable* t, intptr_t* iter) { ent->key = 0; ent->next = NULL; } - -bool upb_inttable_done(const upb_inttable_iter* i) { - if (!i->t) return true; - if (i->array_part) { - return i->index >= i->t->array_size || !upb_arrhas(int_arrent(i)); - } else { - return i->index >= upb_table_size(&i->t->t) || - upb_tabent_isempty(int_tabent(i)); - } -} - -uintptr_t upb_inttable_iter_key(const upb_inttable_iter* i) { - UPB_ASSERT(!upb_inttable_done(i)); - return i->array_part ? i->index : int_tabent(i)->key; -} - -upb_value upb_inttable_iter_value(const upb_inttable_iter* i) { - UPB_ASSERT(!upb_inttable_done(i)); - return _upb_value_val(i->array_part ? i->t->array[i->index].val - : int_tabent(i)->val.val); -} - -void upb_inttable_iter_setdone(upb_inttable_iter* i) { - i->t = NULL; - i->index = SIZE_MAX; - i->array_part = false; -} - -bool upb_inttable_iter_isequal(const upb_inttable_iter* i1, - const upb_inttable_iter* i2) { - if (upb_inttable_done(i1) && upb_inttable_done(i2)) return true; - return i1->t == i2->t && i1->index == i2->index && - i1->array_part == i2->array_part; -} diff --git a/upb/hash/int_table.h b/upb/hash/int_table.h index edfee027c8..7b1d6ccbc5 100644 --- a/upb/hash/int_table.h +++ b/upb/hash/int_table.h @@ -78,71 +78,21 @@ bool upb_inttable_replace(upb_inttable* t, uintptr_t key, upb_value val); // inserting more entries is legal, but will likely require a table resize. void upb_inttable_compact(upb_inttable* t, upb_Arena* a); -/* Iteration over inttable. - * - * intptr_t iter = UPB_INTTABLE_BEGIN; - * uintptr_t key; - * upb_value val; - * while (upb_inttable_next2(t, &key, &val, &iter)) { - * // ... - * } - */ +// Iteration over inttable: +// +// intptr_t iter = UPB_INTTABLE_BEGIN; +// uintptr_t key; +// upb_value val; +// while (upb_inttable_next(t, &key, &val, &iter)) { +// // ... +// } #define UPB_INTTABLE_BEGIN -1 -bool upb_inttable_next2(const upb_inttable* t, uintptr_t* key, upb_value* val, - intptr_t* iter); +bool upb_inttable_next(const upb_inttable* t, uintptr_t* key, upb_value* val, + intptr_t* iter); void upb_inttable_removeiter(upb_inttable* t, intptr_t* iter); -/* DEPRECATED iterators, slated for removal. - * - * Iterators for int tables. We are subject to some kind of unusual - * design constraints: - * - * For high-level languages: - * - we must be able to guarantee that we don't crash or corrupt memory even if - * the program accesses an invalidated iterator. - * - * For C++11 range-based for: - * - iterators must be copyable - * - iterators must be comparable - * - it must be possible to construct an "end" value. - * - * Iteration order is undefined. - * - * Modifying the table invalidates iterators. upb_{str,int}table_done() is - * guaranteed to work even on an invalidated iterator, as long as the table it - * is iterating over has not been freed. Calling next() or accessing data from - * an invalidated iterator yields unspecified elements from the table, but it is - * guaranteed not to crash and to return real table elements (except when done() - * is true). */ - -/* upb_inttable_iter **********************************************************/ - -/* upb_inttable_iter i; - * upb_inttable_begin(&i, t); - * for(; !upb_inttable_done(&i); upb_inttable_next(&i)) { - * uintptr_t key = upb_inttable_iter_key(&i); - * upb_value val = upb_inttable_iter_value(&i); - * // ... - * } - */ - -typedef struct { - const upb_inttable* t; - size_t index; - bool array_part; -} upb_inttable_iter; - -void upb_inttable_begin(upb_inttable_iter* i, const upb_inttable* t); -void upb_inttable_next(upb_inttable_iter* i); -bool upb_inttable_done(const upb_inttable_iter* i); -uintptr_t upb_inttable_iter_key(const upb_inttable_iter* i); -upb_value upb_inttable_iter_value(const upb_inttable_iter* i); -void upb_inttable_iter_setdone(upb_inttable_iter* i); -bool upb_inttable_iter_isequal(const upb_inttable_iter* i1, - const upb_inttable_iter* i2); - #ifdef __cplusplus } /* extern "C" */ #endif diff --git a/upb/hash/str_table.h b/upb/hash/str_table.h index 5219dfd2d2..a53414af41 100644 --- a/upb/hash/str_table.h +++ b/upb/hash/str_table.h @@ -85,9 +85,9 @@ UPB_INLINE bool upb_strtable_remove(upb_strtable* t, const char* key, // Exposed for testing only. bool upb_strtable_resize(upb_strtable* t, size_t size_lg2, upb_Arena* a); -/* Iteration over strtable. +/* Iteration over strtable: * - * intptr_t iter = UPB_INTTABLE_BEGIN; + * intptr_t iter = UPB_STRTABLE_BEGIN; * upb_StringView key; * upb_value val; * while (upb_strtable_next2(t, &key, &val, &iter)) { diff --git a/upb/hash/test.cc b/upb/hash/test.cc index 5451f2bb4b..42d692fbe2 100644 --- a/upb/hash/test.cc +++ b/upb/hash/test.cc @@ -43,313 +43,6 @@ // Must be last. #include "upb/port/def.inc" -// Convenience interface for C++. We don't put this in upb itself because -// the table is not exposed to users. - -namespace upb { - -template -upb_value MakeUpbValue(T val); -template -T GetUpbValue(upb_value val); - -#define FUNCS(name, type_t, enumval) \ - template <> \ - upb_value MakeUpbValue(type_t val) { \ - return upb_value_##name(val); \ - } \ - template <> \ - type_t GetUpbValue(upb_value val) { \ - return upb_value_get##name(val); \ - } - -FUNCS(int32, int32_t, UPB_CTYPE_INT32) -FUNCS(int64, int64_t, UPB_CTYPE_INT64) -FUNCS(uint32, uint32_t, UPB_CTYPE_UINT32) -FUNCS(uint64, uint64_t, UPB_CTYPE_UINT64) -FUNCS(bool, bool, UPB_CTYPE_BOOL) -FUNCS(cstr, char*, UPB_CTYPE_CSTR) -FUNCS(ptr, void*, UPB_CTYPE_PTR) -FUNCS(constptr, const void*, UPB_CTYPE_CONSTPTR) - -#undef FUNCS - -class IntTable { - public: - IntTable() { upb_inttable_init(&table_, arena_.ptr()); } - - size_t count() { return upb_inttable_count(&table_); } - - bool Insert(uintptr_t key, upb_value val) { - return upb_inttable_insert(&table_, key, val, arena_.ptr()); - } - - bool Replace(uintptr_t key, upb_value val) { - return upb_inttable_replace(&table_, key, val); - } - - std::pair Remove(uintptr_t key) { - std::pair ret; - ret.first = upb_inttable_remove(&table_, key, &ret.second); - return ret; - } - - std::pair Lookup(uintptr_t key) const { - std::pair ret; - ret.first = upb_inttable_lookup(&table_, key, &ret.second); - return ret; - } - - std::pair Lookup32(uint32_t key) const { - std::pair ret; - ret.first = upb_inttable_lookup(&table_, key, &ret.second); - return ret; - } - - void Compact() { upb_inttable_compact(&table_, arena_.ptr()); } - - class iterator { - public: - explicit iterator(IntTable* table) { - upb_inttable_begin(&iter_, &table->table_); - } - - static iterator end(IntTable* table) { - iterator iter(table); - upb_inttable_iter_setdone(&iter.iter_); - return iter; - } - - void operator++() { return upb_inttable_next(&iter_); } - - std::pair operator*() const { - std::pair ret; - ret.first = upb_inttable_iter_key(&iter_); - ret.second = upb_inttable_iter_value(&iter_); - return ret; - } - - bool operator==(const iterator& other) const { - return upb_inttable_iter_isequal(&iter_, &other.iter_); - } - - bool operator!=(const iterator& other) const { return !(*this == other); } - - private: - upb_inttable_iter iter_; - }; - - upb::Arena arena_; - upb_inttable table_; -}; - -class StrTable { - public: - StrTable() { upb_strtable_init(&table_, 4, arena_.ptr()); } - - size_t count() { return upb_strtable_count(&table_); } - - bool Insert(const std::string& key, upb_value val) { - return upb_strtable_insert(&table_, key.c_str(), key.size(), val, - arena_.ptr()); - } - - std::pair Remove(const std::string& key) { - std::pair ret; - ret.first = - upb_strtable_remove2(&table_, key.c_str(), key.size(), &ret.second); - return ret; - } - - std::pair Lookup(const std::string& key) const { - std::pair ret; - ret.first = - upb_strtable_lookup2(&table_, key.c_str(), key.size(), &ret.second); - return ret; - } - - void Resize(size_t size_lg2) { - upb_strtable_resize(&table_, size_lg2, arena_.ptr()); - } - - class iterator { - public: - explicit iterator(StrTable* table) { - upb_strtable_begin(&iter_, &table->table_); - } - - static iterator end(StrTable* table) { - iterator iter(table); - upb_strtable_iter_setdone(&iter.iter_); - return iter; - } - - void operator++() { return upb_strtable_next(&iter_); } - - std::pair operator*() const { - std::pair ret; - upb_StringView view = upb_strtable_iter_key(&iter_); - ret.first.assign(view.data, view.size); - ret.second = upb_strtable_iter_value(&iter_); - return ret; - } - - bool operator==(const iterator& other) const { - return upb_strtable_iter_isequal(&iter_, &other.iter_); - } - - bool operator!=(const iterator& other) const { return !(*this == other); } - - private: - upb_strtable_iter iter_; - }; - - upb::Arena arena_; - upb_strtable table_; -}; - -template -class TypedStrTable { - public: - size_t count() { return table_.count(); } - - bool Insert(const std::string& key, T val) { - return table_.Insert(key, MakeUpbValue(val)); - } - - std::pair Remove(const std::string& key) { - std::pair found = table_.Remove(key); - std::pair ret; - ret.first = found.first; - if (ret.first) { - ret.second = GetUpbValue(found.second); - } - return ret; - } - - std::pair Lookup(const std::string& key) const { - std::pair found = table_.Lookup(key); - std::pair ret; - ret.first = found.first; - if (ret.first) { - ret.second = GetUpbValue(found.second); - } - return ret; - } - - void Resize(size_t size_lg2) { table_.Resize(size_lg2); } - - class iterator { - public: - explicit iterator(TypedStrTable* table) : iter_(&table->table_) {} - static iterator end(TypedStrTable* table) { - iterator iter(table); - iter.iter_ = StrTable::iterator::end(&table->table_); - return iter; - } - - void operator++() { ++iter_; } - - std::pair operator*() const { - std::pair val = *iter_; - std::pair ret; - ret.first = val.first; - ret.second = GetUpbValue(val.second); - return ret; - } - - bool operator==(const iterator& other) const { - return iter_ == other.iter_; - } - - bool operator!=(const iterator& other) const { - return iter_ != other.iter_; - } - - private: - StrTable::iterator iter_; - }; - - iterator begin() { return iterator(this); } - iterator end() { return iterator::end(this); } - - StrTable table_; -}; - -template -class TypedIntTable { - public: - size_t count() { return table_.count(); } - - bool Insert(uintptr_t key, T val) { - return table_.Insert(key, MakeUpbValue(val)); - } - - bool Replace(uintptr_t key, T val) { - return table_.Replace(key, MakeUpbValue(val)); - } - - std::pair Remove(uintptr_t key) { - std::pair found = table_.Remove(key); - std::pair ret; - ret.first = found.first; - if (ret.first) { - ret.second = GetUpbValue(found.second); - } - return ret; - } - - std::pair Lookup(uintptr_t key) const { - std::pair found = table_.Lookup(key); - std::pair ret; - ret.first = found.first; - if (ret.first) { - ret.second = GetUpbValue(found.second); - } - return ret; - } - - void Compact() { table_.Compact(); } - - class iterator { - public: - explicit iterator(TypedIntTable* table) : iter_(&table->table_) {} - static iterator end(TypedIntTable* table) { - return IntTable::iterator::end(&table->table_); - } - - void operator++() { ++iter_; } - - std::pair operator*() const { - std::pair val = *iter_; - std::pair ret; - ret.first = val.first; - ret.second = GetUpbValue(val.second); - return ret; - } - - bool operator==(const iterator& other) const { - return iter_ == other.iter_; - } - - bool operator!=(const iterator& other) const { - return iter_ != other.iter_; - } - - private: - IntTable::iterator iter_; - }; - - iterator begin() { return iterator(this); } - iterator end() { return iterator::end(this); } - - IntTable table_; -}; - -} // namespace upb - -#define CPU_TIME_PER_TEST 0.5 - using std::vector; TEST(Table, StringTable) { @@ -374,30 +67,32 @@ TEST(Table, StringTable) { keys.push_back("google.protobuf.UninterpretedOption.NamePart"); /* Initialize structures. */ + upb::Arena arena; + upb_strtable t; + upb_strtable_init(&t, keys.size(), arena.ptr()); std::map m; - typedef upb::TypedStrTable Table; - Table table; std::set all; for (const auto& key : keys) { all.insert(key); - table.Insert(key, key[0]); + upb_value val = {uint64_t(key[0])}; + upb_strtable_insert(&t, key.data(), key.size(), val, arena.ptr()); m[key] = key[0]; } /* Test correctness. */ for (const auto& key : keys) { - std::pair found = table.Lookup(key); - if (m.find(key) != m.end()) { /* Assume map implementation is correct. */ - EXPECT_TRUE(found.first); - EXPECT_EQ(found.second, key[0]); - EXPECT_EQ(m[key], key[0]); - } else { - EXPECT_FALSE(found.first); - } + upb_value val; + bool ok = upb_strtable_lookup2(&t, key.data(), key.size(), &val); + EXPECT_TRUE(ok); + EXPECT_EQ(val.val, uint64_t(key[0])); + EXPECT_EQ(m[key], key[0]); } - for (Table::iterator it = table.begin(); it != table.end(); ++it) { - std::set::iterator i = all.find((*it).first); + intptr_t iter = UPB_STRTABLE_BEGIN; + upb_StringView key; + upb_value val; + while (upb_strtable_next2(&t, &key, &val, &iter)) { + std::set::iterator i = all.find(key.data); EXPECT_NE(i, all.end()); all.erase(i); } @@ -406,16 +101,15 @@ TEST(Table, StringTable) { // Test iteration with resizes. for (int i = 0; i < 10; i++) { - for (Table::iterator it = table.begin(); it != table.end(); ++it) { + intptr_t iter = UPB_STRTABLE_BEGIN; + while (upb_strtable_next2(&t, &key, &val, &iter)) { // Even if we invalidate the iterator it should only return real elements. - EXPECT_EQ((*it).second, m[(*it).first]); + EXPECT_EQ(val.val, m[key.data]); // Force a resize even though the size isn't changing. // Also forces the table size to grow so some new buckets end up empty. - int new_lg2 = table.table_.table_.t.size_lg2 + 1; - // Don't use more than 64k tables, to avoid exhausting memory. - new_lg2 = UPB_MIN(new_lg2, 16); - table.Resize(new_lg2); + bool ok = upb_strtable_resize(&t, 5 + i, arena.ptr()); + EXPECT_TRUE(ok); } } } @@ -442,81 +136,76 @@ class IntTableTest : public testing::TestWithParam { TEST_P(IntTableTest, TestIntTable) { /* Initialize structures. */ - typedef upb::TypedIntTable Table; - Table table; + upb::Arena arena; + upb_inttable t; + upb_inttable_init(&t, arena.ptr()); uint32_t largest_key = 0; std::map m; std::unordered_map hm; for (const auto& key : keys_) { largest_key = UPB_MAX((int32_t)largest_key, key); - table.Insert(key, key * 2); + upb_value val = upb_value_uint32(key * 2); + bool ok = upb_inttable_insert(&t, key, val, arena.ptr()); + EXPECT_TRUE(ok); m[key] = key * 2; hm[key] = key * 2; } + EXPECT_EQ(upb_inttable_count(&t), keys_.size()); /* Test correctness. */ + int count = 0; for (uint32_t i = 0; i <= largest_key; i++) { - std::pair found = table.Lookup(i); - if (m.find(i) != m.end()) { /* Assume map implementation is correct. */ - EXPECT_TRUE(found.first); - EXPECT_EQ(found.second, i * 2); - EXPECT_EQ(m[i], i * 2); - EXPECT_EQ(hm[i], i * 2); - } else { - EXPECT_FALSE(found.first); - } - } - - for (size_t i = 0; i < keys_.size(); i += 2) { - std::pair found = table.Remove(keys_[i]); - EXPECT_EQ(found.first, m.erase(keys_[i]) == 1); - if (found.first) { - EXPECT_EQ(found.second, (uint32_t)keys_[i] * 2); - } - hm.erase(keys_[i]); - m.erase(keys_[i]); - } - - EXPECT_EQ(table.count(), hm.size()); - - /* Test correctness. */ - for (uint32_t i = 0; i <= largest_key; i++) { - std::pair found = table.Lookup(i); - if (m.find(i) != m.end()) { /* Assume map implementation is correct. */ - EXPECT_TRUE(found.first); - EXPECT_EQ(found.second, i * 2); + upb_value val; + bool ok = upb_inttable_lookup(&t, i, &val); + if (ok) { /* Assume map implementation is correct. */ + EXPECT_EQ(val.val, i * 2); EXPECT_EQ(m[i], i * 2); EXPECT_EQ(hm[i], i * 2); - } else { - EXPECT_FALSE(found.first); + count++; } } + EXPECT_EQ(count, keys_.size()); + EXPECT_EQ(count, upb_inttable_count(&t)); // Test replace. + count = 0; for (uint32_t i = 0; i <= largest_key; i++) { - bool replaced = table.Replace(i, i * 3); - if (m.find(i) != m.end()) { /* Assume map implementation is correct. */ - EXPECT_TRUE(replaced); + upb_value val = upb_value_uint32(i * 3); + bool ok = upb_inttable_replace(&t, i, val); + if (ok) { /* Assume map implementation is correct. */ m[i] = i * 3; hm[i] = i * 3; - } else { - EXPECT_FALSE(replaced); + count++; } } + EXPECT_EQ(count, keys_.size()); + EXPECT_EQ(count, upb_inttable_count(&t)); // Compact and test correctness again. - table.Compact(); + upb_inttable_compact(&t, arena.ptr()); + count = 0; for (uint32_t i = 0; i <= largest_key; i++) { - std::pair found = table.Lookup(i); - if (m.find(i) != m.end()) { /* Assume map implementation is correct. */ - EXPECT_TRUE(found.first); - EXPECT_EQ(found.second, i * 3); + upb_value val; + bool ok = upb_inttable_lookup(&t, i, &val); + if (ok) { /* Assume map implementation is correct. */ + EXPECT_EQ(val.val, i * 3); EXPECT_EQ(m[i], i * 3); EXPECT_EQ(hm[i], i * 3); - } else { - EXPECT_FALSE(found.first); + count++; } } + EXPECT_EQ(count, keys_.size()); + EXPECT_EQ(count, upb_inttable_count(&t)); + + for (const auto& key : keys_) { + upb_value val; + bool ok = upb_inttable_remove(&t, key, &val); + EXPECT_TRUE(ok); + EXPECT_EQ(val.val, (uint32_t)key * 3); + count--; + EXPECT_EQ(count, upb_inttable_count(&t)); + } + EXPECT_EQ(0, upb_inttable_count(&t)); } INSTANTIATE_TEST_SUITE_P(IntTableParams, IntTableTest, @@ -550,9 +239,10 @@ TEST(Table, Delete) { upb_inttable_remove(&t, 2, NULL); upb_inttable_remove(&t, 4, NULL); - upb_inttable_iter iter; - for (upb_inttable_begin(&iter, &t); !upb_inttable_done(&iter); - upb_inttable_next(&iter)) { + intptr_t iter = UPB_INTTABLE_BEGIN; + uintptr_t key; + upb_value val; + while (upb_inttable_next(&t, &key, &val, &iter)) { ASSERT_TRUE(false); } } diff --git a/upb/reflection/def_pool.c b/upb/reflection/def_pool.c index b89f396e0b..a088873034 100644 --- a/upb/reflection/def_pool.c +++ b/upb/reflection/def_pool.c @@ -431,14 +431,14 @@ const upb_FieldDef** upb_DefPool_GetAllExtensions(const upb_DefPool* s, // This is O(all exts) instead of O(exts for m). If we need this to be // efficient we may need to make extreg into a two-level table, or have a // second per-message index. - while (upb_inttable_next2(&s->exts, &key, &val, &iter)) { + while (upb_inttable_next(&s->exts, &key, &val, &iter)) { const upb_FieldDef* f = upb_value_getconstptr(val); if (upb_FieldDef_ContainingType(f) == m) n++; } const upb_FieldDef** exts = malloc(n * sizeof(*exts)); iter = UPB_INTTABLE_BEGIN; size_t i = 0; - while (upb_inttable_next2(&s->exts, &key, &val, &iter)) { + while (upb_inttable_next(&s->exts, &key, &val, &iter)) { const upb_FieldDef* f = upb_value_getconstptr(val); if (upb_FieldDef_ContainingType(f) == m) exts[i++] = f; } diff --git a/upb/wire/encode.c b/upb/wire/encode.c index 51be995f25..2ebb89f7a6 100644 --- a/upb/wire/encode.c +++ b/upb/wire/encode.c @@ -428,11 +428,10 @@ static void encode_map(upb_encstate* e, const upb_Message* msg, } _upb_mapsorter_popmap(&e->sorter, &sorted); } else { - upb_strtable_iter i; - upb_strtable_begin(&i, &map->table); - for (; !upb_strtable_done(&i); upb_strtable_next(&i)) { - upb_StringView key = upb_strtable_iter_key(&i); - const upb_value val = upb_strtable_iter_value(&i); + intptr_t iter = UPB_STRTABLE_BEGIN; + upb_StringView key; + upb_value val; + while (upb_strtable_next2(&map->table, &key, &val, &iter)) { upb_MapEntry ent; _upb_map_fromkey(key, &ent.k, map->key_size); _upb_map_fromvalue(val, &ent.v, map->val_size);