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

[Dmitry Venikov via Phabricator via llvm-commits]

Quolyk marked 4 inline comments as done.
Quolyk added inline comments.


================
Comment at: test/DebugInfo/llvm-symbolizer.test:221
 SHORT_FUNCTION_NAME-NOT: _Z1cv
+DEMANGLED_FUNCTION_NAME: c
----------------
jhenderson wrote:
> 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()
> ```
Thanks for detailed explanation, now it becomes clear to me.


================
Comment at: tools/llvm-symbolizer/llvm-symbolizer.cpp:209
+
+  if (ClNoDemangle.getPosition() > ClDemangle.getPosition())
+    ClDemangle = !ClNoDemangle;
----------------
jhenderson wrote:
> Did you mean to remove the comment you had here before?
It's back. Wasn't sure it's necessary.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56773