[PATCH] D122663: Mark identifier prefixes as substitutable

Mon Apr 25 07:12:59 PDT 2022

erichkeane added a comment.

In D122663#3471748 <https://reviews.llvm.org/D122663#3471748>, @hvdijk wrote:

> In D122663#3471741 <https://reviews.llvm.org/D122663#3471741>, @erichkeane wrote:
>
>> Ping me EOW if @rsmith doesn't respond in the meantime.  It is also not clear to me whether you were able to capture/fix the issue he had with the clang-abi-compat.cpp test.
>
> Will do, sure. For the record, for that test I applied @rsmith's suggestion verbatim so it *should* address his concern, but I will wait for him to confirm.

I DID see that and am REASONABLY sure you do, but sometimes folks will ask for test coverage because they suspect the resulting behavior will demonstrate some sort of problem with the current patch.  I DID run this through the demangler and found that the PRE15 case looks odd?

`void test10<X>(X::Y::a, X::Y::b, float*, X::Y)`
vs
`void test10<X>(X::Y::a, X::Y::b, float*, float*)`

Or is that exactly the bug being caught here?  That is, is the `float*` substitution not being properly registered/emitted and being confused with `X::Y`?

PS:, and as an unrelated-lament... A while back I wrote a test regarding mangling where I ALSO had it run us through llvm-cxxfilt so that we had an output of every mangled string that showed the demangled version.  I wish we'd done that from the beginning to make it much less confusing what is going on in these.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122663