rnk added a comment.

Looks good, but we should test both sides of the MSVC behavior.



================
Comment at: clang/test/CodeGenCXX/mangle-ms-auto-templates.cpp:17
+  AutoParmTemplate<0> auto_int;
+  // CHECK: call {{.*}} @"??0?$AutoParmTemplate@$H0A@@@QAE@XZ"
+  AutoParmTemplate<false> auto_bool;
----------------
zequanwu wrote:
> thakis wrote:
> > zequanwu wrote:
> > > thakis wrote:
> > > > Are you sure this is correct? MSVC produces a different mangling 
> > > > (https://godbolt.org/z/VxRfJd) and neither `undname` nor `llvm-undname` 
> > > > / `demumble` can demangle the symbol here (while they demange the msvc 
> > > > output according to godbolt fine).
> > > I use `x64 msvc v19.24` version, which gives 
> > > `@"??0?$AutoParmTemplate@$MH0A@@@QAE@XZ"`. The extra `M` might come from 
> > > qualifier mangling. 
> > > 
> > > For `x86 msvc v19.24(WINE)` version, it produces 
> > > `??0?$AutoParmTemplate@$0A@@@QAE@XZ` for both `AutoParmTemplate<0> 
> > > auto_int` and `AutoParmTemplate<false> auto_int`. Isn't this a bug?
> > The test at the top says -triple=i386. If you have x64 mangling results in 
> > here, you should use a 64-bit triple (ie -tripe=x86_64-pc-windows). If the 
> > mangling is structurally different for different bitnesses, we should 
> > probably have tests for both.
> > 
> > For symbols that can be exported, we need to match msvc's mangling to be 
> > abi-compatible, no matter if we consider the mangling a bug or not.
> In `x64 msvc v19.14`, it still gives the same mangling results as `x86`. From 
> godbolt, it looks like that the mangled type was only added since `msvc 
> v19.20`. (https://godbolt.org/z/xGT3w-). But the `MSVCMajorVersion` is only 
> up to 1914, which I guess means `v19.14`. 
> 
> I will add a new version for `v19.20` which will add `M[type]` for each auto 
> template parameter.
Please add a second RUN line with a new check prefix to test the behavior both 
before and after.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80409

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [P... Reid "Away June-Sep" Kleckner via Phabricator via cfe-commits
    • ... Zequan Wu via Phabricator via cfe-commits
    • ... Reid "Away June-Sep" Kleckner via Phabricator via cfe-commits
    • ... Zequan Wu via Phabricator via cfe-commits
    • ... Reid "Away June-Sep" Kleckner via Phabricator via cfe-commits
    • ... Zequan Wu via Phabricator via cfe-commits
    • ... Zequan Wu via Phabricator via cfe-commits

Reply via email to