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

Extension type new syntax available in dart Dart 3.3.0-174.2.beta, some questions and thoughts #54293

Closed
brilliapps opened this issue Dec 9, 2023 · 12 comments
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. feature-extension-types Implementation of the extension type feature P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@brilliapps
Copy link

brilliapps commented Dec 9, 2023

Normally i ask on the dart official discord server. Comments in the code are describing what is happening and why something else could happen in some cases, what i see, and what in my opinion (limited knowledge) would cause the "extension type" to be perfect.
Please take note that there is here below a new type Int defined, starting from a big letter I (not "int"). Also there may be similar situations like these below, but this is enough for now.

extension type Int(int i) implements int {
  Int operator +(int other) {
    return Int(i + other);
  }
}

void main() {
  int a = 2;
  Int b = Int(8); // coulndn't Int b = 8? and silently cast as Int?

  b = b + a; // wrong, but Int + operator overriden returns Int
  b = a + b; // wrong, because a is int, but couldn't it be silently cast to Int?
  a = a + b; // good as expected.
  print(b + a); // ok, somewhere toString is called for sure.
  print(b + b);
  print(a + b);
}

So, do you think that at least b = b + a shouldn't be marked as error in my VScode? I imagined a reason why it should, but i don't know. Any opinions?

@lrhn
Copy link
Member

lrhn commented Dec 9, 2023

Yes, b = b + a; should not be an error.

The missing piece of the implementation is that the special rules for number operator typing should not apply to the extension type operator.

Normally a + a has type int, which is different from the declared return type of int.operator+, which is num.
That's great, and makes number arithmetics bearable.
While b is-an int, its operator+ is not the one that the special typing applies to. I'm guessing the implementation fails to recognize that.

So, for every special rules about number operation typing, it should be read as having an additional "if the invoked member is a non-extension-type member". That restriction was really always there, it was just not necessary to say so, because it couldn't not be that. Now it can.

@lrhn lrhn added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) area-fe-analyzer-shared Assigned by engineers; when triaging, prefer either area-front-end or area-analyzer. feature-extension-types Implementation of the extension type feature labels Dec 9, 2023
@lrhn
Copy link
Member

lrhn commented Dec 9, 2023

About the silent coercion from int to Int, we've chosen to not do that.
That allows an extension type to expose only public constructors which do more than just inject a value.
If we allow done values to be implicitly downcast to the extension subtype, extension type authors would also need a way to opt out of that.

If we introduce implicit coercions at some point, they will likely also apply to extension types, and allowing some simple type coercions now may get in the way of doing a more general thing in the future.

So not letting you do Int b = 8; is very much deliberate. That's an implicit downcast, and we just got rid of those with null safety.

@brilliapps
Copy link
Author

brilliapps commented Dec 9, 2023

If we allow done values to be implicitly downcast to the extension subtype, extension type authors would also need a way to opt out of that.

If we introduce implicit coercions at some point, they will likely also apply to extension types, and allowing some simple type coercions now may get in the way of doing a more general thing in the future.`

I conclude, speaking in very simple terms from the above that a "feature" can be implemented in some future that will allow maybe not Int b = 8 but at least, based on my initial example code this:
b = a + b; // will be fine one day :)

When might such a thing be implemented if so? A year?, half a year?

@lrhn
Copy link
Member

lrhn commented Dec 9, 2023

I'd say that if we introduce implicit coercions (which a type will have to opt into somehow), then you will be able to declare a coercion from int to Int, and then both Int b = 8; and b = a + b; should work with that.

Absolutely no guarantees that it will happen or, if it does, when.

@brilliapps
Copy link
Author

brilliapps commented Dec 9, 2023

Thank you for your answers. Implementing the coercion (with any "but" and "opt-out") would nail it, imho.

Please, if you sees it fit, you can close this issue. But by not doing it some people may have their opinion on this. And again, please you decide.

@lrhn
Copy link
Member

lrhn commented Dec 9, 2023

The bug is real and should be fixed, so keeping open for that

@brilliapps
Copy link
Author

Ok.

@johnniwinther johnniwinther added the cfe-feature-extension-types Implement extension types feature in the CFE label Dec 12, 2023
@johnniwinther johnniwinther self-assigned this Dec 13, 2023
@johnniwinther johnniwinther added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. and removed area-fe-analyzer-shared Assigned by engineers; when triaging, prefer either area-front-end or area-analyzer. cfe-feature-extension-types Implement extension types feature in the CFE labels Dec 13, 2023
@johnniwinther johnniwinther removed their assignment Dec 13, 2023
@johnniwinther
Copy link
Member

Test to verify (the already correct) CFE behavior added in https://dart-review.googlesource.com/c/sdk/+/341488

copybara-service bot pushed a commit that referenced this issue Dec 14, 2023
In response to #54293

Change-Id: Idbd898b49f631074e0cd7d3321acaeac8a89b819
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/341488
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
@keertip keertip added the P2 A bug or feature request we're likely to work on label Dec 14, 2023
@srawlins
Copy link
Member

srawlins commented Jan 4, 2024

CC @scheglov

@scheglov
Copy link
Contributor

scheglov commented Jan 4, 2024

My reading of the specification is that Int + int should be int.

image
extension type Int(int i) implements int {
  Int operator +(int other) {
    return Int(i + other);
  }
}

void f(Int a, int b) {
  a + b;
}

Here T is Int, and Int <: int, so the third rule in the last group applies.
This makes a + b to have the type int.

@lrhn
Copy link
Member

lrhn commented Jan 8, 2024

The specification doesn't know that extension types exist, and needs to be updated to account for them.

The update is probably just that op must denote an instance member (not an extension-type member or an extension member), otherwise the the exception does not apply.
In this case Int.operator+ denotes an extension type member, so the rule does not apply.

That has to be the behavior, because otherwise it's unsound.

extension type Int(int i) implements int {
  String operator +(int other) => "$this+$other";
}
void main() {
  var x = Int(1) + 2; // Result is `String`, static type *cannot* soundly be `int`.
}

See dart-lang/language#3396, which has sadly not been completed yet.

Not sure "extension member" is possible, when an extension type implements int, it must have a member with the same base name as every member of int, so it cannot also allow an extension member of that name.
But if it was, they also wouldn't count, because the special-casing rules for numbers requires that the code that is actually being run, is the original from the members of int or double. Otherwise the rules are not guaranteed to be sound.

@scheglov
Copy link
Contributor

scheglov commented Jan 8, 2024

@scheglov scheglov self-assigned this Jan 8, 2024
@scheglov scheglov closed this as completed Jan 8, 2024
copybara-service bot pushed a commit that referenced this issue Jan 8, 2024
…e invoked method is from ExtensionTypeElement.

Bug: #54293
Change-Id: Ib23cfe2ff150ef34ad47b78444406d86bc4ddb0c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345360
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. feature-extension-types Implementation of the extension type feature P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

6 participants