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

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 29 17:48:50 PDT 2015


Author: rsmith
Date: Tue Sep 29 19:48:50 2015
New Revision: 248867

URL: http://llvm.org/viewvc/llvm-project?rev=248867&view=rev
Log:
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.

Added:
    cfe/trunk/test/SemaCXX/exception-spec.cpp
Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
    cfe/trunk/test/CXX/drs/dr1xx.cpp
    cfe/trunk/test/CXX/drs/dr5xx.cpp
    cfe/trunk/test/CXX/except/except.spec/p3.cpp
    cfe/trunk/test/CXX/except/except.spec/p4.cpp
    cfe/trunk/test/CXX/except/except.spec/p5-pointers.cpp
    cfe/trunk/test/FixIt/fixit.cpp
    cfe/trunk/test/Misc/warning-flags.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=248867&r1=248866&r2=248867&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 29 19:48:50 2015
@@ -1153,7 +1153,10 @@ def err_incompatible_exception_specs : E
   "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">;

Modified: cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExceptionSpec.cpp?rev=248867&r1=248866&r2=248867&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExceptionSpec.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExceptionSpec.cpp Tue Sep 29 19:48:50 2015
@@ -270,16 +270,31 @@ bool Sema::CheckEquivalentExceptionSpec(
   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.
   }
 
-  // Update the type of the function with the appropriate exception
-  // specification.
-  New->setType(Context.getFunctionType(
-      NewProto->getReturnType(), NewProto->getParamTypes(),
-      NewProto->getExtProtoInfo().withExceptionSpec(ESI)));
+  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(
   SourceLocation FixItLoc;
   if (TypeSourceInfo *TSInfo = New->getTypeSourceInfo()) {
     TypeLoc TL = TSInfo->getTypeLoc().IgnoreParens();
-    if (FunctionTypeLoc FTLoc = TL.getAs<FunctionTypeLoc>())
-      FixItLoc = getLocForEndOfToken(FTLoc.getLocalRangeEnd());
+    // 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(
   if (!Old->getLocation().isInvalid())
     Diag(Old->getLocation(), diag::note_previous_declaration);
 
-  return false;
+  return ReturnValueOnError;
 }
 
 /// CheckEquivalentExceptionSpec - Check if the two types have equivalent

Modified: cfe/trunk/test/CXX/drs/dr1xx.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/dr1xx.cpp?rev=248867&r1=248866&r2=248867&view=diff
==============================================================================
--- cfe/trunk/test/CXX/drs/dr1xx.cpp (original)
+++ cfe/trunk/test/CXX/drs/dr1xx.cpp Tue Sep 29 19:48:50 2015
@@ -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 at -1 {{missing exception specification}}
+    // expected-error at -1 {{missing exception specification}}
 #if __cplusplus >= 201103L
     // expected-error at -3 {{follows constexpr declaration}} expected-note at -10 {{here}}
 #endif

Modified: cfe/trunk/test/CXX/drs/dr5xx.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/dr5xx.cpp?rev=248867&r1=248866&r2=248867&view=diff
==============================================================================
--- cfe/trunk/test/CXX/drs/dr5xx.cpp (original)
+++ cfe/trunk/test/CXX/drs/dr5xx.cpp Tue Sep 29 19:48:50 2015
@@ -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;

Modified: cfe/trunk/test/CXX/except/except.spec/p3.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p3.cpp?rev=248867&r1=248866&r2=248867&view=diff
==============================================================================
--- cfe/trunk/test/CXX/except/except.spec/p3.cpp (original)
+++ cfe/trunk/test/CXX/except/except.spec/p3.cpp Tue Sep 29 19:48:50 2015
@@ -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}}

Modified: cfe/trunk/test/CXX/except/except.spec/p4.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p4.cpp?rev=248867&r1=248866&r2=248867&view=diff
==============================================================================
--- cfe/trunk/test/CXX/except/except.spec/p4.cpp (original)
+++ cfe/trunk/test/CXX/except/except.spec/p4.cpp Tue Sep 29 19:48:50 2015
@@ -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}}
 

Modified: cfe/trunk/test/CXX/except/except.spec/p5-pointers.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p5-pointers.cpp?rev=248867&r1=248866&r2=248867&view=diff
==============================================================================
--- cfe/trunk/test/CXX/except/except.spec/p5-pointers.cpp (original)
+++ cfe/trunk/test/CXX/except/except.spec/p5-pointers.cpp Tue Sep 29 19:48:50 2015
@@ -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}}
 {
 }
 

Modified: cfe/trunk/test/FixIt/fixit.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/fixit.cpp?rev=248867&r1=248866&r2=248867&view=diff
==============================================================================
--- cfe/trunk/test/FixIt/fixit.cpp (original)
+++ cfe/trunk/test/FixIt/fixit.cpp Tue Sep 29 19:48:50 2015
@@ -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 {

Modified: cfe/trunk/test/Misc/warning-flags.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/warning-flags.c?rev=248867&r1=248866&r2=248867&view=diff
==============================================================================
--- cfe/trunk/test/Misc/warning-flags.c (original)
+++ cfe/trunk/test/Misc/warning-flags.c Tue Sep 29 19:48:50 2015
@@ -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

Added: cfe/trunk/test/SemaCXX/exception-spec.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/exception-spec.cpp?rev=248867&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/exception-spec.cpp (added)
+++ cfe/trunk/test/SemaCXX/exception-spec.cpp Tue Sep 29 19:48:50 2015
@@ -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); }
+}




More information about the cfe-commits mailing list