[PATCH] D116256: [-fms-extensions] Make some exception specification warnings/errors compatible with what cl.exe does

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks for the patch! This looks roughly right to me.

Maybe the list of ESTs that are allowed to be mismatched should be opt-in instead of opt-out? (i.e. instead of checking for "not EST_BasicNoexcept /  EST_DependentNoexcept", check for EST_NoThrow (I think?) Not sure which way is better.

(Looks like the old code was added for https://llvm.org/PR25265)

Could you check that we still emit the warning on line 5 in https://godbolt.org/z/bxfx8jsjd ? The test is mostly from https://docs.microsoft.com/en-us/cpp/build/reference/zc-noexcepttypes?view=msvc-170 -- it feels like we might want to have different behavior in c++17 (and later) and c++14 (and earlier) for some of the diags, possibly. Looks like MSVC also has an error on line 15 by default (with /std:c++17, it seems to accept it with /std:c++14), while we only warn.

(I'm a bit surprised the `noexcept` bit doesn't make it into the mangled name in the ms abi, but we're consistent with msvc about this so that's all good.)



================
Comment at: clang/lib/Sema/SemaExceptionSpec.cpp:399
+    return false;
   } else if (New->isReplaceableGlobalAllocationFunction() &&
              ESI.Type != EST_DependentNoexcept) {
----------------
nit: LLVM style says "no else after return", so this should become a regular `if` now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116256