[PATCH] D55685: Update Microsoft name mangling scheme for exception specifiers in the type system

[Zachary Turner via Phabricator via cfe-commits]

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