<div dir="ltr">Hi Richard,<div><br></div><div style>this breaks compilation of this bit of code in WebKit, which does metaprogramming to check if a class implements a given method:</div><div><br></div><div><div>template <typename Type> class IsInstrumented {</div>
<div> class yes { char m; };</div><div> class no { yes m[2]; };</div><div> struct BaseMixin {</div><div> void reportMemoryUsage() const {}</div><div> };</div><div> struct Base : public Type, public BaseMixin {</div>
<div> };</div><div> template <typename T, T t> class Helper {</div><div> };</div><div> template <typename U></div><div> static no</div><div> deduce(U *, Helper<void(BaseMixin::*)() const, &U::reportMemoryUsage> * = 0);</div>
<div> static yes deduce(...);</div><div>public:</div><div> static const bool result = sizeof(yes) == sizeof(deduce((Base *)(0)));</div><div>};</div><div><br></div><div>template <typename T> bool hasReportMemoryUsage(const T *object) {</div>
<div> return IsInstrumented<T>::result;</div><div>}</div><div><br></div><div>class Timer {</div><div> void operator delete(void *p) {}</div><div>public:</div><div> virtual ~Timer();</div><div>};</div><div>bool f() {</div>
<div> Timer m_cacheTimer;</div><div> return hasReportMemoryUsage(&m_cacheTimer);</div><div>}</div></div><div><br></div><div><br></div><div style>The error message looks like this:</div><div style><br></div><div style>
<div>$ clang -c repro.ii -std=gnu++11 # works in older clang</div><div>$ third_party/llvm-build/Release+Asserts/bin/clang -c repro.ii -std=gnu++11</div><div>repro.ii:7:10: error: deleted function '~Base' cannot override a non-deleted function</div>
<div> struct Base : public Type, public BaseMixin {</div><div> ^</div><div>repro.ii:13:51: note: in instantiation of member class 'IsInstrumented<Timer>::Base' requested here</div><div> deduce(U *, Helper<void(BaseMixin::*)() const, &U::reportMemoryUsage> * = 0);</div>
<div> ^</div><div>repro.ii:13:3: note: while substituting deduced template arguments into function template 'deduce' [with U = IsInstrumented<Timer>::Base]</div>
<div> deduce(U *, Helper<void(BaseMixin::*)() const, &U::reportMemoryUsage> * = 0);</div><div> ^</div><div>repro.ii:20:10: note: in instantiation of template class 'IsInstrumented<Timer>' requested here</div>
<div> return IsInstrumented<T>::result;</div><div> ^</div><div>repro.ii:30:10: note: in instantiation of function template specialization 'hasReportMemoryUsage<Timer>' requested here</div><div>
return hasReportMemoryUsage(&m_cacheTimer);</div><div> ^</div><div>repro.ii:26:11: note: overridden virtual function is here</div><div> virtual ~Timer();</div><div> ^</div><div>1 error generated.</div>
<div><br></div></div><div><br></div><div style>Is this expected? I suppose so, but the diagnostic is a bit hard to read (it's not clear to me why ~Base() is deleted).</div><div style><br></div><div style>It compiles fine if I give Base an explicit destructor. Is this the right fix?</div>
<div style><br></div><div style>Thanks,</div><div style>Nico</div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Apr 2, 2013 at 12:38 PM, 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: Tue Apr 2 14:38:47 2013<br>
New Revision: 178563<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=178563&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=178563&view=rev</a><br>
Log:<br>
If a defaulted special member is implicitly deleted, check whether it's<br>
overriding a non-deleted virtual function. The existing check for this doesn't<br>
catch this case, because it fires before we mark the method as deleted.<br>
<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>
cfe/trunk/test/CXX/class.derived/class.abstract/p16.cpp<br>
cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp<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=178563&r1=178562&r2=178563&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=178563&r1=178562&r2=178563&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Apr 2 14:38:47 2013<br>
@@ -4403,7 +4403,7 @@ void Sema::CheckExplicitlyDefaultedSpeci<br>
<br>
if (ShouldDeleteSpecialMember(MD, CSM)) {<br>
if (First) {<br>
- MD->setDeletedAsWritten();<br>
+ SetDeclDeleted(MD, MD->getLocation());<br>
} else {<br>
// C++11 [dcl.fct.def.default]p4:<br>
// [For a] user-provided explicitly-defaulted function [...] if such a<br>
@@ -7586,7 +7586,7 @@ CXXConstructorDecl *Sema::DeclareImplici<br>
DefaultCon->setTrivial(ClassDecl->hasTrivialDefaultConstructor());<br>
<br>
if (ShouldDeleteSpecialMember(DefaultCon, CXXDefaultConstructor))<br>
- DefaultCon->setDeletedAsWritten();<br>
+ SetDeclDeleted(DefaultCon, ClassLoc);<br>
<br>
// Note that we have declared this constructor.<br>
++ASTContext::NumImplicitDefaultConstructorsDeclared;<br>
@@ -7794,7 +7794,7 @@ void Sema::DeclareInheritingConstructors<br>
// Core issue (no number): if the same inheriting constructor is<br>
// produced by multiple base class constructors from the same base<br>
// class, the inheriting constructor is defined as deleted.<br>
- result.first->second.second->setDeletedAsWritten();<br>
+ SetDeclDeleted(result.first->second.second, UsingLoc);<br>
}<br>
continue;<br>
}<br>
@@ -7830,7 +7830,7 @@ void Sema::DeclareInheritingConstructors<br>
NewCtor->setParams(ParamDecls);<br>
NewCtor->setInheritedConstructor(BaseCtor);<br>
if (BaseCtor->isDeleted())<br>
- NewCtor->setDeletedAsWritten();<br>
+ SetDeclDeleted(NewCtor, UsingLoc);<br>
<br>
ClassDecl->addDecl(NewCtor);<br>
result.first->second.second = NewCtor;<br>
@@ -7954,7 +7954,7 @@ CXXDestructorDecl *Sema::DeclareImplicit<br>
Destructor->setTrivial(ClassDecl->hasTrivialDestructor());<br>
<br>
if (ShouldDeleteSpecialMember(Destructor, CXXDestructor))<br>
- Destructor->setDeletedAsWritten();<br>
+ SetDeclDeleted(Destructor, ClassLoc);<br>
<br>
// Note that we have declared this destructor.<br>
++ASTContext::NumImplicitDestructorsDeclared;<br>
@@ -8474,7 +8474,7 @@ CXXMethodDecl *Sema::DeclareImplicitCopy<br>
// there is no user-declared move assignment operator, a copy assignment<br>
// operator is implicitly declared as defaulted.<br>
if (ShouldDeleteSpecialMember(CopyAssignment, CXXCopyAssignment))<br>
- CopyAssignment->setDeletedAsWritten();<br>
+ SetDeclDeleted(CopyAssignment, ClassLoc);<br>
<br>
// Note that we have added this copy-assignment operator.<br>
++ASTContext::NumImplicitCopyAssignmentOperatorsDeclared;<br>
@@ -9277,7 +9277,7 @@ CXXConstructorDecl *Sema::DeclareImplici<br>
// user-declared move assignment operator, a copy constructor is implicitly<br>
// declared as defaulted.<br>
if (ShouldDeleteSpecialMember(CopyConstructor, CXXCopyConstructor))<br>
- CopyConstructor->setDeletedAsWritten();<br>
+ SetDeclDeleted(CopyConstructor, ClassLoc);<br>
<br>
// Note that we have declared this constructor.<br>
++ASTContext::NumImplicitCopyConstructorsDeclared;<br>
@@ -10983,6 +10983,7 @@ void Sema::SetDeclDeleted(Decl *Dcl, Sou<br>
Diag(DelLoc, diag::err_deleted_non_function);<br>
return;<br>
}<br>
+<br>
if (const FunctionDecl *Prev = Fn->getPreviousDecl()) {<br>
// Don't consider the implicit declaration we generate for explicit<br>
// specializations. FIXME: Do not generate these implicit declarations.<br>
@@ -10993,7 +10994,29 @@ void Sema::SetDeclDeleted(Decl *Dcl, Sou<br>
}<br>
// If the declaration wasn't the first, we delete the function anyway for<br>
// recovery.<br>
+ Fn = Fn->getCanonicalDecl();<br>
+ }<br>
+<br>
+ if (Fn->isDeleted())<br>
+ return;<br>
+<br>
+ // See if we're deleting a function which is already known to override a<br>
+ // non-deleted virtual function.<br>
+ if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(Fn)) {<br>
+ bool IssuedDiagnostic = false;<br>
+ for (CXXMethodDecl::method_iterator I = MD->begin_overridden_methods(),<br>
+ E = MD->end_overridden_methods();<br>
+ I != E; ++I) {<br>
+ if (!(*MD->begin_overridden_methods())->isDeleted()) {<br>
+ if (!IssuedDiagnostic) {<br>
+ Diag(DelLoc, diag::err_deleted_override) << MD->getDeclName();<br>
+ IssuedDiagnostic = true;<br>
+ }<br>
+ Diag((*I)->getLocation(), diag::note_overridden_virtual_function);<br>
+ }<br>
+ }<br>
}<br>
+<br>
Fn->setDeletedAsWritten();<br>
}<br>
<br>
<br>
Modified: cfe/trunk/test/CXX/class.derived/class.abstract/p16.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.abstract/p16.cpp?rev=178563&r1=178562&r2=178563&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.abstract/p16.cpp?rev=178563&r1=178562&r2=178563&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CXX/class.derived/class.abstract/p16.cpp (original)<br>
+++ cfe/trunk/test/CXX/class.derived/class.abstract/p16.cpp Tue Apr 2 14:38:47 2013<br>
@@ -14,3 +14,29 @@ struct C: A {<br>
virtual void a();<br>
virtual void b() = delete;<br>
};<br>
+<br>
+struct E;<br>
+struct F;<br>
+struct G;<br>
+struct H;<br>
+struct D {<br>
+ virtual E &operator=(const E &); // expected-note {{here}}<br>
+ virtual F &operator=(const F &);<br>
+ virtual G &operator=(G&&);<br>
+ virtual H &operator=(H&&); // expected-note {{here}}<br>
+ friend struct F;<br>
+<br>
+private:<br>
+ D &operator=(const D&) = default;<br>
+ D &operator=(D&&) = default;<br>
+ virtual ~D(); // expected-note 2{{here}}<br>
+};<br>
+struct E : D {}; // expected-error {{deleted function '~E' cannot override a non-deleted function}} \<br>
+ // expected-error {{deleted function 'operator=' cannot override a non-deleted function}}<br>
+struct F : D {};<br>
+// No move ctor here, because it would be deleted.<br>
+struct G : D {}; // expected-error {{deleted function '~G' cannot override a non-deleted function}}<br>
+struct H : D {<br>
+ H &operator=(H&&) = default; // expected-error {{deleted function 'operator=' cannot override a non-deleted function}}<br>
+ ~H();<br>
+};<br>
<br>
Modified: cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp?rev=178563&r1=178562&r2=178563&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp?rev=178563&r1=178562&r2=178563&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp (original)<br>
+++ cfe/trunk/test/CXX/special/class.dtor/p5-0x.cpp Tue Apr 2 14:38:47 2013<br>
@@ -88,9 +88,10 @@ struct C4 : virtual InaccessibleDtor { C<br>
class D1 {<br>
void operator delete(void*);<br>
public:<br>
- virtual ~D1() = default;<br>
+ virtual ~D1() = default; // expected-note {{here}}<br>
} d1; // ok<br>
-struct D2 : D1 { // expected-note {{virtual destructor requires an unambiguous, accessible 'operator delete'}}<br>
+struct D2 : D1 { // expected-note {{virtual destructor requires an unambiguous, accessible 'operator delete'}} \<br>
+ // expected-error {{deleted function '~D2' cannot override a non-deleted}}<br>
// implicitly-virtual destructor<br>
} d2; // expected-error {{deleted function}}<br>
struct D3 { // expected-note {{virtual destructor requires an unambiguous, accessible 'operator delete'}}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">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>