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

RESOLVED FIXED in Firefox 68

Status

()

defect
P3
normal
RESOLVED FIXED
2 months ago
a month ago

People

(Reporter: lth, Assigned: jseward)

Tracking

(Blocks 1 bug)

unspecified
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

2 months ago

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.

Reporter

Updated

2 months ago
Assignee: lhansen → jseward
Assignee

Comment 1

a month ago

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.

Comment 2

a month ago
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

Comment 3

a month ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: a month ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.