[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