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.

Sign up for GitHub

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

ECMA-402 v1 legacy constructor semantics compromise #84

Merged
merged 2 commits into from
Feb 2, 2017
Merged

ECMA-402 v1 legacy constructor semantics compromise #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b97fabb
ECMA-402 v1 legacy constructor semantics compromise for NumberFormat
littledan Mar 25, 2016
8775fdc
ECMA-402 v1 constructor compatibility semantics for DateTimeFormat
littledan Mar 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions spec/conventions.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ <h1>Notational Conventions</h1>
For ECMAScript objects, this standard may use variable-named internal slots: The notation "[[&lt;_name_&gt;]]" denotes an internal slot whose name is given by the variable name, which must have a String value. For example, if a variable _s_ has the value *"a"*, then [[&lt;_s_&gt;]] denotes the [[&lt;_a_&gt;]] internal slot.
</p>

<p>
This specification uses blocks demarcated as <span class="normative-optional">Normative Optional</span> to denote the sense of <a href="https://tc39.github.io/ecma262/#sec-additional-ecmascript-features-for-web-browsers">Annex B</a> in ECMA 262. That is, normative optional sections are required when the ECMAScript host is a web browser. The content of the section is normative but optional if the ECMAScript host is not a web browser.
Copy link
Contributor

Choose a reason for hiding this comment

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

@erights I think this is the part that @littledan wants to get your feedback.

</p>

<emu-clause id="sec-402-well-known-intrinsic-objects">
<h1>Well-Known Intrinsic Objects</h1>

Expand Down
38 changes: 35 additions & 3 deletions spec/datetimeformat.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,27 @@ <h1>ToLocalTime ( _date_, _calendar_, _timeZone_ )</h1>
It is recommended that implementations use the time zone information of the IANA Time Zone Database.
</emu-note>
</emu-clause>

<emu-clause id="sec-unwrapdatetimeformat" aoid="UnwrapDateTimeFormat">
<h1>UnwrapDateTimeFormat( dtf )</h1>
Copy link
Contributor

Choose a reason for hiding this comment

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

dtf

<p>
The UnwrapDateTimeFormat abstract operation gets the underlying DateTimeFormat operation
for various methods which implement ECMA-402 v1 semantics for supporting initializing
existing Intl objects.
</p>
<emu-normative-optional><span class="normative-optional">Normative Optional</span><div class="normative-optional-contents">
<emu-alg>
1. If _dtf_ does not have an [[initializedDateTimeFormat]] internal slot and ? InstanceofOperator(_dtf_, %DateTimeFormat%),
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to check Type(dtf) is Object because dtf could be a primitive when called from resolvedOptions().

So either:

  1. If Type(dtf) is Object and dtf does not have an [[initializedDateTimeFormat]] internal slot and ? InstanceofOperator(dtf, %DateTimeFormat%) is true, then

Or if instanceof should also be applied on primitive values (also needs GetV instead of Get in that case):

  1. If Type(dtf) is not Object or dtf does not have an [[initializedDateTimeFormat]] internal slot, then
    1. If ? InstanceofOperator(dtf, %DateTimeFormat%) is true, then
      1. Let dtf be ? GetV(dtf, Intl.[[FallbackSymbol]]).

Copy link
Contributor

Choose a reason for hiding this comment

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

I went with the first option because I'm not sure about the primitive values. @littledan can you double check this?

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed by 7eb2c2b

Copy link
Contributor

Choose a reason for hiding this comment

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

... is equal *true*, then

Copy link
Contributor

Choose a reason for hiding this comment

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

in general I haven't seen the use of ? in conditions... but I suspect that's ok.

1. Let _dtf_ be ? RequireObjectCoercible(Get(_dtf_, Intl.[[FallbackSymbol]])).
Copy link
Contributor

Choose a reason for hiding this comment

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

RequireObjectCoercible is not needed here.

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed by 7eb2c2b

Copy link
Contributor

Choose a reason for hiding this comment

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

split this into two lines for readability.

</emu-alg>
</div></emu-normative-optional>
<emu-alg>
2. If _dtf_ does not have an [[initializedDateTimeFormat]] internal slot,
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs type check for object values:

  1. If Type(dtf) is not Object or dtf does not have an [[initializedDateTimeFormat]] internal slot, then

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed by 7eb2c2b

Copy link
Contributor

Choose a reason for hiding this comment

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

1. instead of 2.

Copy link
Contributor

Choose a reason for hiding this comment

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

..., then

1. Throw a *TypeError* exception.
1. Return _dtf_.
</emu-alg>
</emu-clause>
</emu-clause>
</emu-clause>

