Should extension members take precedence over instance members? #556
Comments
|
Just my 2¢, I believe that extensions from other libraries should be opt in, while extensions in your library cannot be opted out of. Something like:
or
Which can declare that the library is opting into those extensions, and that they should override instance members. For Rust traits, it is also true that it needs to be in scope. However, the import discipline is different between the two languages, where Dart prefers to import all (like |
|
@ds84182 It would have to be a new syntax, not That is an option. Define imports as not including extensions by default, but only if they are mentioned in a |
|
There are two topics on the table here:
The priority topic is in the title of this issue. We could change it to "extension wins" or "extension member wins if declared to do so", or "an ambiguity is an error", but all of these would require new syntax for specifying at a call site that the instance method should be selected. Orthogonally, we could reduce the potential for conflicts by improving control over the situations where each extension is enabled, which is again new syntax. We talked about constructs like I'd propose that we keep override declarations ("extension member wins because it's declared to do so") in mind. It should not be hard (or breaking) to add this if the need arises. For controlling which extensions are enabled, we can actually rely on source code organization for now: Extensions can be located in separate libraries (one by one, or as small clusters of extensions that are intended to be used together), and such libraries can be gathered into larger clusters (e.g., by having one library in each relevant package, exporting all extensions in that package). This would give developers the same degree of control as any library-level enablement mechanism. So there's no panic on those issues. But if we want to give extension methods a higher priority than instance methods in general, then we have to do it now, and we need the new override syntax. As you probably know, I'd prefer to keep the high priority of instance methods, primarily because it's conceptually wrong to have a model which says "an extension method is an override of every instance method of the |
|
@lrhn For framework authors, one idea I was considering was having extensions declared in the same library as the target type be use-by-default. Otherwise, dart:ffi wouldn't work as great. Next, for extension resolution. I would like extensions to have a higher priority over instance methods with the same name for many of the reasons described here: dart-lang/sdk#38107. Yes, a user could shoot themselves in the foot by declaring an extension that, say, changes the implementation of Also, it would be rather nice for mod foo {
pub trait Bits {
fn bits() -> u8;
}
impl Bits for i32 {
fn bits() -> u8 { 32 }
}
impl Bits for i64 {
fn bits() -> u8 { 64 }
}
}
// Similar implementation but with a different type
mod bar {
pub trait Bits {
fn bits() -> usize;
}
impl Bits for i32 {
fn bits() -> usize { 32 }
}
impl Bits for i64 {
fn bits() -> usize { 64 }
}
}
pub fn main() {
{
use foo::Bits;
println!("{} {}", i32::bits(), std::mem::size_of_val(&i32::bits())); // 32 1
}
{
use bar::Bits;
println!("{} {}", i32::bits(), std::mem::size_of_val(&i32::bits())); // 32 8
}
} |
That opens a can of worms about what it means to be declared in the same library. Most of our packages declare each class in a separate library, then exports all of them from the single main entry-point of the package. I'd be very very wary about having any semantics depend on the source location of a declaration. Dart semantics are about accessibility of declarations through imports and exports, the original declaration is something you can change in refactoring without changing behavior, and that's a good thing. |
|
To "Should extension members take precedence over instance members?", I would say no. In fact, I would argue that it should be a compile error to define an extension member using the same name as something already defined in the extended type. If an extension mistakenly overrides an instance member, this would be very difficult to track/debug. This also means that upgrading dependencies may suddenly break objects unrelated to the dependency. |
|
@rrousselGit then you have the opposite problem for users of extensions. If a single package updates to add a member to a class, something that typically isn't a breaking change, it now has the ability to break downstream packages if they declare an extension with the same name. This extends for all types, since you can make an extension on The same thing can be said about all packages in general. You're able to add a conflicting top-level member to a package, and it is up to your consumers whether it becomes an error or not. With explicit extension use, this becomes somewhat traceable. You're protected against extensions adding new members and classes adding new members, as long as you don't use a glob import. // optional.dart
extension Optional<T> on T? {
X map<X>(X Function(T) f);
}
// library.dart
// Example syntax:
import 'package:optional/optional.dart' use Optional; // or Optional.*, or Optional.map
int parseAndIncrement(String a) => int.tryParse(a).map((n) => n + 1);If it could become an error to upgrade your dependencies, I can definitely see libraries prefixing all their extension member names with their library name, to avoid conflicts: extension Foo<T> on T {
void fooDoBarThing();
}Kinda unfortunate. The next issue is that for core SDK types, it becomes a breaking change to add a member to even the core types, like extension ToHex on int {
String toHexString([int width]) { ... }
}Which somewhat implements dart-lang/sdk#6618, an issue that has been open since 2012. Extensions should be seen as importing locally scoped methods on existing objects, and is the equivalent to calling a static method with syntactic sugar. |
|
@lrhn What do you think about extensions declared inside the declaring class (I personally don't like it, but w/e): class Pointer<T extends NativeType> {
// Alternatively, "extension where T is _NativeInteger"?
extension on Pointer<_NativeInteger> {
int load();
void store(int v);
}
}Feels extremely weird. |
That's why I added that, IMO, shadowing an exiting member with an extension method should be a compile error. In that situation, worst-case scenario, the app will just not compile. |
I believe that Kotlin supports local extensions (not sure about class local, but I think function local). We considered allowing function local extensions, and I actually kind of like the idea. If you think of extensions as just a way of defining overloaded static functions that get to use the |
|
Ok, thanks for the feedback everybody. We've discussed this fairly extensively at this point, and the consensus on the team is that there are tradeoffs with either, neither design clearly dominates the other, and that on balance we feel that the current design is the correct one for our purposes. Key issues weighing in favor of this decision include (at least) the following.
|
Discussion around the interaction between sealing and extension methods re-raised the question of what the right behavior should be when an invocation
a.mis performed andmis present both as an instance member in the static type ofaand as an extension that otherwise applies toa.If the instance member takes precedence, then adding an instance member is breaking since extensions may exist with that name that will no longer apply. This is true anyway because of extension and implementation: however, if we add
sealedclasses to the language, thensealedwould no longer imply that adding members is non-breaking.If the extension member takes precedence, then adding an extension member is breaking since instance members with that name may exist.
Summary of previous discussion.
In comments, @lrhn makes the following points:
f.thenwherefhas static typeFuture, a reader will be very misled if the.thenis not actually resolving to the member declared onFuture.@nex3 responds that:
@lrhn responds
@eernstg Makes the points that:
@leafpetersen Re-iterated the concern about sealing:
override) in order to allow extensions to be written that do override the instance method, and hence could be allowed to apply to sealed classes .Other languages
Kotlin and C# both resolve this in favor of the instance member when overloading does not otherwise resolve it. Since both languages have overloading, adding an instance member is less likely to break code that uses existing extensions, but it still can break it.
Both Kotlin and C# allow extensions on sealed classes.
cc @nex3 @eernstg @munificent @lrhn @srawlins
The text was updated successfully, but these errors were encountered: