[PATCH] D71202: [test][llvm-cxxfilt] Fix darwin build bot/improve test naming and commenting

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 9 07:19:13 PST 2019


jhenderson marked 2 inline comments as done.
jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-cxxfilt/simple.test:7
+RUN: llvm-cxxfilt -n _Z1fi abc | FileCheck %s
 RUN: echo "Mangled _Z1fi and _Z3foov in string." | llvm-cxxfilt \
 RUN:   | FileCheck %s --check-prefix=CHECK-STRING
----------------
This line should have `llvm-cxxfilt -n`. I've fixed this in rG01d8bb49.


================
Comment at: llvm/test/tools/llvm-cxxfilt/simple.test:9
 RUN:   | FileCheck %s --check-prefix=CHECK-STRING
-RUN: llvm-cxxfilt "CLI remains mangled _Z1fi" \
+RUN: llvm-cxxfilt -n "CLI remains mangled _Z1fi" \
 RUN:   | FileCheck %s --check-prefix=CHECK-MANGLED
----------------
grimar wrote:
> It is probably not obvious that "-n" is the same as `--strip-underscore` (help text does not show it).
> (When I read "Note that this test uses "-n" because on darwin, the default for --strip-underscore is true"
> I wasn't sure if `-n` is the same as `--strip-underscore` or it was a mistype/copy-paste error)
> 
> I'd try stick to the options that are listed in a help text in out tests probably.
> 
> 
-n is what was there before, and is probably better due to being shorter overall. I don't know why it wasn't added to the help text, but hopefully future changes to fix the command-line printing of aliases should fix that. I'll update the comment for additional clarity though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71202





More information about the llvm-commits mailing list