Skip to content

Roll engine to f3ff83a5db71262d240aa5337a2a9a22c73c4749. (dart roll). #21143

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

Merged
merged 4 commits into from
Aug 29, 2018

Conversation

aam
Copy link
Member

@aam aam commented Aug 29, 2018

No description provided.

aam added 2 commits August 28, 2018 19:47
Changes since last roll:
```
5613939 Roll src/third_party/skia 7ba1d64f0706..5f0726b01019 (12 commits) (flutter#6104)
47a1ce0 Allow embedders to set the root surface transformation. (flutter#6085)
```
@aam aam requested a review from a-siva August 29, 2018 03:52
@aam aam merged commit abc1723 into flutter:master Aug 29, 2018
@aam aam deleted the roll-20180828 branch August 29, 2018 04:30
@aam
Copy link
Member Author

aam commented Aug 29, 2018

https://dart-review.googlesource.com/c/sdk/+/70160 (cc @jcollins-g ) caused analyzer warnings reported in https://github.com/flutter/flutter/runs/14496912:

   info • Prefer declare const constructors on `@immutable` classes • packages/flutter/lib/src/widgets/platform_view.dart:53:3 • prefer_const_constructors_in_immutables
   info • Prefer declare const constructors on `@immutable` classes • packages/flutter/lib/src/widgets/scroll_view.dart:53:3 • prefer_const_constructors_in_immutables
   info • Prefer declare const constructors on `@immutable` classes • packages/flutter/lib/src/widgets/single_child_scroll_view.dart:186:3 • prefer_const_constructors_in_immutables

Those were incorrect lints which required adding appropriate //ignore directives.

@devoncarew
Copy link
Member

@danrubel, looks like an issue related to the move to Fasta. Also cc'ing @a14n, who authored the lint.

From the docs here: http://dart-lang.github.io/linter/lints/prefer_const_constructors_in_immutables.html, it looks like we need an @immutable annotation on the definition of the class in order for this lint to apply. I don't see that annotation on these classes, so I'm not sure why the lint would be firing.

@devoncarew
Copy link
Member

I filed dart-lang/sdk#34297 to track this.

aam added a commit to aam/flutter that referenced this pull request Aug 29, 2018
@a14n
Copy link
Contributor

a14n commented Aug 29, 2018

I don't see that annotation on these classes, so I'm not sure why the lint would be firing.

@immutable applies to child classes. Widget (which is a parent of AndroidView) is annotated with @immutable.

@devoncarew
Copy link
Member

It looks like the lint is correct then, and we should change the constructors over to const?

@aam
Copy link
Member Author

aam commented Aug 29, 2018

It looks like the lint is correct then, and we should change the constructors over to const?

It doesn't work as there are other constraints that prevent those constructors to be const.

aam added a commit that referenced this pull request Aug 29, 2018
@devoncarew
Copy link
Member

OK, sounds like there are false positives with the lint, or issues where we don't actually want the constructor to be const even for immutable classes.

@a14n
Copy link
Contributor

a14n commented Aug 29, 2018

It doesn't work as there are other constraints that prevent those constructors to be const.

Once dart-lang/sdk#33408 has landed all constructors pointed out in the log could be const. I think we are in a transition state where CFE and Analyzer differ. /cc @bwilkerson

@a14n
Copy link
Contributor

a14n commented Aug 29, 2018

For instance:

  AndroidView({ // ignore: prefer_const_constructors_in_immutables
    Key key,
    @required this.viewType,
    this.onPlatformViewCreated,
    this.hitTestBehavior = PlatformViewHitTestBehavior.opaque,
    this.layoutDirection,
    this.gestureRecognizers = const <OneSequenceGestureRecognizer> [],
    this.creationParams,
    this.creationParamsCodec
  }) : assert(viewType != null),
       assert(hitTestBehavior != null),
       assert(gestureRecognizers != null),
       assert(creationParams == null || creationParamsCodec != null),
       super(key: key);

creationParams == null prevents const for now but will be const after the landing of dart-lang/sdk#33408

@aam
Copy link
Member Author

aam commented Aug 29, 2018

Thank you, @a14n . Makes sense.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 9, 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.

5 participants