[PATCH] D60134: [llvm-nm]Add support for --no-demangle

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 3 04:47:05 PDT 2019


grimar accepted this revision.
grimar added a comment.
This revision is now accepted and ready to land.

LGTM, comment inline.



================
Comment at: test/tools/llvm-nm/X86/demangle.ll:23
+; RUN: llvm-nm --no-demangle -C --no-demangle %t.o | FileCheck --check-prefix="MANGLED" %s
+; RUN: llvm-nm -C --no-demangle -C %t.o | FileCheck --check-prefix="DEMANGLED" %s
+
----------------
It feels to me a bit excessive to test all possible combination with aliases.
I think lines 4 and 5 of this test are used to demonstrate that `-C` and `--demangle` are equal.
(and the testing of `cl::opt` vs `cl::alias` interaction is done in `llvm\unittests\Support\CommandLineTest.cpp` already).
So I would not do it. And do just:

```
; RUN: llvm-nm --demangle --no-demangle %t.o | FileCheck --check-prefix="MANGLED" %s
; RUN: llvm-nm --no-demangle --demangle %t.o | FileCheck --check-prefix="DEMANGLED" %s
; RUN: llvm-nm --no-demangle --demangle --no-demangle %t.o | FileCheck --check-prefix="MANGLED" %s
; RUN: llvm-nm --demangle --no-demangle --demangle %t.o | FileCheck --check-prefix="DEMANGLED" %s

```

Otherwise, we might end up with a crazy amount of lines in the tests.
Up to you probably.




Repository:
  rL LLVM

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

https://reviews.llvm.org/D60134





More information about the llvm-commits mailing list