[vm] Fix expression evaluation in context of closures

When evaluating an expressino at a breakpoint that's inside a closure,
the closure may refer to anything it closes over. That includes the
`this` from an enclosing method.

So depending on whether a closure's parent chain is an instance method
or a static method, the expression evaluation function is going to be an
instance method or a static method.

=> We walk up the parent chain to determine the correct enclosing class.
=> This avoids making a Type object with a cid from a top-level class (see [0])

Handling this correctly will now try to get the `this` when invoking an
eval function that has an enclosing instance method. Though we may often
have "<optimized out>" the `this` (e.g. due to not capturing it in
closure context chain).

=> We still allow running the expression evaluation function in this
case, but only if the expression being evaluated doesn't access `this`.

A similar issue occurs when trying to use variables in the eval
expression that the closure didn't capture. This results in a confusing
CFE compile-time error. This is a separate issue and tracked in [1].

=> We update the test to distinuish the cases that this CL makes passing
and those that are failing due to [1].

Fixes [0] https://github.com/dart-lang/sdk/issues/53061
See also [1] https://github.com/dart-lang/sdk/issues/53087

TEST=Fixes part of service/evaluate_activation_test failures

Change-Id: I3bb24e7338c7b2f12d5340311d944cb59a455641
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/317540
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>

pkg/vm/lib/target/vm.dart

@@ -398,6 +398,7 @@ class VmTarget extends Target {
   // purposes.
   bool allowPlatformPrivateLibraryAccess(Uri importer, Uri imported) =>
       super.allowPlatformPrivateLibraryAccess(importer, imported) ||
+      importer.path.contains('runtime/observatory/tests') ||
       importer.path.contains('runtime/tests/vm/dart') ||
       importer.path.contains('tests/standalone/io') ||
       importer.path.contains('test-lib') ||

pkg/vm_service/test/common/test_helper.dart

@@ -93,11 +93,15 @@ class _ServiceTesteeRunner {
 class _ServiceTesteeLauncher {
   io.Process? process;
   List<String> args;
+
   bool killedByTester = false;
+  final _exitCodeCompleter = Completer<int>();
 
   _ServiceTesteeLauncher(String script)
       : args = [_getTestUri(script).toFilePath()];
 
+  Future<int> get exitCode => _exitCodeCompleter.future;
+
   // Spawn the testee process.
   Future<io.Process> _spawnProcess(
     bool pause_on_start,
@@ -223,12 +227,7 @@ class _ServiceTesteeLauncher {
           .listen((line) {
         io.stdout.write('>testee>err> ${line}\n');
       });
-      process!.exitCode.then((exitCode) {
-        if ((exitCode != 0) && !killedByTester) {
-          throw "Testee exited with $exitCode";
-        }
-        print("** Process exited: $exitCode");
-      });
+      process!.exitCode.then(_exitCodeCompleter.complete);
       return completer.future;
     });
   }
@@ -320,6 +319,14 @@ class _ServiceTesterRunner {
       print('All service tests completed successfully.');
       process.requestExit();
     });
+
+    final exitCode = await process.exitCode;
+    if (exitCode != 0) {
+      if (!process.killedByTester) {
+        throw "Testee exited with unexpected exitCode: $exitCode";
+      }
+    }
+    print("** Process exited: $exitCode");
   }
 
   Future<IsolateRef> getFirstIsolate(VmService service) async {

pkg/vm_service/test/evaluate_inside_closures_test.dart

@@ -25,10 +25,13 @@ class C {
   void instanceMethod() {
     ((int y) {
       debugger();
+      use(this);
     })(78);
   }
 }
 
+void use(_) {}
+
 testMain() {
   C c = C();
   C.staticMethod();

runtime/observatory/tests/service/evaluate_activation_test.dart

@@ -65,6 +65,26 @@ class C {
       };
     });
   }
