Page MenuHomePhabricator

Changing DenseMapIterator's comparisons to be hidden friends.
AbandonedPublic

Authored by BRevzin on Apr 27 2020, 5:43 AM.

Details

Reviewers
None
Summary

The existing implementation breaks in C++20 (sorry, see this StackOverflow answer), the new implementation works fine under all language versions.

Diff Detail

Event Timeline

BRevzin created this revision.Apr 27 2020, 5:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2020, 5:43 AM
BRevzin edited the summary of this revision. (Show Details)Apr 27 2020, 5:46 AM
BRevzin abandoned this revision.Apr 27 2020, 9:14 AM

This review changed just the one type, I went ahead and fixed the rest of the clang build in a bigger review.

dblaikie added inline comments.
llvm/include/llvm/ADT/DenseMap.h
1247–1259

Should these use ConstIterator parameters? Guess that'd reduce the number of instantiations/versions of this function (rather than having const and non-const versions of the function instantiated)?

BRevzin marked an inline comment as done.Apr 28 2020, 6:05 AM
BRevzin added inline comments.
llvm/include/llvm/ADT/DenseMap.h
1247–1259

FYI I moved this review to https://reviews.llvm.org/D78938.

The problem is, if I declared it using ConstIterator then both the iterator and const iterator versions would be declaring the same function. I don't know of a way to avoid that.