Skip to content

Roll to Dart SDK 1.20.0-dev.5.0. #5911

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.

Sign up for GitHub

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

Jump to bottom
Merged
pq merged 2 commits into from
Sep 16, 2016
Merged

Roll to Dart SDK 1.20.0-dev.5.0. #5911

pq merged 2 commits into from
Sep 16, 2016

Conversation

@pq
Copy link
Contributor

@pq pq commented Sep 16, 2016

  • bumps to 1.20.0-dev.5.0.
  • updates analyzer API use.

@pq
Roll to Dart SDK 1.20.0-dev.5.0.
df18add 
* bumps to `1.20.0-dev.5.0`.
* updates `analyzer` API use.
pq
pq commented Sep 16, 2016
View reviewed changes
packages/flutter_tools/lib/executable.dart
..addCommand(new UpgradeCommand());

return Chain.capture(() async {
return Chain.capture/*<Future<Null>>*/(() async {
Copy link
Contributor Author

@pq pq Sep 16, 2016

Choose a reason for hiding this comment

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

@leafpetersen, @jmesserly: needing this seems odd. Shouldn't inferencing make it unnecessary?

Copy link
Contributor

@leafpetersen leafpetersen Sep 16, 2016

Choose a reason for hiding this comment

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

Possibly because the callback has no return, and hence we can't infer a return type for it? That would make the callback typed to return Future<dynamic>, which would essentially override the context type of Future<Null>. We've been considering ways to prefer the context type when available, but not sure what we're going to do there yet.

Copy link
Contributor Author

@pq pq Sep 16, 2016

Choose a reason for hiding this comment

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

Cool. Thanks. @Hixie : you OK with this as is?

@pq
Copy link
Contributor Author

pq commented Sep 16, 2016

This one needs a few fixes before it can land but wanted to get it up for folks to look at.

[error] Could not infer type parameter T, RenderObject must be of type T. (packages/flutter_test/lib/src/controller.dart, line 222, col 30)
[error] Could not infer type parameter T, Widget must be of type T. (packages/flutter_test/lib/src/controller.dart, line 77, col 30)

/cc @Hixie , @leafpetersen, @jmesserly for input.

@leafpetersen
Copy link
Contributor

leafpetersen commented Sep 16, 2016

In this code (the other one is analogous):

 Iterable<Widget/*=T*/> widgetList/*<T extends Widget>*/(Finder finder) {
    TestAsyncUtils.guardSync();
    return finder.evaluate().map((Element element) => element.widget);
  }

I believe that element.widget is of type Widget (I haven't tracked this down entirely, so correct me if I'm wrong). The strong mode contract that is written there promises to return an Iterable<T>, where T is some arbitrary subtype of Widget. Inference is failing here, since it can't find a choice which satisfies the contract. You can explicitly fill in the type for it:

    return finder.evaluate().map/*<T>*/((Element element) => element.widget);

or put an explicit check:

    return finder.evaluate().map((Element element) => element.widget as dynamic /*=T*/);

or put an implicit check, but since Dart doesn't (yet) have a way to write return types on lambdas, you need to name the function to do that:

    /*=T*/ reader(Element element) => element.widget
    return finder.evaluate().map(reader);

@pq
Copy link
Contributor Author

pq commented Sep 16, 2016

Thanks @leafpetersen!

I believe that element.widget is of type Widget (I haven't tracked this down entirely, so correct me if I'm wrong).

Nope. That's right!

I'm leaning towards your first suggestion of explicit typing and will update the PR.

Cheers!

@pq
Copy link
Contributor Author

pq commented Sep 16, 2016

Build is clean now. @Hixie PTAL at the code contortions and let me know what you think...

@abarth
Copy link
Contributor

abarth commented Sep 16, 2016

LGTM

@pq
Copy link
Contributor Author

pq commented Sep 16, 2016

Thanks @abarth!

@Hixie : feel free to nudge me if you want to follow up with any tweaks to the type error solutions I picked. 👍

@pq pq merged commit 2d4d171 into flutter:master Sep 16, 2016
@Hixie
Copy link
Contributor

Hixie commented Sep 17, 2016

LGTM. In general I'd like the code to not rely on inference at all. The idea is to use inference to verify that the code is correct.

@pq pq deleted the 1.20.0-dev.5.0 branch January 6, 2017 20:51
@rmistry rmistry mentioned this pull request Jul 31, 2018
Closed
rmistry pushed a commit to rmistry/flutter that referenced this pull request Jul 31, 2018
Roll engine af855e0..91ff524 (15 commits)
f7a50b4 
flutter/engine@af855e0...91ff524

git log af855e0..91ff524 --date=short --no-merges --format='%%ad %%ae %%s'
2018-07-31 37626415&flutter#43;skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 74c6ed3d1f17..1935aa3d27cd (11 commits) (flutter#5911)
2018-07-31 jamesderlin@users.noreply.github.com Deprecate -[FlutterDartProject initFromDefaultSourceForConfiguration] (flutter#18886) (flutter#5858)
2018-07-31 37626415&flutter#43;skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 0bf5408286ea..74c6ed3d1f17 (1 commits) (flutter#5910)
2018-07-30 37626415&flutter#43;skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 946a162a5239..0bf5408286ea (1 commits) (flutter#5909)
2018-07-30 jonahwilliams@google.com Add word movement granularities to android (flutter#5902)
2018-07-30 37626415&flutter#43;skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia f7dcd76c552a..946a162a5239 (3 commits) (flutter#5908)
2018-07-30 nathanrogers@google.com Remove the &flutter#34;VSYNC&flutter#34; trace event on Fuchsia (flutter#5907)
2018-07-30 37626415&flutter#43;skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 62501c12c83e..f7dcd76c552a (5 commits) (flutter#5906)
2018-07-30 liyuqian@google.com Replace travis with cirrus (flutter#5899)
2018-07-30 liyuqian@google.com Use container and GKE to accelerate build (flutter#5897)
2018-07-30 liyuqian@google.com Add docker container generation scripts (flutter#5898)
2018-07-30 37626415&flutter#43;skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia c06b8a4e0ca3..62501c12c83e (11 commits) (flutter#5905)
2018-07-30 jason-simmons@users.noreply.github.com Clear the EGL context only if our context is currently active (flutter#5895)
2018-07-30 jason-simmons@users.noreply.github.com Return proper JSON-RPC error responses from service protocol failures (flutter#5889)
2018-07-30 37626415&flutter#43;skia-flutter-autoroll@users.noreply.github.com Roll src/third_party/skia 227424b57d28..c06b8a4e0ca3 (22 commits) (flutter#5904)

The AutoRoll server is located here: http://localhost:8000

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/&#43;/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC&flutter#39;d on the roll, and stop the roller if necessary.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@leafpetersen leafpetersen leafpetersen left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pq @leafpetersen @abarth @Hixie