Closed Bug 1541357 Opened 1 year ago Closed 1 year ago

Change the argument order of table.grow to be spec-compliant

Categories

(Core :: Javascript: WebAssembly, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: lth, Assigned: jseward)

References

Details

Attachments

(1 file)

We implemented table.grow with an initializer argument before there was a spec for it, and the draft spec now takes the arguments in the opposite order of what we have, to follow table.fill and memory.fill: we should pop the delta first, then the init value, ie in the high-level notation, the init value is the first argument and the delta the second. This corresponds to the last two arguments of the fill instructions, which are initializer value and length.

Fixing this in the code is easy; but we have to update a number of test cases too, as well as the documentation on https://github.com/lars-t-hansen/moz-gc-experiments/.

We don't have to rev the gc_feature_opt_in version since that no longer applies to the reftypes features, but some people may experience a little pain if they've started to use the feature.

Assignee: lhansen → jseward

We implemented table.grow with an initializer argument before there was a spec
for it, and the draft spec now takes the arguments in the opposite order of
what we have, to follow table.fill and memory.fill: we should pop the delta
first, then the init value, ie in the high-level notation, the init value is
the first argument and the delta the second. This corresponds to the last two
arguments of the fill instructions, which are initializer value and length.

This commit fixes both the implementation and test cases: it swaps the order
of the 'initial value' and 'delta' arguments.

Pushed by jseward@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/74175527c5ea
Change the argument order of table.grow to be spec-compliant. r=lth
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.