[PATCH] D19746: NFC: An iterator for stepping through CodeView type stream in llvm-readobj

[Reid Kleckner via llvm-commits]

rnk added inline comments.

================
Comment at: tools/llvm-readobj/COFFDumper.cpp:1978-1982
@@ +1977,7 @@
+
+  explicit CodeViewTypeIterator(const SectionRef &Section)
+      : Data(), Magic(0), Current(), AtEnd(false) {
+    const std::error_code Error = Section.getContents(Data);
+    if (Error)
+      throw Error;
+    if (Data.size() >= 4) {
----------------
I think the best way to handle this error case is to have the caller do the getContents call. Then we aren't in the awkward position handling errors in the constructor.

Alternatively, we could store the error code in the iterator (or some parent iterator state object) and check for it after the loop exit.

================
Comment at: tools/llvm-readobj/COFFDumper.cpp:2026
@@ +2025,3 @@
+private:
+  void Next() {
+    assert(!AtEnd && "Attempted to advance more than one past the last rec");
----------------
In LLVM, methods are leading lower-case:
http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

We are famously inconsistent here and the rules don't make a lot of sense, but we're trying to improve.


http://reviews.llvm.org/D19746