[webkit-reviews] review granted: [Bug 22243] WebScriptDebugDelegate should use intptr_t for sourceId, not int : [Attachment 25140] patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted 's request for review:
Bug 22243: WebScriptDebugDelegate should use intptr_t for sourceId, not int
https://bugs.webkit.org/show_bug.cgi?id=22243

Attachment 25140: patch
https://bugs.webkit.org/attachment.cgi?id=25140&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +#define WebSourceId int
> +#ifndef BUILDING_ON_TIGER
> +#ifndef BUILDING_ON_LEOPARD
> +#if 0 // FIXME <rdar://problem/6263293>: Remove this "#if 0" once
<rdar://problem/6263297> is fixed.
> +#undef WebSourceId
> +#define WebSourceId intptr_t
> +#endif
> +#endif
> +#endif

You can do this without #undef very easily, and I think it might read better.

    #if defined BUILDING_ON_TIGER || defined BUILDING_ON_LEOPARD
    #define WebSourceId int
    #else
    #define WebSourceId int /* FIXME <rdar://problem/6263293>: Change this to
intptr_t once <rdar://problem/6263297> is fixed. */
    #endif

I think that's slightly nicer. But also, I suggest using a typedef rather than
a define. For WebNSUInteger we wanted to avoid having a named type that was
just our WebKit copy of NSUInteger, especially since some day we hope to just
switch to using NSUInteger directly. But in this case I think a real typedef
would be appropriate. You could avoid having to define the macro three
different places that way too.

I'm going to say review+ because the patch is OK as is too.