Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document behavior of label name reuse. #210

Closed
mbebenita opened this issue Jan 20, 2016 · 14 comments · Fixed by #216
Closed

Document behavior of label name reuse. #210

mbebenita opened this issue Jan 20, 2016 · 14 comments · Fixed by #216

Comments

@mbebenita
Copy link
Contributor

Currently, the spec does not say if label name reuse is allowed or not:

f3015ed#diff-d372e456aea857e846293c5f4add7c81R156

I'm not sure what the current implemented behavior is, but I suppose there are only two options:

  1. Not allowed.
  2. Allowed, but scoped (not allowed if ambiguous).
@mbebenita mbebenita changed the title Document behavior of label reuse. Document behavior of label name reuse. Jan 20, 2016
@binji
Copy link
Member

binji commented Jan 20, 2016

Just tested this with the spec interpreter:

(module
  (export "f" $f)
  (func $f
    (loop $foo $foo
      (block $foo
        (block $foo
          (br $foo))))
))

No error. It seems that the spec interpreter just uses the first label it finds.

@mbebenita
Copy link
Contributor Author

What about:

(loop $foo $foo
  (br $foo))

Is this a break or continue?

@mbebenita
Copy link
Contributor Author

If this is the behavior we want, then I guess we should spec the label search order. I feel that relying on search order is kind of error prone.

@ghost
Copy link

ghost commented Jan 21, 2016

@mbebenita The loop block label should be the outer label, so it should see the loop repeat label. The binary encoding has no labels here, but you are right that the order should be specified.

@mbebenita
Copy link
Contributor Author

@JSStats I was under the impression that the loop blocks are just blocks with an additional label. So it effectively has two labels, and they are both outer labels as far as the (br $foo) is concerned. Am I missing something here?

(block $foo
  (br $foo) ;; break
)
(loop $foo %bar
  (br $foo) ;; continue
  (br $bar) ;; break
)
(loop $foo %foo
  (br $foo) ;; continue or break?
)

@kripken
Copy link
Member

kripken commented Jan 21, 2016

I'm in favor of disallowing the ambiguous cases (1 or 2 in the first comment).

@binji
Copy link
Member

binji commented Jan 21, 2016

(loop $foo $bar ...)

is sugar for

(block $foo (loop $bar ...))

See https://github.com/WebAssembly/spec/blob/master/ml-proto/README.md#s-expression-syntax.

Testing this in ml-proto:

(module
  (export "f" $f)
  (func $f
    (loop $foo $foo
      (br $foo))
))
(invoke "f")

is an infinite loop.

So the search order is from "outward".

@mbebenita
Copy link
Contributor Author

@kripken option 2 is consistent with JS semantics, so I'd favor that as well.

@mbebenita
Copy link
Contributor Author

@binji ah, I see, that makes sense.

@ghost
Copy link

ghost commented Jan 21, 2016

@binji Not sure if 'outward' is meaningful. Perhaps note labels are a lexical binding, a stack, and that the lookup chooses the first in the stack, so an inner binding shadows an outer binding. But it's just for the spec wast and perhaps people would rather not have such hazards in tests etc.

@rossberg
Copy link
Member

Labels have standard lexical scoping behaviour. That is, they scope over the labelled expression, and can be shadowed within their scope. The loop case is defined by its desugaring.

We could disallow shadowing, but I don't see a strong reason for that. Everything regarding names is not part of the "official" spec semantics anyway.

@kg
Copy link
Contributor

kg commented Jan 21, 2016

We could disallow shadowing, but I don't see a strong reason for that.

What is the advantage? From my perspective this has disadvantages because it is unusual and mostly creates an opportunity for confusion.

Everything regarding names is not part of the "official" spec semantics anyway.

This doesn't match our design history, and it feels like a semantic trick. In the context of tableswitch and a bunch of cases, the best way to describe the numbering of the br targets for the cases would be labels where the names are arbitrary compiler-assigned numbers.

@rossberg
Copy link
Member

On 21 January 2016 at 18:14, Katelyn Gadd [email protected] wrote:

We could disallow shadowing, but I don't see a strong reason for that.

What is the advantage? From my perspective this has disadvantages because
it is unusual and mostly creates an opportunity for confusion.

Allowing shadowing ensures locality and compositionality: without it, you
cannot syntactically insert a piece of code into another piece and expect
it to keep working. At least in general language design, that is a very
useful modularity property. Not saying it's super-important here, but I
also don't see the opposite.

Everything regarding names is not part of the "official" spec semantics

anyway.

This doesn't match our design history,

Not sure why you're saying that. The AST semantics never had names, AFAIR.

and it feels like a semantic trick. In the context of tableswitch and a

bunch of cases, the best way to describe the numbering of the br targets
for the cases would be labels where the names are arbitrary
compiler-assigned numbers.


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

@ghost
Copy link

ghost commented Jan 24, 2016

@rossberg-chromium 'locality and compositionality' seem important for a usable source code. FYI For (module (func $f1 (block $l1 (block $l1 (nop))))) sexp-wasm gives 'redefinition of label "$l1"'!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants