Skip to content

Commit 2cf3c03

Browse files
committed
[DenseMap] Don't capture the BucketEnd pointer before an operation that might change the number of buckets.
This code was added in 887efa5 to fix reverse iteration. The call to InsertIntoBucket/InsertIntoBucketWithLookup can change the number of buckets which will invalidate the BucketEnd. So don't cache it and calculate it when creating the iterator.
1 parent 6235951 commit 2cf3c03

File tree

1 file changed

+30
-12
lines changed

1 file changed

+30
-12
lines changed

llvm/include/llvm/ADT/DenseMap.h

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -220,16 +220,22 @@ class DenseMapBase : public DebugEpochBase {
220220
template <typename... Ts>
221221
std::pair<iterator, bool> try_emplace(KeyT &&Key, Ts &&... Args) {
222222
BucketT *TheBucket;
223-
BucketT *EndBucket =
224-
shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd();
225223
if (LookupBucketFor(Key, TheBucket))
226-
return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),
224+
return std::make_pair(makeIterator(TheBucket,
225+
shouldReverseIterate<KeyT>()
226+
? getBuckets()
227+
: getBucketsEnd(),
228+
*this, true),
227229
false); // Already in map.
228230

229231
// Otherwise, insert the new element.
230232
TheBucket =
231233
InsertIntoBucket(TheBucket, std::move(Key), std::forward<Ts>(Args)...);
232-
return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),
234+
return std::make_pair(makeIterator(TheBucket,
235+
shouldReverseIterate<KeyT>()
236+
? getBuckets()
237+
: getBucketsEnd(),
238+
*this, true),
233239
true);
234240
}
235241

@@ -239,15 +245,21 @@ class DenseMapBase : public DebugEpochBase {
239245
template <typename... Ts>
240246
std::pair<iterator, bool> try_emplace(const KeyT &Key, Ts &&... Args) {
241247
BucketT *TheBucket;
242-
BucketT *EndBucket =
243-
shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd();
244248
if (LookupBucketFor(Key, TheBucket))
245-
return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),
249+
return std::make_pair(makeIterator(TheBucket,
250+
shouldReverseIterate<KeyT>()
251+
? getBuckets()
252+
: getBucketsEnd(),
253+
*this, true),
246254
false); // Already in map.
247255

248256
// Otherwise, insert the new element.
249257
TheBucket = InsertIntoBucket(TheBucket, Key, std::forward<Ts>(Args)...);
250-
return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),
258+
return std::make_pair(makeIterator(TheBucket,
259+
shouldReverseIterate<KeyT>()
260+
? getBuckets()
261+
: getBucketsEnd(),
262+
*this, true),
251263
true);
252264
}
253265

@@ -260,16 +272,22 @@ class DenseMapBase : public DebugEpochBase {
260272
std::pair<iterator, bool> insert_as(std::pair<KeyT, ValueT> &&KV,
261273
const LookupKeyT &Val) {
262274
BucketT *TheBucket;
263-
BucketT *EndBucket =
264-
shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd();
265275
if (LookupBucketFor(Val, TheBucket))
266-
return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),
276+
return std::make_pair(makeIterator(TheBucket,
277+
shouldReverseIterate<KeyT>()
278+
? getBuckets()
279+
: getBucketsEnd(),
280+
*this, true),
267281
false); // Already in map.
268282

269283
// Otherwise, insert the new element.
270284
TheBucket = InsertIntoBucketWithLookup(TheBucket, std::move(KV.first),
271285
std::move(KV.second), Val);
272-
return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),
286+
return std::make_pair(makeIterator(TheBucket,
287+
shouldReverseIterate<KeyT>()
288+
? getBuckets()
289+
: getBucketsEnd(),
290+
*this, true),
273291
true);
274292
}
275293

0 commit comments

Comments
 (0)