[PATCH] D119028: [NFC] Refactor llvm-nm symbol comparing and split sorting

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:249
 
+bool operator>(const NMSymbol &A, const NMSymbol &B) { return B < A; }
+} // anonymous namespace
----------------
MaskRay wrote:
> jhenderson wrote:
> > DiggerLin wrote:
> > > MaskRay wrote:
> > > > Is `operator>` used? If used, can the use site be refactored to only use `operator<`?
> > > yes, the operator> is used to change
> > > 
> > > 
> > > ```
> > >  llvm::sort(SymbolList, [=](const NMSymbol &A, const NMSymbol &B) -> bool {
> > >         return Cmp(B, A);
> > >       });
> > > ```
> > > 
> > > ---->
> > > 
> > > 
> > > 
> > > > llvm::stable_sort(SymbolList, std::greater<NMSymbol>());
> > > 
> > > there is Jame's comment as 
> > > https://reviews.llvm.org/D112735#inline-1137095
> > > 
> > > 
> > > 
> > Just noting that if @MaskRay prefers using the lambda and avoid the `operator>`, I'm okay with that.
> OK, `operator>` is fine since there is a `std::greater<NMSymbol>()`.
Actually, I think it'd be better to do:

```
llvm::sort(Symbols);
if (ReverseSort)
  std::reverse(Symbols.begin(), Symbols.end());
```

NMSymbol is not trivially copyable, so llvm::sort expands to std::sort.
Having two instantiations (less and greater) increase the code size too much.
std::reverse can decrease the code size.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119028