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

initializedIntlObject mechanics still necessary? #115

Closed
littledan opened this issue Nov 29, 2016 · 16 comments
Closed

initializedIntlObject mechanics still necessary? #115

littledan opened this issue Nov 29, 2016 · 16 comments
Assignees
Labels
editorial Involves an editorial fix question

Comments

@littledan
Copy link
Member

In ECMA 402 v1, the internal slot [[initializedIntlObject]] provided the function of ensuring that a single object didn't have the constructor called on it multiple times for different Intl constructors. However, with ECMA 402 v2 and greater, you can't initialize an existing object as in intl object. Are the mechanics still needed? cc @rwaldron

@caridy caridy added question editorial Involves an editorial fix labels Jan 23, 2017
@zbraniecki
Copy link
Member

@allenwb , @annevk @domenic ?

@allenwb
Copy link
Member

allenwb commented Jan 25, 2017

shouldn't be needed, as long as all the constructor starts with a line like:

If NewTarget is undefined, throw a TypeError exception.

see, for example: https://tc39.github.io/ecma262/#sec-map-iterable

(corrected should to shouldn't)

@caridy
Copy link
Contributor

caridy commented Jan 30, 2017

Ok, I can take care of this as part of the clean up process before the next cut.

@caridy caridy self-assigned this Jan 30, 2017
@vanwagonet
Copy link

fwiw JSC does not implement a concrete [[initializedIntlObject]] slot. I believe if we removed the v1 compat fallback, it wouldn't have an observable effect.

As a js dev, I would like to keep the v2+ mechanism of calling without new behaving the same as calling with new. ES2015 classes throwing without the new keyword leads me to stick with ES5 constructors where possible.

@littledan
Copy link
Member Author

@thetalecrafter I was suggesting a purely editorial change, nothing to change observable semantics. It seems like [[initializedIntlObject]] doesn't really do anything anymore in v2+ as the path where it is false is dead code. That doesn't have anything to do with new-less constructors, which are implemented in the first line of constructors as "If NewTarget is undefined, let newTarget be the active function object, else let newTarget be NewTarget."

@vanwagonet
Copy link

@littledan sounds good. @allenwb's comment gave me the impression there would be a necessary change in behavior.

@littledan
Copy link
Member Author

Oh, somehow I missed that.

@allenwb , two questions:

  1. Intl constructors don't start with that line; instead they start with the following line. Does that change anything?

If NewTarget is undefined, let newTarget be the active function object, else let newTarget be NewTarget.

  1. I don't see the Map object having such an internal slot. What are you saying is needed exactly?

@allenwb
Copy link
Member

allenwb commented Feb 14, 2017

@littledan

  1. No, that line is fine. It simply means that calling the constructor as a function is equivalent to using it in a new expression. Since that was the legacy 1.0 behavior, that's presumably want we want.

  2. No internal slot is needed anymore to track initialization. That slot was only necessary when Intl constructors where separated into observably distinct allocation and initialization steps.

  3. (probably off topic) the purpose of the "Normative Optional" steps isn't obvious from the text. Why might or might not an implementation choose to include them? Regardless, of the purpose, step 4 seems buggy. If DateTimeFormat is invoked using [[Construct]] (i.e., if NewTarget is undefined) then "the this value" will be uninitialized and it is meaningless to access its value.

@littledan
Copy link
Member Author

  1. I don't think the Construct case is relevant here, see the bug at [Compatibility Hazard] ECMA 402 2nd Edition Changed the [[Call]] Behavior of Intl Constructors #57 which motivated this case.

That's a good idea to give more context. I'll add a note.

I can't speak to the case for not implementing it, as I work on an implementation which does implement Annex B. Maybe @erights can give more context here, which we could work into the note.

@erights
Copy link

erights commented Feb 15, 2017

@littledan sure:
#57 (comment)
https://bugs.webkit.org/show_bug.cgi?id=153679#c3

But is this entire kludge still necessary? When the issue was first raised, I thought it was described as a transitional issue that would soon be behind us. If we did not do any of this kludge, what would still break?

@littledan
Copy link
Member Author

@erights I meant more about the motivation for why it should be normative-optional rather than simply normative; I couldn't find that detail in the links you provided.

To research whether this compat fix is still necessary, maybe someone could put statistics collections in browsers. The simpler path would be to just try shipping pure v2 semantics again, but I'm hesitant to do that since we know that broke websites the last time, and we've worked out a fix which already made it to multiple implementations and the draft spec.

@erights
Copy link

erights commented Feb 15, 2017

Well, the two are tied together. The reason it is normative-optional is so that implementations are free to stop shipping the kludge as soon as they judge that they no longer need to.

@caridy caridy assigned littledan and unassigned caridy Aug 10, 2017
@caridy
Copy link
Contributor

caridy commented Aug 10, 2017

@littledan send the PR about If NewTarget is undefined, throw a TypeError exception..

littledan added a commit to littledan/ecma402 that referenced this issue Sep 7, 2017
This internal slot has not done anything since ECMA 402 v1.
This patch simply removes it.

Closes tc39#115
@erights
Copy link

erights commented Sep 28, 2017

I notice this issue is closed. What is the answer to the question? Is any of this kludge still needed? Do we know?

@vanwagonet
Copy link

I would love to kill the kludge if we don't need it anymore.

@littledan
Copy link
Member Author

littledan commented Oct 12, 2017

I'm not sure which kludge you're talking about. This bug was all about a purely editorial change, and it was possible to remove that kludge, see the above patch. I don't have any data about whether the kludge in initializing non-Intl objects as DateTimeFormat or NumberFormat is still needed; I'm not so confident, personally.

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

No branches or pull requests

6 participants