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

Evaluations should work for runtimeType #59711

Open
FMorschel opened this issue Dec 12, 2024 · 7 comments
Open

Evaluations should work for runtimeType #59711

FMorschel opened this issue Dec 12, 2024 · 7 comments
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. cfe-expression-compilation Issues related to expression compilation in the CFE triaged Issue has been triaged by sub team

Comments

@FMorschel
Copy link
Contributor

Say you have

class A {}

class B extends A {
  int foo() => 0;
}

class C {
  C(this.a);
  A a;
}

void main() {
  var c = C(B());  // Breakpoint here 
}

Now you try to evaluate c.a.foo() it gives you an error message about foo not being defined for A. But if you do (c.a as dynamic).foo() everything works normally and it can result in 0 here.

Could the evaluations do something like this internally?

// CC @DanTup

@FMorschel FMorschel added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Dec 12, 2024
@mraleph
Copy link
Member

mraleph commented Dec 13, 2024

Right now evaluated expression are compiled as Dart code. So you can only do things which are expressible in Dart.

We used to compile expressions without preserving static types (e.g. c would be dynamic and consequently c.a would be dynamic as well), but that hits a problem that any features which rely on static type and type inference (e.g. extensions and extension types or generics) don't work as one would expect.

Making c.a.foo work reliably across all possibilities requires some sophisticated machinery because it requires parser to somehow be able to introspect runtime state.

@a-siva a-siva added the area-front-end Use area-front-end for front end / CFE / kernel format related issues. label Dec 18, 2024
@a-siva
Copy link
Contributor

a-siva commented Dec 18, 2024

I am not sure if it is a good idea for the VM or CFE to evaluate the expression differently from how it would behave when the code is normally run.

@a-siva a-siva added the triaged Issue has been triaged by sub team label Dec 18, 2024
@FMorschel
Copy link
Contributor Author

The problem currently is that the expression is not running normally, but actually giving out an error. While on code it does run just fine.

@a-siva
Copy link
Contributor

a-siva commented Dec 18, 2024

I modified your code as follows

class A {}

class B extends A {
  int foo() => 0;
}

class C {
  C(this.a);
  A a;
}

void main() {
  var c = C(B());  // Breakpoint here 
  print(c.a.foo());
}

and tried running it, this is what I get


../../../tmp/test.dart:14:13: Error: The method 'foo' isn't defined for the class 'A'.
 - 'A' is from '../../../tmp/test.dart'.
Try correcting the name to the name of an existing method, or defining a method named 'foo'.
  print(c.a.foo());

@FMorschel
Copy link
Contributor Author

Yeah. The code where I saw the example was inside one of the files for the analyzer and it was tested for the type with if case.

It also shows different getters in the evaluation hover and such. That is why I thought it should work.

@a-siva
Copy link
Contributor

a-siva commented Dec 18, 2024

Yeah. The code where I saw the example was inside one of the files for the analyzer and it was tested for the type with if case.

It also shows different getters in the evaluation hover and such. That is why I thought it should work.

The expression evaluator does not have the context of the type testing that the code you were running has.

@johnniwinther johnniwinther added the cfe-expression-compilation Issues related to expression compilation in the CFE label Dec 19, 2024
@DanTup
Copy link
Collaborator

DanTup commented Dec 19, 2024

I'm actually confused... The original code where @FMorschel saw this was like this:

image

And the evaluation failed like this:

image

However, in a simple test of what I think is the same, I actually don't get an error - I am allowed to evaluate a.x even though the static type of a does not have an x:

void main() {
  f(B('test'));
}

void f(A a) {
  print(a);
}

class A {}

class B extends A {
  final String x;
  B(this.x);
}

image

I don't understand why these are different.

@a-siva

I am not sure if it is a good idea for the VM or CFE to evaluate the expression differently from how it would behave when the code is normally run.

I can understand that, but I also feel like having to add ?/! and casts to an expression just for quick evaluations while debugging can be quite annoying - and if you're hovering over an expression in the file, it might not be possible to amend it anyway. However, if it prevents using things like extension methods, maybe it's the right trade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. cfe-expression-compilation Issues related to expression compilation in the CFE triaged Issue has been triaged by sub team
Projects
None yet
Development

No branches or pull requests

5 participants