[PATCH] D38008: [llvm-objcopy] Refactor code to include initialize method

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 21 01:36:12 PDT 2017


jhenderson added a comment.

Looks fine to me. Just a couple of variable names that need dealing with and then I think this can go in.



================
Comment at: tools/llvm-objcopy/Object.cpp:405-406
+                                  Twine TypeErrMsg) {
+  if (T *TSec = llvm::dyn_cast<T>(getSection(Index, IndexErrMsg)))
+    return TSec;
+  error(TypeErrMsg);
----------------
I think you can probably just call "TSec", "Sec".


================
Comment at: tools/llvm-objcopy/Object.h:60
   virtual ~SectionBase() {}
+  virtual void initialize(SectionTableRef Obj);
   virtual void finalize();
----------------
Not sure "Obj" is the right argument name here! See also all the overrides.


Repository:
  rL LLVM

https://reviews.llvm.org/D38008





More information about the llvm-commits mailing list