[PATCH] D47414: [llvm-objcopy] Fix null symbol handling
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu May 31 06:13:11 PDT 2018
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
More information about the llvm-commits
mailing list