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

Initial draft of static extension methods design document #303

Merged
merged 17 commits into from
Jun 18, 2019
Merged

Conversation

lrhn
Copy link
Member

@lrhn lrhn commented Apr 8, 2019

No description provided.

@mit-mit
Copy link
Member

mit-mit commented Apr 9, 2019

Feature is #41


Such a declaration introduces its *name* (the identifier) into the surrounding scope. The name does not denote a type, but it can be used to denote the extension itself in various places. The name can be hidden or shown in `import` or `export` declarations.

The *type* can be any valid Dart type, but not a single type variable. It can refer to the type parameters of the extension. It can be followed by `?` which means that it allows `null` values.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should allow a single type variable - Kotlin uses this for some of their basic building blocks, and I think it's very natural.

Copy link
Member Author

@lrhn lrhn Apr 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Erik also couldn't find any reason not to, and I don't remember why I was worried about it.
It's otherwise equivalent to on Object (or on the bound of the type variable), but you get the static receiver type as a type. That should be simpler than having to deconstruct the receiver type. So, done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other option I've kicked around is always providing the self type. e.g., we just say that there's always an implicit Self type variable in scope that is the matched type. Then, as you say, you could just put it on Object. I think for this use case it's simpler to just allow T as a pattern. What I'm not sure of is whether

extension<T> on TypePattern {}

applies to exactly the same set of things as:

extension<T extends TypePattern> on T {}

It seems like it ought to? And if so, you can always get the self type. But if not, then you can't both get the self type and have a specific pattern.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should work on the same things, yes.

If foo(TypePattern x) {}; foo(expression); is allowed, then so is foo<T extends TypePattern>(T x) {}; foo(expression); for exactly the same expressions. We use exactly the same kind of type inference to match the on type as we do for such a function invocation.

As for getting both, I had hoped that the following would work:

extension Foo<E, S extends List<E>> on S {
   S get self => this;
   List<E> clone() => List<E>.of(this);
}

but I seem to get dynamic for E when I try this with a function. ☹️

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a version that uses a wrapper object rather than a function (I suspect that this will be an even more faithful model for how type inference is performed, given the rest of this document):

class Foo<E, S extends List<E>> {
  S _self;
  Foo(this._self);
  S get self => _self;
  List<E> clone() => List<E>.of(_self);
}

main() {
  List<int> xs = [1];
  // Emulate extension method invocation `xs.clone()`.
  // At first, use the direct desugaring as wrapper:
  var ys = Foo(xs).clone();
  print(ys.runtimeType); // 'List<dynamic>'.
}

In this situation we can't expect type inference to apply any constraints to the first type argument of the Foo instance creation (E), so it becomes dynamic. It would surely be a massively breaking change (and not particularly useful) to "optimize" E to int just because the second type argument has been inferred to be List<int> (assuming that we would do so for type inference everywhere, in order to make sure that we keep the set of special exceptions for extension methods as small as possible).

I believe that the design will satisfy Leaf's rule about applicability ('applies to exactly the same'), but also that we shouldn't expect inference to work the same for extension<T> on TypePattern {} and for extension<T extends TypePattern> on T {}. So I'd recommend that we just don't aim for having this property, such that we can use the normal rules for type inference with extensions.


### Scope

Dart's static extension methods are *scoped*. They only apply to code that the extension itself is accessible to. Being in accessible means that using the extension's name must denote the extension. The extension is not in scope if another declaration with the same name shadows the extension, if the extension's library is not imported, if the library is imported and the extension is hidden, or the library is only imported with a prefix. In other words, if the extension had been a class, it is only in scope if using the name would denote the class&mdash; which will allow you to call static methods on the class, which is exactly what we are going to do.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned about the lack of a way of resolving scope conflicts. For everything else that I can think of, you can always get around a scope conflict by using a qualified name. But as far as I can tell, if there are two extensions named "CoolListExtensions", I can never use both of them in the same scope. This seems really unfortunate - it means that extension names matter in a way that feels bad.

