Closed Bug 1092522 Opened 10 years ago Closed 10 years ago

make_opcode_doc.py needs to be updated to support new XDR_BYTECODE_VERSION notation in Xdr.h.

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: arai, Assigned: arai)

Details

Attachments

(1 file, 2 obsolete files)

Bug 918828 changes XDR_BYTECODE_VERSION notation in Xdr.h, and make_opcode_doc.py does not support it. > static const uint32_t XDR_BYTECODE_VERSION_SUBTRAHEND = 190; > static_assert(XDR_BYTECODE_VERSION_SUBTRAHEND % 2 == 0, "see the comment above"); > static const uint32_t XDR_BYTECODE_VERSION = > uint32_t(0xb973c0de - (XDR_BYTECODE_VERSION_SUBTRAHEND
Updated regex pattern for XDR_BYTECODE_VERSION. Also improved error handling when the pattern does not matched. I already updated document in MDN with patched script. https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Internals/Bytecode
Attachment #8515787 - Flags: review?(jwalden+bmo)
Comment on attachment 8515787 [details] [diff] [review] 0001-Bug-1092522-Update-XDR_BYTECODE_VERSION-pattern-in-m.patch Review of attachment 8515787 [details] [diff] [review]: ----------------------------------------------------------------- A part of me wonders if we should be including some recognition of JS_HAS_SYMBOLS here, but I'm not entirely sure of the simplest way to do it, and hopefully that'll go away at some point soon anyway.
Attachment #8515787 - Flags: review?(jwalden+bmo) → review+
Thank you for reviewing! Yeah, actually I also thought about JS_HAS_SYMBOL. Here is updated patch with JS_HAS_SYMBOL detection, it shows another version number and description when JS_HAS_SYMBOL is detected. If this patch is better than previous one, let's land this instead :)
Attachment #8516427 - Flags: review?(jwalden+bmo)
Comment on attachment 8516427 [details] [diff] [review] Update XDR_BYTECODE_VERSION pattern and support bug 1066322 documentation in make_opcode_doc.py. Review of attachment 8516427 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/vm/make_opcode_doc.py @@ +49,2 @@ > > + return subtrahend, bug1066322 Mild preference for parentheses around tuples. @@ +361,5 @@ > + print(""" > +<p>Until {{{{bug(1066322)}}}} is fixed, JSOP_SYMBOL exists only in Nightly, and it uses different version.</p> > +<p>Bytecode version with JSOP_SYMBOL: <code>{version}</code> > +(<code>0x{actual_version:08x}</code>).</p> > +""".format(source_base=SOURCE_BASE, Remove source_base=... here, as it's not used in this formatting.
Attachment #8516427 - Flags: review?(jwalden+bmo) → review+
Thanks again! Here is fixed patch.
Attachment #8515787 - Attachment is obsolete: true
Attachment #8516427 - Attachment is obsolete: true
Attachment #8518621 - Flags: review?(jwalden+bmo)
Comment on attachment 8518621 [details] [diff] [review] Update XDR_BYTECODE_VERSION pattern and support bug 1066322 documentation in make_opcode_doc.py. Review of attachment 8518621 [details] [diff] [review]: ----------------------------------------------------------------- When someone gives r+ on a patch, that means it's good to go, or it's good to go with the requested changes, and further review of the final patch isn't necessary. :-) You can override that choice if you think the extra changes were complex enough, or if you didn't understand some part of the comments, or whatever. But there's really no way that's the case here. :-) So for simple-enough requested changes in the future, just post the revised patch and the try-run or so and wait for the checkin.
Attachment #8518621 - Flags: review?(jwalden+bmo) → review+
Thank you for letting me know that. Green on try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c094fc25feac
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: