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

[Richard Smith]

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

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

Looks like it's trying to determine whether this will compile:

template <>
template <typename T>
void
MemoryInstrumentation::InstrumentationSelector<true>::reportObjectMemoryUsage(const
T* object, MemoryObjectInfo* memoryObjectInfo)
{
    object->reportMemoryUsage(memoryObjectInfo);
}

For that, it should really be doing SFINAE on the
"object->reportMemoryUsage(memoryObjectInfo)" expression.

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