[PATCH] D67039: [ELF] Add a spell corrector for "undefined symbol" diagnostics
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 3 02:46:09 PDT 2019
grimar added a comment.
It looks fine to me I think. I probably have no more comments except 2 nits below.
================
Comment at: ELF/Relocations.cpp:823
+ size_t i = 0;
+ for (const UndefinedDiag &undef : undefs)
+ if (!undef.locs.empty()) {
----------------
MaskRay wrote:
> grimar wrote:
> > It feels `for` would look a bit better?
> >
> > ```
> > for (size_t I = 0; I<undefs.size(); ++I)
> > if (!undefs[I].locs.empty())
> > reportUndefinedSymbol<ELFT>(undefs[I], i < 2);
> > ```
> I'll use
>
> `for (auto it : enumerate(undefs))`
>
I am not sure `enumerate` is better (it is not consistent and this approach uses `auto`), I'll leave it to others.
================
Comment at: ELF/Relocations.cpp:700
+ DenseMap<StringRef, const Symbol *> map;
+ if (sym.file)
+ for (const Symbol *s : sym.file->getSymbols())
----------------
I think LLD coding style requires curly bracers here, since you have more than one line under `if`.
Repository:
rLLD LLVM Linker
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67039/new/
https://reviews.llvm.org/D67039
More information about the llvm-commits
mailing list