[PATCH] D47414: [llvm-objcopy] Fix null symbol handling

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: tools/llvm-objcopy/Object.cpp:203
 void SymbolTableSection::updateSymbols(function_ref<void(Symbol &)> Callable) {
-  for (auto &Sym : Symbols)
-    Callable(*Sym);
+  for (auto Sym = std::begin(Symbols) + 1; Sym != std::end(Symbols); Sym++)
+    Callable(**Sym);
----------------
Usual style in LLVM is to assign begin and end values in the initialization clause. See [[ https://llvm.org/docs/CodingStandards.html#don-t-evaluate-end-every-time-through-a-loop | Don't evaluate end() every time through a loop]]. Similarly [[ https://llvm.org/docs/CodingStandards.html#prefer-preincrement | use ++Sym instead of Sym++ ]].


================
Comment at: tools/llvm-objcopy/Object.h:370-371
   void addSymbolNames();
-  bool empty() const { return Symbols.empty(); }
+  // We don't want to count the null symbol for the SymbolTable emptyness
+  // as the user might not alter it in anyway.
+  bool empty() const { return Symbols.size() == 1; }
----------------
I'd probably rephrase this, and say something like "An 'empty' symbol table still contains a null symbol." The fact that we don't alter it is not really relevant.


Repository:
  rL LLVM

https://reviews.llvm.org/D47414