Skip to content

[vm] Implement function canonicalization corner cases #46487

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
Closed
eernstg opened this issue Jun 28, 2021 · 4 comments
Closed

[vm] Implement function canonicalization corner cases #46487

eernstg opened this issue Jun 28, 2021 · 4 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@eernstg
Copy link
Member

eernstg commented Jun 28, 2021

This is the VM specific issue for #46485, which has the details.
@eernstg eernstg added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Jun 28, 2021
@dcharkes
Copy link
Contributor

dcharkes commented Jun 29, 2021 •
edited
Loading

I have approved VM failures on this issue.

language/generic_methods/explicit_instantiated_tearoff_test: https://dart-ci.appspot.com/log/vm-kernel-precomp-nnbd-mac-release-arm64/dartkp-strong-mac-release-arm64/2/language/generic_methods/explicit_instantiated_tearoff_test

language/closure/identity_equality_tearoff_test:
https://logs.chromium.org/logs/dart/buildbucket/cr-buildbucket.appspot.com/8843117657258310224/+/u/test_results/new_test_failures__logs_

@eernstg
Copy link
Member Author

eernstg commented Jun 29, 2021 •
edited
Loading

Thanks, @dcharkes!

Checking language/generic_methods/explicit_instantiated_tearoff_test.dart, I can see that these are cases using the new explicit generic instantiation of functions, and the expectations are in line with the newly specified behavior. So fixing this issue might well cause language/generic_methods/explicit_instantiated_tearoff_test.dart to succeed as well.

Regarding language/generic_methods/explicit_instantiated_tearoff_test.dart: I'm not sure why that one would need approval, I just approved it when I landed it yesterday, and the 'Results' page did not show that test as needing any further approvals when I checked. But maybe the approval was needed for a couple of configurations that weren't covered by the trybots, and you just did it before I saw the problem. Thanks again! ;-)

@leafpetersen
Copy link
Member

I believe this changed is gated on some further discussion, see here.

dart-bot pushed a commit that referenced this issue Aug 4, 2021
@alexmarkov
[vm] Fix equality of implicit static closures
f9c8f3c 
There could be multiple instances of implicit static closures due to
instantiations at run time:

void foo<T>(T x) {}

void bar<T>() {
  void Function(T) myfoo1 = foo;
  void Function(T) myfoo2 = foo;
  print(myfoo1 == myfoo2);
}

This change fixes equality to handle this case.

TEST=language/generic_methods/explicit_instantiated_tearoff_test
TEST=language/constructor/tear_off_test

Issue #46231
Issue #46487

Change-Id: I485acc5444d19860ef4d8ebeec2e540fe57776d6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/208981
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
@eernstg eernstg mentioned this issue Sep 3, 2021
Closed
2 tasks
@alexmarkov
Copy link
Contributor

Both language/generic_methods/explicit_instantiated_tearoff_test and language/closure/identity_equality_tearoff_test are currently passing in all configurations, so tentatively closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@dcharkes @leafpetersen @eernstg @alexmarkov