[PATCH] D56637: [llvm-objdump] - Cleanup the code. NFCI.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 14 02:38:12 PST 2019


jhenderson added a comment.

Normally, I wouldn't support whole-sale style changes like this, but the files have a weird mixture of styles, so I'm happy that we're normalising. Thanks!



================
Comment at: llvm-objdump.cpp:571-572
     return getRelocationValueString(ELF32BE, Rel, Result);
-  auto *ELF64BE = cast<ELF64BEObjectFile>(Obj);
-  return getRelocationValueString(ELF64BE, Rel, Result);
+  if (auto *ELF64LE = dyn_cast<ELF64LEObjectFile>(Obj))
+    return getRelocationValueString(ELF64LE, Rel, Result);
+  return getRelocationValueString(cast<ELF64BEObjectFile>(Obj), Rel, Result);
----------------
Any particular reason you've moved this one? The previous order I think was just as good.


================
Comment at: llvm-objdump.cpp:1073-1074
     StringRef Fmt = "\t\t\t%08" PRIx64 ":  ";
-    std::vector<RelocationRef>::const_iterator rel_cur = Rels->begin();
-    std::vector<RelocationRef>::const_iterator rel_end = Rels->end();
+    std::vector<RelocationRef>::const_iterator RelI = Rels->begin();
+    std::vector<RelocationRef>::const_iterator RelE = Rels->end();
 
----------------
I prefer the old names (but LLVM-ified), i.e. RelCur and RelEnd.


================
Comment at: llvm-objdump.cpp:1217-1218
     return Elf32BEObj->getSymbol(Sym.getRawDataRefImpl())->getType();
+  if (auto *Elf64LEObj = dyn_cast<ELF64LEObjectFile>(Obj))
+    return Elf64LEObj->getSymbol(Sym.getRawDataRefImpl())->getType();
   if (auto *Elf64BEObj = cast<ELF64BEObjectFile>(Obj))
----------------
Similar comment to above: does this reordering give us anything?


================
Comment at: llvm-objdump.cpp:1261-1262
     addDynamicElfSymbols(Elf32BEObj, AllSymbols);
+  else if (auto *Elf64LEObj = dyn_cast<ELF64LEObjectFile>(Obj))
+    addDynamicElfSymbols(Elf64LEObj, AllSymbols);
   else if (auto *Elf64BEObj = cast<ELF64BEObjectFile>(Obj))
----------------
As above re. reordering.


================
Comment at: llvm-objdump.cpp:1557-1558
 
-    std::vector<RelocationRef>::const_iterator rel_cur = Rels.begin();
-    std::vector<RelocationRef>::const_iterator rel_end = Rels.end();
+    std::vector<RelocationRef>::const_iterator RelI = Rels.begin();
+    std::vector<RelocationRef>::const_iterator RelE = Rels.end();
     // Disassemble symbol by symbol.
----------------
See above comments re. variable naming.


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

https://reviews.llvm.org/D56637





More information about the llvm-commits mailing list