[webkit-reviews] review granted: [Bug 214389] [WTF] Remove the unnecessary inner class DefaultHash<T>::Hash : [Attachment 404422] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Fujii Hironori
<Hironori.Fujii at sony.com>'s request for review:
Bug 214389: [WTF] Remove the unnecessary inner class DefaultHash<T>::Hash
https://bugs.webkit.org/show_bug.cgi?id=214389

Attachment 404422: Patch

https://bugs.webkit.org/attachment.cgi?id=404422&action=review




--- Comment #11 from Darin Adler <darin at apple.com> ---
Comment on attachment 404422
  --> https://bugs.webkit.org/attachment.cgi?id=404422
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=404422&action=review

>>> Source/JavaScriptCore/assembler/MacroAssemblerCodeRef.h:532
>>> +template<JSC::PtrTag tag> struct
DefaultHash<JSC::MacroAssemblerCodePtr<tag>> :
JSC::MacroAssemblerCodePtrHash<tag> { };
>> 
>> Can we use "using" here instead of inheritance? I’d rather not have a
separate DefaultHash that is not the same as MacroAssemblerCodePtrHash, in case
that might result in a little code bloat.
>> 
>> Same question about the many other places below where we define a new
struct/class instead of doing "using".
> 
> I also had the same idea, but it doesn't work.
> It should be a template specialization of DefaultHash.

This, then, points to the benefit of the old scheme. The DefaultHash expression
evaluated to a type, and it could also be specified directly, and in both cases
it’s the same type.

With this new scheme, DefaultHash itself is a separate type. So that could lead
to a bit of extra code bloat. Maybe in practice that’s not a problem.

I like the benefits of this for reducing the need for class definitions, and
that it’s simpler.

I’d like to hear from C++ template meta programming efforts.