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

[James Henderson via Phabricator via llvm-commits]

jhenderson marked an inline comment as done.
jhenderson added a comment.

I've committed rG9614a7c9391 <https://reviews.llvm.org/rG9614a7c939152909eac40bc1ee8e39c98fd0f483> with the proposed comment update. Please let me know if anybody wants any other changes.



================
Comment at: llvm/test/tools/llvm-cxxfilt/strip-underscore-default.test:8
+CHECK: __Z1fv
+CHECK: ba()
----------------
MaskRay wrote:
> jhenderson wrote:
> > compnerd wrote:
> > > Can you merge this into the previous test and use -triple to test the defaults?  That ensures that the tests always run and people on other hosts can validate that they do not regress tins things.
> > Sounds reasonable. I'll see what I can figure out with that.
> The output of sys::getProcessTriple() is fixed (inferred from LLVM_HOST_TRIPLE).
> 
> ```
> static bool shouldStripUnderscore() {
>   if (StripUnderscore)
>     return true;
>   if (NoStripUnderscore)
>     return false;
>   // If none of them are set, use the default value for platform.
>   // macho has symbols prefix with "_" so strip by default.
>   return Triple(sys::getProcessTriple()).isOSBinFormatMachO();
> }
> ```
> 
> > UNSUPPORTED: system-darwin
> > REQUIRES: system-darwin
> 
> Seem the best to test the platform differences.
> 
Right, as @MaskRay says, --triple isn't an option, since the LLVM_HOST_TRIPLE definition is defined at build time. Additionally, the --triple option isn't available in llvm-cxxfilt. It's a specific option for only certain tools e.g. llvm-objdump and llvm-mc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71202