Bug 23114

Summary: JSValue* should be replaced with a class type.
Product: WebKit Reporter: Gavin Barraclough <barraclough>
Component: JavaScriptCoreAssignee: Gavin Barraclough <barraclough>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
The patch
none
With added 'doze jsc build fix goodness
none
revert emitLoad function name change & fix typo in ChangeLog.
oliver: review+
patch relative to 603, to test on windows [tot was broken earlier.] none

Description Gavin Barraclough 2009-01-04 21:17:30 PST
Presently the representation of JavaScript values varies depending on the platform, (e.g. on x86-64 we can represent 32-bit integers in JSValue*s, but on x86 we can only represent 31-bit integers).  Furthermore we cannot override casting operators to control conversions – which, for example, means that noValue() must be 0 (since the code relies on being able to ask "if (someJSValue) {" meaning "if (someJSValue != noValue()) {", and also means we cannot control casting between pointer types, making it problematic to store pointers in anything but their canonical form.

Encapsulating JSValue*s in a smart pointer type will allow us greater flexibility in and control over the internal representation.
Comment 1 Gavin Barraclough 2009-01-04 21:18:19 PST
Created attachment 26422 [details]
The patch

Testing on windows before setting review flag.
Comment 2 Gavin Barraclough 2009-01-05 14:58:58 PST
Created attachment 26443 [details]
With added 'doze jsc build fix goodness
Comment 3 Gavin Barraclough 2009-01-05 17:06:51 PST
Created attachment 26447 [details]
revert emitLoad function name change & fix typo in ChangeLog.

revert emitLoad name change & fix type in ChangeLog.
Comment 4 Oliver Hunt 2009-01-06 12:00:51 PST
Comment on attachment 26447 [details]
revert emitLoad function name change & fix typo in ChangeLog.

> +        value now encapsulaed it will likely make sense to migrate the functionality
tyop

Looks good, am only rubber stamping the rename portion of it, r=me on the JSValuePtr (and EncodedAsPtr) conversion logic, assuming you've tested jit + interpreter code paths, and perf is good
Comment 5 Gavin Barraclough 2009-01-06 16:30:55 PST
Created attachment 26480 [details]
patch relative to 603, to test on windows [tot was broken earlier.]
Comment 6 Gavin Barraclough 2009-01-06 21:12:34 PST
Transmitting file data .........................................................................................................................................................................................................................................................................................................
Committed revision 39670.