I think we may need a way to rename extensions or use them qualified. So for example, we could allow you to alias them:

import `pkg:list` as list;
import `pkg:quiver` as quiver;

extension CoolList = list.CoolList;
extension QuiverList = quiver.CoolList;

Or we could allow you to bring the whole thing (or parts of it) into scope on the import line:

import `pkg:list` as list show CoolList;  //bring all of list.CoolList extensions into scope
import `pkg:quiver` as quiver show CoolList; // bring all of the quiver.CoolList extensions into scope
import `pkg:cool` as cool show CoolList.map; // bring cool.CoolList.map (only) into scope

Copy link
Member Author

@lrhn lrhn Apr 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can always do scope override with import prefixes, you just have to do the explicit quiver.CoolList(myList).foo() override. You don't get the implicit suffix invocation when you override.

It is true that a conflict of name will prevent you from accessing the extension, just as it would prevent you from accessing a class. If you have two different classes named CoolList, then you can't call a static method on either.

We could choose to allow conflicts where all the names are extensions, as long as the member resolution gives a unique result, so if you have two CoolList extensions in scope, but only one of them has a foo member, then [42].foo() is not considered a conflict. Or if both have a foo, but one is more specific than the other, then it is not considered a conflict.
I am wary at allowing this because it would suggest that declaring those in the same scope should also be allowed (you can have them in the same scope, so why not declared them in the same scope?), and then people might think the following is a reasonable design:

extension CoolList<T> on List<T> { T foo() ... }
extension CoolList on List<int> { int foo() ...int specific code ... }
extension CoolList on List<String> { String foo() ...String specific code ... }

If library authors start doing that, then you cannot do an explicit override any more because that depends on explicit name resolution.

I think keeping the "at most one definition of a name in each scope" rule is important for language sanity.

That leaves us with the ability to open more than one extension with the same name in a specific scope, which means renaming.

We can introduce the "extension alias" you suggest. We could also go one step further and introduce a renaming operation directly in the import modifiers:

import "package:list/list.dart" show CoolList;
import "package:quiver/quiver.dart" show CoolList as QuiverList;
// or: import "package:quiver/quiver.dart" hide CoolList as QuiverList;

That would allow renaming any import. It's a trivial rename, all uses of the new name will refer to the imported declaration.
In the importing library, if the name QuiverList is in scope and not shadowed, then the quiver CoolList extension applies.

I'm sure people will have lots of fun with import "dart:core" hide dynamic as Any;. 😈

Then again, I'm not sure why we will need to give more ways to avoid scope conflict for extensions than we do for other declarations. You can already have scope conflicts, you resolve them by putting one of the things into a prefix scope, then use the prefix to explicitly name the things. We can do that, we just have to do the explicit extension override, the same you would use if two differently named extensions conflict on a specific member.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then again, I'm not sure why we will need to give more ways to avoid scope conflict for extensions than we do for other declarations.

My argument would be that prefixing other kinds of things doesn't change their affordances. There is nothing you can do with a class Foo that you can't do with foo.Foo, as far as I know. And similarly for functions, getters, setters, etc. But with extensions, there is: prefixing it stops the implicit open of the extension in scope. I'm really worried that if providing extensions as part of APIs becomes really common, then basically everyone will have to name mangle their extension names to be globally unique or else clients won't be able to use them as intended.

I'd be fine with import based renaming. I'd be fine with explicit renaming.

Another approach would be to allow "opening" and extension explicitly. So you would do:

import "package:list/list.dart" show CoolList;  // Bring CoolList into scope, but not CoolList.foo
open CoolList;  // Bring CoolList.foo into the extension method resolution scope.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a scope override for opting in to an extension, but none for opting out. You need to remove the extension from the scope for that. That is probably unavoidable since we use the same syntax for everything, so there is no syntax we can write to do "the normal thing", because that is the syntax we are using already.

Copy link
Member

@eernstg eernstg May 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use generalized type aliases to perform renaming, just like they can rename classes:

