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

%Intl%.[[FallbackSymbol]] per-realm #686

Open
acutmore opened this issue Jun 1, 2022 · 23 comments
Open

%Intl%.[[FallbackSymbol]] per-realm #686

acutmore opened this issue Jun 1, 2022 · 23 comments
Labels
c: meta Component: intl-wide issues needs tests s: help wanted Status: help wanted; needs proposal champion
Milestone

Comments

@acutmore
Copy link

acutmore commented Jun 1, 2022

This has come up a few times during proposal-symbols-as-weakmap-keys. Creating an issue here as the 402 repo feels like a more central point to discuss this.

Related discussions in original PR: #84
Test262 issue: tc39/test262#3420


The Intl spec says that %Intl%.[[FallbackSymbol]] is created in the current realm. Implying that each realm has it's own unique IntlLegacyConstructedSymbol.

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

This can be observed:

let getFallbackSymbol_code = `Reflect.ownKeys(
  Intl.DateTimeFormat.call(Object.create(Intl.DateTimeFormat.prototype))
).filter(v => typeof v === 'symbol' && v.description === 'IntlLegacyConstructedSymbol')[0];`;

// using ShadowRealms:
(new ShadowRealm).evaluate(getFallbackSymbol_code) !== (new ShadowRealm).evaluate(getFallbackSymbol_code)

// Or iframes:
const frame = document.createElement('iframe');
document.body.appendChild(frame);
eval(getFallbackSymbol_code) !== frame.contentWindow.eval(getFallbackSymbol_code);

However implementations differ.

  • V8 : same across realms ref
  • JSC : same across realms ref
  • SM : unique per realm ref

It is unclear if this is an issue in practice. The symbol is there to ensure backwards compatibility with existing code, rather than for new code to use. However having a concrete issue to capture/track the subtle inconsistency feels appropriate.

cc: @ljharb

@ljharb
Copy link
Member

ljharb commented Jun 1, 2022

If this is a well-known symbol (it’s cross-realm, like in v8 and jsc) then it belongs in the WKS table. If not, there should be a test262 test that fails on v8 and jsc.

An additional concern is that this is a built-in value, with identity, that isn’t reachable directly from the global. It’d be nice to address that as well (cc @erights @kriskowal)

@caridy
Copy link
Contributor

caridy commented Jun 1, 2022

It should be per Realm instead of a well-known symbol. The fact that Intl is optional, having it as a well-known is just odd because you might have the symbol but not Intl in some environments. It was never intended to be a well-known symbol IIRC.

@ljharb
Copy link
Member

ljharb commented Jun 1, 2022

@caridy that's fine, if v8 and jsc are willing to change. either way tho, i still think it should be directly accessible somewhere from the global without having to invoke functions.

@caridy
Copy link
Contributor

caridy commented Jun 1, 2022

@ljharb

i still think it should be directly accessible somewhere from the global without having to invoke functions.

What about putting that realm specific symbol into Intl instead?

@ljharb
Copy link
Member

ljharb commented Jun 1, 2022

@caridy whether it's well-known or not, that seems like the appropriate place to put it to me!

@anba
Copy link
Contributor

anba commented Jun 8, 2022

