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

Amy Huang via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 5 17:56:34 PST 2022


akhuang added a comment.

In D116256#3215801 <https://reviews.llvm.org/D116256#3215801>, @thakis wrote:

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

It did not still emit the warning but I've changed it now. I'm not sure why https://llvm.org/PR25265 added a new warning (ext_ms_missing_exception_specification) instead of just using ext_missing_exception_specification; I guess I'll get rid of it anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116256



More information about the cfe-commits mailing list