From d459fe7137411d13852229e5eb269e1190f869f1 Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Mon, 7 Nov 2022 12:29:04 -0800 Subject: Fix some invalid iterator bugs in btree_test.cc for multi{set,map} emplace{_hint} tests. Note that multi{set,map}::emplace doesn't specify in which order the new element is inserted if there are equivalent keys in the table, whereas emplace_hint specifies that the new element must be before the hint if possible. https://en.cppreference.com/w/cpp/container/multiset/emplace https://en.cppreference.com/w/cpp/container/multiset/emplace_hint Also refactor to rely on IsAssertEnabled instead of checking for NDEBUG. PiperOrigin-RevId: 486733450 Change-Id: Ie90d33c584a6caccd8301ad6fc0396234dbbfac4 --- absl/container/btree_test.cc | 68 +++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 29 deletions(-) (limited to 'absl/container') diff --git a/absl/container/btree_test.cc b/absl/container/btree_test.cc index e6d4e360..05bbcf2f 100644 --- a/absl/container/btree_test.cc +++ b/absl/container/btree_test.cc @@ -74,6 +74,16 @@ void CheckPairEquals(const std::pair &x, const std::pair &y) { CheckPairEquals(x.first, y.first); CheckPairEquals(x.second, y.second); } + +bool IsAssertEnabled() { + // Use an assert with side-effects to figure out if they are actually enabled. + bool assert_enabled = false; + assert([&]() { // NOLINT + assert_enabled = true; + return true; + }()); + return assert_enabled; +} } // namespace // The base class for a sorted associative container checker. TreeType is the @@ -1651,10 +1661,9 @@ TEST(Btree, BtreeMultisetEmplace) { auto iter = s.emplace(value_to_insert); ASSERT_NE(iter, s.end()); EXPECT_EQ(*iter, value_to_insert); - auto iter2 = s.emplace(value_to_insert); - EXPECT_NE(iter2, iter); - ASSERT_NE(iter2, s.end()); - EXPECT_EQ(*iter2, value_to_insert); + iter = s.emplace(value_to_insert); + ASSERT_NE(iter, s.end()); + EXPECT_EQ(*iter, value_to_insert); auto result = s.equal_range(value_to_insert); EXPECT_EQ(std::distance(result.first, result.second), 2); } @@ -1665,44 +1674,45 @@ TEST(Btree, BtreeMultisetEmplaceHint) { auto iter = s.emplace(value_to_insert); ASSERT_NE(iter, s.end()); EXPECT_EQ(*iter, value_to_insert); - auto emplace_iter = s.emplace_hint(iter, value_to_insert); - EXPECT_NE(emplace_iter, iter); - ASSERT_NE(emplace_iter, s.end()); - EXPECT_EQ(*emplace_iter, value_to_insert); + iter = s.emplace_hint(iter, value_to_insert); + // The new element should be before the previously inserted one. + EXPECT_EQ(iter, s.lower_bound(value_to_insert)); + ASSERT_NE(iter, s.end()); + EXPECT_EQ(*iter, value_to_insert); } TEST(Btree, BtreeMultimapEmplace) { const int key_to_insert = 123456; const char value0[] = "a"; - absl::btree_multimap s; - auto iter = s.emplace(key_to_insert, value0); - ASSERT_NE(iter, s.end()); + absl::btree_multimap m; + auto iter = m.emplace(key_to_insert, value0); + ASSERT_NE(iter, m.end()); EXPECT_EQ(iter->first, key_to_insert); EXPECT_EQ(iter->second, value0); const char value1[] = "b"; - auto iter2 = s.emplace(key_to_insert, value1); - EXPECT_NE(iter2, iter); - ASSERT_NE(iter2, s.end()); - EXPECT_EQ(iter2->first, key_to_insert); - EXPECT_EQ(iter2->second, value1); - auto result = s.equal_range(key_to_insert); + iter = m.emplace(key_to_insert, value1); + ASSERT_NE(iter, m.end()); + EXPECT_EQ(iter->first, key_to_insert); + EXPECT_EQ(iter->second, value1); + auto result = m.equal_range(key_to_insert); EXPECT_EQ(std::distance(result.first, result.second), 2); } TEST(Btree, BtreeMultimapEmplaceHint) { const int key_to_insert = 123456; const char value0[] = "a"; - absl::btree_multimap s; - auto iter = s.emplace(key_to_insert, value0); - ASSERT_NE(iter, s.end()); + absl::btree_multimap m; + auto iter = m.emplace(key_to_insert, value0); + ASSERT_NE(iter, m.end()); EXPECT_EQ(iter->first, key_to_insert); EXPECT_EQ(iter->second, value0); const char value1[] = "b"; - auto emplace_iter = s.emplace_hint(iter, key_to_insert, value1); - EXPECT_NE(emplace_iter, iter); - ASSERT_NE(emplace_iter, s.end()); - EXPECT_EQ(emplace_iter->first, key_to_insert); - EXPECT_EQ(emplace_iter->second, value1); + iter = m.emplace_hint(iter, key_to_insert, value1); + // The new element should be before the previously inserted one. + EXPECT_EQ(iter, m.lower_bound(key_to_insert)); + ASSERT_NE(iter, m.end()); + EXPECT_EQ(iter->first, key_to_insert); + EXPECT_EQ(iter->second, value1); } TEST(Btree, ConstIteratorAccessors) { @@ -3005,8 +3015,9 @@ TEST(Btree, ConstructImplicitlyWithUnadaptedComparator) { absl::btree_set set = {{}, MultiKeyComp{}}; } -#ifndef NDEBUG TEST(Btree, InvalidComparatorsCaught) { + if (!IsAssertEnabled()) GTEST_SKIP() << "Assertions not enabled."; + { struct ZeroAlwaysLessCmp { bool operator()(int lhs, int rhs) const { @@ -3054,7 +3065,6 @@ TEST(Btree, InvalidComparatorsCaught) { EXPECT_DEATH(set.insert({0, 1, 2}), "lhs_comp_rhs < 0 -> rhs_comp_lhs > 0"); } } -#endif #ifndef _MSC_VER // This test crashes on MSVC. @@ -3340,13 +3350,13 @@ TEST(Btree, IteratorSubtraction) { } } -#ifndef NDEBUG TEST(Btree, DereferencingEndIterator) { + if (!IsAssertEnabled()) GTEST_SKIP() << "Assertions not enabled."; + absl::btree_set set; for (int i = 0; i < 1000; ++i) set.insert(i); EXPECT_DEATH(*set.end(), R"regex(Dereferencing end\(\) iterator)regex"); } -#endif } // namespace } // namespace container_internal -- cgit v1.2.3