Closed Bug 922070 Opened 6 years ago Closed 6 years ago

Source note table in BytecodeEmitter.cpp should be generated by higher-order macro

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: wingo, Assigned: wingo)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 2 obsolete files)

The js_SrcNoteSpec[] table in BytecodeEmitter.cpp should be generated by a higher-order macro, so that it will always be in sync with SourceNotes.h.
Assignee: nobody → wingo
Attachment #824608 - Flags: review?(jorendorff)
Attachment #824608 - Attachment is obsolete: true
Attachment #824608 - Flags: review?(jorendorff)
Comment on attachment 824613 [details] [diff] [review]
Define SrcNoteType, js_SrcNoteSpec using higher-order macro v2

Better version; previous one depended on bug 932312 needlessly.
Attachment #824613 - Attachment description: Define SrcNoteType, js_SrcNoteSpec using higher-order macro → Define SrcNoteType, js_SrcNoteSpec using higher-order macro v2
Attachment #824613 - Flags: review?(jorendorff)
Blocks: 932312
Comment on attachment 824613 [details] [diff] [review]
Define SrcNoteType, js_SrcNoteSpec using higher-order macro v2

Review of attachment 824613 [details] [diff] [review]:
-----------------------------------------------------------------

Stealing review.

::: js/src/frontend/SourceNotes.h
@@ +40,5 @@
> +#define FOR_EACH_SRC_NOTE_TYPE(M) \
> +    M(SRC_NULL, "null", 0)              /* terminates a note vector */ \
> +    M(SRC_IF, "if", 0)                  /* JSOP_IFEQ bytecode is from an if-then */ \
> +    M(SRC_IF_ELSE, "if-else", 1)        /* JSOP_IFEQ bytecode is from an if-then-else */ \
> +    M(SRC_COND, "cond", 1)              /* JSOP_IFEQ is from conditional ?: operator */ \

Please add whitespace so the columns vertically align.

I'd probably also use C++-style comments in this table.
Attachment #824613 - Flags: review?(jorendorff) → review+
Attachment #824613 - Attachment is obsolete: true
Comment on attachment 827981 [details] [diff] [review]
Define SrcNoteType, js_SrcNoteSpec using higher-order macro  r=njn

Updated patch indents the columns to align.  You have to use C-style comments though, because of the continuation lines.
Attachment #827981 - Attachment description: Define SrcNoteType, js_SrcNoteSpec using higher-order macro → Define SrcNoteType, js_SrcNoteSpec using higher-order macro r=njn
Attachment #827981 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d0d87190415e
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.