<emu-clause id="sec-intl-datetimeformat-constructor">
Expand All @@ -379,7 +400,18 @@ <h1>Intl.DateTimeFormat ( [ _locales_ [ , _options_ ] ] )</h1>
<emu-alg>
1. If NewTarget is *undefined*, let _newTarget_ be the active function object, else let _newTarget_ be NewTarget.
1. Let _dateTimeFormat_ be ? OrdinaryCreateFromConstructor(_newTarget_, `"%DateTimeFormatPrototype%"`, &laquo; [[initializedIntlObject]], [[initializedDateTimeFormat]], [[locale]], [[calendar]], [[numberingSystem]], [[timeZone]], [[weekday]], [[era]], [[year]], [[month]], [[day]], [[hour]], [[minute]], [[second]], [[timeZoneName]], [[hour12]], [[hourNo0]], [[pattern]], [[boundFormat]] &raquo;).
1. Return ? InitializeDateTimeFormat(_dateTimeFormat_, _locales_, _options_).
1. Perform ? InitializeDateTimeFormat(_dateTimeFormat_, _locales_, _options_).
</emu-alg>
<emu-normative-optional><span class="normative-optional">Normative Optional</span><div class="normative-optional-contents">
<emu-alg>
4. Let _this_ be the *this* value.
1. If NewTarget is *undefined* and ? InstanceofOperator(_this_, %NumberFormat%),
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing "then" after comma.

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed by 7eb2c2b

1. Perform ? DefineOwnPropertyOrThrow(_this_, Intl.[[FallbackSymbol]], { [[Value]]: _dateTimeFormat_, [[Writable]]: *false*, [[Enumerable]]: *false*, [[Configurable]]: *false* }).
Copy link
Contributor

Choose a reason for hiding this comment

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

This also needs a type check, because the this-value could be a primitive value in which case it's not valid to call DefineOwnPropertyOrThrow.

Copy link
Contributor

Choose a reason for hiding this comment

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

That is covered by InstanceofOperator(_this_, %NumberFormat%) in the previous step I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not if Intl.NumberFormat[Symbol.hasInstance] was redefined to add a custom instanceof behaviour, like in this test: https://github.com/mozilla/gecko-dev/blob/3ed98bee2a88a4d953d22520e727344bf8e617b8/js/src/tests/Intl/NumberFormat/call.js#L62-L85

Copy link
Member Author

Choose a reason for hiding this comment

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

That was all unintentional, but I agree with @caridy's reading.

Copy link
Contributor

Choose a reason for hiding this comment

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

@anba will something like this be sufficient?

image

or should we just throw a TypeError if this-value is not an object before calling DefineOwnPropertyOrThrow?

Copy link
Member Author

Choose a reason for hiding this comment

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

@caridy I don't see why that's necessary; I'm fine with this throwing due to InstanceOf.

Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly, I feel the same @littledan, but @anba seems to be convinced about a potential issue here, and I trust him :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I missed the above comment--yeah, the text you wrote sounds good and (I think) it should guard against it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@anba can you confirm?

Copy link
Contributor

Choose a reason for hiding this comment

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

Both solutions (guard or throwing) work, and I'm fine with either of them. 😄

1. Return _this_.
</emu-alg>
</div></emu-normative-optional>
<emu-alg>
1. Return _dateTimeFormat_.
</emu-alg>
</emu-clause>
</emu-clause>
Expand Down Expand Up @@ -512,7 +544,7 @@ <h1>get Intl.DateTimeFormat.prototype.format</h1>
<emu-alg>
1. Let _dtf_ be *this* value.
1. If Type(_dtf_) is not Object, throw a *TypeError* exception.
1. If _dtf_ does not have an [[initializedDateTimeFormat]] internal slot, throw a *TypeError* exception.
1. Let _dtf_ be ? UnwrapDateTimeFormat(_dtf_).
1. If _dtf_.[[boundFormat]] is *undefined*, then
1. Let _F_ be a new built-in function object as defined in DateTime Format Functions (<emu-xref href="#sec-datetime-format-functions"></emu-xref>).
1. Let _bf_ be BoundFunctionCreate(_F_, _dft_, &laquo; &raquo;).
Expand Down Expand Up @@ -545,7 +577,7 @@ <h1>Intl.DateTimeFormat.prototype.formatToParts ( [ _date_ ] )</h1>
<h1>Intl.DateTimeFormat.prototype.resolvedOptions ()</h1>

<p>
This function provides access to the locale and formatting options computed during initialization of the object.
This function provides access to the locale and formatting options computed during initialization of the object. This function initially invokes the internal algorithm UnwrapDateTimeFormat to get the %DateTimeFormat% object on which to operate.
</p>

<p>
Expand Down
14 changes: 14 additions & 0 deletions spec/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,20 @@
margin-bottom: 10px;
padding-left: 10px;
}

