[webkit-reviews] review granted: [Bug 180762] Enhance Ref and RefPtr to be able to work with smart pointers. : [Attachment 329325] proposed patch.

[bugzilla-daemon at webkit.org]

JF Bastien <jfbastien at apple.com> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 180762: Enhance Ref and RefPtr to be able to work with smart pointers.
https://bugs.webkit.org/show_bug.cgi?id=180762

Attachment 329325: proposed patch.

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




--- Comment #15 from JF Bastien <jfbastien at apple.com> ---
Comment on attachment 329325
  --> https://bugs.webkit.org/attachment.cgi?id=329325
proposed patch.

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

Added Darin and Chris, just because it's hard to see mistakes in code like this
sometimes, despite having shoulder-coded with you on this code for a bit, so
even deferred review would be useful :-)
As I like to say, C++ templates are their own reward! Not sure that's a good
thing, but I like how the patch turned out.

Should we also update http://webkit.org/coding/RefPtr.html ? It would be useful
to document when to poison such pointers, and which keys should / shouldn't be
shared.

Would be good to figure out the crashes.

Otherwise r=me

> Source/WTF/wtf/DumbPtrTraits.h:43
> +using WTF::DumbPtrTraits;

I don't think you need this, since it's only used in WTF? The other traits
aren't injected in the global namespace.

> Source/WTF/wtf/Forward.h:24
> +#include <wtf/RefForward.h>

I'm not sure I understand why this is a useful addition. We can forward
templates.

> Source/WTF/wtf/RefForward.h:28
> +#include <wtf/DumbPtrTraits.h>

I think you can forward-declare the traits as well, and use them below in the
forward declaration? Not that the header is big right now, but I'm wary of
include-creep in Forward.h.