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

Editorial: Define "Let"/"Set"; correct usage #958

Merged
merged 2 commits into from
Jul 25, 2017
Merged

Conversation

jugglinmike
Copy link
Contributor

No description provided.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's kind of nice to keep text concise if "let" spec vars are hoisted; but it's consistent with JS if they're not - your change is good but I'm not sure whether I like it or not yet :-/

@bterlson bterlson merged commit 77b8451 into tc39:master Jul 25, 2017
@jmdyck
Copy link
Collaborator

jmdyck commented Jul 26, 2017

"Once declared, aliases may be referenced in any subsequent step or substep thereof, but they may not be referenced from steps at a higher level."

I'm pretty sure there are lots of places where the spec doesn't hold to that rule.

@bterlson
Copy link
Member

bterlson commented Jul 26, 2017

Yeah, on reflection I don't really want this algorithm convention. The semantics need to be as simple as possible to reason about and I think we can rely on editorial discretion (and PRs) to fix confusing alias references. One namespace per algorithm seems sufficient. Let could be spec'd as something like "an assertion that this is the first reference of an alias in any execution of this algorithm", but I can also see an argument for saying let/set mean the same thing with let being more of an editorial suggestion that this is the first time you're seeing a binding.

That said, things get hard in larger algorithms and I'm not against the other changes in this PR. It's helpful to know the "default" case up front.

@jugglinmike objections to just adding something like "Once declared, aliases may be referenced in any subsequent step" to the previous paragraph?

@jmdyck
Copy link
Collaborator

jmdyck commented Jul 26, 2017

"Once declared, aliases may be referenced in any subsequent step"

Does 'subsequent' mean lexically or temporally? And should we infer that aliases may not be referenced in any non-subsequent step?

@bterlson
Copy link
Member

@jmdyck both, I'd hope. And yes, access prior to let/set should not be allowed.

@domenic
Copy link
Member

domenic commented Jul 26, 2017

FWIW we decided for web specs that variables are block scoped. https://infra.spec.whatwg.org/#variables

@bterlson
Copy link
Member

@domenic can you link to the relevant discussion? Do you make use of the block scoping (e.g. with shadowing)? Any experience you can share would be nice.

@domenic
Copy link
Member

domenic commented Jul 26, 2017

whatwg/infra#88 is most of the discussion. We never really discussed shadowing; that'd be a good discussion to have. (Edit: opened whatwg/infra#139.) It's mostly about people not wanting to have variables "declared" in ifs and then used later "outside the if scope", as that's confusing for most developers reading the spec.

@domenic
Copy link
Member

domenic commented Jul 26, 2017

In the same area of "formalizing variables in specs": whatwg/infra#139

@bterlson
Copy link
Member

bterlson commented Jul 26, 2017

Looking at the examples, I'm not sure it's confusing in all cases. E.g. given the following:

If x is *true*, then
  Let y be 0.
Else,
  Let y be 1.
Return y.

I think this is fine, personally, over the alternative that declares it before the If (then you have to think about it's default value too, even though the default is never used).

That said, haven't read the linked threads yet. Will do so! Thank you :)

@domenic
Copy link
Member

domenic commented Jul 26, 2017

Well, our intuition was that such code was confusing, as to us it implied that y had gone out of scope, but was now being used.

We've been fine using "null" as such a default when necessary, although in such if/else cases it's often easier to just set the default to what it would be in the else branch.

@bterlson
Copy link
Member

That's if you assume that steps create scopes rather than sharing a single namespace for the algorithm. I think you wouldn't assume that in the example above because if you assume it you are looking at two useless declarations. It can be confusing though not because of scoping per se (e.g. I think the reduce fixes in this PR are good). Also worth noting that anyone with experience in 262-spec-reading will expect the status quo.

@domenic
Copy link
Member

domenic commented Jul 26, 2017

. I think you wouldn't assume that in the example above because...

The problem is if you read other code, or other parts of the spec, with one interpretation, and then you end up reading such sections, my first reaction is that I've found a spec bug, or at least editorial sloppiness.

Also worth noting that anyone with experience in 262-spec-reading will expect the status quo.

I don't think this is necessarily true, given how rarely you encounter these special cases. Especially given how many readers are programmers of other languages besides ECMASpeak.

@jugglinmike
Copy link
Contributor Author

@jugglinmike objections to just adding something like "Once declared, aliases may be referenced in any subsequent step" to the previous paragraph?

Personally, I prefer the rigor of block-scoped variables. That said, this was just a drive-by commit. I trust your judgment about what makes more sense from a project maintenance standpoint. (Though I will say that the tooling could be updated to enforce it moving forward--I believe Bikeshed does this--making it easier for future contributors to comply.)

FWIW we decided for web specs that variables are block scoped. https://infra.spec.whatwg.org/#variables

That was actually what inspired this change. I misremembered reading that text in ECMA262, and when I couldn't find it, I thought it was just an oversight. It's starting to sound like that isn't necessarily true, though.

@jmdyck
Copy link
Collaborator

jmdyck commented Jul 26, 2017

And yes, access prior to let/set should not be allowed.

I have a vague memory of something in the spec that didn't follow this rule. Something like:

  While <condition>, do
    If <not the first iteration>, then
        Do something with _foo_
    ...
    If <the first iteration>, then
        Let _foo_ be ...

So the "Do something" reference is prior to the "Let" lexically but not temporally.

@jmdyck
Copy link
Collaborator

jmdyck commented Jul 26, 2017

@domenic:

We never really discussed shadowing; that'd be a good discussion to have. (Edit: opened whatwg/infra#139.)

Presumably you meant to link to whatwg/infra#141.

anba added a commit to anba/ecma262 that referenced this pull request Aug 28, 2017
This reverts commit 77b8451.

The newly added rule for variables does not match how variables are currently used, cf.

7.1.1.1 OrdinaryToPrimitive
7.1.17 ToIndex
7.2.12 Abstract Relational Comparison
7.3.14 SetIntegrityLevel
7.4.2 IteratorNext
8.1.1.4.18 CreateGlobalFunctionBinding
9.2.1.2 OrdinaryCallBindThis
9.2.2 [[Construct]]
9.2.12 FunctionDeclarationInstantiation
9.4.2.4 ArraySetLength
etc.
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 this pull request may close these issues.

5 participants