Inconsistencies with regard to cases about spec HOT 7 CLOSED

On Thu, Nov 19, 2015 at 7:19 PM, jcbeyler [email protected] wrote:

When reading the design document, I saw two things:

A) The if, br, br_if, case, and return constructs do not yield values.

B) A case node consists of an expression

Now, when looking at the testsuite, I saw this:

labels.wast contains:
(case $0 (i32.const 5))

Which seems to go against (A).

And:
(case $3 (set_local $j (i32.sub (i32.const 0) (get_local $i))) (br 0))

which seems to go against (B) if we are pedantic that an expression is
different to a list of expressions.

My questions are the following:
A) should we fix this in the design or the spec?
B) should we fix this in the design or the spec?

I can do both if you wish (though again my Ocaml is a bit rusty but I can
try :)); I just would need to know what we want for each. My thoughts are:

A) I like that case can return a value, I think there are little gains to
not doing it and forcing a br there

Cases can't return values, except the last one, because we've defined
fall-through semantics.

B) I think that a case should respect the design document and only have
one expression, if you want to have more complex things, add a block node

Case is already defined to take a single subexpression.

Opinions?

—
Reply to this email directly or view it on GitHub
#186.

from spec.

On Thu, Nov 19, 2015 at 7:19 PM, jcbeyler [email protected] wrote:

When reading the design document, I saw two things:

A) The if, br, br_if, case, and return constructs do not yield values.

That is true; they don't produce a value that flows to the enclosing
expression. For br and br_if, the value flows to the exit point of the
block or loop which they reference. For return, the value flows out of
(i.e. is returned by) the function. If cannot produce a value since it only
has one arm (i.e. no else clause).

B) A case node consists of an expression

Now, when looking at the testsuite, I saw this:

labels.wast contains:
(case $0 (i32.const 5))

Which seems to go against (A).

And:
(case $3 (set_local $j (i32.sub (i32.const 0) (get_local $i))) (br 0))

which seems to go against (B) if we are pedantic that an expression is
different to a list of expressions.

My questions are the following:
A) should we fix this in the design or the spec?
B) should we fix this in the design or the spec?

I can do both if you wish (though again my Ocaml is a bit rusty but I can
try :)); I just would need to know what we want for each. My thoughts are:

A) I like that case can return a value, I think there are little gains to
not doing it and forcing a br there
B) I think that a case should respect the design document and only have
one expression, if you want to have more complex things, add a block node

Opinions?

—
Reply to this email directly or view it on GitHub
#186.

from spec.

Ok for (A) then but I still have a problem with:

(case $3 (set_local $j (i32.sub (i32.const 0) (get_local $i))) (br 0))

which has two subexpressions (This is from switch.wast).

I also have another issue (also from switch.wast):

(tableswitch $l (i32.const 0)
  (table (br $l)) (case $default)
  (case $default (set_local $x (i32.add (get_local $x) (i32.const 1))))
)
(tableswitch $l (i32.const 1)
  (table (case $0)) (br $l)
  (case $0 (set_local $x (i32.add (get_local $x) (i32.const 2))))
)

This code defines twice the label $l. Should this be allowed?

from spec.

On Thu, Nov 19, 2015 at 9:19 PM, jcbeyler [email protected] wrote:

Ok for (A) then but I still have a problem with:

(case $3 (set_local $j (i32.sub (i32.const 0) (get_local $i))) (br 0))

which has two subexpressions (This is from switch.wast).

I also have another issue (also from switch.wast):

(tableswitch $l (i32.const 0)
(table (br $l)) (case $default)
(case $default (set_local $x (i32.add (get_local $x) (i32.const 1))))
)
(tableswitch $l (i32.const 1)
(table (case $0)) (br $l)
(case $0 (set_local $x (i32.add (get_local $x) (i32.const 2))))
)

This code defines twice the label $l. Should this be allowed?

Labels are always scoped to be only referencable from their children, so
yes, this is valid.

—
Reply to this email directly or view it on GitHub
#186 (comment).

from spec.

Ok for (A) then but I still have a problem with:

(case $3 (set_local $j (i32.sub (i32.const 0) (get_local $i))) (br 0))

which has two subexpressions (This is from switch.wast).

There were a few cases of control operators where it wasn't super clear from the design doc at the time whether they should allow sequences of expressions, or whether explicit blocks should be required. So I opted on the side of allowing implicit blocks as sugar in most cases; of those, loop, case, and func still remain.

Should we make them all unary? It would be a simplification.

from spec.

I think I don't mind, it just is another small inconsistency between the design doc and the spec repo.

I think being consistent just makes more sense and putting block won't change much but at least will be explicit, no?

On the other hand, I could argue that we can just imply that loop, case, and func just start with an unmentioned block.

This issue is simply to state an inconsistent state that we should rectifty: either put in the design document that there is an implicit block for these three cases, or we do the opposite and fix the testsuite and spec.

from spec.

Obsoleted by br_table.

from spec.