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

Positional fields for Records should be indexed at 1 not 0 #51451

Closed
elliette opened this issue Feb 17, 2023 · 3 comments
Closed

Positional fields for Records should be indexed at 1 not 0 #51451

elliette opened this issue Feb 17, 2023 · 3 comments
Assignees
@derekxu16
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 vm-service The VM Service Protocol, both the specification and its implementation
Milestone
Dart 3 beta 2

Comments

@elliette
Copy link
Contributor

This would conform with the recent spec update to Records: https://github.com/dart-lang/language/blob/master/accepted/future-releases/records/records-feature-specification.md#120

FYI @derekxu16 @bkonyi
@elliette elliette added the vm-service The VM Service Protocol, both the specification and its implementation label Feb 17, 2023
@mraleph mraleph added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Feb 18, 2023
@a-siva a-siva added this to the Dart 3 beta 2 milestone Feb 20, 2023
@a-siva a-siva added the P2 A bug or feature request we're likely to work on label Feb 20, 2023
@stevemessick stevemessick mentioned this issue Feb 22, 2023
Closed
@mit-mit
Copy link
Member

mit-mit commented Feb 23, 2023

I think this should be a P1 for beta 2 given it's such a big API change

@derekxu16
Copy link
Member

The change will probably land today.

https://dart-review.googlesource.com/c/sdk/+/284722

copybara-service bot pushed a commit that referenced this issue Feb 23, 2023
@derekxu16
Revert "[VM/Service] Start indexing positional record fields at 1"
c1f3a5a 
This reverts commit 1be893c.

Reason for revert: Broke pkg/dds/test/dap/integration/debug_variables_test

Original change's description:
> [VM/Service] Start indexing positional record fields at 1
>
> TEST=CI
>
> Fixes: #51451
> Change-Id: I0e00e0ffb35aeb40affbbd5544542723bafc747c
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284722
> Reviewed-by: Ben Konyi <bkonyi@google.com>
> Commit-Queue: Derek Xu <derekx@google.com>

Change-Id: Ib82f3fa8ca970eb4accb8f6a86b9ce36e72d9bc8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284861
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Derek Xu <derekx@google.com>
@derekxu16
Copy link
Member

about to be relanded: https://dart-review.googlesource.com/c/sdk/+/284862

copybara-service bot pushed a commit that referenced this issue Feb 23, 2023
@derekxu16
Reland "[VM/Service] Start indexing positional record fields at 1"
c0c5231 
This is a reland of commit 1be893c

Patchset 2 fixes the failure that was in
pkg/dds/test/dap/integration/debug_variables_test.

TEST=CI

Original change's description:
> [VM/Service] Start indexing positional record fields at 1
>
> TEST=CI
>
> Fixes: #51451
> Change-Id: I0e00e0ffb35aeb40affbbd5544542723bafc747c
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284722
> Reviewed-by: Ben Konyi <bkonyi@google.com>
> Commit-Queue: Derek Xu <derekx@google.com>

Change-Id: I4cdee610646b3389d7d02ee6d905a66ae6e0329d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/284862
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@derekxu16 derekxu16

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 vm-service The VM Service Protocol, both the specification and its implementation
Projects
None yet
Milestone
Dart 3 beta 2
Development

No branches or pull requests
5 participants
@mraleph @derekxu16 @a-siva @mit-mit @elliette