Bug 23114

Summary:

JSValue* should be replaced with a class type.

Product:

WebKit

Reporter:

Gavin Barraclough <barraclough>

Component:

JavaScriptCore

Assignee:

Gavin Barraclough <barraclough>

Status:

RESOLVED FIXED

Severity:

Normal

Priority:

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

Gavin Barraclough

Reported 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.

Attachments
The patch (898.54 KB, patch) 2009-01-04 21:18 PST, Gavin Barraclough	no flags	Details Formatted Diff Diff
With added 'doze jsc build fix goodness (898.75 KB, patch) 2009-01-05 14:58 PST, Gavin Barraclough	no flags	Details Formatted Diff Diff
revert emitLoad function name change & fix typo in ChangeLog. (897.25 KB, patch) 2009-01-05 17:06 PST, Gavin Barraclough	oliver: review+	Details Formatted Diff Diff
patch relative to 603, to test on windows [tot was broken earlier.] (897.26 KB, patch) 2009-01-06 16:30 PST, Gavin Barraclough	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Gavin Barraclough

Comment 1 2009-01-04 21:18:19 PST

Created attachment 26422 [details] The patch Testing on windows before setting review flag.

Gavin Barraclough

Comment 2 2009-01-05 14:58:58 PST

Created attachment 26443 [details] With added 'doze jsc build fix goodness

Gavin Barraclough

Comment 3 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.

Oliver Hunt

Comment 4 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

Gavin Barraclough

Comment 5 2009-01-06 16:30:55 PST

Created attachment 26480 [details] patch relative to 603, to test on windows [tot was broken earlier.]

Gavin Barraclough

Comment 6 2009-01-06 21:12:34 PST

Transmitting file data ......................................................................................................................................................................................................................................................................................................... Committed revision 39670.

Note You need to log in before you can comment on or make changes to this bug.