import `pkg:list` as list;
import `pkg:quiver` as quiver;

typedef CoolList<X> = list.CoolList<X>;
typedef QuiverList<X> = quiver.CoolList<X>;

These type aliases would be semantically isolated from other type aliases (which is good because it means that we don't promise being able to do anything new just because we're accessing a renamed extension), but it might also be useful because it can be used to pre-resolve some conflicts in a way that will work for the given application:

import `pkg:list` as list;
import `pkg:quiver` as quiver;

typedef CoolList<X> = list.CoolList<X>;
typedef QuiverList<X extends SomeQuiverishType> = quiver.CoolList<X>;

So if quiver.CoolList admits all type arguments, but this application will actually only use it when X extends SomeQuiverishType then we could use the above to ensure that there is no conflict with list.CoolList for any type argument which does not extend SomeQuiverishType.

We'd need to handle inference based on a renamed extension, but that shouldn't be worse than handling inference of an instance creation based on a renamed class.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels really off to me to use typedef for renaming these since they... aren't types. It feels really misleading and confusing. If we want a declaration, maybe:

extension CoolList<X> = list.CoolList<X>;

?


Here all three extensions apply. The most specific one is `BoxSpec` which is specialized for lists of numbers. If `BoxSpec` had not been in scope, then neither of `BoxCom` or `BoxList` would be more specific than the other. Their `on` types, when instantiated to bounds, are `Box<Iterable<Comparable<dynamic>>>` and `Box<List<dynamic>>` which are unrelated by subtyping.

This is also why we use the instantiated-to-bounds type for comparison. If we used the actual type, bound to the type variable by the current use-case, we would consider `BoxList<int>` to be more specific than `BoxSpec`, but in practice, the `BoxList<int>` extension would not be able to *use* the integer-ness of the type in its code, so `BoxSpec`, which can specialize its code for lists of numbers, is the most precise match.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Counter-argument: if you used the actual type, and BoxSpec wasn't in scope, then BoxList would be most specific, and it could take advantage of the Listness of the type. But if you use the instantiated-to-bounds type, then neither is most specific and you get an error.

I'm not sure what the right answer is here. Do we know what other languages do?

