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