There are two other options:

  1. We could check if we actually still need the legacy fallback. (ECMA-402 v1 legacy constructor semantics compromise #84 mentions that we hope we can remove this kludge one day.)
  2. Now that engines have support for private fields, %Intl%.[[FallbackSymbol]] could be changed to a private symbol. That way it can't be observed in the first place.

@ljharb
Copy link
Member

ljharb commented Jun 8, 2022

What is a private symbol?

@acutmore
Copy link
Author

acutmore commented Jun 8, 2022

What is a private symbol?

Some engines, like v8, internally implement private fields using symbols that are hidden from things like Reflect.ownKeys

@anba
Copy link
Contributor

anba commented Jun 8, 2022

Yes, exactly that. "Private Name" should be the correct spec term.

@ljharb
Copy link
Member

ljharb commented Jun 8, 2022

If removing it is web compatible, that seems ideal. If not, imo it should be directly exposed on Intl, whether it remains per-realm or not.

@erights
Copy link

erights commented Jun 8, 2022

First, I agree that simply removing it, if possible, would be ideal.

Otherwise, the spec and SpiderMonkey are both already correct --- it is per realm. We need a test added to test262 which v8 and jsc will fail until they implement this correctly.

Separately, the spec currently happens to obey the useful invariant that the string-named symbol-valued properties of %Symbol%, the Symbol constructor, are exactly the well-known symbols. I have already written code that depends on this invariant, and I suspect others have too. @codehag , I suggest this is a nice self-contained clear invariant that could go first in our efforts to document invariants and make some normative. (Attn @mhofman )

@ryzokuken
Copy link
Member

We could check if we actually still need the legacy fallback.

How would this practically work? I suppose at this point browser teams have experience around finding websites that utilize a certain feature, right?

@bakkot
Copy link
Contributor

bakkot commented Jul 11, 2022

Separately, the spec currently happens to obey the useful invariant that the string-named symbol-valued properties of %Symbol%, the Symbol constructor, are exactly the well-known symbols. I have already written code that depends on this invariant, and I suspect others have too. I suggest this is a nice self-contained clear invariant that could go first in our efforts to document invariants and make some normative.

I am very strongly opposed to treating this as an invariant. We should not constrain the language in this way. For example, in the Set methods proposal, I am considering having Sets speak to Sets by accessing a cross-realm symbol to get the method to test membership (which avoids the problem of using a string-based has, which conflicts with Maps), but it makes absolutely no sense for such a symbol to live on Symbol, since it is Set-specific.

If we need to treat this as an invariant it will severely constrain our ability to evolve the language. @erights, please, please revise your code that depends on this, and avoid writing such code in the future.

(If we need to have a list of all cross-realm symbols somewhere, that's fine, I just don't think that list should be "string-named properties of Symbol".)

@bathos
Copy link

bathos commented Jul 20, 2022

it makes absolutely no sense for such a symbol to live on Symbol, since it is Set-specific.

More than half of the well-known symbols concern built-in-specific contracts. Is this one different from the others or do you mean you’d have preferred none of them to live there?

@bakkot
Copy link
Contributor

bakkot commented Jul 20, 2022

I would also have preferred the RegExp ones live somewhere related to RegExp and that Symbol.isConcatSpreadable lived somewhere related to Array, yes. Putting everything on Symbol is not sustainable.

@ljharb
Copy link
Member

ljharb commented Jul 20, 2022

I 100% agree with this.

@FrankYFTang
Copy link
Contributor

so... can someone make a test262 test to verify the per-realm behavior?

@sffc
Copy link
Contributor

sffc commented Aug 16, 2022

I'm told that the web reality in V8 is that this is a cross-realm symbol, so making it per-realm would require a change in V8. Should we revisit this and change the spec to be consistent with web reality (at least in V8)? What do other engines do?

@ljharb
Copy link
Member

ljharb commented Aug 16, 2022

@sffc we revisited this in the recent plenary, and consensus was to keep it per-realm, which indeed forces v8 to change (since v8 is the only engine that implemented it in this fashion)

@syg
Copy link

syg commented Aug 16, 2022

The original post suggests JSC also has the cross-realm behavior. Is this no longer the case?

@sffc
Copy link
Contributor

sffc commented Aug 16, 2022

Got it; according to the OP, the web reality is inconsistent, so someone needs to change. Let's stick with the TC39 conclusion of per-realm.

@ljharb
Copy link
Member

ljharb commented Aug 16, 2022

@syg ah, my mistake, thanks for clarifying - both v8 and jsc would need to change, then, yes.

@FrankYFTang
Copy link
Contributor

FrankYFTang commented Aug 16, 2022

I have already expressed in TC39 that I am willing to make the code change in v8, assuming there will be a test262 test which can validate/ verify my fix. So I am waiting for someone to write a test262 test to help me for that.

@sffc sffc added the s: help wanted Status: help wanted; needs proposal champion label Dec 14, 2023
@sffc sffc added this to the ES 2024 milestone Dec 14, 2023
@sffc sffc added c: meta Component: intl-wide issues needs tests labels Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: meta Component: intl-wide issues needs tests s: help wanted Status: help wanted; needs proposal champion
Projects
None yet
Development

No branches or pull requests