MyList(object).quickSort();
```

The syntax looks like a constructor invocation, but it does not create a new object.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably if MyList is generic you should be able to pass the explicit generic arguments as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, or have it inferred like here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty clever and looks really nice. I'm somewhat worried users will be surprised that this doesn't work:

MyList(object).quickSort(); // Fine.

var list = MyList(object); // Error.
list.quickSort();

This is probably acceptable, but again something to keep an eye on.

Is there any chance this syntax will collide unpleasantly with static extension types?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt it will conflict with extension types. Those would rather be MyList list = object;

We can define MyList(object) as a cast to the matched on type. Then:

var list = MyList(object);

would be equivalent to:

var list = object as List<dynamic>;

Not sure it's useful.


If an extension is found to be the one applying to a member invocation, then at run-time, the invocation will perform a method invocation of the corresponding instance member of the extension, with `this` bound to the receiver value and type parameters bound to the types found by static inference.

If the receiver is `null`, then that invocation is an immediate run-time error unless the `on` type of the extension has a trailing `?`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I maybe understand your previous comment better - you're not assuming NNBD. In post NNBD world, does this mean "unless the on type is nullable"? Or "unless the on type is potentially nullable"?

e.g.

extension E1<T extends Object?> on T {
  void m() {};
}
extension E2 on FutureOr<Object?> {
  void n() {};
}
void test() {
  null.m(); // OK?
  null.n(); // OK?
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In NNBD-world, if the on type is nullable, then it applies to a nullable static receiver type, if not, it doesn't. There won't be any unsafe null errors (except in the presence of legacy unsafe-nullable * types).

So, the example is OK. Since null is a subtype of Object? and of FutureOr<Object?>, the n and m extension methods match the null.m() (with T inferred as Null) and null.n() invocations.

Since the on types are nullable, the this references inside the methods will be nullable as well, so it's no surprise that they might be null.

If the receiver type is potentially nullable

So, using NNBD:

extension E1 on Object { m1() { assert(this != null); } }
extension E2 on Object?  { m2() {} }
void main() {
  Object o1 = 1;
  o1.m1(); // OK, Object <: Object
  o1.m2(); // OK, Object <: Object?
  Object? p1 = 1;
  p1.m1(); // compile-time error, there is no m1 method matching Object?.
  p1.m2(); // OK, Object? <: Object?
  Object? p2 = null;
  p2.m1(); // compile-time error, there is no m1 method matching Object?.
  p2.m2(); // OK, Object? <: Object?.
}

Using non-NNBD semantics, where the ? on the type means "accepts null":

extension E1 on Object { m1() { assert(this != null); } }
extension E2 on Object?  { m2() {} }
void main() {
  Object o1 = 1;
  o1.m1(); // OK, o1 != null
  o1.m2(); // OK.
  Object? o2 = null;
  o2.m1(); // run-time error, o2 == null at runtime.
  o2.m2(); // OK, o2 == null at runtime is accepted and this == null.
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be sure I understand correctly:

pre-NNBD:

extension E<T> on T {
  void foo(){}
}

void main() {
  null.foo();
}

is a runtime error, but post-NNBD will be accepted?

Copy link
Member Author

@lrhn lrhn Jun 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. If you want the pre-migration behavior, you have to migrate it to:

extension E<T extends Object> on T {
  void foo() {}
}

(which is also valid pre-migration and does the same thing),
and if you want the post-NNBD behavior prior to migration, you have to write;

extension E<T> on T? {
  void foo() {}
}

(which will still work post-migration, the ? will just be redundant).

This obviously assumes that extension methods is released before NNBD. That does seem likely.


With NNBD types, we will not allow a non-nullable extension `on` type to apply to a member invocation with a nullable receiver type.

### Semantics of Extension Members
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I buy all of this, but it's very finicky stuff: by which I mean, we should think hard about this, and then think hard about it again... :)

@Hixie
Copy link

Hixie commented May 2, 2019

I'm concerned about the "COMEFROM" nature of extension methods (where do i go to find where this method i'm using is implemented?).

I think overall in the Flutter framework we'd probably avoid using it, except for one thing, which is adding methods to specific specializations. For example, right now Animation<T> has a drive method that asserts T == double. I'd be happy to add an extension AnimationDouble on Animation<double> that added drive.

My biggest question is, how will this look in the dartdocs?

@lrhn
Copy link
Member Author

lrhn commented May 2, 2019

My biggest question is, how will this look in the dartdocs?

It will be documented where the extension is declared. The DartDoc tool may want to special-case the situation where an extension is defined in the same library as a class that it applies to, and give the class at least a link to the extension methods.

Apart from that, we just have to figure out how to document the type. I'll leave that to the DartDoc team, they have much more experience with presenting the relevant information.

Perhaps extension Foo<T extends Comparable<T>> on List<T> { void fastSort() { ... } } could be documented as

extension Foo on List<T extends Comparable<T>> 
instance members:
  void fastSort() { ... }

?

static smartHelper(Object o) { ... }
}
...
MySmart.smartHelper(someObject); // valid
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we disallow references to generic type parameters in static methods? Otherwise we need to allow MyGenericSmart<int>.smartHelper(someObject).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, Dart already forbids that. Static members exist on the class's namespace, and don't hang off the instantiated type or have access to the class's type arguments.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Also MyGenericSmart<int>.smartHelper(someObject) isn't valid syntax (unless it's a constructor invocation). I want to change that at some point, to allow List<int> as a type literal, or list.fold<int> as instantiated generic tear-off. Until than, id<type> is only allowed in constructor invocations or type expressions.

}
```

Here the `list.isEven` is guaranteed to hit the `isEven` of the same extension (unless someone puts an `isEven` member on `List`), an extension has more specificity than any other extension inside itself.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably say explicitly that you can use MyUnaryNumber(object).isEven inside the definition of MyUnaryNumber to reach the extension even in the presence of an instance method.

something.doStuff().doMyStuff().doOtherStuff().doMyOtherStuff()
```

The code is also much less discoverable. An IDE can suggest `doMyStuff()` after `something.doStuff().`, but will be unlikely to suggest putting `doMyOherStuff(…)` around the expression.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doMyOherStuff(…) -> doMyOtherStuff(…)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.


The declaration introduces an extension. The extension's `on` type defines which types are being extended.

For any member access, `x.foo`, `x.bar()`, `x.baz = 42`, `x(42)` or `x + y`, the language first checks whether the static type of `x` has a member with the same base name as the operation. That is, if it has a corresponding instance member, respectively, a `foo` method or getter or a `foo=` setter. a `bar` member or `bar=` setter, a `baz` member or `baz=` setter, a `call` method, or a `+` method. If so, then the operation is unaffected by extensions. *This check does not care whether the invocation is otherwise correct, based on number or type of the arguments, it only checks if there is a member at all.*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does "member access" include null-aware and cascade too? I assume so. Also implicit this self calls? Maybe call this out?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does. I'll make it explicit.
A this should work, even an implicit one, although that makes me slightly worried.
Someone declaring:

extension Gotcha on Object {
   dynamic get typo {
     print("Gotcha");
     return null;
   }
}

Then anywhere you write typo instead of type, it will match and hide the typo.

Having to write this.typo at least proves that you are intending a member invocation.

May work, not sure it's completely safe.

… acts like a normal method where `this` has the `on` type as static type)

Like for a class or mixin member declaration, the names of the extension members, both static and instance, are in the *lexical* scope of the extension member body. That is why `MySmart` above can invoke the static `smartHelper` without prefixing it by the extension name. In the same way, *instance* members are in the lexical scope.

If an unqualified identifier may lexically resolves to an extension method, the invocation becomes an explicit invocation of that extension method on `this` (which we already know has a compatible type for the extension).
Copy link
Member

@eernstg eernstg May 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: resolves --> resolve.

This does not fit well with the current rules (that is, the treatment of this in Dart of today).

With the current rules, any unqualified identifier m (used as an expression or as the first token of an unqualified function invocation) is desugared to this.m when all possible scoped resolutions except instance methods have been tried and did not apply (so this also occurs when an instance method named m is in scope). The same rule applied to an extension would make m and m(...) desugar to this.m/this.m(...), that is, an instance member of the syntactic receiver of the extension method invocation, not an extension method in the enclosing extension. (If this.m is an error then it would presumably be processed once more and become MyExtension(this).m.)

So for consistency with the current rules, and in order to maintain the rule "instance method always wins over extension-instance method", we should actually not have this rule.

Alternatively, we could maintain another rule even more prominently: "Implicit access to the enclosing declarations (say, of the current class or extension) always win over remote declarations".

That's already true today (because implicit access is only supported with the built-in this), but as soon as we introduce new ways to specify the binding of this (e.g., a static extension, or a Kotlin style function-literal/type-with-receiver) then we might very well want to give preference to the lexically enclosing declarations.

Bob gave this example, arguing that a Kotlin style function with receiver will be inconvenient if it means that we can't access an instance method from the enclosing class.

I certainly agree, and my proposal #328 about the treatment of "custom" bindings of this actually does have that property, and we might use that as a foundation for how to deal with the "custom" binding of this in a static extension.

With a straightforward application of the rules in #328 to static extensions, an unqualified invocation will target an extension method which is in scope, not the instance method. This works as follows:

  • We introduce a new syntax <identifier> '.' 'this' (meaning ClassName.this and maybe class.this as an abbreviation) that allows for accessing the enclosing meaning of this when some other mechanism has introduced a meaning for this that shadows that outer one. It's only intended to target the enclosing class (not several nested "custom" declarations of this). So C.this.m in a regular class C is just the same thing as this.m, but C.this.m keeps the same meaning also in the case where some other meaning is given to this in some scope that is nested inside C.

  • The treatment of unqualified identifiers (stand-alone or as an unqualified function invocation) is adjusted to desugar m to C.this.m rather than this.m, when the interface of C has an m; only 'otherwise' do we desugar m to this.m (that's the old rule). So the "normal" this always wins over a "custom" this, when the requested member exists.

This will give us a consistent foundation for deciding to use the rule as specified in line 263 (for more details check #328).

So we can have consistency in two ways:

(1) If we generalize the current rules, an extension-instance method m will not win over an instance method m for an unqualified call inside the body of an extension method.

(2) If we do something similar to the proposal in #328 in order to have a robust foundation for "custom" bindings of this in a broader sense, then we would also be able to let the extension method win, and the intuition would be "because the extension method is in scope".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec does not say "except instance methods" anywhere. It does not ignore instance member declarations (they shadow global declarations).. Instead it explicitly says what happens when the identifier refers to an instance method, and that happens to be the same thing which happens when it refers to nothing (or, in Dart 2, to a member of the interface of this).

The current rules are that an unqualified identifier which refers to a lexically scoped declaration of an instance methods becomes an invocation of exactly that (virtual) method on the current object. That is, your alternative is what I already consider the rule of Dart.
I then generalize that to say that if the unqualified identifier lexically refers to an extension method, then it becomes an invocation of exactly that extension method on the current object.

Nobody has made it explicit which interpretation of the mechanical rules is correct, they simply say what happens, not why it happens, so we can both be right.
I'm picking your alternative version as the explanataion for the current behavior.

The fact that there is a rewrite to this.foo() or MyExtension(this).foo is an implementation detail, the rule is that an unqualified identifier refers to what it means in the lexical scope. Only if it refers to nothing, then it uses the this interface scope.

Bob's example does not disagree with this. You can still refer to the lexical scope, and you can also refer to this.something for the instance scope.

I believe the rule, as written here, is consistent.


The unqualified `length` of `isEven` is not defined in the current lexical scope, so is equivalent to `this.length`, which is valid since `List<Object>` has a `length` getter.

The unqualified `isEven` of `isOdd` resolves lexically to the `isEvent` getter above it, so it is equivalent to `MyUnaryNumber(this).isEven`, even if there are other extensions in scope which define an `isEven` on `List<Object>`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would work exactly as described if we use the rules in #328, cf. my comment on line 263.

Otherwise, I think we'd need to introduce complex and inconsistent rules (in the sense that we couldn't defend a description starting with "We always treat this like that").

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need anything new, even #328. we can consistently say that a lexically bound identifier refers to the thing in the lexical scope. Whether we do that by a rewrite to C.foo, this.foo or Extension(this).foo is a matter of what is in the lexical scope (a static member, an instance member or an extension member).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static extensions are not (except for some variants discussed briefly at the end) related in any way that resembles the subtype and subclass relation of classes, but I think we need a foundation for how to deal with custom bindings of this that will also work for classes (and everything else) in a consistent manner.

In a class, I believe that it would be confusing (and inconvenient) if you can call methods of the enclosing class when they are in the lexical scope, but not if they are inherited from a superclass or just added to the interface of the class by the implements clause. #328 specifies that we select the instance member of the enclosing class in the case where the interface of that class has a member of the requested name, and that's not something that we would get from a rule that is based solely on lexical scoping.

`}'
```

where `extension` becomes a built-in identifier, `<type>` must not be a type variable, and `<memberDeclaration>` does not allow instance fields or constructors. It does allow static members.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cf. comment #303 (review): I believe there's consensus on allowing the lone type variable, in which case we presumably don't need to say anything about <type>.

Suggested change
where `extension` becomes a built-in identifier, `<type>` must not be a type variable, and `<memberDeclaration>` does not allow instance fields or constructors. It does allow static members.
where `extension` becomes a built-in identifier and `<memberDeclaration>` does not allow instance fields or constructors. It does allow static members.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, I allowed it elsewhere, but missed it here.


- Otherwise an invocation of an extension method runs the instance method with `this` bound to the receiver and with type variables bound to the types found by type inference (or written explicitly for an override invocation). The static type of `this` is the `on` type of the extension.

- Inside an instance extension member, extension members accessed by unqualified name are treated as extension override accesses on `this`. Otherwise invocations on `this` are treated as any other invocations on the same static type.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in comment on line 263, I believe that this rule would be consistent with the approach proposed in #328, but not consistent with the current rules for the treatment of unqualified identifiers and this.

In short, the current rules stipulate that an unqualified name m that resolves to an instance member in the lexical scope will not be taken to denote that instance member directly, it will be desugared to this.m and then processed as such.

- An extension only applies if the receiver type is a subtype of *all* `on` types.
- An extension is more specific than another if for every `on` type in the latter, there is an `on` type in the former which is a proper subtype of that type, or the two are equivalent, and the former is a proper subtype of the latter when instantiated to bounds.
- The trailing `?` makes the most sense if it is applied only once (it's the extension which accepts and understands `null` as a receiver), but for forwards compatibility, we will need to put it on every `on` type individually. All `on` types must be nullable in order to accept a nullable receiver.
- There is no clear type to assign to `this` inside an instance extension method. For a mixin that's not a problem because it introduces a type by itself, and the combined super-interface is only used for `super` invocations. For extension, a statement like `var self = this;` needs to be assigned a useful type.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superinvocations are otherwise no longer mentioned in this document. Maybe it should be deleted from here, or some extra explanation should be added in order to specify what it would mean?

My first thought would be that this.m() would invoke m on the syntactic receiver of the current extension method invocation, and that would be checked against the combined interface of the on types, and there's nothing "superinvocated" about this.

Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good! And there are lots and lots of things that we have already agreed on.

However, I still think it's worth taking one more round on the treatment of this (in particular: how and whether to allow implicit access to an extension-instance member m). My main concern is that we should ensure a high degree of consistency within the language, including the situation where we'd add more than one way to achieve a non-standard binding for this (e.g., for a "function with receiver" as well as for a static extension).


This approach allows related extensions to declare functionality on a number of types, without accidentally allowing a conflict with an unrelated extension.

The extending extension must be defined on a subtype of its super-extension. In the above, the `on` type of `Bar<T>` is `List<T>`, which is a subtype of the `on` type of `Foo<T>`, which is `Iterable<T>`. Also, the declared extension methods in the extending extension must be valid overrides of the same-named super-extensions extension methods, as if it was a subclass relationship.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean subtype in general, or proper subtype? Can I do

extension Baz<T> extends Foo<T> on Iterable<T> { twizzle() { ... }}

What about

extension A<T> on T { twizzle() { ... }}
extension B<T> extends A<T> on T { twizzle() { ... } }

The latter would give you a way to override completely generic extensions, which we wouldn't otherwise have, I think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just subtype. You can completely shadow the super-extension, but I don't think avoiding that isn't worth much effort.


```dart
extension Foo<T> on Iterable<T> { twizzle() {...} fromp() { ... }}
extension Bar<T> extends Foo<T> on List<T> { twizzle() { ... } }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need "implements" as well? Otherwise, it feels like I can get stuck:

extension Foo on A { twizzle() {...} }
extension Bar on I  { twizzle() { ... } } 
class B extends A implements I {}
extension Baz extends Foo on A { twizzle() {...} } 
B().twizzle() // Error, because Baz doesn't cover I, right?

Copy link
Member Author

@lrhn lrhn Jun 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want some way to extend more than one other extension, yes. Adding that.

Whether to use extends ext1, ext2 or extends ext1 implements ext2, that only matters if we inherit implementation from the superextension. I have not found a good use of that yet.


This is a simple feature, but with very low impact. It only allows you to omit a single private name for an extension that is only used in a single library.

### Scope
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we provide a way to show/hide individual members of an extension? Otherwise you're somewhat stuck if you have two extensions that provide overlapping names and you want to pick and choose.


### Scope

Dart's static extension methods are *scoped*. They only apply to code that the extension itself is accessible to. Being accessible means that using the extension's name must denote the extension. The extension is not in scope if another declaration with the same name shadows the extension, if the extension's library is not imported, if the library is imported and the extension is hidden, or the library is only imported with a prefix. In other words, if the extension had been a class, it is only in scope if using the name would denote the class&mdash; which will allow you to call static methods on the class, which is exactly what we are going to do.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a private extension in a library's export scope? If not, we should probably be sure to say so explicitly. If so, does importing that library bring any public methods in that private extension into scope?

