[PATCH] RFC: fail-fast iterators for DenseMap
Sanjoy Das
sanjoy at playingwithpointers.com
Mon Mar 2 10:00:21 PST 2015
Thanks for the review! I'll land this sometime in the next couple of days.
================
Comment at: include/llvm/ADT/DenseMap.h:1014
@@ -1005,3 +1013,3 @@
- DenseMapIterator(pointer Pos, pointer E, bool NoAdvance = false)
- : Ptr(Pos), End(E) {
+ DenseMapIterator(pointer Pos, pointer E, const DebugEpochBase *Epoch,
+ bool NoAdvance = false)
----------------
chandlerc wrote:
> Is Epoch ever false here? If not, pass by reference?
Will do.
================
Comment at: include/llvm/ADT/EpochTracker.h:25-35
@@ +24,13 @@
+
+class DebugEpochBase {
+public:
+ void bumpEpoch() {}
+
+ class HandleBase {
+ public:
+ HandleBase() {}
+ explicit HandleBase(const DebugEpochBase *) {}
+ bool isHandleInSync() { return true; }
+ };
+};
+
----------------
chandlerc wrote:
> Looking at what this will do with doxygen and other things, I think it would be better to sink the #ifdef stuff inside the class and method bodies. That will ensure code navigation and such always have a consistent target. A very minor point though.
Frankly, I'd rather have an easy to understand .h file over easy to read doxygen, so I went against too many `#ifdef`s. I'll push the `#ifdef`s inside class bodies if people actually complain about bad doxygen. :)
================
Comment at: include/llvm/ADT/EpochTracker.h:56
@@ +55,3 @@
+ /// instance.
+ void bumpEpoch() { ++Epoch; }
+
----------------
chandlerc wrote:
> What do you think about calling this "next"? or "increment"? I don't feel strongly, but "bump" seems fairly imprecise...
Will change to `incrementEpoch`
http://reviews.llvm.org/D7931
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list