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

Normative: Redefine CatchParameter as FormalParameter #1126

Closed

Conversation

chicoxyzzy
Copy link
Member

This change removes default value restriction for catch parameter.

Closes #1121

@ljharb ljharb added normative change Affects behavior required to correctly evaluate some ECMAScript source text needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Mar 3, 2018
@allenwb
Copy link
Member

allenwb commented Mar 3, 2018

Did you review all of the static and runtime semantics rules associated with FormatParameter to make sure the will work for CatchParameter. They are probably fine, but it needs to be proactively verified.

@littledan
Copy link
Member

@chicoxyzzy Thanks for preparing this patch. At a glance, it seems correct, though I'm not sure why we should bother having a CatchParameter production at this point rather than just using FormalParameter directly. This would require updating many more little parts of the spec, unfortunately.

Would you be interested in writing test262 tests for this change? (I think the test wouldn't change regardless of whether you take that editorial suggestion I made above.)

@chicoxyzzy
Copy link
Member Author

@littledan sure, no problem. I will create PR in test262 repo within the next few days.

@anba
Copy link
Contributor

anba commented Mar 5, 2018

I know Allen mentioned in #1121 to use FormalParameter, but I still don't see why we can't use BindingElement directly.

@chicoxyzzy
Copy link
Member Author

chicoxyzzy commented Mar 9, 2018

@anba I think that using FormalParameter could be guarantee of consistency

@littledan IMO it's better to keep CatchParameter name because we also have a lot of notes about CatchParameter behavior in spec

@littledan
Copy link
Member

I don't have a strong opinion about this editorial question, but in general, I don't think patches like this need to use a minimal number of lines of spec changed as a goal. It would be a little bit of work, but it'd be fine to update all the usages of CatchParameter, even though it's a bigger patch.

@leobalter
Copy link
Member

leobalter commented Mar 12, 2018

what happens when the initializer is/has the binding name?

try {
  throw undefined;
} catch (x = x) {}

@ljharb
Copy link
Member

ljharb commented Mar 12, 2018

@leobalter i think that'd just be the same as (x), and altho we could forbid that, we couldn't likely forbid catch(x = (x)) or catch(x = (0, x)) so it might end up being a linter thing (like throw throw)

@leobalter
Copy link
Member

Let me expand this a bit more:

var x;
try {
  throw undefined;
} catch (x = x) {}
var y = 42;
try {
  throw undefined;
} catch (x = y) {
  let y = 42;
}
try {
  throw undefined;
} catch ([x = y, y] = x) {}

In functions we have the IteratorBindingInitialization to solve these issues.

I believe the catch clause evaluation should get some 👀 as well.

@chicoxyzzy
Copy link
Member Author

I'll take a look at it

@anba
Copy link
Contributor

anba commented Mar 15, 2018

@anba I think that using FormalParameter could be guarantee of consistency

Can you clarify what you mean by "consistency"? FormalParameter is more or less only an alias for BindingElement, when ignoring its additional static semantics. And for the runtime semantics it seems unlikely we want to use https://tc39.github.io/ecma262/#sec-function-definitions-runtime-semantics-iteratorbindinginitialization here.

@chicoxyzzy
Copy link
Member Author

Sorry, I was too busy at work. I'll try to get back to this in next few weeks.

@ljharb
Copy link
Member

ljharb commented Aug 22, 2018

@chicoxyzzy have you had a chance to get to this?

@chicoxyzzy
Copy link
Member Author

I'm trying to make python test generation tool work, but no luck yet

@chicoxyzzy
Copy link
Member Author

Do I have any chance to find a champion who can present this normative change to the committee? I'm going to make another attempt at tests soon.

@ljharb
Copy link
Member

ljharb commented Mar 28, 2019

@chicoxyzzy Once tests are written I'm happy to present it to the committee on your behalf; I'll probably be able to do that in July.

@ljharb
Copy link
Member

ljharb commented Apr 26, 2019

@chicoxyzzy ping; we've still got some time, but if you can get those tests written I can add this to the July agenda :-)

@chicoxyzzy
Copy link
Member Author

chicoxyzzy commented May 7, 2019

Unfortunately I won't have time for that until the end of this week 😞

@ljharb
Copy link
Member

ljharb commented May 7, 2019

@chicoxyzzy that would work just fine! looking forward to it.

@ljharb
Copy link
Member

ljharb commented Oct 1, 2019

The committee discussed this today and could not come to consensus on merging the PR - there was skepticism about the value of this change, and concern about it being confusing. If @chicoxyzzy or anyone is still interested in this change, a full proposal would be appropriate.

I'll close this PR, but we can reopen it if this becomes a proposal that advances.

@chicoxyzzy
Copy link
Member Author

Yes, I'd like to create a proposal repo later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 normative change Affects behavior required to correctly evaluate some ECMAScript source text
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow set default value to CatchParameter
6 participants