lrhn added 7 commits June 12, 2019 07:38
Extension names may conflict without affecting the implicit use of their declared members. Only access to the *name* is an error (and that can be double-imported with a prefix if necessary).

Pre-NNBBD, an `on` type of `Object` or `Null` will allow a `null` value at run-time, even without a trailing `?`.

Removed the "explicit extension" variants. We are not going with those.
…atform library declarations.

Also state the a trailing `?` does not affects specificity pre-NNBD. Since all types are effectively nullable, the `?` merely describes run-time behavior.
@lrhn
Copy link
Member Author

lrhn commented Jun 17, 2019

Landing so the text can be used as reference.

@lrhn lrhn merged commit 5b3d43e into master Jun 18, 2019
@lrhn lrhn deleted the lrhn-sem-spec branch June 18, 2019 08:01
Copy link
Member

@leafpetersen leafpetersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!


This is equivalent to giving the extension a fresh private name.

We may need to make `on` a built-in identifier, and not allow those as names of extensions, then there should not be any parsing issue. Even without that, the grammar should be unambiguous because `extension on on on { … }` and `extension on on { … }` are distinguishable, and the final type cannot be empty. It may be *harder* to parse, though.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the plan to resolve this question? Base it on experimentation in the parser?


Here all three extensions apply to both invocations.

