Re: [heycam/webidl] Allow extended attributes to apply to types (#286)

tobie requested changes on this pull request.

As discussed over irc, I think we need to clarify the relationship and naming between the annotated type and it's "inner type" and figure out what incidence that has over the overload resolution algorithm.

Other than that and the few nits mentioned above, this is LGTM (and is pretty awesome :)).

Additionally, filed #334, because extended attribute usage *as a whole* lacks examples (but we shouldn't fix it here).



> +Additional types can be created from existing ones by specifying certain [=extended attributes=] on
+the existing types. For example, <code>[Clamp] long</code> defines a new type whose behavior is
+based on that of the {{long}} type, but modified as specified by the [{{Clamp}}] extended attribute.
+
+<p id="extended-attributes-applicable-to-types">
+    The following extended attributes are applicable to types:
+    [{{Clamp}}],
+    [{{EnforceRange}}], and
+    [{{LegacyTreatNullAsEmptyString}}].
+</p>
+
+<div algorithm>
+    The <dfn for="IDL type" lt="extended attribute associated with|extended attributes associated with">extended attributes associated with</dfn>
+    a [=type=] |type| are determined as follows:
+
+    1.  Let |extended attributes| be the empty set.

Link to infra ordered set?

> +        <emu-nt><a href="#prod-TypeWithExtendedAttributes">TypeWithExtendedAttributes</a></emu-nt>
+        production, add the [=extended attributes=] present in the production's
+        <emu-nt><a href="#prod-ExtendedAttributeList">ExtendedAttributeList</a></emu-nt> to
+        |extended attributes|.
+    1.  If |type| is a [=member type=] of a [=union type=] |U|, add the
+        [=extended attributes associated with=] |U| to |extended attributes|.
+    1.  If |type| appears as part of a <emu-nt><a href="#prod-Type">Type</a></emu-nt> production
+        within an <emu-nt><a href="#prod-Argument">Argument</a></emu-nt> production, add to
+        |extended attributes| all of the [=extended attributes=] present in the production's
+        <emu-nt><a href="#prod-ExtendedAttributeList">ExtendedAttributeList</a></emu-nt> that are
+        <a href="#extended-attributes-applicable-to-types">applicable to types</a>.
+    1.  If |type| appears as part of a <emu-nt><a href="#prod-Type">Type</a></emu-nt> production
+        within an <emu-nt><a href="#prod-DictionaryMember">DictionaryMember</a></emu-nt> production,
+        add to |extended attributes| all of the [=extended attributes=] present in the production's
+        <emu-nt><a href="#prod-ExtendedAttributeList">ExtendedAttributeList</a></emu-nt> that are
+        <a href="#extended-attributes-applicable-to-types">applicable to types</a>.

Shouldn't we have a (possibly non-exported) dfn for this?

> +        <emu-nt><a href="#prod-ExtendedAttributeList">ExtendedAttributeList</a></emu-nt> to
+        |extended attributes|.
+    1.  If |type| is a [=member type=] of a [=union type=] |U|, add the
+        [=extended attributes associated with=] |U| to |extended attributes|.
+    1.  If |type| appears as part of a <emu-nt><a href="#prod-Type">Type</a></emu-nt> production
+        within an <emu-nt><a href="#prod-Argument">Argument</a></emu-nt> production, add to
+        |extended attributes| all of the [=extended attributes=] present in the production's
+        <emu-nt><a href="#prod-ExtendedAttributeList">ExtendedAttributeList</a></emu-nt> that are
+        <a href="#extended-attributes-applicable-to-types">applicable to types</a>.
+    1.  If |type| appears as part of a <emu-nt><a href="#prod-Type">Type</a></emu-nt> production
+        within an <emu-nt><a href="#prod-DictionaryMember">DictionaryMember</a></emu-nt> production,
+        add to |extended attributes| all of the [=extended attributes=] present in the production's
+        <emu-nt><a href="#prod-ExtendedAttributeList">ExtendedAttributeList</a></emu-nt> that are
+        <a href="#extended-attributes-applicable-to-types">applicable to types</a>.
+    1.  If |type| is a [=typedef=], add the [=extended attributes associated with=] the type being
+        given a new name to |extended attributes|.

