-
Notifications
You must be signed in to change notification settings - Fork 214
Add first version of extension type feature specification #1452
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
Conversation
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.
All in all, I think the definitions can work, but the structure could be more approachable.
Not entirely sold on the protected extension type
phrasing. Maybe just use typedef N on T { ... }
instead.
Consider allowing named constructors and show/hide on non-protected extension types.
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.
Review response. Several issues not resolved, probably not many that are really controversial.
possible to reason about this dynamic code and navigate the tree correctly | ||
according to the schema. But there is no encapsulation, and the code where | ||
this kind of careful reasoning is required may be fragmented into many | ||
different locations. |
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.
We need to maintain a specific discipline manually (because the type system doesn't understand that a List<dynamic>
should be treated as a node in a tree that satisfies a specific schema).
The problem is that it is much harder to do this correctly if the discipline must be followed manually in a large number of locations than it is if it is done in a small amount of code in one location. Hence, if we centralize the unsafe code into an extension type declaration then it is only a small group of developers (the ones who are writing the extension type declaration) who need to maintain the special discipline in a small amount of code which is easy to identify (because it is inside a single declaration). That's simply less error-prone and more maintainable.
I tried to make this point clearer.
|
||
This approach does not admit any member accesses for a receiver of type | ||
`void`, and it isn't assignable to anything else without a cast. Just like | ||
`void` of today. |
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.
Nice analysis! I think I agree with all the points made here, but I also think the whole idea about defining void
as an extension type would need some further scrutiny.
|
||
This approach does not admit any member accesses for a receiver of type | ||
`void`, and it isn't assignable to anything else without a cast. Just like | ||
`void` of today. |
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.
I've rewritten the part about void
to indicate the known problems.
Assume that _E_ is an extension declaration of the following form: | ||
|
||
```dart | ||
extension Ext<X1 extends B1, .. Xk extends Bk> on T { |
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.
I'm a bit confused here. Are you proposing to change normal extensions to define types now? That feels very surprising.
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.
Yes, I see no reason why we should prevent using an existing extension declaration with a name as an extension type.
Do you see any drawbacks in allowing that?
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.
As noted in discussion, I think my concerns boil down to the following.
First, it's not clear to me that this will work out. It's not clear to me that it won't, but we need to be sure that the semantics we want are the ones we get already from extension methods, and that we won't break existing code by doing this.
Second, it's not clear to me that we want this. A whole bunch of people wrote extension methods without expecting to be introducing a new type. How will this affect existing code (and future uses of extension methods). Maybe it's ok? You can sort of think of it as a more convenient way of applying the explicit extension syntax? But it needs some substantial discussion, I think.
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.
I'm fairly sure it will work. I considered it while designing extension declarations. That's also why explicit extension invocation looks like a constructor invocation, I wanted Ext(o)
to be a valid expression of the extension type if we introduce extension types with constructors. Doing Ext(o).method
and (o as Ext).method
will mean the same thing.
Whether we want it ...
It's true that the extensions were not designed to be used as types, and I'm slightly worried about unintended uses being potentially a longer term support issue (like using classes as mixins even if they weren't intended for it).
The biggest argument against is that most extensions don't work particularly well as types. They just add individual methods. I'd want to do a show *
by default on those types so that the extension type actually works just like calling directly on the object, just with only extensions from that extension. Hiding the object's own methods is not going to be useful.
The names also don't sound like type names.
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.
We could use the keyword type
to bundle the two things: (1) use as a type, and (2) enable implicit invocations. We would then have the following combinations:
- Without
type
: "You cannot use it as a type, but you can invoke members implicitly". This is the traditional extension declaration. - With
type
: "You can use it as a type, but you cannot invoke members implicitly".
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.
At this point I've kept the ability to use an extension declared using the current syntax (since Dart 2.6) as a type.
The name generally occurs in the namespace already, so we're not proposing to change the behavior of the current extension
declarations with respect to scoping and accessibility. If any given extension is not very useful as a type then developers can just continue to not use it as a type.
<code>Ext<S<sub>1</sub>, .. S<sub>k</sub>>(v).m(args)</code> | ||
where `v` is a fresh variable whose declared type | ||
is the on-type corresponding to `Ext<S1, .. Sm>`, and similarly for | ||
instance getters and operators. This rule also applies when a member access |
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.
I'm still a bit confused about where you're talking about extensions vs extension types, but assuming you really mean for this to be about extensions, I think this isn't coherent with the existing resolution rules for extensions. Specifically, in an extension definition (now), an implicit this
access always goes to the method on the extension, not to the on
type method. The rule you describe here would change that when the on
type was an extension type, which seems broken.
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.
implicit this access always goes to the method on
the extension, not to theon
type method
Right, this is already true for a non-explicit extension (that is, one that does not have the type
keyword).
For an explicit extension type it did not work. To fix this, I added a paragraph to the section about explicit extension types.
The effect is that an explicit extension type E
is treated as a non-explicit extension type in the body of E
, but only when the receiver is an implicit this
.
This ensures that it is still possible to insist on using the instance member m
of the on-type by calling this.m()
, even when the enclosing extension type also declares a member named m
.
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.
Sorry, I'm not following your reply. The text that I read here is as follows:
If e is an expression whose static type is the extension type Ext<S1, .. Sk>, then a member access like e.m(args) is treated as Ext<S1, .. Sk>(v).m(args) where v is a fresh variable whose declared type is the on-type corresponding to Ext<S1, .. Sm>, and similarly for instance getters and operators. This rule also applies when a member access implicitly has the receiver this, and the static type of this is an extension type (which can only occur in an extension type member declaration).
This specifies a semantics which is inconsistent with the existing extension method semantics, right? So the fix, if there is one, needs to be here. Or am I misunderstanding something?
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.
An "implicit this access" for extension methods is really a successful lexical scope look-up.
If that lookup finds an extension method declaration, then we don't add a this.
which is then implicitly invoked with an extension method. We convert it directly to Ext<...>(this).method(...)
.
We always look up an unqualified identifier, like method
in method(...)
, in the lexical scope. What we then do depends on what we find. If it's a static method, it's invoked as ClassName.method(...)
. If it's a top-level method, it's invoked just as method(...)
. If it's an instance method it's invoked as this.method(...)
. And if it's an extension method, it's invoked as Ext<...>(this).method(...)
. If we find nothing, and we are in an instance scope, then we check the interface of the surrounding class, and if we find something there, we do this.method(...)
. This is a different step from finding the instance method in the lexical lookup, it just ends up looking the same. If we find nothing and we are in an extension method scope, we check the interface of this
and if we find something, we do this.method(...)
. Again a different situation with a similar outcome. (Not sure we documented the last case very well, but that's how it's supposed to work).
I see no change to the existing specified behavior here.
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.
@lrhn I didn't follow that sorry. I don't think you're responding to what I said? Let me try to be as explicit as possible. Consider this code:
extension E1 on int {
bool get isEven => true;
void test() {
if (isEven) {} // this code is type correct, because implicit this access resolves to the extension
}
}
extension E2 on E1 {
List<int> get isEven => [0];
void test2() {
isEven[0]; // This code, per the text that I reference here, is invalid
}
}
I claim that the call to isEven in the second extension is an error per this text, because first:
If e is an expression whose static type is the extension type Ext<S1, .. Sk>, then a member access like e.m(args) is treated as Ext<S1, .. Sk>(v).m(args) ... This rule also applies when a member access implicitly has the receiver this
In the test2, the reference to isEven
has implicit receiver this
(so the new rule from @eernstg applies), and so this is treated as E1(this).isEven
.
This semantics is inconsistent. It is not reasonable to change the method look up in an extension depending on whether or not the on
type is an extension type or not.
That's my concern. Am I misunderstanding something?
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.
I certainly did not intend to specify a rule which is inconsistent with the current extension methods declaration. However, the case where the on-type is an extension type is already new, so no particular treatment of that case can be a breaking change. Let's consider the example:
extension E1 on int {
bool get isEven => true;
void test() {
if (isEven) {} // (1)
}
}
extension E2 on E1 {
List<int> get isEven => [0];
void test2() {
isEven[0]; // (2)
}
}
At (1), I agree that
this code is type correct, because implicit this access resolves to the extension
We have an identifier reference isEven
. Lexical lookup finds the extension member, because it occurs in an enclosing scope, and that extension member has a suitable type.
I actually forgot to specify the identifier reference static/dynamic semantics for this case. PR #1489 fixes that. I do not expect that specification to be surprising or controversial.
At (2), I don't agree with this:
This code, per the text that I reference here, is invalid
We have an unqualified function invocation, isEven[0]
, we perform lexical lookup on isEven
, we find the declaration E2.isEven
, and we treat the expression as E2(this).isEven[0]
. Why would that be invalid?
PS: I'm using the newly added specification of extensions in the language specification to arrive at these results. If the text in the proposed feature specification seems to contradict this then I'll need to fix the feature specification.
Note that I'll update the proposal to make constructors available for some non-protected extension types. I haven't yet found a good way to handle the interactions with explicit extension method invocations, but some ideas are developing. |
The proposal has now been updated to enable constructors for all explicit extension types (previously it was only supported with protected extension types). |
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.
Review response.
Assume that _E_ is an extension declaration of the following form: | ||
|
||
```dart | ||
extension Ext<X1 extends B1, .. Xk extends Bk> on T { |
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.
Yes, I see no reason why we should prevent using an existing extension declaration with a name as an extension type.
Do you see any drawbacks in allowing that?
<code>Ext<S<sub>1</sub>, .. S<sub>k</sub>>(v).m(args)</code> | ||
where `v` is a fresh variable whose declared type | ||
is the on-type corresponding to `Ext<S1, .. Sm>`, and similarly for | ||
instance getters and operators. This rule also applies when a member access |
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.
implicit this access always goes to the method on
the extension, not to theon
type method
Right, this is already true for a non-explicit extension (that is, one that does not have the type
keyword).
For an explicit extension type it did not work. To fix this, I added a paragraph to the section about explicit extension types.
The effect is that an explicit extension type E
is treated as a non-explicit extension type in the body of E
, but only when the receiver is an implicit this
.
This ensures that it is still possible to insist on using the instance member m
of the on-type by calling this.m()
, even when the enclosing extension type also declares a member named m
.
|
||
```ebnf | ||
<extensionDeclaration> ::= | ||
'protected'? 'extension' ('type'? <typeIdentifier> <typeParameters>?)? |
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.
What do you think of using class
instead of type
? It feels more Darty.
Revised the proposal: Removed everything about protected extension types (moved that material to a separate document), and corrected a number of smaller issues with the remaining parts. |
I just uploaded a new version that includes a section about |
has two errors.* | ||
|
||
If `e` is an expression whose static type is the extension type | ||
<code>Ext<S<sub>1</sub>, .. S<sub>k</sub>></code>, |
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.
I don't think this is really true if e
is this
, and m
is in the lexical scope.
extension Ext1 on int {}
extension Ext2 on Ext1 {
void test() {
this.test(); // `this` has static type `Ext1` but this is not treated as `invokeExtensionMethod(Ext1, this).test()`
}
}
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.
Right, I added the requirement that Ext
declares an m
(in which case we must invoke the member declared by Ext
, or fail because we request a getter and find a setter etc.).
In the case where Ext
does not declare an m
we may be able to invoke an extension method.
However, that's not exactly the situation we have with Ext1
and Ext2
in your example: this.test()
must be handled according to the static type of this
(that is, Ext1
). Ext1
does not declare a test
. We can't see any extensions on Ext1
or on T
where Ext1 <: T
that declares a test
, so I'd say this.test()
is a compile-time error.
The lexical lookup that finds test
in the enclosing scope is not performed for an expression like this.test()
, we just don't use lexical scoping on an explicit member access a.m...
, we use the interface of a
(plus extension methods, if any).
I removed the (rather fuzzy) sentence about "this also applies" and explained the treatment of implicit this instead.
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.
I think this could resolve the thread, but I'm keeping it open because of the amount of discussion we've had about this point.
instantiated on-type of `E`. In the case where the instantiated on-type of | ||
`E` is not a top type, `E` is also a proper subtype of `Object?`. | ||
|
||
*That is, the underlying on-type can only be recovered by an explicit cast |
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.
explicit or implicit cast.
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.
I don't see what would be wrong with this. When the on-type is a top type it is always an upcast, so there is no requirement to make it explicit; that case is covered in line 390.
When the on-type is not a top type we have a proper subtype relationship on-type <: E
, and the downcast must be made explicit.
Do we have a case where that is not true?
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.
To implicitDownCast<From, To extends From>(From x) {
var l = <To>[];
List<From> castThrough = l;
castThrough.add(x);
return l[0];
}
void main() {
num x = 3;
print(implicitDownCast<num, int>(x));
x = 3.3;
print(implicitDownCast<num, int>(x));
}
different type for it by forgetting everything (going to a top type), or by | ||
means of an explicit cast, typically a downcast to the on-type.* | ||
|
||
When `E` is an extension type, a type test `o is E` or `o is! E` and a 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.
So to be clear: the only time that o is E
promotes is if o
is a top type, since that's the only super type of E
?
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.
True.
|
||
When `E` is an extension type, a type test `o is E` or `o is! E` and a type | ||
check `o as E` can be performed. Such checks performed on a local variable | ||
can promote the variable to the extension type using the normal rules for |
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 just to be clear, given extension E on T {}
and E x = ...
then x is T
will promote to T
, since T <: E
?
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.
Yes.
*This means that members of `E` can be invoked implicitly on `this` inside | ||
`E`, just like the members in a non-explicit extension declaration. Another | ||
way to describe this rule is that it makes `E` non-explicit inside the body | ||
of `E`, but only when the receiver is an implicit `this`.* |
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 unlike normal extensions, extension types don't allow explicit
this` calls?
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.
Right, this.m(args)
in a normal extension as well as in an extension type is resolved using first the interface of this
(the on-type) and then any accessible and applicable extension methods. With a normal (non-explicit) extension, this.m(args)
could invoke a method in the enclosing extension (if it is most specific, but there could also be a conflict). In an extension type E
the syntax this.m(args)
can not invoke an m
declared in E
, but (this as E).m(args)
could do it, as well as m(args)
.
Ok, I took a pass - some small comments, but my only main issue right now is that I still don't think the treatment of |
Signing off on this to land in working, since it feels like it's lingering here. I'm not really very comfortable with this direction yet though, so we should figure out how to proceed. |
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.
Review response. Several things are considerably clearer now!
Assume that _E_ is an extension declaration of the following form: | ||
|
||
```dart | ||
extension Ext<X1 extends B1, .. Xk extends Bk> on T { |
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.
At this point I've kept the ability to use an extension declared using the current syntax (since Dart 2.6) as a type.
The name generally occurs in the namespace already, so we're not proposing to change the behavior of the current extension
declarations with respect to scoping and accessibility. If any given extension is not very useful as a type then developers can just continue to not use it as a type.
different type for it by forgetting everything (going to a top type), or by | ||
means of an explicit cast, typically a downcast to the on-type.* | ||
|
||
When `E` is an extension type, a type test `o is E` or `o is! E` and a 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.
True.
|
||
When `E` is an extension type, a type test `o is E` or `o is! E` and a type | ||
check `o as E` can be performed. Such checks performed on a local variable | ||
can promote the variable to the extension type using the normal rules for |
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.
Yes.
*This means that members of `E` can be invoked implicitly on `this` inside | ||
`E`, just like the members in a non-explicit extension declaration. Another | ||
way to describe this rule is that it makes `E` non-explicit inside the body | ||
of `E`, but only when the receiver is an implicit `this`.* |
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.
Right, this.m(args)
in a normal extension as well as in an extension type is resolved using first the interface of this
(the on-type) and then any accessible and applicable extension methods. With a normal (non-explicit) extension, this.m(args)
could invoke a method in the enclosing extension (if it is most specific, but there could also be a conflict). In an extension type E
the syntax this.m(args)
can not invoke an m
declared in E
, but (this as E).m(args)
could do it, as well as m(args)
.
@@ -458,15 +504,11 @@ etc.* | |||
Let `E` be an explicit extension type declaration, and consider an | |||
occurrence of an identifier expression `id` in the body of an instance | |||
member of `E`. If a lexical lookup of `id` yields a declaration of a |
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're being persnickety, we probably should say "yields a declaration of an instance member of", since id
could be a static method of the extension 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.
Argh, I didn't see this comment when I landed the PR. I'll fix it later.
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.
LGTM, thanks!
This CL adds a draft version of the specification for extension types, based on the feature proposal #1426. The intention is that this document will be discussed and updated as a consequence of discussions. We may later move the document to accepted, which would imply overall acceptance of the proposal.
Discussions about specific topics in this area would take place in issues in this repo, marked with the label
extension-types
.