emu-normative-optional {
margin: 1em 0;
border-left: 5px solid #ff6600;
display: block;
background: #ffeedd;
}

span.normative-optional {
padding-left: 5px;
text-transform: uppercase;
color: #884400;
}

</style>
<img src="img/ecma-logo.jpg">
<pre class=metadata>
Expand Down
4 changes: 4 additions & 0 deletions spec/intl.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ <h1>The Intl Object</h1>
The Intl object is not a function object. It does not have a [[Construct]] internal method; it is not possible to use the Intl object as a constructor with the *new* operator. The Intl object does not have a [[Call]] internal method; it is not possible to invoke the Intl object as a function.
</p>

<p>
The Intl object has an internal slot, [[FallbackSymbol]], which is a new %Symbol% in the current realm.
Copy link
Contributor

Choose a reason for hiding this comment

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

@jswalden asked in https://bugzilla.mozilla.org/show_bug.cgi?id=1328386 if [[FallbackSymbol]] should get a [[Description]]?

Copy link
Contributor

Choose a reason for hiding this comment

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

@littledan can you comment here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Would undefined be a reasonable [[Description]] value here? In V8, I used "IntlFallback", but for no good reason. There's no way that you can access this symbol as the property of something else, and I don't see a reason to make one, so "Intl.fallback" or "Symbol.intlFallback" would not be all that compelling in my opinion.

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume @jswalden asked about adding a description for debugging purposes, I can try to catch him later on IRC, so he can chime in.

Copy link
Contributor

Choose a reason for hiding this comment

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

solved by 44107b6

Copy link
Member

Choose a reason for hiding this comment

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

The Intl object has an internal slot, [[FallbackSymbol]], which is a new %Symbol% in the current realm.

In this case, shouldn't something like %FallbackSymbol% be added to Table 1 and the accesses performed like: Get(%Intl%,%FallbackSymbol)? Basically, internal slots are not identified using symbols. It either needs to be an internal slot named FallBackSymbol or a own property whose key is %FallBackSymbol%. You should be muddling these distinct concepts.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see no problem with putting it as %FallbackSymbol%, but I also don't quite understand the problem with the current phrasing. When I wrote it out with this internal slot notation, I was picturing it as being "in a separate namespace"--it's just an internal slot that is called [[FallbackSymbol]] which contains a symbol. Is the problem that this is a one-off internal slot, rather than a name that's reused over a class of objects?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I misunderstood what you were trying to say (which is the root problem). I thought you were saying that the "new %Symbol%" was the key/name of the internal slot. If the value is a realm specific intrinsic symbol, it probably needs to be added to Table 1 (of this spec). Note that in Ecma-262 we don't have any realm-specific intrinsic symbols so rather than appearing in the corresponding Table 6 (Well known intrinsic objects) intrinsic symbols are listed in Ecma-262 Table 1 (Well known symbols).

We should probably also refer it the symbol as @@FallbackSymbol.

So, the spec statement could be: "The Intl object has an internal slot, [[FallbackSymbol]] whose value is initialized to @@FallbackSymbol

Copy link
Member Author

Choose a reason for hiding this comment

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

This is an interesting point about being realm-specific vs cross-realm. In V8, I ended up implementing it as a cross-realm symbol, actually. @anba What did you do for SpiderMonkey? Do you have an opinion about which way it should be?

Copy link
Contributor

Choose a reason for hiding this comment

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

What did you do for SpiderMonkey? Do you have an opinion about which way it should be?

It's currently implemented as a realm-specific symbol in SpiderMonkey. Hm, if it isn't a realm-specific symbol, I wonder if we should remove the InstanceofOperator call in the unwrapping operations, so that a cross-realm object initialized as a Intl.DateTimeFormat/NumberFormat object works the same way as a proper cross-realm Intl.DateTimeFormat/NumberFormat object. (Intl.NumberFormat.prototype.resolvedOptions.call(crossRealmNumberFormat) currently works, but Intl.NumberFormat.prototype.resolvedOptions.call(crossRealmObjectInitializedAsNumberFormat) throws a TypeError.)

</p>

<emu-clause id="sec-constructor-properties-of-the-intl-object">
<h1>Constructor Properties of the Intl Object</h1>

Expand Down
37 changes: 34 additions & 3 deletions spec/numberformat.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,26 @@ <h1>ToRawFixed( _x_, _minInteger_, _minFraction_, _maxFraction_ )</h1>
1. Return _m_.
</emu-alg>
</emu-clause>

<emu-clause id="sec-unwrapnumberformat" aoid="UnwrapNumberFormat">
<h1>UnwrapNumberFormat(nf)</h1>
Copy link
Contributor

Choose a reason for hiding this comment

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

nf

