[PATCH] D56773: [llvm-symbolizer] Add -no-demangle as alias for -demangle=false

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/DebugInfo/llvm-symbolizer.test:221
 SHORT_FUNCTION_NAME-NOT: _Z1cv
+DEMANGLED_FUNCTION_NAME: c
----------------
Sorry, I realised that I misread the test. The existing --demangle=false tests are testing that the "short" name is NOT matched (that's what the -NOT suffix is for on the line above, so this isn't testing what you think it is testing.

Instead, you'll want some new tests below this and the corresponding checks. The --functions=short means that it is printing the short version of the function name, taken from the debug information, I think. In the new tests you write, you don't want the --functions switch at all, I believe.

Try experimenting with running llvm-symbolizer on %t.input7 and look at the output. You should see a function name c() appear somewhere, if I'm not mistaken. Assuming you do, double-check that --no-demangle turns it into _Z1cv, and then add something like the following tests at the end of the file:

```
; Check that the last of --demangle and --no-demangle wins.
RUN: llvm-symbolizer --demangle < %t.input7 \
RUN:    | FileCheck %s --check-prefix=DEMANGLED_FUNCTION_NAME
RUN: llvm-symbolizer --no-demangle < %t.input7 \
RUN:    | FileCheck %s --check-prefix=MANGLED_FUNCTION_NAME
RUN: llvm-symbolizer --demangle --no-demangle < %t.input7 \
RUN:    | FileCheck %s --check-prefix=MANGLED_FUNCTION_NAME
RUN: llvm-symbolizer --no-demangle --demangle < %t.input7 \
RUN:    | FileCheck %s --check-prefix=DEMANGLED_FUNCTION_NAME

MANGLED_FUNCTION_NAME: _Z1cv
DEMANGLED_FUNCTION_NAME: c()
```


================
Comment at: tools/llvm-symbolizer/llvm-symbolizer.cpp:209
+
+  if (ClNoDemangle.getPosition() > ClDemangle.getPosition())
+    ClDemangle = !ClNoDemangle;
----------------
Did you mean to remove the comment you had here before?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56773