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.

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

[Patterns] VM implementation #49755

Closed
itsjustkevin opened this issue Aug 19, 2022 · 4 comments
Closed

[Patterns] VM implementation #49755

itsjustkevin opened this issue Aug 19, 2022 · 4 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. P2 A bug or feature request we're likely to work on triaged Issue has been triaged by sub team
Milestone

Comments

@itsjustkevin
Copy link
Contributor

No description provided.

@a-siva a-siva added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Aug 19, 2022
@a-siva a-siva assigned alexmarkov and unassigned a-siva Aug 19, 2022
@a-siva a-siva added the P2 A bug or feature request we're likely to work on label Aug 19, 2022
@a-siva a-siva added this to the Dart 3 beta milestone Oct 28, 2022
@a-siva a-siva added triaged Issue has been triaged by sub team and removed vm-triaged labels Dec 20, 2022
copybara-service bot pushed a commit that referenced this issue Jan 31, 2023
…scope

When lowering patterns, front-end can add multiple distinct local
variables with the same name into the same local scope.
Previously, VM identified local variables in a local scope by name.
However, this no longer works with patterns.

With this change, local variables are now identified by pair (name,
kernel offset). Name is still taken into account as compiler can add
extra variables which do not correspond to kernel variables,
such as 'this'.

TEST=runtime/tests/vm/dart/regress_51091_test.dart
Fixes #51091
Issue #49755

Change-Id: I0263769cb31f3f8d9652f5d6534800510ac882fb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279650
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
copybara-service bot pushed a commit that referenced this issue Mar 20, 2023
… of a compatible shape

Compiler can see an unreachable code like this:

v1 <- Constant(Record (C, C))
if v1 is (C, C, C) {
  v2 <- LoadField (v1.$3)
}

So it should check that record type of the receiver has enough fields
before querying field type in LoadField.

TEST=co19/LanguageFeatures/Patterns/matching_record_*

Fixes #51767
Issue #49755

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-nnbd-linux-release-x64-try
Change-Id: Iac42c8edf3d22dce0061b6d75e8bdf51b7bf88e6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/289540
Auto-Submit: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
copybara-service bot pushed a commit that referenced this issue Mar 30, 2023
…ing through patterns

TEST=pkg/vm_service/test/step_through_patterns_test.dart
Issue: #51812
Issue: #49755

Change-Id: Ieb2e6f49c613c7543e7e76119cefbc4d8f9ae3de
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291840
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
@itsjustkevin
Copy link
Contributor Author

Friendly ping @alexmarkov

@alexmarkov
Copy link
Contributor

@itsjustkevin We don't have any planned work to do, and currently we don't have any open bugs in the VM related to patterns. However, there is still a significant number of failing tests, seemingly because tests were not updated after the recent spec change (dart-lang/co19#1976). So we're waiting for co19 roll to land in Dart SDK and all VM bots to cycle in order to confirm that all tests now pass (or discover more bugs which could be hidden beyond the existing failures).

@a-siva
Copy link
Contributor

a-siva commented Apr 5, 2023

Marking this as closed, if we find issues with the new set of co19 tests we can create new issues specific to the failures.

@a-siva a-siva closed this as completed Apr 5, 2023
@alexmarkov
Copy link
Contributor

All pattern tests are now passing on the VM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. P2 A bug or feature request we're likely to work on triaged Issue has been triaged by sub team
Projects
None yet
Development

No branches or pull requests

3 participants