For `x.best()`, the most specific one is `BestList`. Because `List<int>` is a proper subtype of both ` iterable<int>` and `<List<num>`, we expect `BestList` to be the best implementation. The return type causes `v` to have type `int`. If we had chosen `BestSpec` instead, the return type could only be `num`, which is one of the reasons why we choose the most specific instantiated type as the winner.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iterable<int> -> Iterable<int>


The unqualified `length` of `isEven` is not defined in the current lexical scope, so is equivalent to `this.length`, which is valid since `List<Object>` has a `length` getter.

The unqualified `isEven` of `isOdd` resolves lexically to the `isEvent` getter above it, so it is equivalent to `MyUnaryNumber(this).isEven`, even if there are other extensions in scope which define an `isEven` on `List<Object>`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isEvent -> isEven. Also, maybe make explicit that even if someone added an isEven instance member to List, that the unqualified isEven reference would resolve to the extension method?


Like for a class or mixin member declaration, the names of the extension members, both static and instance, are in the *lexical* scope of the extension member body. That is why `MySmart` above can invoke the static `smartHelper` without prefixing it by the extension name. In the same way, *instance* member declarations (the extension members) are in the lexical scope.

If an unqualified identifier lexically resolves to an extension method of the surrounding extension, then that identifier is not equivalent to `this.id`, rather the invocation is equivalent to an explicit invocation of that extension method on `this` (which we already know has a compatible type for the extension): `Ext<T1,…,Tn>(this).id`, where `Ext` is the surrounding extension and `T1` through `Tn` are its type parameters, if any. The invocation works whether or not the names of the extension or parameters are actually accessible, it is not a syntactic rewrite.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A corner case here: if I write an extension on dynamic, I think this implies that internal unqualified references to the members that I am defining will resolve to the extension members and not as dynamic calls. Worth calling out, and testing for?

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

Successfully merging this pull request may close these issues.

8 participants