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

[CP] [stable] Request cherry pick of commit f5080bedb38c77bd99b5caa0bc012007ab7158e3 into stable #53747

Closed
a-siva opened this issue Oct 12, 2023 · 4 comments
Closed

[CP] [stable] Request cherry pick of commit f5080bedb38c77bd99b5caa0bc012007ab7158e3 into stable #53747

a-siva opened this issue Oct 12, 2023 · 4 comments
Assignees
@vsmenon @itsjustkevin @mit-mit @athomas
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. cherry-pick-approved Label for approved cherrypick request cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. cherry-pick-review Issue that need cherry pick triage to approve merge-to-stable P1 A high priority bug; for example, a single project is unusable or has many test failures triaged Issue has been triaged by sub team

Comments

@a-siva
Copy link
Contributor

a-siva commented Oct 12, 2023 •
edited
Loading

Commit(s) to merge

f5080be

Target

stable

Prepared changelist for beta/stable

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

Issue Description

During debug, hovering a value doesn't show its value anymore.

What is the fix

Fix behaviour of ActivationFrame::EvaluateCompiledExpression when paused inside a closure

Why cherry-pick

A number of flutter users have upvoted in the comment that asks if folks would like a cherry pick of the issue into Flutter stable. It seems to affect developer productivity.

Risk

low

Issue link(s)

#53654

Extra Info

No response
@a-siva a-siva added the cherry-pick-review Issue that need cherry pick triage to approve label Oct 12, 2023
@a-siva a-siva added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. merge-to-stable triaged Issue has been triaged by sub team P1 A high priority bug; for example, a single project is unusable or has many test failures labels Oct 12, 2023
@a-siva
Copy link
Contributor Author

a-siva commented Oct 13, 2023

@itsjustkevin if you see the comments in the original issue #53654 folks would like for a hot fix to the stable channel, I hope this change can make it for the stable hot fix release next week.

@a-siva a-siva mentioned this issue Oct 13, 2023
Closed
@itsjustkevin
Copy link
Contributor

@alexmarkov could you take a look at this cherry-pick request?

@alexmarkov
Copy link
Contributor

Cherry-pick lgtm.

There was another fix which canceled this change in ActivationFrame::EvaluateCompiledExpression - https://dart-review.googlesource.com/c/sdk/+/317540. Should we cherry-pick Martin's fix as well? /cc @mkustermann

@a-siva
Copy link
Contributor Author

a-siva commented Oct 16, 2023

Cherry-pick lgtm.

There was another fix which canceled this change in ActivationFrame::EvaluateCompiledExpression - https://dart-review.googlesource.com/c/sdk/+/317540. Should we cherry-pick Martin's fix as well? /cc @mkustermann

This seems like a big change compared to the other one, not sure if it is worth the risk considering we have another stable release coming up soon.

@itsjustkevin itsjustkevin added cherry-pick-approved Label for approved cherrypick request cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. labels Oct 17, 2023
copybara-service bot pushed a commit that referenced this issue Oct 17, 2023
@a-siva
[stable] [VM/Debugger] Fix behaviour of ActivationFrame::EvaluateComp…
1ce0cc8 
…iledExpression when paused inside a closure

TEST=pkg/vm_service/test/evaluate_inside_closures_test.dart, pkg tryjob

Fixes: #52430
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/313001
Cherry-pick-request: #53747
Change-Id: Ib3479a0b39377ea20765752fd91fef26e8f46454
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330248
Reviewed-by: Derek Xu <derekx@google.com>
Commit-Queue: Kevin Chisholm <kevinjchisholm@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vsmenon vsmenon

@itsjustkevin itsjustkevin

@mit-mit mit-mit

@athomas athomas

Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. cherry-pick-approved Label for approved cherrypick request cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. cherry-pick-review Issue that need cherry pick triage to approve merge-to-stable P1 A high priority bug; for example, a single project is unusable or has many test failures triaged Issue has been triaged by sub team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@vsmenon @a-siva @itsjustkevin @mit-mit @athomas @alexmarkov