Promote a warning on ill-formed code (redeclaration missing an exception

specification) to an error. No compiler other than Clang seems to allow this,
and it doesn't seem like a useful thing to accept as an extension in general.

The current behavior was added for PR5957, where the problem was specifically
related to mismatches of the exception specification on the implicitly-declared
global operator new and delete. To retain that workaround, we downgrade the
error to an ExtWarn when the declaration is of a replaceable global allocation
function.

Now that this is an error, stop trying (and failing) to recover from a missing
computed noexcept specification. That recovery didn't work, and led to crashes
in code like the added testcase.

llvm-svn: 248867

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -1153,7 +1153,10 @@ def err_incompatible_exception_specs : Error<
   "target exception specification is not superset of source">;
 def err_deep_exception_specs_differ : Error<
   "exception specifications of %select{return|argument}0 types differ">;
-def warn_missing_exception_specification : Warning<
+def ext_missing_exception_specification : ExtWarn<
+  "%0 is missing exception specification '%1'">,
+  InGroup<DiagGroup<"missing-exception-spec">>;
+def err_missing_exception_specification : Error<
   "%0 is missing exception specification '%1'">;
 def err_noexcept_needs_constant_expression : Error<
   "argument to noexcept specifier must be a constant expression">;

clang/lib/Sema/SemaExceptionSpec.cpp

