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

Explicitly include annotated types in the various type categories. #747

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Ms2ger
Copy link
Member

@Ms2ger Ms2ger commented Jun 27, 2019

Fixes #670.


Preview | Diff

@bzbarsky
Copy link
Collaborator

@Ms2ger I don't think this fixes #670. This PR makes [Clamp] long be an integer type, but the question in #670 is around whether long? is an integer type, no?

@Ms2ger
Copy link
Member Author

Ms2ger commented Jun 28, 2019

You're correct, I got confused. I think this is better.

@Ms2ger
Copy link
Member Author

Ms2ger commented Aug 7, 2019

@bzbarsky any more thoughts?

Copy link
Collaborator

@bzbarsky bzbarsky left a 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}},
Copy link
Collaborator

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:

  1. 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.

  2. 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}},
Copy link
Collaborator

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=],
Copy link
Collaborator

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Is long? an integer type?
2 participants