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

[akhuang]

Summary

Make clang-cl error when a function definition is missing 'noexcept',
and succeed without warnings when missing '__declspec(nothrow)' or 'throw'.

Fixes pr52860

[thakis]

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

clang/lib/Sema/SemaExceptionSpec.cpp

397

nit: LLVM style says "no else after return", so this should become a regular if now.

[akhuang]

Fix warning behavior

[akhuang]

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

[thakis]

clang/test/SemaCXX/MicrosoftCompatibility.cpp

4

Could you add a run line with -std=c++17?

[akhuang]

Add std=c++17 to the test.

[thakis]

Thanks!

clang/include/clang/Basic/DiagnosticSemaKinds.td

def err_deep_exception_specs_differ : Error<
   "exception specifications of %select{return|argument}0 types differ">;
 def warn_deep_exception_specs_differ : Warning<
   err_deep_exception_specs_differ.Text>, InGroup<IncompatibleExceptionSpec>;
 def err_missing_exception_specification : Error<
   "%0 is missing exception specification '%1'">;
 def ext_missing_exception_specification : ExtWarn<
   err_missing_exception_specification.Text>,
   InGroup<DiagGroup<"missing-exception-spec">>;
-def ext_ms_missing_exception_specification : ExtWarn<
-  err_missing_exception_specification.Text>,
-  InGroup<MicrosoftExceptionSpec>;
 def err_noexcept_needs_constant_expression : Error<
   "argument to noexcept specifier must be a constant expression">;
 def err_exception_spec_not_parsed : Error<
   "exception specification is not available until end of class definition">;
 def err_exception_spec_cycle : Error<
   "exception specification of %0 uses itself">;
 def err_exception_spec_incomplete_type : Error<
   "exception specification needed for member of incomplete class %0">;

clang/lib/Sema/SemaExceptionSpec.cpp

bool Sema::CheckEquivalentExceptionSpec(FunctionDecl *Old, FunctionDecl *New) {
   } else {
     // Update the type of the function with the appropriate exception
     // specification.
     New->setType(Context.getFunctionType(
         NewProto->getReturnType(), NewProto->getParamTypes(),
         NewProto->getExtProtoInfo().withExceptionSpec(ESI)));
   }
 
-  if (getLangOpts().MSVCCompat && ESI.Type != EST_DependentNoexcept) {
-    // Allow missing exception specifications in redeclarations as an extension.
-    DiagID = diag::ext_ms_missing_exception_specification;
+  if (getLangOpts().MSVCCompat && isDynamicExceptionSpec(ESI.Type)) {
+    DiagID = diag::ext_missing_exception_specification;
     ReturnValueOnError = false;
   } else if (New->isReplaceableGlobalAllocationFunction() &&
              ESI.Type != EST_DependentNoexcept) {
     // Allow missing exception specifications in redeclarations as an extension,
     // when declaring a replaceable global allocation function.
     DiagID = diag::ext_missing_exception_specification;
     ReturnValueOnError = false;
   } else if (ESI.Type == EST_NoThrow) {
+    // Don't emit any warning for missing 'nothrow' in MSVC.
+    if (getLangOpts().MSVCCompat) {
+      return false;
+    }
     // Allow missing attribute 'nothrow' in redeclarations, since this is a very
     // common omission.
     DiagID = diag::ext_missing_exception_specification;
     ReturnValueOnError = false;
   } else {
     DiagID = diag::err_missing_exception_specification;
     ReturnValueOnError = true;
   }

clang/test/SemaCXX/MicrosoftCompatibility.cpp

 // RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++11 -Wmicrosoft -verify -fms-compatibility -fexceptions -fcxx-exceptions -fms-compatibility-version=19.28
 // RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++11 -Wmicrosoft -verify -fms-compatibility -fexceptions -fcxx-exceptions -fms-compatibility-version=19.27
 // RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++11 -Wmicrosoft -verify -fms-compatibility -fexceptions -fcxx-exceptions -fms-compatibility-version=19.00
 // RUN: %clang_cc1 %s -triple i686-pc-win32 -fsyntax-only -std=c++11 -Wmicrosoft -verify -fms-compatibility -fexceptions -fcxx-exceptions -fms-compatibility-version=18.00
 
 #if defined(_HAS_CHAR16_T_LANGUAGE_SUPPORT) && _HAS_CHAR16_T_LANGUAGE_SUPPORT
 char16_t x;
 char32_t y;
 #else
 typedef unsigned short char16_t;
 typedef unsigned int char32_t;
 #endif

 
 class B : public A {
   virtual ~B();
 #if __cplusplus <= 199711L
   // expected-warning@-2 {{exception specification of overriding function is more lax than base version}}
 #endif
 };
 
-}
+void f4() throw(); // expected-note {{previous declaration is here}}
+void f4() {}       // expected-warning {{'f4' is missing exception specification 'throw()'}}
 
-namespace PR25265 {
-struct S {
-  int fn() throw(); // expected-note {{previous declaration is here}}
-};
+__declspec(nothrow) void f5();
+void f5() {}
 
-int S::fn() { return 0; } // expected-warning {{is missing exception specification}}
+void f6() noexcept; // expected-note {{previous declaration is here}}
+void f6() {}        // expected-error {{'f6' is missing exception specification 'noexcept'}}
 }
 
 namespace PR43265 {
 template <int N> // expected-note {{template parameter is declared here}}
 struct Foo {
   static const int N = 42; // expected-warning {{declaration of 'N' shadows template parameter}}
 };
 }