+
+  method5(methodParam) {
+    var methodTemp = 4;
+    use(methodTemp);
+    [5].forEach((outerParam) {
+      var outerTemp = 6;
+      use(outerTemp);
+      closureWithReturnedHome = (innerParam) {
+        use(this);
+        use(methodParam);
+        use(methodTemp);
+        use(outerParam);
+        use(outerTemp);
+        use(innerParam);
+        var innerTemp = 8;
+        use(innerTemp);
+        breakHere();
+      };
+    });
+  }
 }
 
 Future testMethod(Isolate isolate) async {
@@ -85,20 +105,20 @@ Future testMethod(Isolate isolate) async {
     dynamic frameNumber = 1, r;
     r = await isolate.evalFrame(frameNumber, '123');  //# instance: ok
     expect(r.valueAsString, equals('123'));  //# instance: continued
-    r = await isolate.evalFrame(frameNumber, 'this');  //# scope: ok
-    expect(r.clazz.name, equals('C'));  //# scope: continued
+    r = await isolate.evalFrame(frameNumber, 'this');  //# instance: continued
+    expect(r.valueAsString, equals('<optimized out>'));  //# instance: continued
     r = await isolate.evalFrame(frameNumber, 'instVar');  //# instance: continued
-    expect(r.valueAsString, equals('1'));  //# instance: continued
+    expect(r.valueAsString, equals('<optimized out>'));  //# instance: continued
     r = await isolate.evalFrame(frameNumber, 'classVar'); //# instance: continued
     expect(r.valueAsString, equals('2'));  //# instance: continued
-    r = await isolate.evalFrame(frameNumber, 'methodParam');  //# scope: continued
-    expect(r.valueAsString, equals('3'));  //# scope: continued
+    r = await isolate.evalFrame(frameNumber, 'methodParam');  //# scope: ok
+    expect(r.valueAsString, equals('<optimized out>'));  //# scope: continued
     r = await isolate.evalFrame(frameNumber, 'methodTemp');  //# scope: continued
-    expect(r.valueAsString, equals('4'));  //# scope: continued
+    expect(r.valueAsString, equals('<optimized out>'));  //# scope: continued
     r = await isolate.evalFrame(frameNumber, 'outerParam');  //# scope: continued
-    expect(r.valueAsString, equals('5'));  //# scope: continued
+    expect(r.valueAsString, equals('<optimized out>'));  //# scope: continued
     r = await isolate.evalFrame(frameNumber, 'outerTemp');  //# scope: continued
-    expect(r.valueAsString, equals('6'));  //# scope: continued
+    expect(r.valueAsString, equals('<optimized out>'));  //# scope: continued
     r = await isolate.evalFrame(frameNumber, 'innerParam');  //# instance: continued
     expect(r.valueAsString, equals('7'));  //# instance: continued
     r = await isolate.evalFrame(frameNumber, 'innerTemp');  //# instance: continued
@@ -137,13 +157,13 @@ Future testMethod2(Isolate isolate) async {
     r = await isolate.evalFrame(frameNumber, 'classVar');
     expect(r.valueAsString, equals('2'));
     r = await isolate.evalFrame(frameNumber, 'methodParam');
-    expect(r.valueAsString, equals('3'));  //# scope: continued
-    r = await isolate.evalFrame(frameNumber, 'methodTemp');
-    expect(r.valueAsString, equals('4'));  //# scope: continued
-    r = await isolate.evalFrame(frameNumber, 'outerParam');
-    expect(r.valueAsString, equals('5'));  //# scope: continued
-    r = await isolate.evalFrame(frameNumber, 'outerTemp');
-    expect(r.valueAsString, equals('6'));  //# scope: continued
+    expect(r.valueAsString, equals('<optimized out>'));  //# scope: continued
+    r = await isolate.evalFrame(frameNumber, 'methodTemp');  //# scope: continued
+    expect(r.valueAsString, equals('<optimized out>'));  //# scope: continued
+    r = await isolate.evalFrame(frameNumber, 'outerParam');  //# scope: continued
+    expect(r.valueAsString, equals('<optimized out>'));  //# scope: continued
+    r = await isolate.evalFrame(frameNumber, 'outerTemp');  //# scope: continued
+    expect(r.valueAsString, equals('<optimized out>'));  //# scope: continued
     r = await isolate.evalFrame(frameNumber, 'innerParam');
     expect(r.valueAsString, equals('7'));
     r = await isolate.evalFrame(frameNumber, 'innerTemp');
@@ -213,10 +233,10 @@ Future testMethod4(Isolate isolate) async {
     dynamic frameNumber = 1, r;
     r = await isolate.evalFrame(frameNumber, '123');  //# instance: continued
     expect(r.valueAsString, equals('123'));  //# instance: continued
-    r = await isolate.evalFrame(frameNumber, 'this');  //# scope: continued
-    expect(r.clazz.name, equals('C'));  //# scope: continued
+    r = await isolate.evalFrame(frameNumber, 'this');  //# instance: continued
+    expect(r.valueAsString, equals('<optimized out>'));  //# instance: continued
     r = await isolate.evalFrame(frameNumber, 'instVar');  //# instance: continued
-    expect(r.valueAsString, equals('1'));  //# instance: continued
+    expect(r.valueAsString, equals('<optimized out>'));  //# instance: continued
     r = await isolate.evalFrame(frameNumber, 'classVar');  //# instance: continued
     expect(r.valueAsString, equals('2'));  //# instance: continued
     r = await isolate.evalFrame(frameNumber, 'methodParam');  //# scope: continued
@@ -243,11 +263,58 @@ Future testMethod4(Isolate isolate) async {
   expect(hitBreakpoint, isTrue);
 }
 
+Future testMethod5(Isolate isolate) async {
+  Library rootLib = await isolate.rootLibrary.load() as Library;
+  ServiceFunction function =
+      rootLib.functions.singleWhere((f) => f.name == 'breakHere');
+  Breakpoint bpt = await isolate.addBreakpointAtEntry(function);
+  print("Breakpoint: $bpt");
+
+  bool hitBreakpoint = false;
+  var stream = await isolate.vm.getEventStream(VM.kDebugStream);
+  stream.firstWhere((event) {
+    print("Event $event");
+    return event.kind == ServiceEvent.kPauseBreakpoint;
+  }).then((event) async {
+    dynamic frameNumber = 1, r;
+    r = await isolate.evalFrame(frameNumber, '123');  //# instance: continued
+    expect(r.valueAsString, equals('123'));  //# instance: continued
+    r = await isolate.evalFrame(frameNumber, 'this');  //# instance: continued
+    expect(r.clazz.name, equals('C'));  //# instance: continued
+    r = await isolate.evalFrame(frameNumber, 'instVar');  //# instance: continued
+    expect(r.valueAsString, equals('1'));  //# instance: continued
+    r = await isolate.evalFrame(frameNumber, 'classVar');  //# instance: continued
+    expect(r.valueAsString, equals('2'));  //# instance: continued
+    r = await isolate.evalFrame(frameNumber, 'methodParam');  //# instance: continued
+    expect(r.valueAsString, equals('3'));  //# instance: continued
+    r = await isolate.evalFrame(frameNumber, 'methodTemp');  //# instance: continued
+    expect(r.valueAsString, equals('4'));  //# instance: continued
+    r = await isolate.evalFrame(frameNumber, 'outerParam');  //# instance: continued
+    expect(r.valueAsString, equals('5'));  //# instance: continued
+    r = await isolate.evalFrame(frameNumber, 'outerTemp');  //# instance: continued
+    expect(r.valueAsString, equals('6'));  //# instance: continued
+    r = await isolate.evalFrame(frameNumber, 'innerParam');  //# instance: continued
+    expect(r.valueAsString, equals('7'));  //# instance: continued
+    r = await isolate.evalFrame(frameNumber, 'innerTemp');  //# instance: continued
+    expect(r.valueAsString, equals('8'));  //# instance: continued
+    r = await isolate.evalFrame(frameNumber, 'math.sqrt');  //# instance: continued
+    expect(r.isClosure, isTrue);  //# instance: continued
+
+    hitBreakpoint = true;
+  }).whenComplete(isolate.resume);
+
+  var result = await rootLib.evaluate(
+      '(){ new C().method5(3); C.closureWithReturnedHome(7); }()');
+  print("Result $result");
+  expect(hitBreakpoint, isTrue);
+}
+
 var tests = <IsolateTest>[
   testMethod,
   testMethod2,
   testMethod3,
   testMethod4,
+  testMethod5,
 ];
 
 main(args) => runIsolateTests(args, tests);

runtime/observatory/tests/service/evaluate_expression_with_optimized_out_receiver_on_stack_test.dart

@@ -0,0 +1,125 @@
+// Copyright (c) 2023, 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.
+
+// VMOptions=--deterministic
+
+import 'dart:async';
+import 'dart:_internal'; // ignore: import_internal_library, unused_import
+
+import 'package:observatory/service_io.dart';
+import 'package:test/test.dart';
+
+import 'test_helper.dart';
+
+String examineStackExpression(String variableName) {
+  // The returned string is the evaluation expression. We try to make it so that
+  // the evaluated expression exercises OSR, Deopt, StackTrace.
+  //
+  // Even though the expression we build doesn't use `this`, the expression is
+  // evaluated in the context of a closure that's inside an instance method. So
+  // the evaluation function will be an instance method that does receive a
+  // receiver that was "<optimized out>".
+  // => The purpose of the test is to see if having the sentinal as receiver
+  // causes any issues.
+  final entries = [
+    // Obtain stack while eval function is unoptimized
+    'triggerStackTrace()',
+
+    // Get eval function OSRed
+    'for (int i = 0; i < 100 * 1000; ++i) i',
+
+    // Obtain stack while eval function is optimized.
+    'triggerStackTrace()',
+
+    // Deopt eval function.
+    'triggerDeopt()',
+  ];
+  final round = entries.join(', ');
+  return 'returnFirst([$variableName, $round, $round, $round])';
+}
+
+@pragma('vm:never-inline')
+dynamic triggerDeopt() {
+  print('triggerDeopt');
+  VMInternalsForTesting.deoptimizeFunctionsOnStack();  // ignore: undefined_identifier
+}
+
+@pragma('vm:never-inline')
+dynamic triggerStackTrace() {
+  print('triggerStackTrace');
+  return StackTrace.current;
+}
+
+@pragma('vm:never-inline')
+dynamic returnFirst(List l) {
+  print('returnFirst');
+  return l.first;
+}
+
+breakHere() {}
+
+use(dynamic v) => v;
+
+class C {
+  var instVar = 1;
+
+  method(methodParam) {
+    var methodTemp = 3;
+    use(methodTemp);
+    [4].forEach((outerParam) {
+      var outerTemp = 5;
+      use(outerTemp);
+      [6].forEach((innerParam) {
+        var innerTemp = 7;
+        use(innerTemp);
+        breakHere();
+      });
+    });
+  }
+}
+
+Future testMethod(Isolate isolate) async {
+  final rootLib = await isolate.rootLibrary.load() as Library;
+  final function = rootLib.functions.singleWhere((f) => f.name == 'breakHere');
+  final bpt = await isolate.addBreakpointAtEntry(function);
+  print("Breakpoint: $bpt");
+
+  final stream = await isolate.vm.getEventStream(VM.kDebugStream);
+  final hitBreakpointFuture = stream.firstWhere((event) {
+    print("Event $event");
+    return event.kind == ServiceEvent.kPauseBreakpoint;
+  });
+
+  Future handleBreakpoint() async {
+    Future checkValue(String expr, String value) async {
+      print('Evaluating "$expr".');
+      const frameNumber = 1;
+      final dynamic r = await isolate.evalFrame(frameNumber, expr);
+      print(' -> result $r');
+      expect(r.valueAsString, equals(value));
+    }
+
+    print('waiting for breakpoint');
+    await hitBreakpointFuture;
+    print('got breakpoint');
+    await checkValue(examineStackExpression('this'), '<optimized out>');
+    await checkValue(examineStackExpression('instVar'), '<optimized out>');
+    await checkValue(examineStackExpression('innerParam'), '6');
+    await checkValue(examineStackExpression('innerTemp'), '7');
+
+    await isolate.resume();
+    print('resuming ');
+  }
+
+  await Future.wait([
+    handleBreakpoint(),
+    rootLib.evaluate('C().method(2)'),
+  ]);
+}
+
+final tests = <IsolateTest>[
+  testMethod,
+];
+
+main(args) => runIsolateTests(args, tests);

runtime/vm/compiler/frontend/scope_builder.cc

@@ -1928,6 +1928,8 @@ void ScopeBuilder::HandleLoadReceiver() {
     // use-site also includes the [receiver].
     scope_->CaptureVariable(parsed_function_->receiver_var());
   }
+
+  parsed_function_->set_receiver_used();
 }
 
 void ScopeBuilder::HandleSpecialLoad(LocalVariable** variable,

runtime/vm/debugger.cc

@@ -1055,28 +1055,27 @@ ObjectPtr ActivationFrame::EvaluateCompiledExpression(
     const Array& type_definitions,
     const Array& arguments,
     const TypeArguments& type_arguments) {
-  if (function().IsClosureFunction()) {
-    return Library::Handle(Library()).EvaluateCompiledExpression(
-        kernel_buffer, type_definitions, arguments, type_arguments);
-  } else if (function().is_static()) {
-    const Class& cls = Class::Handle(function().Owner());
-    return cls.EvaluateCompiledExpression(kernel_buffer, type_definitions,
-                                          arguments, type_arguments);
-  } else {
-    const Object& receiver = Object::Handle(GetReceiver());
-    if (receiver.ptr() == Object::optimized_out().ptr()) {
-      // Cannot execute an instance method without a receiver.
-      return Object::optimized_out().ptr();
-    }
-    const Class& method_cls = Class::Handle(function().Owner());
-    ASSERT(receiver.IsInstance() || receiver.IsNull());
-    if (!(receiver.IsInstance() || receiver.IsNull())) {
-      return Object::null();
-    }
-    const Instance& inst = Instance::Cast(receiver);
-    return inst.EvaluateCompiledExpression(
-        method_cls, kernel_buffer, type_definitions, arguments, type_arguments);
+  auto thread = Thread::Current();
+  auto zone = thread->zone();
+
+  // The expression evaluation function will get all it's captured state passed
+  // as parameters (with `this` being the exception). As a result, we treat the
+  // expression evaluation function as either a top-level, static or instance
+  // method.
+  const auto& outermost =
+      Function::Handle(zone, function().GetOutermostFunction());
+  const auto& klass = Class::Handle(zone, outermost.Owner());
+  const auto& library = Library::Handle(zone, klass.library());
+
+  auto& receiver = Object::Handle(zone);
+  if (!klass.IsTopLevel() && !outermost.is_static()) {
+    receiver = GetReceiver();
+    RELEASE_ASSERT(receiver.IsInstance() ||
+                   receiver.ptr() == Object::optimized_out().ptr());
   }
+  return Instance::EvaluateCompiledExpression(thread, receiver, library, klass,
+                                              kernel_buffer, type_definitions,
+                                              arguments, type_arguments);
 }
 
 TypeArgumentsPtr ActivationFrame::BuildParameters(

runtime/vm/object.cc

@@ -4800,32 +4800,164 @@ ObjectPtr Class::Invoke(const String& function_name,
   return DartEntry::InvokeFunction(function, args, args_descriptor_array);
 }
 
+#if !defined(DART_PRECOMPILED_RUNTIME)
+
+static ObjectPtr LoadExpressionEvaluationFunction(
+    Zone* zone,
+    const ExternalTypedData& kernel_buffer,
+    const String& library_url,
+    const String& klass) {
+  std::unique_ptr<kernel::Program> kernel_pgm =
+      kernel::Program::ReadFromTypedData(kernel_buffer);
+
+  if (kernel_pgm == nullptr) {
+    return ApiError::New(String::Handle(
+        zone, String::New("Kernel isolate returned ill-formed kernel.")));
+  }
+
+  auto& result = Object::Handle(zone);
+  {
+    kernel::KernelLoader loader(kernel_pgm.get(),
+                                /*uri_to_source_table=*/nullptr);
+    result = loader.LoadExpressionEvaluationFunction(library_url, klass);
+    kernel_pgm.reset();
+  }
+  if (result.IsError()) return result.ptr();
+  return Function::Cast(result).ptr();
+}
+
+static bool EvaluationFunctionNeedsReceiver(Thread* thread,
+                                            Zone* zone,
+                                            const Function& eval_function) {
+  auto parsed_function = new ParsedFunction(
+      thread, Function::ZoneHandle(zone, eval_function.ptr()));
+  parsed_function->EnsureKernelScopes();
+  return parsed_function->is_receiver_used();
+}
+
 static ObjectPtr EvaluateCompiledExpressionHelper(
+    Zone* zone,
+    const Function& eval_function,
+    const Array& type_definitions,
+    const Array& arguments,
+    const TypeArguments& type_arguments) {
+  // type_arguments is null if all type arguments are dynamic.
+  if (type_definitions.Length() == 0 || type_arguments.IsNull()) {
+    return DartEntry::InvokeFunction(eval_function, arguments);
+  }
+
+  intptr_t num_type_args = type_arguments.Length();
+  const auto& real_arguments =
+      Array::Handle(zone, Array::New(arguments.Length() + 1));
+  real_arguments.SetAt(0, type_arguments);
+  Object& arg = Object::Handle(zone);
+  for (intptr_t i = 0; i < arguments.Length(); ++i) {
+    arg = arguments.At(i);
+    real_arguments.SetAt(i + 1, arg);
+  }
+
+  const Array& args_desc =
+      Array::Handle(zone, ArgumentsDescriptor::NewBoxed(
+                              num_type_args, arguments.Length(), Heap::kNew));
+  return DartEntry::InvokeFunction(eval_function, real_arguments, args_desc);
+}
+
+#endif  // !defined(DART_PRECOMPILED_RUNTIME)
+
+ObjectPtr Library::EvaluateCompiledExpression(
     const ExternalTypedData& kernel_buffer,
     const Array& type_definitions,
-    const String& library_url,
-    const String& klass,
     const Array& arguments,
-    const TypeArguments& type_arguments);
+    const TypeArguments& type_arguments) const {
+  const auto& klass = Class::Handle(toplevel_class());
+  return klass.EvaluateCompiledExpression(kernel_buffer, type_definitions,
+                                          arguments, type_arguments);
+}
 
 ObjectPtr Class::EvaluateCompiledExpression(
     const ExternalTypedData& kernel_buffer,
     const Array& type_definitions,
     const Array& arguments,
     const TypeArguments& type_arguments) const {
-  ASSERT(Thread::Current()->IsDartMutatorThread());
-  if (IsInternalOnlyClassId(id()) || (id() == kTypeArgumentsCid)) {
-    const Instance& exception = Instance::Handle(String::New(
-        "Expressions can be evaluated only with regular Dart instances"));
-    const Instance& stacktrace = Instance::Handle();
-    return UnhandledException::New(exception, stacktrace);
+  auto thread = Thread::Current();
+  const auto& library = Library::Handle(thread->zone(), this->library());
+  return Instance::EvaluateCompiledExpression(
+      thread, Instance::null_object(), library, *this, kernel_buffer,
+      type_definitions, arguments, type_arguments);
+}
+
+ObjectPtr Instance::EvaluateCompiledExpression(
+    const Class& klass,
+    const ExternalTypedData& kernel_buffer,
+    const Array& type_definitions,
+    const Array& arguments,
+    const TypeArguments& type_arguments) const {
+  auto thread = Thread::Current();
+  auto zone = thread->zone();
+  const auto& library = Library::Handle(zone, klass.library());
+  return Instance::EvaluateCompiledExpression(thread, *this, library, klass,
+                                              kernel_buffer, type_definitions,
+                                              arguments, type_arguments);
+}
+
+ObjectPtr Instance::EvaluateCompiledExpression(
+    Thread* thread,
+    const Object& receiver,
+    const Library& library,
+    const Class& klass,
+    const ExternalTypedData& kernel_buffer,
+    const Array& type_definitions,
+    const Array& arguments,
+    const TypeArguments& type_arguments) {
+  auto zone = Thread::Current()->zone();
+#if defined(DART_PRECOMPILED_RUNTIME)
+  const auto& error_str = String::Handle(
+      zone,
+      String::New("Expression evaluation not available in precompiled mode."));
+  return ApiError::New(error_str);
+#else
+  if (IsInternalOnlyClassId(klass.id()) || (klass.id() == kTypeArgumentsCid)) {
+    const auto& exception = Instance::Handle(
+        zone, String::New("Expressions can be evaluated only with regular Dart "
+                          "instances/classes."));
+    return UnhandledException::New(exception, StackTrace::null_instance());
   }
 
-  return EvaluateCompiledExpressionHelper(
-      kernel_buffer, type_definitions,
-      String::Handle(Library::Handle(library()).url()),
-      IsTopLevel() ? String::Handle() : String::Handle(UserVisibleName()),
-      arguments, type_arguments);
+  const auto& url = String::Handle(zone, library.url());
+  const auto& klass_name = klass.IsTopLevel()
+                               ? String::null_string()
+                               : String::Handle(zone, klass.UserVisibleName());
+
+  const auto& result = Object::Handle(
+      zone,
+      LoadExpressionEvaluationFunction(zone, kernel_buffer, url, klass_name));
+  if (result.IsError()) return result.ptr();
+
+  const auto& eval_function = Function::Cast(result);
+
+  auto& all_arguments = Array::Handle(zone, arguments.ptr());
+  if (!eval_function.is_static()) {
+    // `this` may be optimized out (e.g. not accessible from breakpoint due to
+    // not being captured by closure). We allow this as long as the evaluation
+    // function doesn't actually need `this`.
+    if (receiver.IsNull() || receiver.ptr() == Object::optimized_out().ptr()) {
+      if (EvaluationFunctionNeedsReceiver(thread, zone, eval_function)) {
+        return Object::optimized_out().ptr();
+      }
+    }
+
+    all_arguments = Array::New(1 + arguments.Length());
+    auto& param = PassiveObject::Handle();
+    all_arguments.SetAt(0, receiver);
+    for (intptr_t i = 0; i < arguments.Length(); i++) {
+      param = arguments.At(i);
+      all_arguments.SetAt(i + 1, param);
+    }
+  }
+
+  return EvaluateCompiledExpressionHelper(zone, eval_function, type_definitions,
+                                          all_arguments, type_arguments);
+#endif  // !defined(DART_PRECOMPILED_RUNTIME)
 }
 
 void Class::EnsureDeclarationLoaded() const {
@@ -14573,16 +14705,6 @@ ObjectPtr Library::Invoke(const String& function_name,
   return DartEntry::InvokeFunction(function, args, args_descriptor_array);
 }
 
-ObjectPtr Library::EvaluateCompiledExpression(
-    const ExternalTypedData& kernel_buffer,
-    const Array& type_definitions,
-    const Array& arguments,
-    const TypeArguments& type_arguments) const {
-  return EvaluateCompiledExpressionHelper(
-      kernel_buffer, type_definitions, String::Handle(url()), String::Handle(),
-      arguments, type_arguments);
-}
-
 void Library::InitNativeWrappersLibrary(IsolateGroup* isolate_group,
                                         bool is_kernel) {
   const int kNumNativeWrappersClasses = 4;
@@ -14638,65 +14760,6 @@ class LibraryLookupTraits {
 };
 typedef UnorderedHashMap<LibraryLookupTraits> LibraryLookupMap;
 
-static ObjectPtr EvaluateCompiledExpressionHelper(
-    const ExternalTypedData& kernel_buffer,
-    const Array& type_definitions,
-    const String& library_url,
-    const String& klass,
-    const Array& arguments,
-    const TypeArguments& type_arguments) {
-  Thread* thread = Thread::Current();
-  Zone* zone = thread->zone();
-#if defined(DART_PRECOMPILED_RUNTIME)
-  const String& error_str = String::Handle(
-      zone,
-      String::New("Expression evaluation not available in precompiled mode."));
-  return ApiError::New(error_str);
-#else
-  std::unique_ptr<kernel::Program> kernel_pgm =
-      kernel::Program::ReadFromTypedData(kernel_buffer);
-
-  if (kernel_pgm == nullptr) {
-    return ApiError::New(String::Handle(
-        zone, String::New("Kernel isolate returned ill-formed kernel.")));
-  }
-
-  auto& result = Object::Handle(zone);
-  {
-    kernel::KernelLoader loader(kernel_pgm.get(),
-                                /*uri_to_source_table=*/nullptr);
-    result = loader.LoadExpressionEvaluationFunction(library_url, klass);
-    kernel_pgm.reset();
-  }
-
-  if (result.IsError()) return result.ptr();
-
-  const auto& callee = Function::CheckedHandle(zone, result.ptr());
-
-  // type_arguments is null if all type arguments are dynamic.
-  if (type_definitions.Length() == 0 || type_arguments.IsNull()) {
-    result = DartEntry::InvokeFunction(callee, arguments);
-  } else {
-    intptr_t num_type_args = type_arguments.Length();
-    Array& real_arguments =
-        Array::Handle(zone, Array::New(arguments.Length() + 1));
-    real_arguments.SetAt(0, type_arguments);
-    Object& arg = Object::Handle(zone);
-    for (intptr_t i = 0; i < arguments.Length(); ++i) {
-      arg = arguments.At(i);
-      real_arguments.SetAt(i + 1, arg);
-    }
-
-    const Array& args_desc =
-        Array::Handle(zone, ArgumentsDescriptor::NewBoxed(
-                                num_type_args, arguments.Length(), Heap::kNew));
-    result = DartEntry::InvokeFunction(callee, real_arguments, args_desc);
-  }
-
-  return result.ptr();
-#endif
-}
-
 // Returns library with given url in current isolate, or nullptr.
 LibraryPtr Library::LookupLibrary(Thread* thread, const String& url) {
   Zone* zone = thread->zone();
@@ -20344,28 +20407,6 @@ ObjectPtr Instance::Invoke(const String& function_name,
                                 inst_type_args);
 }
 
-ObjectPtr Instance::EvaluateCompiledExpression(
-    const Class& method_cls,
-    const ExternalTypedData& kernel_buffer,
-    const Array& type_definitions,
-    const Array& arguments,
-    const TypeArguments& type_arguments) const {
-  const Array& arguments_with_receiver =
-      Array::Handle(Array::New(1 + arguments.Length()));
-  PassiveObject& param = PassiveObject::Handle();
-  arguments_with_receiver.SetAt(0, *this);
-  for (intptr_t i = 0; i < arguments.Length(); i++) {
-    param = arguments.At(i);
-    arguments_with_receiver.SetAt(i + 1, param);
-  }
-
-  return EvaluateCompiledExpressionHelper(
-      kernel_buffer, type_definitions,
-      String::Handle(Library::Handle(method_cls.library()).url()),
-      String::Handle(method_cls.UserVisibleName()), arguments_with_receiver,
-      type_arguments);
-}
-
 ObjectPtr Instance::HashCode() const {
   // TODO(koda): Optimize for all builtin classes and all classes
   // that do not override hashCode.

runtime/vm/object.h

@@ -8201,17 +8201,31 @@ class Instance : public Object {
                          bool respect_reflectable = true,
                          bool check_is_entrypoint = false) const;
 
+  ObjectPtr EvaluateCompiledExpression(
+      const Class& klass,
+      const ExternalTypedData& kernel_buffer,
+      const Array& type_definitions,
+      const Array& arguments,
+      const TypeArguments& type_arguments) const;
+
   // Evaluate the given expression as if it appeared in an instance method of
-  // this instance and return the resulting value, or an error object if
+  // [receiver] and return the resulting value, or an error object if
   // evaluating the expression fails. The method has the formal (type)
   // parameters given in (type_)param_names, and is invoked with the (type)
   // argument values given in (type_)param_values.
-  ObjectPtr EvaluateCompiledExpression(
-      const Class& method_cls,
+  //
+  // We allow [receiver] to be null/<optimized out> if
+  //   * the evaluation function doesn't access `this`
+  //   * the evaluation function is static
+  static ObjectPtr EvaluateCompiledExpression(
+      Thread* thread,
+      const Object& receiver,
+      const Library& library,
+      const Class& klass,
       const ExternalTypedData& kernel_buffer,
       const Array& type_definitions,
       const Array& param_values,
-      const TypeArguments& type_param_values) const;
+      const TypeArguments& type_param_values);
 
   // Equivalent to invoking hashCode on this instance.
   virtual ObjectPtr HashCode() const;

runtime/vm/parser.h

@@ -141,6 +141,13 @@ class ParsedFunction : public ZoneAllocated {
   }
   bool has_receiver_var() const { return receiver_var_ != nullptr; }
 
+  void set_receiver_used() { receiver_used_ = true; }
+  bool is_receiver_used() const {
+    ASSERT(kernel_scopes_ != nullptr);
+    ASSERT(!receiver_used_ || receiver_var() != nullptr);
+    return receiver_used_;
+  }
+
   LocalVariable* expression_temp_var() const {
     ASSERT(has_expression_temp_var());
     return expression_temp_var_;
@@ -302,6 +309,7 @@ class ParsedFunction : public ZoneAllocated {
   DynamicClosureCallVars* dynamic_closure_call_vars_;
   mutable FieldSet guarded_fields_;
   ZoneGrowableArray<const Instance*>* default_parameter_values_;
+  bool receiver_used_ = false;
 
   LocalVariable* raw_type_arguments_var_;
   ZoneGrowableArray<LocalVariable*>* raw_parameters_ = nullptr;

runtime/vm/scopes.cc

@@ -341,7 +341,7 @@ LocalVariable* LocalScope::LookupVariable(const String& name,
     LocalVariable* var =
         current_scope->LocalLookupVariable(name, kernel_offset);
     // If testing only, return the variable even if invisible.
-    if ((var != nullptr) && (!var->is_invisible_ || test_only)) {
+    if ((var != nullptr) && (!var->is_invisible() || test_only)) {
       if (!test_only && (var->owner()->function_level() != function_level())) {
         CaptureVariable(var);
       }

runtime/vm/scopes.h

@@ -91,13 +91,7 @@ class LocalVariable : public ZoneAllocated {
         parameter_type_(parameter_type),
         parameter_value_(parameter_value),
         const_value_(nullptr),
-        is_final_(false),
-        is_captured_(false),
-        is_invisible_(false),
-        is_captured_parameter_(false),
-        is_forced_stack_(false),
         covariance_mode_(kNotCovariant),
-        is_late_(false),
         late_init_offset_(0),
         type_check_mode_(kDoTypeCheck),
         index_(),
@@ -132,11 +126,11 @@ class LocalVariable : public ZoneAllocated {
   CompileType* parameter_type() const { return parameter_type_; }
   const Object* parameter_value() const { return parameter_value_; }
 
-  bool is_final() const { return is_final_; }
-  void set_is_final() { is_final_ = true; }
+  bool is_final() const { return IsFinalBit::decode(bitfield_); }
+  void set_is_final() { bitfield_ = IsFinalBit::update(true, bitfield_); }
 
-  bool is_captured() const { return is_captured_; }
-  void set_is_captured() { is_captured_ = true; }
+  bool is_captured() const { return IsCapturedBit::decode(bitfield_); }
+  void set_is_captured() { bitfield_ = IsCapturedBit::update(true, bitfield_); }
 
   bool ComputeIfIsAwaiterLink(const Library& library);
   void set_is_awaiter_link(bool value) {
@@ -147,11 +141,13 @@ class LocalVariable : public ZoneAllocated {
   // CaptureLocalVariables - which iterates scope chain between two scopes
   // and indiscriminately marks all variables as captured.
   // TODO(27590) remove the hardcoded list of names from CaptureLocalVariables
-  bool is_forced_stack() const { return is_forced_stack_; }
-  void set_is_forced_stack() { is_forced_stack_ = true; }
+  bool is_forced_stack() const { return IsForcedStackBit::decode(bitfield_); }
+  void set_is_forced_stack() {
+    bitfield_ = IsForcedStackBit::update(true, bitfield_);
+  }
 
-  bool is_late() const { return is_late_; }
-  void set_is_late() { is_late_ = true; }
+  bool is_late() const { return IsLateBit::decode(bitfield_); }
+  void set_is_late() { bitfield_ = IsLateBit::update(true, bitfield_); }
 
   intptr_t late_init_offset() const { return late_init_offset_; }
   void set_late_init_offset(intptr_t late_init_offset) {
@@ -205,11 +201,17 @@ class LocalVariable : public ZoneAllocated {
 
   // Invisible variables are not included into LocalVarDescriptors
   // and not displayed in the debugger.
-  void set_invisible(bool value) { is_invisible_ = value; }
-  bool is_invisible() const { return is_invisible_; }
+  bool is_invisible() const { return IsInvisibleBit::decode(bitfield_); }
+  void set_invisible(bool value) {
+    bitfield_ = IsInvisibleBit::update(value, bitfield_);
+  }
 
-  bool is_captured_parameter() const { return is_captured_parameter_; }
-  void set_is_captured_parameter(bool value) { is_captured_parameter_ = value; }
+  bool is_captured_parameter() const {
+    return IsCapturedParameterBit::decode(bitfield_);
+  }
+  void set_is_captured_parameter(bool value) {
+    bitfield_ = IsCapturedParameterBit::update(value, bitfield_);
+  }
 
   bool IsConst() const { return const_value_ != nullptr; }
 
@@ -226,6 +228,18 @@ class LocalVariable : public ZoneAllocated {
   bool Equals(const LocalVariable& other) const;
 
  private:
+  // If true, this variable is readonly.
+  using IsFinalBit = BitField<uint32_t, bool, 0, 1>;
+  // If true, this variable lives in the context, otherwise
+  // in the stack frame.
+  using IsCapturedBit = BitField<uint32_t, bool, IsFinalBit::kNextBit, 1>;
+  using IsInvisibleBit = BitField<uint32_t, bool, IsCapturedBit::kNextBit, 1>;
+  using IsCapturedParameterBit =
+      BitField<uint32_t, bool, IsInvisibleBit::kNextBit, 1>;
+  using IsForcedStackBit =
+      BitField<uint32_t, bool, IsCapturedParameterBit::kNextBit, 1>;
+  using IsLateBit = BitField<uint32_t, bool, IsForcedStackBit ::kNextBit, 1>;
+
   enum CovarianceMode {
     kNotCovariant,
     kImplicit,
@@ -250,14 +264,8 @@ class LocalVariable : public ZoneAllocated {
 
   const Instance* const_value_;  // nullptr or compile-time const value.
 
-  bool is_final_;     // If true, this variable is readonly.
-  bool is_captured_;  // If true, this variable lives in the context, otherwise
-                      // in the stack frame.
-  bool is_invisible_;
-  bool is_captured_parameter_;
-  bool is_forced_stack_;
+  uint32_t bitfield_ = 0;
   CovarianceMode covariance_mode_;
-  bool is_late_;
   intptr_t late_init_offset_;
   TypeCheckMode type_check_mode_;
   VariableIndex index_;

runtime/vm/service.cc

@@ -3370,43 +3370,38 @@ static void EvaluateCompiledExpression(Thread* thread, JSONStream* js) {
       }
       return;
     }
+    const auto& type_params_names_fixed =
+        Array::Handle(zone, Array::MakeFixedLength(type_params_names));
+    const auto& param_values_fixed =
+        Array::Handle(zone, Array::MakeFixedLength(param_values));
+
     TypeArguments& type_arguments = TypeArguments::Handle(zone);
     if (obj.IsLibrary()) {
-      const Library& lib = Library::Cast(obj);
-      const Object& result = Object::Handle(
+      const auto& lib = Library::Cast(obj);
+      const auto& result = Object::Handle(
           zone,
-          lib.EvaluateCompiledExpression(
-              kernel_data,
-              Array::Handle(zone, Array::MakeFixedLength(type_params_names)),
-              Array::Handle(zone, Array::MakeFixedLength(param_values)),
-              type_arguments));
+          lib.EvaluateCompiledExpression(kernel_data, type_params_names_fixed,
+                                         param_values_fixed, type_arguments));
       result.PrintJSON(js, true);
       return;
     }
     if (obj.IsClass()) {
-      const Class& cls = Class::Cast(obj);
-      const Object& result = Object::Handle(
+      const auto& cls = Class::Cast(obj);
+      const auto& result = Object::Handle(
           zone,
-          cls.EvaluateCompiledExpression(
-              kernel_data,
-              Array::Handle(zone, Array::MakeFixedLength(type_params_names)),
-              Array::Handle(zone, Array::MakeFixedLength(param_values)),
-              type_arguments));
+          cls.EvaluateCompiledExpression(kernel_data, type_params_names_fixed,
+                                         param_values_fixed, type_arguments));
       result.PrintJSON(js, true);
       return;
     }
     if ((obj.IsInstance() || obj.IsNull()) && !ContainsNonInstance(obj)) {
-      // We don't use Instance::Cast here because it doesn't allow null.
-      Instance& instance = Instance::Handle(zone);
-      instance ^= obj.ptr();
-      const Class& receiver_cls = Class::Handle(zone, instance.clazz());
-      const Object& result = Object::Handle(
-          zone,
-          instance.EvaluateCompiledExpression(
-              receiver_cls, kernel_data,
-              Array::Handle(zone, Array::MakeFixedLength(type_params_names)),
-              Array::Handle(zone, Array::MakeFixedLength(param_values)),
-              type_arguments));
+      const auto& instance =
+          Instance::Handle(zone, Instance::RawCast(obj.ptr()));
+      const auto& receiver_cls = Class::Handle(zone, instance.clazz());
+      const auto& result = Object::Handle(
+          zone, instance.EvaluateCompiledExpression(
+                    receiver_cls, kernel_data, type_params_names_fixed,
+                    param_values_fixed, type_arguments));
       result.PrintJSON(js, true);
       return;
     }