<p>
The UnwrapNumberFormat abstract operation gets the underlying NumberFormat operation
for various methods which implement ECMA-402 v1 semantics for supporting initializing
existing Intl objects.
</p>
<emu-normative-optional><span class="normative-optional">Normative Optional</span><div class="normative-optional-contents">
<emu-alg>
1. If _nf_ does not have an [[initializedNumberFormat]] internal slot and ? InstanceofOperator(_nf_, %NumberFormat%),
Copy link
Contributor

Choose a reason for hiding this comment

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

Same issues here and below like for DateTimeFormat.

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed by 7eb2c2b

1. Let _nf_ be ? RequireObjectCoercible(Get(_nf_, Intl.[[FallbackSymbol]])).
Copy link
Contributor

Choose a reason for hiding this comment

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

split...

</emu-alg>
</div></emu-normative-optional>
<emu-alg>
2. If _nf_ does not have an [[initializedNumberFormat]] internal slot,
Copy link
Contributor

Choose a reason for hiding this comment

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

1.

Copy link
Member Author

Choose a reason for hiding this comment

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

Starting at 2 is deliberate since it follows earlier algorithm steps.

1. Throw a *TypeError* exception.
1. Return _nf_.
</emu-alg>
</emu-clause>
</emu-clause>

<emu-clause id="sec-intl-numberformat-constructor">
Expand All @@ -467,7 +487,18 @@ <h1>Intl.NumberFormat ( [ _locales_ [ , _options_ ] ] )</h1>
<emu-alg>
1. If NewTarget is *undefined*, let _newTarget_ be the active function object, else let _newTarget_ be NewTarget.
1. Let _numberFormat_ be ? OrdinaryCreateFromConstructor(_newTarget_, `"%NumberFormatPrototype%"`, &laquo; [[initializedIntlObject]], [[initializedNumberFormat]], [[locale]], [[numberingSystem]], [[style]], [[currency]], [[currencyDisplay]], [[minimumIntegerDigits]], [[minimumFractionDigits]], [[maximumFractionDigits]], [[minimumSignificantDigits]], [[maximumSignificantDigits]], [[useGrouping]], [[positivePattern]], [[negativePattern]], [[boundFormat]] &raquo;).
1. Return ? InitializeNumberFormat(_numberFormat_, _locales_, _options_).
1. Perform ? InitializeNumberFormat(_numberFormat_, _locales_, _options_).
</emu-alg>
<emu-normative-optional><span class="normative-optional">Normative Optional</span><div class="normative-optional-contents">
<emu-alg>
4. Let _this_ be the *this* value.
1. If NewTarget is *undefined* and ? InstanceofOperator(_this_, %NumberFormat%),
1. Perform ? DefineOwnPropertyOrThrow(_this_, Intl.[[FallbackSymbol]], { [[Value]]: _numberFormat_, [[Writable]]: *false*, [[Enumerable]]: *false*, [[Configurable]]: *false* }).
1. Return _this_.
</emu-alg>
</div></emu-normative-optional>
<emu-alg>
6. Return _numberFormat_.
</emu-alg>
</emu-clause>
</emu-clause>
Expand Down Expand Up @@ -577,7 +608,7 @@ <h1>get Intl.NumberFormat.prototype.format</h1>
<emu-alg>
1. Let _nf_ be *this* value.
1. If Type(_nf_) is not Object, throw a *TypeError* exception.
1. If _nf_.[[initializedNumberFormat]] is *true*, throw a *TypeError* exception.
1. Let _nf_ be ? UnwrapNumberFormat(_nf_);
1. If _nf_.[[boundFormat]] is *undefined*, then
1. Let _F_ be a new built-in function object as defined in Number Format Functions (<emu-xref href="#sec-number-format-functions"></emu-xref>).
1. Let _bf_ be BoundFunctionCreate(_F_, _nf_, &laquo; &raquo;).
Expand All @@ -591,7 +622,7 @@ <h1>get Intl.NumberFormat.prototype.format</h1>
<h1>Intl.NumberFormat.prototype.resolvedOptions ()</h1>

<p>
This function provides access to the locale and formatting options computed during initialization of the object.
This function provides access to the locale and formatting options computed during initialization of the object. This function initially invokes the internal algorithm UnwrapNumberFormat to get the %NumberFormat% object on which to operate.
</p>
<p>
The function returns a new object whose properties and attributes are set as if constructed by an object literal assigning to each of the following properties the value of the corresponding internal slot of this NumberFormat object (see <emu-xref href="#sec-properties-of-intl-numberformat-instances"></emu-xref>): locale, numberingSystem, style, currency, currencyDisplay, minimumIntegerDigits, minimumFractionDigits, maximumFractionDigits, minimumSignificantDigits, maximumSignificantDigits, and useGrouping. Properties whose corresponding internal slots have the value *undefined* are not assigned.
Expand Down