Bug 72071 - Add jsCast to replace static_cast
Summary: Add jsCast to replace static_cast
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-10 16:00 PST by Mark Hahnenberg
Modified: 2011-11-11 12:41 PST (History)
6 users (show)

See Also:


Attachments
Patch (186.37 KB, patch)
2011-11-10 18:57 PST, Mark Hahnenberg
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2011-11-10 16:00:59 PST
We do a lot of downcasts in JSC, especially now that we've done away with virtual methods in favor of static ones that take a generic first argument (e.g. JSCell) and then downcast to type the function expects.  We currently don't do any checking to make sure that the object is indeed of the type expected.  In order to remedy this, we should add a new custom cast that ASSERTs we have the correct object, then performs the cast and returns the result.  jsCast will behave like a static_cast when performance matters and a dynamic_cast using our own custom RTTI in our debug builds.
Comment 1 Mark Hahnenberg 2011-11-10 18:57:20 PST
Created attachment 114615 [details]
Patch
Comment 2 Geoffrey Garen 2011-11-10 22:15:57 PST
Comment on attachment 114615 [details]
Patch

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

r=me

> Source/JavaScriptCore/runtime/JSCell.h:333
> +    template <typename To, typename From>

Should be no space before "<". (Think of template arguments just like function arguments.)

> Source/JavaScriptCore/runtime/JSCell.h:336
> +        ASSERT(from->classInfo() == &WTF::RemovePointer<To>::Type::s_info || from->inherits(&WTF::RemovePointer<To>::Type::s_info));

Since you're checking inherits(), I don't think you need to check ==. == is a subset of inherits().
Comment 3 Gavin Barraclough 2011-11-11 09:14:15 PST
(In reply to comment #0)
> jsCast will behave like a static_cast when performance matters and a dynamic_cast using our own custom RTTI in our debug builds.

Presumably you mean we'll ASSERT in debug builds? - I wouldn't make sense to change behavior in debug/ndebug - that would just mask bugs.
Comment 4 Mark Hahnenberg 2011-11-11 09:40:46 PST
> Presumably you mean we'll ASSERT in debug builds? - I wouldn't make sense to change behavior in debug/ndebug - that would just mask bugs.

Right.  Since we're using an ASSERT, it will actually perform the check at runtime that the object is of the correct type in debug builds.  Sorry about comparing to dynamic_cast, I just meant that we check dynamically.  It won't return null or any of that stuff.
Comment 5 Mark Hahnenberg 2011-11-11 12:41:10 PST
Committed r100006: <http://trac.webkit.org/changeset/100006>