[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