Skip to content

Bump Dart SDK to 1.22.0-dev.10.1. #7741

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

Closed
wants to merge 1 commit into from
Closed

Conversation

pq
Copy link
Contributor

@pq pq commented Jan 30, 2017

Fixes: #7736

Unblocks #7734 (migration to using covariant syntax).

@Hixie @abarth @tvolkert

FYI @leafpetersen for strong mode suppressions

Fixes: flutter#7736

Unblocks flutter#7734 (migration to using `covariant` syntax).
@tvolkert
Copy link
Contributor

What's the story on the strong_mode_not_instantiated_bound suppressions?

@@ -143,7 +143,7 @@ class WidgetController {
///
/// * Use [firstState] if you expect to match several states but only want the first.
/// * Use [stateList] if you expect to match several states and want all of them.
T state<T extends State>(Finder finder) {
T state<T extends State>(Finder finder) { // ignore: strong_mode_not_instantiated_bound
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a breaking change in the latest dev release to make strong mode be more conservative about guessing default type arguments when code leaves off a type argument. Here, State is defined as

State<T extends StatefulWidget>

Previously, strong mode would just guess that you meant State<StatefulWidget> when you wrote State. Writing this explicitly here will fix the error.

This error is primarily aimed at eliminating cases where we simply can't guess a reasonable answer, e.g.

class Foo<T extends Foo<T>> {}

Foo x; // What does Foo mean here?

There is some ongoing discussion as to whether to allow cases like State here where there is an answer we can guess.

@Hixie, @sethladd any thoughts about this?

Copy link
Contributor

Choose a reason for hiding this comment

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

State is marked @optionalTypeArgs. I would hope it would default to the broadest type possible (dynamic, or, in the case of a <T extends Q>, Q).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, that's good to know. In general that's what we do (that is, fill in with the bound), but we're being a bit more conservative with types used in bounds.

Would you expect to be able to use my example Foo above with no type arguments? And if so what would you expect it to mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't wrap my head around your example. I think I'd be ok with the compiler falling over when faced with that type. :-)

@pq
Copy link
Contributor Author

pq commented Jan 30, 2017

What's the story on the strong_mode_not_instantiated_bound suppressions?

My understanding is that this warning is still a work in progress. In any event @bwilkerson and I looked at a proper fix and I got shy since it would likely involve some invasive changes.

Maybe @leafpetersen can guide us to the light?

(EDIT: he already has! 😊 )

@@ -278,7 +278,7 @@ class IOSDevice extends Device {

Future<int> launch = runCommandAndStreamOutput(launchCommand, trace: true);

List<Uri> uris = await launch.then((int result) async {
List<Uri> uris = await launch.then<List<Uri>>((int result) async {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this an inference regression, or an unrelated fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah. Good catch. I should have flagged this. Regression. flutter analyze reported:

[error] Missing type arguments for generic method 'then<S>'. (packages/flutter_tools/lib/src/commands/daemon.dart, line 467, col 57)
[error] Missing type arguments for generic method 'then<S>'. (packages/flutter_tools/lib/src/ios/devices.dart, line 281, col 37)

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any chance that this is a holdover from dev.10.0? We had a regression related to async/await, but I can't reproduce this error in either dev.10.1 or bleeding edge.

I'm not testing the actual code, so it's possible there's an issue I'm not seeing, but I have a repro that should be capturing everything relevant and I don't see any issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was definitely failing flutter analyze w/ dev.10.1 , though interestingly I was not seeing the errors in IntelliJ.

Maybe @danrubel can comment on that disconnect. I haven't had time to follow-up on it...

@Hixie
Copy link
Contributor

Hixie commented Jan 30, 2017

Four new // ignores will cost us $8012 (as measured by our technical debt benchmark).

@pq
Copy link
Contributor Author

pq commented Jan 31, 2017

Thanks for all the guidance @leafpetersen !

@Hixie : do we have a next-step? It'd be great to land this so you all can start playing with covariants (right @sethladd 😉 ?) Are there any tracking bugs I can add to the ignores to provide more context for future cleanup? (Alternatively, I could link back to this PR I suppose...)

@sethladd
Copy link
Contributor

sethladd commented Jan 31, 2017

Testing covariant is not a P0 for us, but when the new SDK lands for Flutter, we'll be happy to try it out. (In other words: no need to rush the fix to the SDK just for us :)

@leafpetersen
Copy link
Contributor

Filed dart-lang/sdk#28580 to track the relaxation on the restriction on type bounds.

@goderbauer
Copy link
Member

The latest engine version requires Dart SDK 1.22.0-dev.10.1. So, this is currently blocking the engine roll (#7777), which includes fixes for Flutter on Windows.

What is the path forward on bumping the Dart SDK?

@Hixie
Copy link
Contributor

Hixie commented Feb 1, 2017

Closing this in favour of #7791.

@Hixie Hixie closed this Feb 1, 2017
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Roll Dart SDK to 1.22.0-dev.10 (so we can format code with covariant)
7 participants