[wasm] Update WebAssembly pilot format to support semicolons and parens

RESOLVED FIXED in Firefox 49

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: yury, Assigned: yury)

Tracking

Trunk
mozilla49
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Based on new update to the grammar (see [1]) few items needs to be changed:

- Add semicolons (after each drop value statement?).
- Add parenthesis for every operation that is not expressed as ASCII-art operator.
- Change load and store operations to be array-like.

  [1] https://github.com/sunfishcode/design/commit/0141d6420bba8b47ac15943a9d670f0516ec85fc#diff-fd84284c66f9d51232ae21604e3e4d4b
(Assignee)

Comment 1

2 years ago
Created attachment 8758514 [details]
Bug 1277063 - Update WebAssembly pilot format to support semicolons and parens.

Review commit: https://reviewboard.mozilla.org/r/56788/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/56788/
Attachment #8758514 - Flags: review?(sunfish)
(Assignee)

Comment 2

2 years ago
(Also includes fixes for indenture, missing parens, and test for operator precedence)
Comment on attachment 8758514 [details]
Bug 1277063 - Update WebAssembly pilot format to support semicolons and parens.

https://reviewboard.mozilla.org/r/56788/#review53688

Looks good.

One additional tweak request: Could you add a space after the ":" in PrintSignature and PrintFunction when they're printing arguments and locals? It's not syntactically significant, but it is the prevalent style in TypeScript, which the current type syntax is trying to resemble somewhat. Eg. "x: i32" rather than "x:i32".
Attachment #8758514 - Flags: review?(sunfish) → review+
(Assignee)

Comment 4

2 years ago
Comment on attachment 8758514 [details]
Bug 1277063 - Update WebAssembly pilot format to support semicolons and parens.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56788/diff/1-2/
(Assignee)

Comment 5

2 years ago
Comment on attachment 8758514 [details]
Bug 1277063 - Update WebAssembly pilot format to support semicolons and parens.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56788/diff/2-3/
The updates look good to me. We might also want a space after the "," now, but I'm ok addressing that in a followup if you prefer.
(Assignee)

Comment 7

2 years ago
Comment on attachment 8758514 [details]
Bug 1277063 - Update WebAssembly pilot format to support semicolons and parens.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56788/diff/3-4/
Attachment #8758514 - Attachment description: MozReview Request: Bug 1277063 - Update WebAssembly pilot format to support semicolons and parens. r?sunfish → Bug 1277063 - Update WebAssembly pilot format to support semicolons and parens.
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 8

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/71530f6566b9
Update WebAssembly pilot format to support semicolons and parens. r=sunfish
Keywords: checkin-needed

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/71530f6566b9
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox49: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.