hvdijk added a comment.

In D122663#3471763 <https://reviews.llvm.org/D122663#3471763>, @erichkeane 
wrote:

> 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`?

Yes, that is exactly it. That is what @rsmith was getting at with his comment 
about regarding this as two separate bugs (he can correct me if I am misstating 
things), one that we do not add the substitution and as such use wrong numbers 
for subsequent substitutions, the other that we do not use the substitution. 
The other is what I had covered in my initial test, his addition covers that 
first bug where we wrongly used S4 when we should be using S5.

> 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.

llvm-cxxfilt has no option to be compatible with earlier incorrect mangling 
though, so it would not help with this particular test. But it could help with 
other tests, agreed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122663

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to