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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.

Thanks!



================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:655
+  if (ReverseSort)
+    llvm::sort(SymbolList, std::greater<NMSymbol>());
+  else
----------------
Optional: this can be written as `llvm::sort(SymbolList, std::greater<>());` to reduce boilerplate.


================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:249
 
+bool operator>(const NMSymbol &A, const NMSymbol &B) { return B < A; }
+} // anonymous namespace
----------------
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>()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119028