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

Review breakpoint type resolving scope #56301

Open
FMorschel opened this issue Jul 22, 2024 · 14 comments
Open

Review breakpoint type resolving scope #56301

FMorschel opened this issue Jul 22, 2024 · 14 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. P3 A lower priority bug or feature request triaged Issue has been triaged by sub team type-enhancement A request for a change that isn't a bug vm-debugger

Comments

@FMorschel
Copy link
Contributor

FMorschel commented Jul 22, 2024

Hello. Please consider #56300.

Summary: The is operator in Dart evaluates to true when checking against an unresolved identifier, instead of throwing an error. This behavior is observed in Flutter, but not in standalone Dart, leading to unexpected results in conditional breakpoints and expression evaluation.

Now, if this ends up being fixed as non-Flutter Dart, for every type that is not in scope:

For non-Flutter dart, you get a reasonable error:

image

I'd like to question here if there could be a way for breakpoint expressions to "ignore" that scope.

Use case:

I have an extension of FocusNode, wich is a Flutter (third party/package) class. But as an extended class, some of the logic is done inside Flutter files and they have no way of knowing about my subclass. I'd like to place a breakpoint there and only stop there when my class stops there so I can see if I made anything wrong or I am missing anything. But in my case, all FocusNode/FocusScopes in my screen stop there and that makes me have to skip a lot of breaks and hope for me to not missclick on the right one.

In this specific case, I can of course do a simple empty wrapper on the exact method I'd like to see and break point there and go inward to see whats going on. Well, two more things here:

  • If the method I'd like to see is private (which in this case I know there are some - and made an issue about that already), I'm not able to do that.
  • If I'd like to test the behaviour inside a Flutter Widget (third party interacting class), I'm not able to do so as well since I would then need to basically rewrite all of the inner workings of TextFormField, for example, (which has a lot going on) just to get to the Focus widget State (aka FocusState).
@FMorschel FMorschel changed the title Create an issue Review breakpoint type resolving scope Jul 22, 2024
@dart-github-bot
Copy link
Collaborator

Summary: The is operator in Dart incorrectly evaluates to true when checking against an unresolved identifier in Flutter, leading to unexpected behavior in conditional breakpoints and expression evaluation. This issue is not observed in standalone Dart. The user requests a way for breakpoint expressions to ignore scope, allowing them to debug code within extended classes without being interrupted by unrelated breakpoints.

@dart-github-bot dart-github-bot added area-front-end Use area-front-end for front end / CFE / kernel format related issues. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-enhancement A request for a change that isn't a bug labels Jul 22, 2024
@DanTup
Copy link
Collaborator

DanTup commented Jul 22, 2024

What makes this complicated is that if the type is not in-scope, it could be ambiguous (for example you could have two class Foos in your project, neither of which are in-scope.. which one should be used?).

C# has something where you can use global::Namespace.Foo to reference Foo in Namespace even if it's not "imported" (although I don't know if you can use it in debugger evaluation expressions), but AFAIK there's nothing similar in Dart.

I think the VM Service's evaluate methods may also allow us to pass additional names to have in-scope, but I don't know a) if it would work here or b) how the debugger/editor would be able to build scopes to satisfy this (we can't parse the expression so we don't really know that it has type names in, even if we had a way to search for them).

@FMorschel
Copy link
Contributor Author

Maybe if we could indicate what it should import in some way?

@johnniwinther johnniwinther added the cfe-expression-compilation Issues related to expression compilation in the CFE label Jul 23, 2024
@johnniwinther
Copy link
Member

cc @jensjoha

@lrhn lrhn added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. vm-debugger and removed area-front-end Use area-front-end for front end / CFE / kernel format related issues. cfe-expression-compilation Issues related to expression compilation in the CFE triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. labels Jul 23, 2024
@a-siva
Copy link
Contributor

a-siva commented Jul 24, 2024

Not sure if this is a VM issue, as indicated in comment-2243272766 the client tool would have to build scopes appropriately.

@a-siva a-siva added triaged Issue has been triaged by sub team P3 A lower priority bug or feature request labels Jul 24, 2024
@bkonyi
Copy link
Contributor

bkonyi commented Jul 25, 2024

@DanTup, if you make use of the evaluteInFrame RPC, the type should be in scope without having to explicitly provide it.

@DanTup
Copy link
Collaborator

DanTup commented Jul 25, 2024

@bkonyi we do use evaluateInFrame, but the issue here is that the class used in the expression is not in-scope.

The issue here is not a bug, but a request to try and improve the situation where you have a variable of some type and want to do an is check (for example in a conditional breakpoint), but there's nothing you can write for the type that will work, because it isn't imported and in-scope.

I've hit this myself before, and often end up rewriting the expression to be .runtimeType.name == 'Foo', but it's not equivalent so isn't always possible. As far as I know, there's no expression we can write to reference a type that's not in scope (for example like .NET's global::Namespace.Type).

