On Thu, Apr 18, 2013 at 10:50 AM, Nico Weber <[email protected]> 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 <[email protected]>wrote: > >> On Thu, Apr 18, 2013 at 10:09 AM, Nico Weber <[email protected]> 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. >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
