[PATCH] D22079: Refactor Archive-child iteration.

Rafael Ávila de Espíndola via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 7 06:53:20 PDT 2016


rafael added inline comments.

================
Comment at: include/llvm/Object/Archive.h:110
@@ -110,1 +109,3 @@
+    Child C;
+    Error *E;
 
----------------
Can this be "Error &"?

================
Comment at: include/llvm/Object/Archive.h:113
@@ -111,6 +112,3 @@
   public:
-    child_iterator() : child(Child(nullptr, nullptr, nullptr)) {}
-    child_iterator(const Child &c) : child(c) {}
-    child_iterator(std::error_code EC) : child(EC) {}
-    const ErrorOr<Child> *operator->() const { return &child; }
-    const ErrorOr<Child> &operator*() const { return child; }
+    child_iterator() : C(Child(nullptr, nullptr, nullptr)), E(nullptr) {}
+    child_iterator(const Child &C, Error *E) : C(C), E(E) {}
----------------
And delete this.

================
Comment at: include/llvm/Object/Archive.h:132
@@ -132,3 +131,3 @@
     child_iterator &operator++() { // Preincrement
-      assert(child && "Can't increment iterator with error");
-      child = child->getNext();
+      assert(E && "Can't increment iterator with no Error attached");
+      if (auto ChildOrErr = C.getNext())
----------------
and this assert?

Basically this code has the invariant that E can be null, but we know it is not when needed, which is non-obvious.


Repository:
  rL LLVM

http://reviews.llvm.org/D22079





More information about the llvm-commits mailing list