I suspect for the huge majority of cases, "the first class found anywhere with the name Foo" would do just what the user wanted, but ofc that's not very sound (or understandable to the user).

I'm not sure (without having some way to reference a type that's not in scope) there's a lot we can do here, but maybe others have ideas? 🤔

@bkonyi
Copy link
Contributor

bkonyi commented Jul 25, 2024

@DanTup can you share some sample code to help me better understand the context?

@DanTup
Copy link
Collaborator

DanTup commented Jul 26, 2024

@bkonyi this is a bit contrived, but imagine this code:

void main() {
  foo(<String>{});
}

void foo(Set s) {
  print(s); // Conditional breakpoint here "s is LinkedHashSet"
}

If I add a breakpoint to the marked line with the condition s is LinkedHashSet, it doesn't work because LinkedHashSet isn't in scope (even though it is the type of the variable):

image

While the error is clear (at least for Dart - #56300 makes this really confusing for Flutter, because it just silently doesn't trigger), besides adding an unused import to your code and reloading, there's not a clear way to make the breakpoint trigger when you want.

I don't know what a good fix it, but it would be nice if there was a way to fix this, by being able to write an expression like s is LinkedHashSet (and have some way to specify where LinkedHashSet comes from) without having to add an unnecessary import and reload the app.

@bkonyi
Copy link
Contributor

bkonyi commented Jul 26, 2024

Is this a problem if the declaration or import is present in the file? Or is this only a problem with types defined in dart:core?

@FMorschel
Copy link
Contributor Author

FMorschel commented Jul 26, 2024

Or is this only a problem with types defined in dart:core?

It happens with any type that the declaration is unknown.

Is this a problem if the declaration or import is present in the file?

No. When this happens, it does work.

The following example works for both B and C because their type declarations are in the scope of the breakpoint (present in file like B or imported like C).

File bug.dart:

import 'c.dart';

void main() {
  foo(<String>{});
  bar(B());
  bar(C());
}

void foo(Set s) {
  print(s); // Conditional breakpoint here "s is LinkedHashSet"
}

void bar(A a) {
  print(a); // Conditional breakpoint here "a is B" or "a is C"
}

class A {}

class B extends A {}

File c.dart:

import 'bug.dart';

class C extends A {}

@FMorschel
Copy link
Contributor Author

FMorschel commented Jul 26, 2024

Just to be clear, about DanTup comment above:

While the error is clear (at least for Dart - #56300 makes this really confusing for Flutter, because it just silently doesn't trigger), [...]

It should be:

For Flutter, currently (until #56300 is resolved) it silently (does) triggers.

Which is really weird, it means that for the current state, it behaves strangely.


I'll explain this with a Flutter case but any third party package would fit here.

Lets say Flutter has a type B and a type D that extends/implements B.

If I'm debugging my Flutter project and I've created some class C for example, extending B because some Flutter widget accepts any B.

Now I go inside that widget declaration to see its current behaviour with my new class instance (that is C) and do this test (foo is C), it sounds to me like every instance of the type the widget accepts (B) is an instance of my new class C. Even when the framework itself passes a D instance somewhere else.

@DanTup
Copy link
Collaborator

DanTup commented Jul 26, 2024

@bkonyi

Is this a problem if the declaration or import is present in the file? Or is this only a problem with types defined in dart:core?

My example may have been slightly misleading - LinkedHashSet is not in dart:core but in dart:collection, so that name is not in-scope for this file/frame because there's no import for it. In any case where you have a variable of type X but X is not imported and in-scope causes the issue (and any time the type is in scope, it's fine).

@FMorschel

For Flutter, currently (until #56300 is resolved) it silently (does) triggers.

Sorry yes, I got it the wrong way around. That bug results in the condition returning true even when it's not, and that is more confusing than the opposite, because it pauses the debugger and you assume the variable is of the type you wrote, but it might not be. Hopefully that's an easy fix though, whereas this request seems a little tricker 😄

@FMorschel
Copy link
Contributor Author

Maybe if we could indicate what it should import in some way?

Like the following:

image

Currently, it gives an error:

Debugger failed to evaluate breakpoint condition "import 'dart:collection'; s is LinkedHashSet": evaluateInFrame: (113) Expression compilation error
org-dartlang-debug:synthetic_debug_expression:1:1: Error: Undefined name 'import'.
import 'dart:collection'; s is LinkedHashSet
^^^^^^
org-dartlang-debug:synthetic_debug_expression:1:8: Error: Expected one expression, but found additional input.
import 'dart:collection'; s is LinkedHashSet
       ^^^^^^^^^^^^^^^^^

It is not the best possible option, but it would at least make sure that we could run these evaluation tests in breakpoints and such.

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. P3 A lower priority bug or feature request triaged Issue has been triaged by sub team type-enhancement A request for a change that isn't a bug vm-debugger
Projects
None yet
Development

No branches or pull requests

7 participants