Bug 58198 – Clean up JSValue implementation for JSVALUE64.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Gavin Barraclough]

Remove JSNumberCell, JSImmediate, unify some methods between JSVALUE32_64/JSVALUE64

JSNumberCell.h largely just contained the constructors for JSValue on JSVALUE64, which should not have been here.  JSImmediate mostly contained uncalled methods, along with the internal implementation of the JSValue constructors split unnecessarily across a number of layers of function calls.  These could largely be merged back together.  Many methods and constructors from JSVALUE32_64 and JSVALUE64 can by unified.  The .cpp files are empty.

Moving all these methods into JSValue.h seems to be a repro measurable regression, so I have kept these methods in a separate JSValueInlineMethods.h.  Adding the 64-bit tag values as static const members of JSValue also measures as a repro regression, so I have made these #defines.

[Gavin Barraclough]

Created attachment 88944 [details]
The patch

[Cameron Zwarich (cpst)]

It's unfortunate that you have to make those things #defines...

[Gavin Barraclough]

(In reply to comment #2)
> It's unfortunate that you have to make those things #defines...

Yeah. :-(  Still, on balance I think its clear that this is a really great clean up still, and these values are not used in ways that the lack of type checking is particularly concerning in this case, so I'm not too worried about this overall.

But I guess I really should be filing a bug against the compiler on this – adding a few static const integers to a class really shouldn't be a 2% regression.  Do you know what radar component I should be filing this against?

cheers, G.

[Geoffrey Garen]

I love it!

[Gavin Barraclough]

Fixed in r83459