zturner added a comment.

BTW, as far as updating the demangler, as long as it doesn't crash or generate 
an error on a valid `_E` mangling, that should be sufficient (with a test).  If 
you want bonus points you can make it print out `noexcept` when you see the 
`_E`, but I won't require it as it's a bit of extra work.  If you decide not to 
do that, I'll just file a bug for it so that we don't forget.



================
Comment at: lib/AST/MicrosoftMangle.cpp:2311-2314
+  if (FT->canThrow())
+    Out << 'Z';
+  else
+    Out << "_E";
----------------
rnk wrote:
> zahen wrote:
> > zturner wrote:
> > > I knew that the mangling changed whenever a pointer to a `noexcept` 
> > > function is passed as an argument, and we don't yet handle that, but I'm 
> > > surprised to hear that they changed an existing mangling, since it's a 
> > > hard ABI break.
> > > 
> > > Do you know the major and minor version numbers that this changed in?  
> > > I'd like to test it out for starters, but also since this is an ABI break 
> > > we would need to put it behind `-fms-compatibility-version` and only 
> > > mangle using the new scheme when the compatibility version is 
> > > sufficiently high.
> > It's only when a function is used as a type.  My original rathole was 
> > trying to enumerate all of the places where that could be, but instead I 
> > settled on "everywhere but the initial definition".  It's why false is 
> > passed in the 4th parameter on line 516.
> > 
> > I've confirmed this changed in 15.5 so I'll use that as the compat version.
> I see existing code that uses this pattern: 
> `getLangOpts().isCompatibleWithMSVC(LangOptions::MSVC2015)`
> 
> The MSVCMajorVersion enum is symbolic, so I think you might have to multiply 
> it by a hundred and modify LangOptions::isCompatibleWithMSVC to multiply by 
> two fewer places.
> 
> I guess to fit with the existing enums we'd say MSVC2017_5, even though that 
> conflates VS and VC version numbers.
Ok, I see it now.  That should be fine.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55685



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

Reply via email to