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

Nico Weber thakis at chromium.org
Thu Apr 18 10:50:04 PDT 2013


Filed PR15783, thanks.

The WebKit code was added here and seems to be doing what the people who
wrote it want:
https://bugs.webkit.org/show_bug.cgi?id=105026
https://bugs.webkit.org/attachment.cgi?id=179485&action=review


On Thu, Apr 18, 2013 at 10:43 AM, Richard Smith <richard at metafoo.co.uk>wrote:

> 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/c673f530/attachment.html>


More information about the cfe-commits mailing list