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.

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

TypeEnvironment.forInElementType doesn't handle type parameters #39565

Closed
sjindel-google opened this issue Nov 28, 2019 · 1 comment
Closed

TypeEnvironment.forInElementType doesn't handle type parameters #39565

sjindel-google opened this issue Nov 28, 2019 · 1 comment
Assignees
@johnniwinther
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues.

Comments

@sjindel-google
Copy link
Contributor

sjindel-google commented Nov 28, 2019 •
edited
Loading

The following code breaks TypeEnvironment.forInElementType because it assumes that the static type of the iterable expression should be an interface type (by passing it into getTypeAsInstanceOf):

void test<T extends Iterable<T>>(T x) {
  for (var y in x) {
    // ...
  }
}

/cc @mkustermann @stefantsov
@sjindel-google sjindel-google added the area-front-end Use area-front-end for front end / CFE / kernel format related issues. label Nov 28, 2019
@sjindel-google
Copy link
Contributor Author

Should we maybe use getStaticTypeAsInstanceOf instead?

dart-bot pushed a commit that referenced this issue Nov 29, 2019
@sjindel-google
[vm/cfe] Elaborate for-in statements during async transform
63333de 
This allows us TFA to analyze the iterator calls and we generate much tighter
code in AOT.

However, due to the increased inlining opportunities, we end up emitting 0.5%
more code. Inlining of the _GrowableList iterator specifically also includes the
concurrent modification check and error handling.

Calls to get:iterator, moveNext and get:current account for 7.12% of all InstanceCall
instructions in Flutter Gallery.

Fixes #39516
Issue #39566
Issue #39565

Change-Id: I8dcc08b7571137e869a16ceea8cc73539eb02a5a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/126381
Commit-Queue: Samir Jindel <sjindel@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
dart-bot pushed a commit that referenced this issue Dec 3, 2019
Revert "[vm/cfe] Elaborate for-in statements during async transform"
89e3106 
This reverts commit 63333de.

Reason for revert: Causes failures on Flutter HHH CI in the "flutter test hostonly_devicelab_tests" step due to kernel binary format version mismatch.

Original change's description:
> [vm/cfe] Elaborate for-in statements during async transform
> 
> This allows us TFA to analyze the iterator calls and we generate much tighter
> code in AOT.
> 
> However, due to the increased inlining opportunities, we end up emitting 0.5%
> more code. Inlining of the _GrowableList iterator specifically also includes the
> concurrent modification check and error handling.
> 
> Calls to get:iterator, moveNext and get:current account for 7.12% of all InstanceCall
> instructions in Flutter Gallery.
> 
> Fixes #39516
> Issue #39566
> Issue #39565
> 
> Change-Id: I8dcc08b7571137e869a16ceea8cc73539eb02a5a
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/126381
> Commit-Queue: Samir Jindel <sjindel@google.com>
> Reviewed-by: Martin Kustermann <kustermann@google.com>

TBR=kustermann@google.com,sjindel@google.com,johnniwinther@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I89b88c3d9f7c743fc340ee73a45c3f57059bcf30
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/126734
Reviewed-by: Clement Skau <cskau@google.com>
Reviewed-by: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Clement Skau <cskau@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@johnniwinther johnniwinther

Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@johnniwinther @sjindel-google