<div dir="ltr">This was causing some failures of the assert in operator*/operator-> in the iterator for me. I believe I've fixed the issue in 2cf3c033f3a1d4c2c36a94a80b7ba6bcba1e8711<div><br></div><div>~Craig<br><div><br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 20, 2020 at 5:31 PM Chris Bieneman via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: Chris Bieneman<br>
Date: 2020-04-20T19:31:32-05:00<br>
New Revision: 887efa51c1e0e43ca684ed78b92dbc3a0720881b<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/887efa51c1e0e43ca684ed78b92dbc3a0720881b" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/887efa51c1e0e43ca684ed78b92dbc3a0720881b</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/887efa51c1e0e43ca684ed78b92dbc3a0720881b.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/887efa51c1e0e43ca684ed78b92dbc3a0720881b.diff</a><br>
<br>
LOG: Fix DenseMap iterator asserts when shouldReverseIterate==true<br>
<br>
This patch gets the asserts working correctly when LLVM_REVERSE_ITERATION=On by fixing the iterators returned by the DenseMap::find* methods so that they return well-formed iterators that work with reverse iteration, and satisfy the assertions.<br>
<br>
Added: <br>
<br>
<br>
Modified: <br>
llvm/include/llvm/ADT/DenseMap.h<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h<br>
index f453ce539e2b..ff9dab15291f 100644<br>
--- a/llvm/include/llvm/ADT/DenseMap.h<br>
+++ b/llvm/include/llvm/ADT/DenseMap.h<br>
@@ -149,13 +149,19 @@ class DenseMapBase : public DebugEpochBase {<br>
iterator find(const_arg_type_t<KeyT> Val) {<br>
BucketT *TheBucket;<br>
if (LookupBucketFor(Val, TheBucket))<br>
- return makeIterator(TheBucket, getBucketsEnd(), *this, true);<br>
+ return makeIterator(TheBucket,<br>
+ shouldReverseIterate<KeyT>() ? getBuckets()<br>
+ : getBucketsEnd(),<br>
+ *this, true);<br>
return end();<br>
}<br>
const_iterator find(const_arg_type_t<KeyT> Val) const {<br>
const BucketT *TheBucket;<br>
if (LookupBucketFor(Val, TheBucket))<br>
- return makeConstIterator(TheBucket, getBucketsEnd(), *this, true);<br>
+ return makeConstIterator(TheBucket,<br>
+ shouldReverseIterate<KeyT>() ? getBuckets()<br>
+ : getBucketsEnd(),<br>
+ *this, true);<br>
return end();<br>
}<br>
<br>
@@ -168,14 +174,20 @@ class DenseMapBase : public DebugEpochBase {<br>
iterator find_as(const LookupKeyT &Val) {<br>
BucketT *TheBucket;<br>
if (LookupBucketFor(Val, TheBucket))<br>
- return makeIterator(TheBucket, getBucketsEnd(), *this, true);<br>
+ return makeIterator(TheBucket,<br>
+ shouldReverseIterate<KeyT>() ? getBuckets()<br>
+ : getBucketsEnd(),<br>
+ *this, true);<br>
return end();<br>
}<br>
template<class LookupKeyT><br>
const_iterator find_as(const LookupKeyT &Val) const {<br>
const BucketT *TheBucket;<br>
if (LookupBucketFor(Val, TheBucket))<br>
- return makeConstIterator(TheBucket, getBucketsEnd(), *this, true);<br>
+ return makeConstIterator(TheBucket,<br>
+ shouldReverseIterate<KeyT>() ? getBuckets()<br>
+ : getBucketsEnd(),<br>
+ *this, true);<br>
return end();<br>
}<br>
<br>
@@ -208,17 +220,17 @@ class DenseMapBase : public DebugEpochBase {<br>
template <typename... Ts><br>
std::pair<iterator, bool> try_emplace(KeyT &&Key, Ts &&... Args) {<br>
BucketT *TheBucket;<br>
+ BucketT *EndBucket =<br>
+ shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd();<br>
if (LookupBucketFor(Key, TheBucket))<br>
- return std::make_pair(<br>
- makeIterator(TheBucket, getBucketsEnd(), *this, true),<br>
- false); // Already in map.<br>
+ return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),<br>
+ false); // Already in map.<br>
<br>
// Otherwise, insert the new element.<br>
TheBucket =<br>
InsertIntoBucket(TheBucket, std::move(Key), std::forward<Ts>(Args)...);<br>
- return std::make_pair(<br>
- makeIterator(TheBucket, getBucketsEnd(), *this, true),<br>
- true);<br>
+ return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),<br>
+ true);<br>
}<br>
<br>
// Inserts key,value pair into the map if the key isn't already in the map.<br>
@@ -227,16 +239,16 @@ class DenseMapBase : public DebugEpochBase {<br>
template <typename... Ts><br>
std::pair<iterator, bool> try_emplace(const KeyT &Key, Ts &&... Args) {<br>
BucketT *TheBucket;<br>
+ BucketT *EndBucket =<br>
+ shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd();<br>
if (LookupBucketFor(Key, TheBucket))<br>
- return std::make_pair(<br>
- makeIterator(TheBucket, getBucketsEnd(), *this, true),<br>
- false); // Already in map.<br>
+ return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),<br>
+ false); // Already in map.<br>
<br>
// Otherwise, insert the new element.<br>
TheBucket = InsertIntoBucket(TheBucket, Key, std::forward<Ts>(Args)...);<br>
- return std::make_pair(<br>
- makeIterator(TheBucket, getBucketsEnd(), *this, true),<br>
- true);<br>
+ return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),<br>
+ true);<br>
}<br>
<br>
/// Alternate version of insert() which allows a <br>
diff erent, and possibly<br>
@@ -248,17 +260,17 @@ class DenseMapBase : public DebugEpochBase {<br>
std::pair<iterator, bool> insert_as(std::pair<KeyT, ValueT> &&KV,<br>
const LookupKeyT &Val) {<br>
BucketT *TheBucket;<br>
+ BucketT *EndBucket =<br>
+ shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd();<br>
if (LookupBucketFor(Val, TheBucket))<br>
- return std::make_pair(<br>
- makeIterator(TheBucket, getBucketsEnd(), *this, true),<br>
- false); // Already in map.<br>
+ return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),<br>
+ false); // Already in map.<br>
<br>
// Otherwise, insert the new element.<br>
TheBucket = InsertIntoBucketWithLookup(TheBucket, std::move(KV.first),<br>
std::move(KV.second), Val);<br>
- return std::make_pair(<br>
- makeIterator(TheBucket, getBucketsEnd(), *this, true),<br>
- true);<br>
+ return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),<br>
+ true);<br>
}<br>
<br>
/// insert - Range insertion of pairs.<br>
@@ -1200,20 +1212,16 @@ class DenseMapIterator : DebugEpochBase::HandleBase {<br>
<br>
reference operator*() const {<br>
assert(isHandleInSync() && "invalid iterator access!");<br>
- if (shouldReverseIterate<KeyT>()) {<br>
- assert(Ptr != &End[-1] && "dereferencing end() iterator");<br>
- return Ptr[-1];<br>
- }<br>
assert(Ptr != End && "dereferencing end() iterator");<br>
+ if (shouldReverseIterate<KeyT>())<br>
+ return Ptr[-1];<br>
return *Ptr;<br>
}<br>
pointer operator->() const {<br>
assert(isHandleInSync() && "invalid iterator access!");<br>
- if (shouldReverseIterate<KeyT>()) {<br>
- assert(Ptr != &End[-1] && "dereferencing end() iterator");<br>
- return &(Ptr[-1]);<br>
- }<br>
assert(Ptr != End && "dereferencing end() iterator");<br>
+ if (shouldReverseIterate<KeyT>())<br>
+ return &(Ptr[-1]);<br>
return Ptr;<br>
}<br>
<br>
@@ -1234,13 +1242,12 @@ class DenseMapIterator : DebugEpochBase::HandleBase {<br>
<br>
inline DenseMapIterator& operator++() { // Preincrement<br>
assert(isHandleInSync() && "invalid iterator access!");<br>
+ assert(Ptr != End && "incrementing end() iterator");<br>
if (shouldReverseIterate<KeyT>()) {<br>
- assert(Ptr != &End[-1] && "dereferencing end() iterator");<br>
--Ptr;<br>
RetreatPastEmptyBuckets();<br>
return *this;<br>
}<br>
- assert(Ptr != End && "incrementing end() iterator");<br>
++Ptr;<br>
AdvancePastEmptyBuckets();<br>
return *this;<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>