r178563 - If a defaulted special member is implicitly deleted, check whether it's

[Richard Smith]

On Thu, Apr 18, 2013 at 10:09 AM, Nico Weber <thakis at chromium.org> wrote:

> 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).
>

Yes, the diagnostic seems to be correct, if unhelpful. Feel free to file a
bug on the diagnostic (if not, I'll try to remember to fix this but can't
promise!). We should be pointing out that the function is implicitly
deleted because it would otherwise try to call an inaccessible
class-specific 'operator delete'.


> It compiles fine if I give Base an explicit destructor. Is this the right
> fix?
>

It's inelegant, but yes, that should work (unless you use this with a final
class or similar...). The check itself looks a bit odd -- usually, one
would check whether a certain expression is valid (something like putting
sizeof(declval<Type>().reportMemoryUsage()) into your SFINAE context)
rather than checking whether a member with the given name exists -- but
assuming it's getting the effect you want, I'll have a think about a more
reliable way to perform the test.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130418/34ad5373/attachment.html>