[PATCH] RFC: fail-fast iterators for DenseMap
Chandler Carruth
chandlerc at gmail.com
Mon Mar 2 09:49:13 PST 2015
Minor nits only. Feel free to submit, but probably submit when you can clean up substantial fallout... I expect this to cause chaos when it lands.
================
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)
----------------
Is Epoch ever false here? If not, pass by reference?
================
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; }
+ };
+};
+
----------------
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.
================
Comment at: include/llvm/ADT/EpochTracker.h:56
@@ +55,3 @@
+ /// instance.
+ void bumpEpoch() { ++Epoch; }
+
----------------
What do you think about calling this "next"? or "increment"? I don't feel strongly, but "bump" seems fairly imprecise...
http://reviews.llvm.org/D7931
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list