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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
