On Tue, Jul 16, 2013 at 3:24 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">According to the commit message, it sounds like <a href="http://llvm.org/bugs/show_bug.cgi?id=16638" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=16638</a> is working as intended. Is that correct? I found this diagnostic very confusing. (The code in the bug is reduced from v8 code, built with stlport.)</div>
</blockquote><div><br></div><div>Yes, this is working as intended, although the diagnostic could be clearer. I wouldn't object to downgrading the operator delete case from an ExtWarn to an Extension, if you feel so inclined.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><div class="gmail_quote">On Sat, Oct 20, 2012 at 1:26 AM, Richard Smith <span dir="ltr"><<a href="mailto:richard-llvm@metafoo.co.uk" target="_blank">richard-llvm@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rsmith<br>
Date: Sat Oct 20 03:26:51 2012<br>
New Revision: 166372<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=166372&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=166372&view=rev</a><br>
Log:<br>
Rework implementation of DR1492: Apply the resolution to operator delete too,<br>
since it also has an implicit exception specification. Downgrade the error to<br>
an extwarn, since at least for operator delete, system headers like to declare<br>
it as 'noexcept' whereas the implicit definition does not have an explicit<br>
exception specification. Move the exception specification for user-declared<br>
'operator delete' functions from the type-as-written into the type, to reflect<br>
reality and to allow us to detect whether there was an implicit exception spec<br>
or not.<br>
<br>
Added:<br>
cfe/trunk/test/CXX/except/except.spec/p4.cpp<br>
Modified:<br>
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
cfe/trunk/lib/Sema/SemaDecl.cpp<br>
cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
cfe/trunk/lib/Sema/SemaExceptionSpec.cpp<br>
cfe/trunk/lib/Sema/SemaType.cpp<br>
cfe/trunk/test/CXX/except/except.spec/p15.cpp<br>
cfe/trunk/test/CXX/special/class.dtor/p3.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=166372&r1=166371&r2=166372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=166372&r1=166371&r2=166372&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat Oct 20 03:26:51 2012<br>
@@ -5425,6 +5425,10 @@<br>
"defaulting this %select{default constructor|copy constructor|move "<br>
"constructor|copy assignment operator|move assignment operator|destructor}0 "<br>
"would delete it after its first declaration">;<br>
+def ext_implicit_exception_spec_mismatch : ExtWarn<<br>
+ "function previously declared with an %select{explicit|implicit}0 exception "<br>
+ "specification redeclared with an %select{implicit|explicit}0 exception "<br>
+ "specification">, InGroup<DiagGroup<"implicit-exception-spec-mismatch">>;<br>
<br>
def warn_ptr_arith_precedes_bounds : Warning<<br>
"the pointer decremented by %0 refers before the beginning of the array">,<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=166372&r1=166371&r2=166372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=166372&r1=166371&r2=166372&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Sat Oct 20 03:26:51 2012<br>
@@ -5519,6 +5519,20 @@<br>
diag::err_static_out_of_line)<br>
<< FixItHint::CreateRemoval(D.getDeclSpec().getStorageClassSpecLoc());<br>
}<br>
+<br>
+ // C++11 [except.spec]p15:<br>
+ // A deallocation function with no exception-specification is treated<br>
+ // as if it were specified with noexcept(true).<br>
+ const FunctionProtoType *FPT = R->getAs<FunctionProtoType>();<br>
+ if ((Name.getCXXOverloadedOperator() == OO_Delete ||<br>
+ Name.getCXXOverloadedOperator() == OO_Array_Delete) &&<br>
+ getLangOpts().CPlusPlus0x && FPT && !FPT->hasExceptionSpec()) {<br>
+ FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();<br>
+ EPI.ExceptionSpecType = EST_BasicNoexcept;<br>
+ NewFD->setType(Context.getFunctionType(FPT->getResultType(),<br>
+ FPT->arg_type_begin(),<br>
+ FPT->getNumArgs(), EPI));<br>
+ }<br>
}<br>
<br>
// Filter out previous declarations that don't match the scope.<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=166372&r1=166371&r2=166372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=166372&r1=166371&r2=166372&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Sat Oct 20 03:26:51 2012<br>
@@ -9379,7 +9379,7 @@<br>
}<br>
<br>
static bool<br>
-CheckOperatorDeleteDeclaration(Sema &SemaRef, const FunctionDecl *FnDecl) {<br>
+CheckOperatorDeleteDeclaration(Sema &SemaRef, FunctionDecl *FnDecl) {<br>
// C++ [basic.stc.dynamic.deallocation]p1:<br>
// A program is ill-formed if deallocation functions are declared in a<br>
// namespace scope other than global scope or declared static in global<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaExceptionSpec.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExceptionSpec.cpp?rev=166372&r1=166371&r2=166372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExceptionSpec.cpp?rev=166372&r1=166371&r2=166372&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaExceptionSpec.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaExceptionSpec.cpp Sat Oct 20 03:26:51 2012<br>
@@ -120,21 +120,22 @@<br>
return SourceDecl->getType()->castAs<FunctionProtoType>();<br>
}<br>
<br>
-/// Get the type that a function had prior to adjustment of the exception<br>
+/// Determine whether a function has an implicitly-generated exception<br>
/// specification.<br>
-static const FunctionProtoType *getUnadjustedFunctionType(FunctionDecl *Decl) {<br>
- if (isa<CXXDestructorDecl>(Decl) && Decl->getTypeSourceInfo()) {<br>
- const FunctionProtoType *Ty =<br>
- Decl->getTypeSourceInfo()->getType()->getAs<FunctionProtoType>();<br>
- if (!Ty->hasExceptionSpec())<br>
- // The type will be adjusted. Use the EST_None exception specification<br>
- // from the type as written.<br>
- return Ty;<br>
- }<br>
+static bool hasImplicitExceptionSpec(FunctionDecl *Decl) {<br>
+ if (!isa<CXXDestructorDecl>(Decl) &&<br>
+ Decl->getDeclName().getCXXOverloadedOperator() != OO_Delete &&<br>
+ Decl->getDeclName().getCXXOverloadedOperator() != OO_Array_Delete)<br>
+ return false;<br>
+<br>
+ // If the user didn't declare the function, its exception specification must<br>
+ // be implicit.<br>
+ if (!Decl->getTypeSourceInfo())<br>
+ return true;<br>
<br>
- // Use whatever type the function now has. The TypeSourceInfo does not contain<br>
- // an instantiated exception specification for a function template,<br>
- return Decl->getType()->getAs<FunctionProtoType>();<br>
+ const FunctionProtoType *Ty =<br>
+ Decl->getTypeSourceInfo()->getType()->getAs<FunctionProtoType>();<br>
+ return !Ty->hasExceptionSpec();<br>
}<br>
<br>
bool Sema::CheckEquivalentExceptionSpec(FunctionDecl *Old, FunctionDecl *New) {<br>
@@ -150,18 +151,31 @@<br>
// specification adjustment is applied.<br>
if (!CheckEquivalentExceptionSpec(<br>
PDiag(DiagID), PDiag(diag::note_previous_declaration),<br>
- getUnadjustedFunctionType(Old), Old->getLocation(),<br>
- getUnadjustedFunctionType(New), New->getLocation(),<br>
+ Old->getType()->getAs<FunctionProtoType>(), Old->getLocation(),<br>
+ New->getType()->getAs<FunctionProtoType>(), New->getLocation(),<br>
&MissingExceptionSpecification, &MissingEmptyExceptionSpecification,<br>
- /*AllowNoexceptAllMatchWithNoSpec=*/true, IsOperatorNew))<br>
+ /*AllowNoexceptAllMatchWithNoSpec=*/true, IsOperatorNew)) {<br>
+ // C++11 [except.spec]p4 [DR1492]:<br>
+ // If a declaration of a function has an implicit<br>
+ // exception-specification, other declarations of the function shall<br>
+ // not specify an exception-specification.<br>
+ if (getLangOpts().CPlusPlus0x &&<br>
+ hasImplicitExceptionSpec(Old) != hasImplicitExceptionSpec(New)) {<br>
+ Diag(New->getLocation(), diag::ext_implicit_exception_spec_mismatch)<br>
+ << hasImplicitExceptionSpec(Old);<br>
+ if (!Old->getLocation().isInvalid())<br>
+ Diag(Old->getLocation(), diag::note_previous_declaration);<br>
+ }<br>
return false;<br>
+ }<br>
<br>
// The failure was something other than an empty exception<br>
// specification; return an error.<br>
if (!MissingExceptionSpecification && !MissingEmptyExceptionSpecification)<br>
return true;<br>
<br>
- const FunctionProtoType *NewProto = getUnadjustedFunctionType(New);<br>
+ const FunctionProtoType *NewProto =<br>
+ New->getType()->getAs<FunctionProtoType>();<br>
<br>
// The new function declaration is only missing an empty exception<br>
// specification "throw()". If the throw() specification came from a<br>
@@ -186,7 +200,8 @@<br>
}<br>
<br>
if (MissingExceptionSpecification && NewProto) {<br>
- const FunctionProtoType *OldProto = getUnadjustedFunctionType(Old);<br>
+ const FunctionProtoType *OldProto =<br>
+ Old->getType()->getAs<FunctionProtoType>();<br>
<br>
FunctionProtoType::ExtProtoInfo EPI = NewProto->getExtProtoInfo();<br>
EPI.ExceptionSpecType = OldProto->getExceptionSpecType();<br>
@@ -310,6 +325,10 @@<br>
<br>
/// CheckEquivalentExceptionSpec - Check if the two types have compatible<br>
/// exception specifications. See C++ [except.spec]p3.<br>
+///<br>
+/// \return \c false if the exception specifications match, \c true if there is<br>
+/// a problem. If \c true is returned, either a diagnostic has already been<br>
+/// produced or \c *MissingExceptionSpecification is set to \c true.<br>
bool Sema::CheckEquivalentExceptionSpec(const PartialDiagnostic &DiagID,<br>
const PartialDiagnostic & NoteID,<br>
const FunctionProtoType *Old,<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaType.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=166372&r1=166371&r2=166372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=166372&r1=166371&r2=166372&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaType.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaType.cpp Sat Oct 20 03:26:51 2012<br>
@@ -2169,16 +2169,6 @@<br>
ASTContext &Context = S.Context;<br>
const LangOptions &LangOpts = S.getLangOpts();<br>
<br>
- bool ImplicitlyNoexcept = false;<br>
- if (D.getName().getKind() == UnqualifiedId::IK_OperatorFunctionId &&<br>
- LangOpts.CPlusPlus0x) {<br>
- OverloadedOperatorKind OO = D.getName().OperatorFunctionId.Operator;<br>
- /// In C++0x, deallocation functions (normal and array operator delete)<br>
- /// are implicitly noexcept.<br>
- if (OO == OO_Delete || OO == OO_Array_Delete)<br>
- ImplicitlyNoexcept = true;<br>
- }<br>
-<br>
// The name we're declaring, if any.<br>
DeclarationName Name;<br>
if (D.getIdentifier())<br>
@@ -2578,12 +2568,6 @@<br>
Exceptions,<br>
EPI);<br>
<br>
- if (FTI.getExceptionSpecType() == EST_None &&<br>
- ImplicitlyNoexcept && chunkIndex == 0) {<br>
- // Only the outermost chunk is marked noexcept, of course.<br>
- EPI.ExceptionSpecType = EST_BasicNoexcept;<br>
- }<br>
-<br>
T = Context.getFunctionType(T, ArgTys.data(), ArgTys.size(), EPI);<br>
}<br>
<br>
<br>
Modified: cfe/trunk/test/CXX/except/except.spec/p15.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p15.cpp?rev=166372&r1=166371&r2=166372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p15.cpp?rev=166372&r1=166371&r2=166372&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CXX/except/except.spec/p15.cpp (original)<br>
+++ cfe/trunk/test/CXX/except/except.spec/p15.cpp Sat Oct 20 03:26:51 2012<br>
@@ -9,16 +9,20 @@<br>
delete[] new int[1];<br>
}<br>
<br>
-void operator delete(void*) noexcept;<br>
-void operator delete[](void*) noexcept;<br>
+void operator delete(void*);<br>
+void operator delete[](void*);<br>
+<br>
+static_assert(noexcept(operator delete(0)), "");<br>
+static_assert(noexcept(operator delete[](0)), "");<br>
<br>
// Same goes for explicit declarations.<br>
void operator delete(void*, float);<br>
-void operator delete(void*, float) noexcept;<br>
-<br>
void operator delete[](void*, float);<br>
-void operator delete[](void*, float) noexcept;<br>
+<br>
+static_assert(noexcept(operator delete(0, 0.f)), "");<br>
+static_assert(noexcept(operator delete[](0, 0.f)), "");<br>
<br>
// But explicit specs stay.<br>
void operator delete(void*, double) throw(int); // expected-note {{previous}}<br>
+static_assert(!noexcept(operator delete(0, 0.)), "");<br>
void operator delete(void*, double) noexcept; // expected-error {{does not match}}<br>
<br>
Added: cfe/trunk/test/CXX/except/except.spec/p4.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p4.cpp?rev=166372&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p4.cpp?rev=166372&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/CXX/except/except.spec/p4.cpp (added)<br>
+++ cfe/trunk/test/CXX/except/except.spec/p4.cpp Sat Oct 20 03:26:51 2012<br>
@@ -0,0 +1,36 @@<br>
+// RUN: %clang_cc1 -std=c++11 %s -verify -fcxx-exceptions<br>
+<br>
+// We permit overriding an implicit exception specification with an explicit one<br>
+// as an extension, for compatibility with existing code.<br>
+<br>
+struct S {<br>
+ void a(); // expected-note {{here}}<br>
+ ~S(); // expected-note {{here}}<br>
+ void operator delete(void*); // expected-note {{here}}<br>
+};<br>
+<br>
+void S::a() noexcept {} // expected-error {{does not match previous}}<br>
+S::~S() noexcept {} // expected-warning {{function previously declared with an implicit exception specification redeclared with an explicit exception specification}}<br>
+void S::operator delete(void*) noexcept {} // expected-warning {{function previously declared with an implicit exception specification redeclared with an explicit exception specification}}<br>
+<br>
+struct T {<br>
+ void a() noexcept; // expected-note {{here}}<br>
+ ~T() noexcept; // expected-note {{here}}<br>
+ void operator delete(void*) noexcept; // expected-note {{here}}<br>
+};<br>
+<br>
+void T::a() {} // expected-warning {{missing exception specification 'noexcept'}}<br>
+T::~T() {} // expected-warning {{function previously declared with an explicit exception specification redeclared with an implicit exception specification}}<br>
+void T::operator delete(void*) {} // expected-warning {{function previously declared with an explicit exception specification redeclared with an implicit exception specification}}<br>
+<br>
+<br>
+// The extension does not extend to function templates.<br>
+<br>
+template<typename T> struct U {<br>
+ T t;<br>
+ ~U(); // expected-note {{here}}<br>
+ void operator delete(void*); // expected-note {{here}}<br>
+};<br>
+<br>
+template<typename T> U<T>::~U() noexcept(true) {} // expected-error {{exception specification in declaration does not match previous declaration}}<br>
+template<typename T> void U<T>::operator delete(void*) noexcept(false) {} // expected-error {{exception specification in declaration does not match previous declaration}}<br>
<br>
Modified: cfe/trunk/test/CXX/special/class.dtor/p3.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.dtor/p3.cpp?rev=166372&r1=166371&r2=166372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.dtor/p3.cpp?rev=166372&r1=166371&r2=166372&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CXX/special/class.dtor/p3.cpp (original)<br>
+++ cfe/trunk/test/CXX/special/class.dtor/p3.cpp Sat Oct 20 03:26:51 2012<br>
@@ -4,10 +4,10 @@<br>
// the exception specification adjustment occurs.<br>
namespace DR1492 {<br>
struct A { ~A(); }; // expected-note {{here}}<br>
- A::~A() noexcept {} // expected-error {{does not match previous declaration}}<br>
+ A::~A() noexcept {} // expected-warning {{previously declared with an implicit exception specification}}<br>
<br>
struct B { ~B() noexcept; }; // expected-note {{here}}<br>
- B::~B() {} // expected-warning {{~B' is missing exception specification 'noexcept'}}<br>
+ B::~B() {} // expected-warning {{previously declared with an explicit exception specification}}<br>
<br>
template<typename T> struct C {<br>
T t;<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br>