Propagate a type's extended attributes to a nullable inner type

[nicolo-ribaudo]

This patch makes types such as [Clamp] long? legal, propagating the extended attribute to the inner type of the nullable type.

It relaxes the definitions of [AllowResizable], [AllowShared], [Clamp] and [EnforceRange] so that they can be used as such; while it leaves [LegacyNullToEmptyString] as is since it does not really make sense to apply it to a nullable string (since then the attribute would be a no-op, given that the null is already handled).

Closes #670.

---

My motivation for doing this is that I will need it in #1568.

---

• At least two implementers are interested (and none opposed):
  • …
  • …
• Tests are written and can be reviewed and commented upon at:
  • …
• Implementation bugs are filed:
  • Chromium: …
  • Gecko: …
  • WebKit: …
  • Deno: …
  • Node.js: …
  • webidl2.js: …
  • widlparser: …
• MDN issue is filed: …
• The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)

---

Preview | Diff

[saschanaz]

I'm not sure I like this kind of syntactic ambiguity, can we force ([Clamp] long)?

[nicolo-ribaudo]

Probably yes (I actually first started doing that, before changing my mind 😅). The main reasons why I went with this approach are that:

• it's a much larger change, affecting a lot of the WebIDL grammar
• it's kind of weird to have something that normally is space-separate having tighter precedence than something that is not

I can finish my attempt and open a PR to compare.

Note that the approach taken by this PR is similar to what we do for union types, e.g. [Attr] (SomeType or undefined) foo

[nicolo-ribaudo]

Actually, I'm not sure it would be possible*. There are currently productions that are not nullable (e.g. Promise<T>), which are however children of TypeWithExtendedAttributes. If I made TypeWithExtendedAttributes a child of "something ?", i.e.

NullableType:
  TypeWithExtendedAttributes `?`
  TypeWithExtendedAttributes

TypeWithExtendedAttributes :
  ExtendedAttributes Type
  Type

Type :
  ...

* well, it is, but it further complicates the grammar because I'd need to have a TypeWithExtendedAttributesForInsideNullable and TypeWithExtendedAttributesForOutsideNullable with different allowed types inside of them.

[nicolo-ribaudo]

@saschanaz Another problem with that approach is with types in arguments. Per #691, we can have both extended attributes that apply to arguments as well as extended attributes that apply to types. This means that in an argument [AttrForArg, AttrForType] Type? x, [AttrForArg, AttrForType] cannot bind tighter than ?, because AttrForArg needs to apply to the whole argument.

[saschanaz]

By forcing I was thinking about "this extended attribute is valid only on ..." type of thing rather than forcing syntactically. That should be doable.

This means that in an argument [AttrForArg, AttrForType] Type? x, [AttrForArg, AttrForType] cannot bind tighter than ?, because AttrForArg needs to apply to the whole argument.

It can be [AttrForArg] ([AttrForType] Type)? x right? Yes it's kinda awkward to read, but maybe less ambiguous?

[nicolo-ribaudo]

By forcing I was thinking about "this extended attribute is valid only on ..." type of thing rather than forcing syntactically. That should be doable.

Isn't this what this PR is doing? It marks the extended attributes with, e.g.

A type that is not an [=integer type=], and that is not a [=nullable type=] whose [=nullable types/inner type=] is an [=integer type=], must not be [=extended attributes associated with|associated with=] the [{{Clamp}}] extended attribute.

[saschanaz]

Yes except the propagation through nullable type.

[nicolo-ribaudo]

Ok I'm very confused because that would be effectively "not do this PR", I think maybe I misunderstood your original comment:

I'm not sure I like this kind of syntactic ambiguity, can we force ([Clamp] long)?

Where you suggesting "can we force [Clamp] long? to be parsed with [Clamp] ... having higher priority than ... ?, effectively as if it was ([Clamp] long)?, or were you suggesting to actually allow writing parentheses there?

I interpreted it as the first one but I'm now guessing you meant the second one? 😅

[annevk]

This seems like the right direction, WebGL already relies on it (it has [AllowShared] ArrayBufferView?), and engines already behave this way.

Unless @saschanaz is prepared to drive the alternative approach and get the world to adopt that, I think we should go ahead with the direction this PR proposes. It's also not clear the alternative is better as it would allow types such as (long) potentially.

AI found a couple of issues:

• For each of the four attributes there are two kinds of sentence: a "must not be associated with" restriction, and an "appears on X, it creates a new IDL type…" definition. The PR edits only the former. The clean fix is to change "appears on" → "is associated with" in those six sentences, which also repairs the same staleness for the existing union propagation.
• There is no NullableType production. Null : "?" | ε (index.bs:5845) hangs off DistinguishableType : PrimitiveType Null (index.bs:5770). So in long?, the only type node is the whole DistinguishableType; the inner long is a PrimitiveType
  — not a Type, SingleType, or DistinguishableType. It exists only as the §6187 concept.
• UnionMemberType : ExtendedAttributeList DistinguishableType (index.bs:5758) is not a TypeWithExtendedAttributes production, and no step consumes it. Step 3 only pushes down from the union. So [AllowShared] ArrayBufferView written as a
  union member has nothing associating the attribute with it — which is what Web IDL's own AllowSharedBufferSource typedef does at index.bs:15087. That's a hole in the standard today, in the algorithm this PR edits.

[nicolo-ribaudo]

• For each of the four attributes there are two kinds of sentence: a "must not be associated with" restriction, and an "appears on X, it creates a new IDL type…" definition. The PR edits only the former. The clean fix is to change "appears on" → "is associated with" in those six sentences, which also repairs the same staleness for the existing union propagation.

I think this is fine, actually. Those sentences (e.g. second paragraph of https://webidl.spec.whatwg.org/#AllowResizable) are descriptive; the actual normative behavior is given by associated with which already covers all cases.

We can make it more precise and use is associated with rather than appears on, but that's a pre-existing issue as it covers every case of extended attributes moving from a type to a nested one (e.g. with type unions). Would you want it here or in a separate editorial PR?

• There is no NullableType production. Null : "?" | ε (index.bs:5845) hangs off DistinguishableType : PrimitiveType Null (index.bs:5770). So in long?, the only type node is the whole DistinguishableType; the inner long is a PrimitiveType
  — not a Type, SingleType, or DistinguishableType. It exists only as the §6187 concept.

I agree with this observation, but do you think it implies some concrete change for this PR?

• UnionMemberType : ExtendedAttributeList DistinguishableType (index.bs:5758) is not a TypeWithExtendedAttributes production, and no step consumes it. Step 3 only pushes down from the union. So [AllowShared] ArrayBufferView written as a
  union member has nothing associating the attribute with it — which is what Web IDL's own AllowSharedBufferSource typedef does at index.bs:15087. That's a hole in the standard today, in the algorithm this PR edits.

This is a pre-existing bug on main, right? Extended attributes are meant to be put on the whole union (and then they are associated with its members) rather than on the members themselves.

[saschanaz]

I interpreted it as the first one but I'm now guessing you meant the second one? 😅

I did mean the second one, but I realize that needs parser change as currently the rules do not allow a single item inside parentheses - it must always be a union right now. Maybe changing that is not worth just to fix this 🤔

[nicolo-ribaudo]

• "Introduce concept of a "potentially nullable" type" solves the first of those three points by also making things a bit nicer.
• "Clairfy that the inner types are "directly within" ..." I think solves the second point, but I'm not 100% sure it does not have unintended side effects by making things stricter.
• For the third point, there is already Type association restrictions for extended attributes break with unions #827. How it interacts with potentially nullable types really depends on what solution we'll have for that issue.