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

Behavior around upcasts in Dart2 are not documented well #33947

Closed
RedBrogdon opened this issue Jul 23, 2018 · 6 comments
Closed

Behavior around upcasts in Dart2 are not documented well #33947

RedBrogdon opened this issue Jul 23, 2018 · 6 comments
Labels
area-documentation Prefer using 'type-documentation' and a specific area label. needs-info We need additional information from the issue author (auto-closed after 14 days if no response)

Comments

@RedBrogdon
Copy link

I have the following code running in DartPad (Chrome, OSX, Dart SDK 2.0.0-dev.63.0):

class Shape {
  final id;

  Shape(this.id);

  void log() { 
    print('Logging Shape #$id.');
  }
}

class Square extends Shape {
  Square(int id) : super(id);
  
  @override
  void log() {
    print('Logging Square #$id.');
  }
}

void main() {
  print('*** Squares -> Shapes with "as":');
  var listOfSquares = <Square>[Square(1), Square(2), Square(3)];
  
  print('1) Original list of Squares:');
  listOfSquares.forEach((s) => s.log());

  final listOfShapes = listOfSquares as List<Shape>;
  print('2) List of Shapes:');
  listOfShapes.forEach((s) => s.log());
  
  print('3) List of Shapes after adding a Shape to new list:');
  listOfShapes.add(Shape(5));
  listOfShapes.forEach((s) => s.log());

  print('4) Original List after adding a Shape to new list:');
//   try {
    listOfSquares.forEach((s) => s.log());
//   } on CastError {
//     print ('Caught a CastError!');
//   }
}

In short, it creates a list of a subtype, uses as to cast it to the supertype, then adds a new instance of the super type to the new list reference. I would expect this to always fail at Step 3, when the new item is added to the new list reference. It's a casted version and still backed by the original, which can't accept an instance of the supertype.

Instead, this code fails at Step 4, when it tries to print out the original list. If I remove the comments, adding a try/catch around Step 4, then it begins to fail at Step 3, which is just weird.

FWIW, this issue does not occur in a Flutter unit test. If executed in that environment, it always fails as expected at Step 3.

@matanlurey
Copy link
Contributor

matanlurey commented Jul 23, 2018

Hi @RedBrogdon!

(For those that don't know, @RedBrogdon is on the Flutter DevRel team. Welcome!)

A couple of things. Thanks for filing, and if you think docs could be improved anywhere in the future please file some issues.

var listOfSquares = <Square>[Square(1), Square(2), Square(3)];

nit: In Dart2 this is the same as:

var listOfSquares = [Square(1), Square(2), Square(3)];

It's easy to get tripped up in Dart2 if you try to manually specify types. The various style guides on the subject have been moving to being more authoritative to trust type inference. I personally recommend (re)-reading the Types section of effective Dart.

final listOfShapes = listOfSquares as List<Shape>;

This is fine because it is an upcast, a List<Square> is-a List<Shape> due to covariance.

@RedBrogdon:

I would expect this to always fail at Step 3... It's a casted version and still backed by the original

listOfShapes.add(Shape(5));

Ok, so you ran into a combination of bugs and misconceptions around Dart2.

First, Dart SDK 2.0.0-dev.63.0, which uses Dart2JS, had a bug in previous SDKs (including Dart SDK 2.0.0-dev.63.0). I've filed a bug to upgrade it, but in newer versions of Dart2 (and Flutter, which uses the Dart VM), this triggers a covariance check, at runtime, that will throw a CastError (or is it TypeError?).

The way I explained it to myself (and others) is covariance is safe for reads, not writes. That is, it's reasonable to expose a List<Square> as-a List<Shape>, assuming only reads are done (not writes, which are checked, and fail if invalid).

If Dart ever gets immutable data structures or ways to specify variance, it could help:

... so, tl;dr, in "correct" implementations of Dart2, the list .add(...) will fail at runtime.

It's a casted version and still backed by the original

Careful with this language. What a cast is in Dart2 can be a little confusing. For example:

var x = <num>[1, 2, 3];
List<int> x = y as List<int>;

... is invalid (a runtime error). To cast generic data structures, the rules are a little different:

@RedBrogdon:

Instead, this code fails at Step 4, when it tries to print out the original list:

print('4) Original List after adding a Shape to new list:');
listOfSquares.forEach((s) => s.log());

This is because function types are contravariant, which hits an error case because it is (at runtime) a List<Square> not a List<Shape>. This is harder to explain in the context of this issue, so come grab me for coffee or take a look at the issues below:

I also filed a request for more docs.

@matanlurey matanlurey added the area-documentation Prefer using 'type-documentation' and a specific area label. label Jul 23, 2018
@matanlurey
Copy link
Contributor

/cc @munificent, you might want to merge this with dart-lang/site-www#1017.

@matanlurey matanlurey changed the title Appending supertype to to list upcasted with "as" causes unexpected behavior Behavior around upcasts in Dart2 are not documented well Jul 23, 2018
@RedBrogdon
Copy link
Author

Thanks for all the detail! I've taken another look at this now that I have some time on the shuttle (and after, admittedly, refreshing myself on co- and contravariance), and I think I've got a grasp of it.

Dart's notion of covariance allows me to upcast lists and read from them:

final listOfSquares = [Square(1), Square(2), Square(3)];
final listOfShapes = listOfSquares as List<Shape>;
listOfShapes.forEach((s) => s.log());

But should stop me from doing this:

final listOfSquares = [Square(1), Square(2), Square(3)];
final listOfShapes = listOfSquares as List<Shape>;
listOfShapes.add(Shape(4));

And contravariance of function types allows me to do something like this:

final listOfSquares = [Square(1), Square(2), Square(3)];
listOfSquares.forEach((Shape s) => s.log());

What I still don't understand is this bit at the end of my original question (which I now realize I should have foregrounded, since it's the reason I posted this whole thing):

