-
Notifications
You must be signed in to change notification settings - Fork 29
#3203. Add VM - CFE interaction tests for primary initializer scope #3359
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| // Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file | ||
| // for details. All rights reserved. Use of this source code is governed by a | ||
| // BSD-style license that can be found in the LICENSE file. | ||
|
|
||
| /// @assertion A declaring constructor declaration is a declaration that | ||
| /// contains a `<declaringConstructorSignature>` with a | ||
| /// `<declaringParameterList>`, or a declaration that contains a | ||
| /// `<declaringConstantConstructorSignature>`, or it is a | ||
| /// `<primaryConstructorNoConst>` in the header of a class, enum, or extension | ||
| /// type declaration, together with a declaration in the body that contains a | ||
| /// `<declaringConstructorSignature>`. | ||
| /// | ||
| /// @description Check that members initialized in primary initializer scope can | ||
| /// be debugged. | ||
| /// @author [email protected] | ||
|
|
||
| // SharedOptions=--enable-experiment=declaring-constructors | ||
|
|
||
| import 'dart:developer'; | ||
| import 'package:vm_service/vm_service.dart'; | ||
|
|
||
| import '../../../../pkg/vm_service/test/common/service_test_common.dart'; | ||
| import '../../../../pkg/vm_service/test/common/test_helper.dart'; | ||
| import '../Utils/expect.dart'; | ||
|
|
||
| const String shortFile = 'declaring_constructors_t03.dart'; | ||
|
|
||
| // AUTOGENERATED START | ||
| // | ||
| // Update these constants by running: | ||
| // | ||
| // dart pkg/vm_service/test/update_line_numbers.dart tests/co19/src/VM/declaring_constructors_t02.dart | ||
| // | ||
| const LINE_A = 41; | ||
| const LINE_B = 44; | ||
| const LINE_C = 48; | ||
| const LINE_D = 49; | ||
| const LINE_E = 50; | ||
| // AUTOGENERATED END | ||
|
|
||
| class C1(var String x); // LINE_A | ||
|
|
||
| class C2 { | ||
| this(final String x); // LINE_B | ||
| } | ||
|
|
||
| void testeeMain() { | ||
| debugger(); // LINE_C | ||
| C1("xxx"); // LINE_D | ||
| C2("yyy"); // LINE_E | ||
| } | ||
|
|
||
| final tests = <IsolateTest>[ | ||
| hasStoppedAtBreakpoint, | ||
| stoppedAtLine(LINE_C), | ||
| stepInto, | ||
| stoppedAtLine(LINE_D), | ||
| stepInto, | ||
| stoppedAtLine(LINE_A), | ||
| (VmService service, IsolateRef isolateRef) async { | ||
| final isolateId = isolateRef.id!; | ||
| final xRef1 = | ||
| await service.evaluateInFrame(isolateId, 0, 'x') as InstanceRef; | ||
| Expect.equals("null", xRef1.valueAsString); | ||
| final xRef2 = | ||
| await service.evaluateInFrame(isolateId, 0, 'this.x') as InstanceRef; | ||
| Expect.equals("null", xRef2.valueAsString); | ||
| }, | ||
| stepInto, | ||
| (VmService service, IsolateRef isolateRef) async { | ||
| final isolateId = isolateRef.id!; | ||
| final xRef1 = | ||
| await service.evaluateInFrame(isolateId, 0, 'x') as InstanceRef; | ||
| Expect.equals('xxx', xRef1.valueAsString); | ||
| final xRef2 = | ||
| await service.evaluateInFrame(isolateId, 0, 'this.x') as InstanceRef; | ||
| Expect.equals('xxx', xRef2.valueAsString); | ||
| }, | ||
| stepInto, | ||
| stoppedAtLine(LINE_E), | ||
| stepInto, | ||
| stoppedAtLine(LINE_B), | ||
| (VmService service, IsolateRef isolateRef) async { | ||
| final isolateId = isolateRef.id!; | ||
| final xRef1 = | ||
| await service.evaluateInFrame(isolateId, 0, 'x') as InstanceRef; | ||
| Expect.equals("null", xRef1.valueAsString); | ||
| final xRef2 = | ||
| await service.evaluateInFrame(isolateId, 0, 'this.x') as InstanceRef; | ||
| Expect.equals("null", xRef2.valueAsString); | ||
| }, | ||
| stepInto, | ||
| (VmService service, IsolateRef isolateRef) async { | ||
| final isolateId = isolateRef.id!; | ||
| final xRef1 = | ||
| await service.evaluateInFrame(isolateId, 0, 'x') as InstanceRef; | ||
| Expect.equals('yyy', xRef1.valueAsString); | ||
| final xRef2 = | ||
| await service.evaluateInFrame(isolateId, 0, 'this.x') as InstanceRef; | ||
| Expect.equals('yyy', xRef2.valueAsString); | ||
| }, | ||
| ]; | ||
|
|
||
| void main([args = const <String>[]]) => runIsolateTests( | ||
| args, | ||
| tests, | ||
| 'declaring_constructors_t02.dart', | ||
| pauseOnExit: true, | ||
| testeeConcurrent: testeeMain, | ||
| ); | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may be overlooking something but is this testing what is in scope when paused at the primary constructor (
LINE_A)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I'm afraid not. Seems that you are expecting the test like the below (example is written without primary constructors):
Please confirm that it's what you'd expect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting! In
LINE_Awe have the constructor declaration, with no initializer list and no constructor body (but we could have those, too), andxin the initializer list would denote the formal parameter whereasxin the constructor body would denote the instance variable. I think this means thatLINE_Ais simply too ambiguous to answer questions like "do we have access tox?". The parameter and the instance variable may have different values (for instance, withC(String x): assert(x != ""), x = x { x = ""; }, the instance variable does not have a value until the second element in the initializer list has been executed, and it gets a new value in the body.It would be possible to make the location in code more fine grained by inserting line breaks, e.g.:
At the comment, the formal parameter
xis not in scope, and the instance variablexmay or may not have been initialized (depends on whether we're before or afterthis.xin that line).However, it certainly makes sense to show the value of a formal parameter at the time where it is used (in any way, including the case where it's used to initialize an instance variable as it is here).
I think this implies that "what's in scope?" is a (slightly) wrong question to ask when it is decided what to show in a debugger, which is probably also reflected by the implemented behavior.
@nshahan, WDYT? Is there a concept which is similar to the language-level notion of a scope which is used for this purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sgrekhov
That looks like what I thought the description comment at the top of the file was saying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eernstg
I'm trying to ask what values we think would be useful to provide to users when stopped at a breakpoint and what names would be used to access those values.
I don't have a proper name for this concept but "scope" comes close because it largely overlaps when stopped at breakpoints at most locations.
I realize now that the language does define scope but not breakable points so there are times when the defined scope isn't always enough to define what values should be available in the debugger when stopped at a breakpoint.
My original questions about breakpoints at primary constructor lines are all really asking the following:
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that would be meaningful and useful. Also, a breakpoint on the body part of a header constructor should probably give rise to the same behavior, and there may other reasons to stop on that line even in the case where there's no code to run in order to "execute" the parameters.
Here, I'd expect a breakpoint on the
thisline to allow me to step into the evaluation offooas well as the invocation ofprint. Presumably a breakpoint onA()would do the same (because it allows me to single step through the code associated with this constructor invocation even if there's no code associated withA()itself).You could provide support for evaluating variables in scope using their declared name. An expression like
this.xcould be used to evaluate an instance variable which is declared in the enclosing class or inherited, even in the case where the instance variable is shadowed by some other declaration (such as a formal parameter).This would allow the developer to see the value of some variables whose value is not available to the code in the program at the current breakpoint, but it would be useful and meaningful to be able to access them even in the case where there is no value (because the variable hasn't been initialized yet), and also in the case where a declaration exists but is shadowed.
Is there code that implements the side effect associated with passing an actual argument to a formal parameter which is initializing (e.g.,
this.x) or declaring (e.g.,final int x)? The initialization of variables based on these syntactic forms might be considered to be part of the initializer list, in which case it's fine. Otherwise I'd expect to stop before the parameter-based initialization code as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nshahan, I think this thread is clarifying a broader set of topics that aren't blocking for this test to land. Do you agree?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, except it is unclear to me what properties of the stepping and debugging experience this test attempts to assert. I'm probably just getting lost in the details so don't consider it blocking.