From: Richard Smith Date: Wed, 30 Sep 2015 00:48:50 +0000 (+0000) Subject: Promote a warning on ill-formed code (redeclaration missing an exception X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=73d9148e4de2e2aee56b7b639df12c474fee7797;p=clang 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@248867 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 162f699809..63343b0382 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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>; +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">; diff --git a/lib/Sema/SemaExceptionSpec.cpp b/lib/Sema/SemaExceptionSpec.cpp index 6182ad934e..8e1c88458e 100644 --- a/lib/Sema/SemaExceptionSpec.cpp +++ b/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. } - // 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(FunctionDecl *Old, FunctionDecl *New) { SourceLocation FixItLoc; if (TypeSourceInfo *TSInfo = New->getTypeSourceInfo()) { TypeLoc TL = TSInfo->getTypeLoc().IgnoreParens(); - if (FunctionTypeLoc FTLoc = TL.getAs()) - 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()) + 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 diff --git a/test/CXX/drs/dr1xx.cpp b/test/CXX/drs/dr1xx.cpp index d8d9307a5e..377bfc94ff 100644 --- a/test/CXX/drs/dr1xx.cpp +++ b/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 diff --git a/test/CXX/drs/dr5xx.cpp b/test/CXX/drs/dr5xx.cpp index 5bf085f522..0cf67e6b8c 100644 --- a/test/CXX/drs/dr5xx.cpp +++ b/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; diff --git a/test/CXX/except/except.spec/p3.cpp b/test/CXX/except/except.spec/p3.cpp index d77aea4060..03f1d7626c 100644 --- a/test/CXX/except/except.spec/p3.cpp +++ b/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}} diff --git a/test/CXX/except/except.spec/p4.cpp b/test/CXX/except/except.spec/p4.cpp index 8d1b75fbdd..04b2bd9bf2 100644 --- a/test/CXX/except/except.spec/p4.cpp +++ b/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}} diff --git a/test/CXX/except/except.spec/p5-pointers.cpp b/test/CXX/except/except.spec/p5-pointers.cpp index f855520aa2..fe4a264587 100644 --- a/test/CXX/except/except.spec/p5-pointers.cpp +++ b/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}} { } diff --git a/test/FixIt/fixit.cpp b/test/FixIt/fixit.cpp index 512713aa52..686cc23d63 100644 --- a/test/FixIt/fixit.cpp +++ b/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 { diff --git a/test/Misc/warning-flags.c b/test/Misc/warning-flags.c index f61e966612..bd97c4ae80 100644 --- a/test/Misc/warning-flags.c +++ b/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 diff --git a/test/SemaCXX/exception-spec.cpp b/test/SemaCXX/exception-spec.cpp new file mode 100644 index 0000000000..f301a63503 --- /dev/null +++ b/test/SemaCXX/exception-spec.cpp @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -fcxx-exceptions -std=c++11 %s + +namespace MissingOnTemplate { + template void foo(T) noexcept(true); // expected-note {{previous}} + template void foo(T); // expected-error {{missing exception specification 'noexcept(true)'}} + void test() { foo(0); } +}