-
Notifications
You must be signed in to change notification settings - Fork 163
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
Explicitly include annotated types in the various type categories. #747
base: main
Are you sure you want to change the base?
Conversation
You're correct, I got confused. I think this is better. |
@bzbarsky any more thoughts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do this, we should make some more fixups to overload resolution and a few other places, but my main concern is that I think this makes it harder to reason about the types the spec is talking about.
It seems to me that we do want to be able to refer to the underlying, non-nullable and non-annotated, types somehow. If we then want a term that refers to "that thing, with maybe a nullable or annotation attached", that would make sense.
One option is to define the "essential type" (please suggest other names as desired) of a type to be the type with all the nullable/annotation bits stripped off, then use that concept when defining [Clamp] and [EnforceRange] and [AllowShared] as well as some other places that currently worry about needing to strip off those bits by hand. I suspect that will make it clearer what's really going on with the various consumers....
Thoughts?
|
||
The following types are known as <dfn id="dfn-numeric-type" export>numeric types</dfn>: | ||
the [=integer types=], | ||
{{float}}, | ||
{{unrestricted float}}, | ||
{{double}} and | ||
{{unrestricted double}}. | ||
{{double}}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So... With this change there are a few things that are a bit odd:
-
https://heycam.github.io/webidl/#es-overloads step 12.12 no longer needs to worry about "a nullable numeric type" or "an annotated type whose inner type is one of the above types", since those will be included under "numeric type", no? Same thing for step 12.14. And 12.13 given the "string type" changes below.
-
There are various other places in the spec that mention "numeric types" but they all already handle unwrapping nullable and annotated types. Maybe this is OK; it's just a little confusing to consider the table in the distinguishability section, for example, when "numeric types" could in theory include
long?
but in practice can't because we unwrapped that already...
|
||
The <dfn id="dfn-primitive-type" export>primitive types</dfn> are | ||
{{boolean}} and the [=numeric types=]. | ||
{{boolean}}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, this leads to the slightly odd situation that the PrimitiveType
production no longer matches the set of "primitive types". Further, the prose around ConstType says:
The type of a constant (matching ConstType) must not be any type other than a primitive type.
with that last it linking to this definition but it would now actually means a stronger restriction than what that link provides.
{{Float64Array}}. | ||
{{Float32Array}}, | ||
{{Float64Array}}, | ||
any [=nullable types=] whose [=nullable types/inner type=] is a [=typed array type=], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But here we couldn't simplify overload resolution step 12.7 because for the annotated/nullable types the name would not match...
|
||
The <dfn id="dfn-buffer-source-type" export>buffer source types</dfn> | ||
are {{ArrayBuffer}}, | ||
The <dfn id="dfn-buffer-source-type" export>buffer source types</dfn> are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this part changed, the claim that "buffer source types are simply references to objects" stops being true, no?
And "Values of the IDL buffer source types are represented by objects of the corresponding ECMAScript class" also stops beign true.
Fixes #670.
Preview | Diff