"the type being given a new name" probably also deserves its own dfn.

> +        <a href="#extended-attributes-applicable-to-types">applicable to types</a>.
+    1.  If |type| appears as part of a <emu-nt><a href="#prod-Type">Type</a></emu-nt> production
+        within an <emu-nt><a href="#prod-DictionaryMember">DictionaryMember</a></emu-nt> production,
+        add to |extended attributes| all of the [=extended attributes=] present in the production's
+        <emu-nt><a href="#prod-ExtendedAttributeList">ExtendedAttributeList</a></emu-nt> that are
+        <a href="#extended-attributes-applicable-to-types">applicable to types</a>.
+    1.  If |type| is a [=typedef=], add the [=extended attributes associated with=] the type being
+        given a new name to |extended attributes|.
+    1. Return |extended attributes|.
+</div>
+
+For any [=type=], the [=extended attributes associated with=] it must only contain
+[=extended attributes=] that are
+<a href="#extended-attributes-applicable-to-types">applicable to types</a>.
+
+The [=type name=] of a type associated with [=extended attributes=] is the concatenation of the

I know this doesn't matter a lot, but it would be nice to be a tad more explicit in which order this concatenation happens, e.g.:

```
[…] [=extended attribute associated with=] the type, in this order.
```

…and how it interacts with nullable types; i.e. for `[Clamped] Foo?` is it: `FooClampedOrNull` or `FooOrNullClamped`? It's the former imho, but this needs to be explicitly specified somewhere.



> @@ -6099,6 +6129,7 @@ type is the concatenation of the type name for |T| and the string
 An <dfn id="dfn-extended-attribute" export>extended attribute</dfn> is an annotation
 that can appear on
 definitions,
+[=types=],

\o/

> @@ -5953,6 +5935,54 @@ and joining them with the string “Or”.
 <div data-fill-with="grammar-NonAnyType"></div>
 
 
+<h4 id="idl-types-with-extended-attributes">Types with extended attributes</h4>
+
+Additional types can be created from existing ones by specifying certain [=extended attributes=] on
+the existing types. For example, <code>[Clamp] long</code> defines a new type whose behavior is
+based on that of the {{long}} type, but modified as specified by the [{{Clamp}}] extended attribute.

Should we add a dfn to name those new types?

It seem you're using "the type *annotated* with the extended attribute" and "the extended attribute *associated* with type x".

As discussed over irc, nullables have the "inner" and "nullable type" concepts. We probably need something similar here.

Do we need to go all the way and merge the two concepts like you suggested on irc? Maybe? It might turn out to be simpler.

Either way, we'll need to figure out how an annotated nullable type behaves in the overload resolution algorithm.

> @@ -6505,6 +6536,9 @@ when passed to a [=platform object=] expecting that type, and how IDL values
 of that type are <dfn id="dfn-convert-idl-to-ecmascript-value" export lt="converted to an ECMAScript value|converted to ECMAScript values">converted to ECMAScript values</dfn>
 when returned from a platform object.
 
+Note that the sub-sections and algorithms below also apply to the related types created by applying

Dfns would allow making that explicit in the algo, no? E.g.:

```
1.  If |type| is an [=annotated type=], set |type| to |type|'s [=inner type=].
```

> @@ -8555,6 +8553,66 @@ entails.
 </div>
 
 
+<h4 id="LegacyTreatNullAsEmptyString" extended-attribute lt="LegacyTreatNullAsEmptyString" oldids="TreatNullAs">[LegacyTreatNullAsEmptyString]</h4>

Wondering whether the name change belongs in a different PR. I guess this depends on what makes it easier to file and track this in specs and implementations. Thoughts?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/heycam/webidl/pull/286#pullrequestreview-29144768

Received on Tuesday, 28 March 2017 09:08:30 UTC