From 0064d9db90d32d35e9f9d70e2df4ddf8d0ab1257 Mon Sep 17 00:00:00 2001 From: Evan Brown Date: Tue, 1 Nov 2022 13:26:41 -0700 Subject: Improve error messages when dereferencing invalid swisstable iterators. - Separate the failure cases into different assertions: end/default constructed vs rehashed or erased. - Update the assertion error for AssertIsValid to not mention the end iterator case because end iterators are considered valid by AssertIsValid. - Also fix an out-of-date comment for skip_empty_or_deleted. PiperOrigin-RevId: 485402559 Change-Id: I593056abdc6c3565d0396fb885923fef643bf4e4 --- absl/container/internal/raw_hash_set.h | 38 ++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 18 deletions(-) (limited to 'absl/container/internal/raw_hash_set.h') diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index 93de2221..676cebd7 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -797,15 +797,22 @@ size_t SelectBucketCountForIterRange(InputIter first, InputIter last, return 0; } -#define ABSL_INTERNAL_ASSERT_IS_FULL(ctrl, msg) \ - ABSL_HARDENING_ASSERT((ctrl != nullptr && IsFull(*ctrl)) && msg) +#define ABSL_INTERNAL_ASSERT_IS_FULL(ctrl, operation) \ + do { \ + ABSL_HARDENING_ASSERT( \ + (ctrl != nullptr) && operation \ + " called on invalid iterator. The iterator might be an end() " \ + "iterator or may have been default constructed."); \ + ABSL_HARDENING_ASSERT( \ + (IsFull(*ctrl)) && operation \ + " called on invalid iterator. The element might have been erased or " \ + "the table might have rehashed."); \ + } while (0) inline void AssertIsValid(ctrl_t* ctrl) { - ABSL_HARDENING_ASSERT( - (ctrl == nullptr || IsFull(*ctrl)) && - "Invalid operation on iterator. The element might have " - "been erased, the table might have rehashed, or this may " - "be an end() iterator."); + ABSL_HARDENING_ASSERT((ctrl == nullptr || IsFull(*ctrl)) && + "Invalid operation on iterator. The element might have " + "been erased or the table might have rehashed."); } struct FindInfo { @@ -1034,22 +1041,19 @@ class raw_hash_set { // PRECONDITION: not an end() iterator. reference operator*() const { - ABSL_INTERNAL_ASSERT_IS_FULL(ctrl_, - "operator*() called on invalid iterator."); + ABSL_INTERNAL_ASSERT_IS_FULL(ctrl_, "operator*()"); return PolicyTraits::element(slot_); } // PRECONDITION: not an end() iterator. pointer operator->() const { - ABSL_INTERNAL_ASSERT_IS_FULL(ctrl_, - "operator-> called on invalid iterator."); + ABSL_INTERNAL_ASSERT_IS_FULL(ctrl_, "operator->"); return &operator*(); } // PRECONDITION: not an end() iterator. iterator& operator++() { - ABSL_INTERNAL_ASSERT_IS_FULL(ctrl_, - "operator++ called on invalid iterator."); + ABSL_INTERNAL_ASSERT_IS_FULL(ctrl_, "operator++"); ++ctrl_; ++slot_; skip_empty_or_deleted(); @@ -1081,7 +1085,7 @@ class raw_hash_set { // Fixes up `ctrl_` to point to a full by advancing it and `slot_` until // they reach one. // - // If a sentinel is reached, we null both of them out instead. + // If a sentinel is reached, we null `ctrl_` out instead. void skip_empty_or_deleted() { while (IsEmptyOrDeleted(*ctrl_)) { uint32_t shift = Group{ctrl_}.CountLeadingEmptyOrDeleted(); @@ -1601,8 +1605,7 @@ class raw_hash_set { // This overload is necessary because otherwise erase(const K&) would be // a better match if non-const iterator is passed as an argument. void erase(iterator it) { - ABSL_INTERNAL_ASSERT_IS_FULL(it.ctrl_, - "erase() called on invalid iterator."); + ABSL_INTERNAL_ASSERT_IS_FULL(it.ctrl_, "erase()"); PolicyTraits::destroy(&alloc_ref(), it.slot_); erase_meta_only(it); } @@ -1636,8 +1639,7 @@ class raw_hash_set { } node_type extract(const_iterator position) { - ABSL_INTERNAL_ASSERT_IS_FULL(position.inner_.ctrl_, - "extract() called on invalid iterator."); + ABSL_INTERNAL_ASSERT_IS_FULL(position.inner_.ctrl_, "extract()"); auto node = CommonAccess::Transfer(alloc_ref(), position.inner_.slot_); erase_meta_only(position); -- cgit v1.2.3