[PATCH] D55352: [elfabi] Introduce tool for ELF TextAPI

Jake Ehrlich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 11 17:58:58 PST 2018


jakehehrlich added inline comments.


================
Comment at: llvm/tools/llvm-elfabi/ErrorCollector.cpp:36
+  log(RawSS);
+  UnhandledErrIdx = Errors.size();
+  return createStringError(errc::interrupted, RawSS.str().c_str());
----------------
Rather than this, when you make the error you should be left with an empty vector of errors. Long term I'd like to see this be a move of the vector into an aggregate error but for now I'm fine just consuming and clearing.


================
Comment at: llvm/tools/llvm-elfabi/ErrorCollector.cpp:63
+  }
+  Errors.clear();
+}
----------------
You don't need to clear since the destructor will be called after this anyway.


================
Comment at: llvm/tools/llvm-elfabi/ErrorCollector.h:44
+  /// Logs all errors to a raw_ostream.
+  void log(raw_ostream &OS);
+
----------------
I think I'd prefer this be marked private.


================
Comment at: llvm/tools/llvm-elfabi/ErrorCollector.h:61
+  /// false if errors have been added since the last makeError() call.
+  bool allErrsHandled() const;
+
----------------
I think id prefer this marked private for now.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55352/new/

https://reviews.llvm.org/D55352





More information about the llvm-commits mailing list