@@ -270,16 +270,31 @@ bool Sema::CheckEquivalentExceptionSpec(FunctionDecl *Old, FunctionDecl *New) {
   FunctionProtoType::ExceptionSpecInfo ESI = OldProto->getExceptionSpecType();
   if (ESI.Type == EST_Dynamic) {
     ESI.Exceptions = OldProto->exceptions();
-  } else if (ESI.Type == EST_ComputedNoexcept) {
-    // FIXME: We can't just take the expression from the old prototype. It
-    // likely contains references to the old prototype's parameters.
   }
 
+  if (ESI.Type == EST_ComputedNoexcept) {
+    // For computed noexcept, we can't just take the expression from the old
+    // prototype. It likely contains references to the old prototype's
+    // parameters.
+    New->setInvalidDecl();
+  } else {
     // Update the type of the function with the appropriate exception
     // specification.
     New->setType(Context.getFunctionType(
         NewProto->getReturnType(), NewProto->getParamTypes(),
         NewProto->getExtProtoInfo().withExceptionSpec(ESI)));
+  }
+
+  // Allow missing exception specifications in redeclarations as an extension,
+  // when declaring a replaceable global allocation function.
+  if (New->isReplaceableGlobalAllocationFunction() &&
+      ESI.Type != EST_ComputedNoexcept) {
+    DiagID = diag::ext_missing_exception_specification;
+    ReturnValueOnError = false;
+  } else {
+    DiagID = diag::err_missing_exception_specification;
+    ReturnValueOnError = true;
+  }
 
   // Warn about the lack of exception specification.
   SmallString<128> ExceptionSpecString;
@@ -322,17 +337,18 @@ bool Sema::CheckEquivalentExceptionSpec(FunctionDecl *Old, FunctionDecl *New) {
   SourceLocation FixItLoc;
   if (TypeSourceInfo *TSInfo = New->getTypeSourceInfo()) {
     TypeLoc TL = TSInfo->getTypeLoc().IgnoreParens();
-    if (FunctionTypeLoc FTLoc = TL.getAs<FunctionTypeLoc>())
+    // FIXME: Preserve enough information so that we can produce a correct fixit
+    // location when there is a trailing return type.
+    if (auto FTLoc = TL.getAs<FunctionProtoTypeLoc>())
+      if (!FTLoc.getTypePtr()->hasTrailingReturn())
         FixItLoc = getLocForEndOfToken(FTLoc.getLocalRangeEnd());
   }
 
   if (FixItLoc.isInvalid())
-    Diag(New->getLocation(), diag::warn_missing_exception_specification)
+    Diag(New->getLocation(), DiagID)
       << New << OS.str();
   else {
-    // FIXME: This will get more complicated with C++0x
-    // late-specified return types.
-    Diag(New->getLocation(), diag::warn_missing_exception_specification)
+    Diag(New->getLocation(), DiagID)
       << New << OS.str()
       << FixItHint::CreateInsertion(FixItLoc, " " + OS.str().str());
   }
@@ -340,7 +356,7 @@ bool Sema::CheckEquivalentExceptionSpec(FunctionDecl *Old, FunctionDecl *New) {
   if (!Old->getLocation().isInvalid())
     Diag(Old->getLocation(), diag::note_previous_declaration);
 
-  return false;
+  return ReturnValueOnError;
 }
 
 /// CheckEquivalentExceptionSpec - Check if the two types have equivalent

clang/test/CXX/drs/dr1xx.cpp

@@ -234,7 +234,7 @@ namespace dr125 {
     friend dr125_A::dr125_B (::dr125_C)(); // ok
     friend dr125_A (::dr125_B::dr125_C)(); // ok
     friend dr125_A::dr125_B::dr125_C(); // expected-error {{did you mean the constructor name 'dr125_B'?}}
-    // expected-warning@-1 {{missing exception specification}}
+    // expected-error@-1 {{missing exception specification}}
 #if __cplusplus >= 201103L
     // expected-error@-3 {{follows constexpr declaration}} expected-note@-10 {{here}}
 #endif

clang/test/CXX/drs/dr5xx.cpp

@@ -7,7 +7,7 @@
 // pointing at the implicit operator new. We can't match such a diagnostic
 // with -verify.
 __extension__ typedef __SIZE_TYPE__ size_t;
-void *operator new(size_t); // expected-warning 0-1{{missing exception spec}} expected-note{{candidate}}
+void *operator new(size_t); // expected-error 0-1{{missing exception spec}} expected-note{{candidate}}
 
 namespace dr500 { // dr500: dup 372
   class D;

clang/test/CXX/except/except.spec/p3.cpp

@@ -24,9 +24,9 @@ extern void (*r4)() throw(...);
 extern void (*r5)() throw(int); // expected-note {{previous declaration}}
 extern void (*r5)(); // expected-error {{exception specification in declaration does not match}}
 
-// For functions, we accept this with a warning.
+// throw(int) and no spec are not compatible
 extern void f5() throw(int); // expected-note {{previous declaration}}
-extern void f5(); // expected-warning {{missing exception specification}}
+extern void f5(); // expected-error {{missing exception specification}}
 
 // Different types are not compatible.
 extern void (*r7)() throw(int); // expected-note {{previous declaration}}

clang/test/CXX/except/except.spec/p4.cpp

@@ -19,7 +19,7 @@ struct T {
   void operator delete(void*) noexcept; // expected-note {{here}}
 };
 
-void T::a() {} // expected-warning {{missing exception specification 'noexcept'}}
+void T::a() {} // expected-error {{missing exception specification 'noexcept'}}
 T::~T() {} // expected-warning {{function previously declared with an explicit exception specification redeclared with an implicit exception specification}}
 void T::operator delete(void*) {} // expected-warning {{function previously declared with an explicit exception specification redeclared with an implicit exception specification}}
 

clang/test/CXX/except/except.spec/p5-pointers.cpp

@@ -73,7 +73,7 @@ void fnptrs()
 // Member function stuff
 
 struct Str1 { void f() throw(int); }; // expected-note {{previous declaration}}
-void Str1::f() // expected-warning {{missing exception specification}}
+void Str1::f() // expected-error {{missing exception specification}}
 {
 }
 

clang/test/FixIt/fixit.cpp

@@ -45,7 +45,7 @@ class B : public A {
 };
 
 void f() throw(); // expected-note{{previous}}
-void f(); // expected-warning{{missing exception specification}}
+void f(); // expected-error{{missing exception specification}}
 
 namespace rdar7853795 {
   struct A {

clang/test/Misc/warning-flags.c

@@ -18,7 +18,7 @@ This test serves two purposes:
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (90):
+CHECK: Warnings without flags (89):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -80,7 +80,6 @@ CHECK-NEXT:   warn_maynot_respond
 CHECK-NEXT:   warn_method_param_redefinition
 CHECK-NEXT:   warn_missing_case_for_condition
 CHECK-NEXT:   warn_missing_dependent_template_keyword
-CHECK-NEXT:   warn_missing_exception_specification
 CHECK-NEXT:   warn_missing_whitespace_after_macro_name
 CHECK-NEXT:   warn_mt_message
 CHECK-NEXT:   warn_no_constructor_for_refconst

clang/test/SemaCXX/exception-spec.cpp

@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fcxx-exceptions -std=c++11 %s
+
+namespace MissingOnTemplate {
+  template<typename T> void foo(T) noexcept(true); // expected-note {{previous}}
+  template<typename T> void foo(T); // expected-error {{missing exception specification 'noexcept(true)'}}
+  void test() { foo(0); }
+}