...this code fails at Step 4, when it tries to print out the original list. If I remove the comments, adding a try/catch around Step 4, then it begins to fail at Step 3, which is just weird. FWIW, this issue does not occur in a Flutter unit test.

If you run the code I posted above in DartPad, you get this output:

*** Squares -> Shapes with "as":
1) Original list of Squares:
Logging Square #1.
Logging Square #2.
Logging Square #3.
2) List of Shapes:
Logging Square #1.
Logging Square #2.
Logging Square #3.
3) List of Shapes after adding a Shape to new list:
Logging Square #1.
Logging Square #2.
Logging Square #3.
Logging Shape #5.
4) Original List after adding a Shape to new list:
Logging Square #1.
Logging Square #2.
Logging Square #3.
Uncaught exception:
TypeError: Instance of 'Shape': type 'Shape' is not a subtype of type 'Square'    

If you uncomment the four lines toward the end, wrapping the last statement in a try/catch, and re-run it, you get this:

*** Squares -> Shapes with "as":
1) Original list of Squares:
Logging Square #1.
Logging Square #2.
Logging Square #3.
2) List of Shapes:
Logging Square #1.
Logging Square #2.
Logging Square #3.
3) List of Shapes after adding a Shape to new list:
Uncaught exception:
TypeError: Instance of 'Shape': type 'Shape' is not a subtype of type 'Square'

I don't understand how uncommenting code at the end of main is causing the method to throw before the uncommented code is ever reached. What is it that I'm missing here?

@matanlurey
Copy link
Contributor

The current version of DartPad is based on a buggy version of Dart2 on Dart2JS.

I believe the VM has the correct behavior of the newer versions of Dart2JS mimic the VM.

@srawlins
Copy link
Member

Any more to do on this issue? The only thing I see remaining is perhaps an article on casting in Dart?

@srawlins srawlins added the needs-info We need additional information from the issue author (auto-closed after 14 days if no response) label Oct 18, 2019
@no-response
Copy link

no-response bot commented Apr 21, 2020

Without additional information, we are unfortunately not sure how to resolve this issue. We are therefore reluctantly going to close this bug for now. Please don't hesitate to comment on the bug if you have any more information for us; we will reopen it right away!
Thanks for your contribution.

@no-response no-response bot closed this as completed Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-documentation Prefer using 'type-documentation' and a specific area label. needs-info We need additional information from the issue author (auto-closed after 14 days if no response)
Projects
None yet
Development

No branches or pull requests

3 participants