Consider restricting attribute keys syntax to identifiers and strings again

[nicolo-ribaudo]

BigInt attribute keys (i.e. import "x" with { 10n: "val" }) need to be converted to strings while parsing, to check for duplicate keys. In SpiderMonkey this would require allocating the bigint at parse time, which is something that does not currently happen (the object { 10n: x } is parsed as { [10n]: x }).

The motivation for allowing number&bigint literals in import attribute keys is for symmetry with object literals.

[mgaudet]

The other thing that I find sort of strange here is that the argument is that we're trying to be symmetric with the object literal version for dynamic import, but the syntax ends up still being a subset of the object literal syntax. For example,

let x = "type"
import "foo" with { [x]: "js"} 

won't parse -- IMO having a tighter simpler rule for the WithClause makes more sense than trying to align it with the dynamic import case.

[nicolo-ribaudo]

It looks like JSC currently allocates bigints while parsing, but they would like to stop doing so:
https://github.com/WebKit/WebKit/blob/4b27f17f5ade44bc02f5d8707c8140a762bb9253/Source/JavaScriptCore/parser/ParserArena.cpp#L98-L112

V8 allocates bigints while parsing without problems

[nicolo-ribaudo]

Also cc @tc39/ecma262-editors, since the keys syntax was relaxed due to feedback from the stage 3 review 🙂

[michaelficarra]

I don't understand the problem. BigInts are only allocated when someone uses them as keys.

[mgaudet]

The spec requires that duplicate assertion keys are detected and reported as syntax errors; but that means that we need to be able to report that import "f.js" with { 1_000n: "value", 1000n: "value" } are the same. In the arbitrary case of a BigInt, we don't want our parser to have to have a full understanding of BigInt semantics, so we would have to allocate a BigInt.

However, as a design principle we are not allocating GC things during parse; this would be an instance of us having to do so.

[mgaudet]

(for regular numeric values we have specialized machinery to handle this, but since a bigint is arbitrary precision, we would need the full arbitrary precision comparison)

[ljharb]

Forgive me if i don't understand some nuance of parsers, but if it's successfully parsed as a bigint, you can't just remove the _s and compare literal source text to see if it's the same?

[michaelficarra]

@ljharb No, the BigInt grammar is not otherwise canonicalising. 0xFFn === 255n

[ljharb]

ah, i forgot about hex literals, fair point

[michaelficarra]

Regardless, though, I am skeptical of the claim that a parser must instantiate an actual JS BigInt to be able to detect duplicate keys. You don't need to allocate an actual BigInt to compute BigInt::toString.

edit: And this cost, even if excessive, is only paid when the program actually uses this combination of features (import assertions with bigint keys), which will be exceptionally rare.

[bakkot]

It's definitely possible in principle. Seems like an awful lot of work to ask of implementations for something which isn't ever actually going to be used, though.

[ghost]

Just implemented an attributes parser, with numeric literal support. I was surprised it was there. Just some thoughts:

• BigInt support adds no value. If you need a numeric key, you can get it with a standard number or a string.
• There is precedent in modules of designing the (static) syntax specifically for easy parsing. It could be argued that a more restricted syntax maintains alignment with the established precedent.

I don't have strong feelings. Following this so my parser can stay aligned with whatever happens.