387286 - Consistent naming for JOF_ type flags

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Igor Bukanov]

[This is a spin-off of bug 385729 comment 37 and 39.]

Bug 385729 introduced separated tables for object and regexp that are stored in JSScript* in addition to JSScript.atomMap. In view of that names of JOF_CONST or JOF_INDEXCONST bytecode types becomes rather misleading. It would be nice for consistency to rename them and related functions to names that better reflects the nature of bytecodes.

Comment 1

[Rich Dougherty]

Attachment: Patch v1

Here's a patch which makes the following changes:

JOF_CONST -> JOF_ATOM
JOF_INDEXCONST -> JOF_SLOTATOM
JOF_INDEXOBJECT -> JOF_SLOTOBJECT
EmitIndexConstOp -> EmitSlotIndexOp

Comment 2

[Igor Bukanov]

Comment on attachment 271483 [details] [diff] [review]
Patch v1

Thanks for looking into this!

Comment 3

[Rich Dougherty]

Attachment: Patch v2

Patch updated to apply against HEAD.

Comment 4

[Igor Bukanov]

(In reply to comment #3)
> Created an attachment (id=275650) [details]
> Patch v2

Rich: Could you attach a plain diff between v1 and v2 to simplify reviewing? Bugzilla interrdiff does not work here. Note also that the new check in rules require either blocking 1.9 flag on the bug or approval 1.9 flag on the patch, see http://tinderbox.mozilla.org/Firefox/ .

Brendan: This should be included in 1.9, so could you set the blocking flag?

Comment 5

[Brendan Eich [:brendan]]

I'll just approve the patch once it's r+.

/be

Comment 6

[Rich Dougherty]

Attachment: Differences between v1 and v2

I diffed the patches - is that what you wanted? I think the only changes between patches are the inclusion of more context, and handling the renaming of JSOP_NUMBER to JSOP_DOUBLE.

Comment 7

[Igor Bukanov]

(In reply to comment #6)
> Created an attachment (id=275661) [details]
> Differences between v1 and v2

Could you just add a plain diff, not -u one, as in:

diff v1.patch v2.patch > v1_v2.diff

Since the plain diff uses >< as opposed to +- it is very easy to see the patch evolution.

Comment 8

[Rich Dougherty]

Attachment: Differences between v1 and v2 (plain)

Ok, that makes sense. Here's a plain diff.

Comment 9

[Igor Bukanov]

Comment on attachment 275668 [details]
Differences between v1 and v2 (plain)

This is not a patch.

Comment 10

[Igor Bukanov]

(In reply to comment #8)
> Ok, that makes sense. Here's a plain diff.

Thanks. The diff clearly shows that the update was necessary due to that JSOP_NUMBER->JSOP_DOUBLE rename.

Comment 11

[Igor Bukanov]

I checked in the patch from comment 3 to the trunk:

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1186579980&maxdate=1186580280&who=igor%mir2.org