[CFE] Expression evaluation will use static type when given 'dynamic'

Fixes #57040

Change-Id: I0f954390c364a91816f66af821e12bee93d4746a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/400302
Commit-Queue: Jens Johansen <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
Reviewed-by: Derek Xu <[email protected]>

Author: jensjoha

pkg/front_end/lib/src/base/incremental_compiler.dart

@@ -27,6 +27,7 @@ import 'package:kernel/kernel.dart'
         Class,
         Component,
         DartType,
+        DynamicType,
         Expression,
         Extension,
         ExtensionType,
@@ -1868,11 +1869,16 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
               library, scriptUriAsUri, cls, offset);
           // For now, if any definition is (or contains) an Extension Type,
           // we'll overwrite the given (runtime?) definitions so we know about
-          // the extension type.
+          // the extension type. If any definition is said to be dynamic we'll
+          // overwrite as well because that mostly means that the value is
+          // currently null. This can also mean that the VM can't send over the
+          // information - this for instance happens for function types.
           for (MapEntry<String, DartType> def
               in foundScope.definitions.entries) {
-            if (!usedDefinitions.containsKey(def.key)) continue;
-            if (_ExtensionTypeFinder.isOrContainsExtensionType(def.value)) {
+            DartType? existingType = usedDefinitions[def.key];
+            if (existingType == null) continue;
+            if (existingType is DynamicType ||
+                _ExtensionTypeFinder.isOrContainsExtensionType(def.value)) {
               usedDefinitions[def.key] = def.value;
             }
           }

pkg/front_end/testcases/expression/issue_56911_01.expression.yaml

@@ -5,9 +5,9 @@
 # https://github.com/dart-lang/sdk/issues/56911
 
 # Definition, offset, method etc extracted by starting the VM with
-# `-DDFE_VERBOSE=false`, e.g.
+# `-DDFE_VERBOSE=true`, e.g.
 # ```
-# out/ReleaseX64/dart -DDFE_VERBOSE=false --enable-vm-service \
+# out/ReleaseX64/dart -DDFE_VERBOSE=true --enable-vm-service \
 # --serve-observatory --disable-service-auth-codes --pause_isolates_on_start \
 # inputFile.dart
 # ```

pkg/front_end/testcases/expression/issue_57040_01.expression.yaml

@@ -0,0 +1,47 @@
+# Copyright (c) 2024, 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.
+
+# https://github.com/dart-lang/sdk/issues/57040
+
+# Definition, offset, method etc extracted by starting the VM with
+# `-DDFE_VERBOSE=true`, e.g.
+# ```
+# out/ReleaseX64/dart -DDFE_VERBOSE=true --enable-vm-service \
+# --serve-observatory --disable-service-auth-codes --pause_isolates_on_start \
+# inputFile.dart
+# ```
+# and then issuing the expression compilation. It will then print stuff like
+# `DFE: request[6]: [dart:core, _OneByteString, 1, 0]` in the terminal.
+
+sources: |
+  import "dart:developer";
+
+  extension on String? {
+    bool get isNullOrEmpty {
+      var str = this;
+      return str == null || str.isEmpty;
+    }
+  }
+
+  void main() {
+    String? str = "hello";
+    debugger();
+    print(str.isNullOrEmpty);
+    str = null;
+    debugger();
+    print(str.isNullOrEmpty);
+  }
+
+definitions: ["str"]
+# String
+definition_types: ["dart:core", "_OneByteString", "1", "0"]
+type_definitions: []
+type_bounds: []
+type_defaults: []
+method: "main"
+static: true
+offset: 183 # at the first 'debugger' call.
+scriptUri: main.dart
+expression: |
+  str.isNullOrEmpty

pkg/front_end/testcases/expression/issue_57040_01.expression.yaml.expect

@@ -0,0 +1,4 @@
+Errors: {
+}
+static method /* from org-dartlang-debug:synthetic_debug_expression */ debugExpr(dart.core::_OneByteString str) → dynamic
+  return #lib1::_extension#0|get#isNullOrEmpty(str);

pkg/front_end/testcases/expression/issue_57040_02.expression.yaml

@@ -0,0 +1,47 @@
+# Copyright (c) 2024, 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.
+
+# https://github.com/dart-lang/sdk/issues/57040
+
+# Definition, offset, method etc extracted by starting the VM with
+# `-DDFE_VERBOSE=true`, e.g.
+# ```
+# out/ReleaseX64/dart -DDFE_VERBOSE=true --enable-vm-service \
+# --serve-observatory --disable-service-auth-codes --pause_isolates_on_start \
+# inputFile.dart
+# ```
+# and then issuing the expression compilation. It will then print stuff like
+# `DFE: request[6]: [dart:core, _OneByteString, 1, 0]` in the terminal.
+
+sources: |
+  import "dart:developer";
+
+  extension on String? {
+    bool get isNullOrEmpty {
+      var str = this;
+      return str == null || str.isEmpty;
+    }
+  }
+
+  void main() {
+    String? str = "hello";
+    debugger();
+    print(str.isNullOrEmpty);
+    str = null;
+    debugger();
+    print(str.isNullOrEmpty);
+  }
+
+definitions: ["str"]
+# dynamic
+definition_types: ["null"]
+type_definitions: []
+type_bounds: []
+type_defaults: []
+method: "main"
+static: true
+offset: 239 # at the second 'debugger' call.
+scriptUri: main.dart
+expression: |
+  str.isNullOrEmpty

pkg/front_end/testcases/expression/issue_57040_02.expression.yaml.expect

@@ -0,0 +1,4 @@
+Errors: {
+}
+static method /* from org-dartlang-debug:synthetic_debug_expression */ debugExpr(dart.core::String? str) → dynamic
+  return #lib1::_extension#0|get#isNullOrEmpty(str);

pkg/pkg.status

@@ -161,6 +161,7 @@ vm_service/test/issue_27238_test: SkipByDesign # Debugger is disabled in AOT mod
 vm_service/test/issue_27287_test: SkipByDesign # Debugger is disabled in AOT mode.
 vm_service/test/issue_30555_test: SkipByDesign # Debugger is disabled in AOT mode.
 vm_service/test/issue_56911_test: SkipByDesign # Debugger is disabled in AOT mode.
+vm_service/test/issue_57040_test: SkipByDesign # Debugger is disabled in AOT mode.
 vm_service/test/issue_59661_test: SkipByDesign # Debugger is disabled in AOT mode.
 vm_service/test/kill_paused_test: SkipByDesign # Debugger is disabled in AOT mode.
 vm_service/test/library_dependency_test: SkipByDesign # Uses 'dart:mirrors' library.

pkg/vm_service/test/issue_57040_test.dart

@@ -0,0 +1,96 @@
+// Copyright (c) 2024, 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.
+
+import 'dart:developer';
+import 'package:test/test.dart';
+import 'package:vm_service/vm_service.dart';
+import 'common/service_test_common.dart';
+import 'common/test_helper.dart';
+
+// AUTOGENERATED START
+//
+// Update these constants by running:
+//
+// dart pkg/vm_service/test/update_line_numbers.dart <test.dart>
+//
+const LINE_A = 30;
+const LINE_B = 33;
+// AUTOGENERATED END
+
+extension on String? {
+  bool get isNullOrEmpty {
+    final str = this;
+    return str == null || str.isEmpty;
+  }
+}
+
+void code() {
+  String? str = 'hello';
+  debugger(); // LINE_A
+  print(str.isNullOrEmpty);
+  str = null;
+  debugger(); // LINE_B
+  print(str.isNullOrEmpty);
+}
+
+Future<void> Function(VmService, IsolateRef) test(
+  String topFrameName,
+  List<String> availableVariables,
+  List<(String evaluate, String evaluationResult)> evaluations,
+) {
+  return (VmService service, IsolateRef isolateRef) async {
+    final isolateId = isolateRef.id!;
+    final stack = await service.getStack(isolateId);
+
+    // Make sure we are in the right place.
+    expect(stack.frames!.length, greaterThanOrEqualTo(1));
+    expect(stack.frames![0].function!.name, topFrameName);
+
+    // Check variables.
+    expect(
+      (stack.frames![0].vars ?? []).map((v) => v.name).toList(),
+      equals(availableVariables),
+    );
+
+    // Evaluate.
+    for (final (expression, expectedResult) in evaluations) {
+      final result = await service.evaluateInFrame(
+        isolateId,
+        /* frame = */ 0,
+        expression,
+      ) as InstanceRef;
+      print(result.valueAsString);
+      expect(result.valueAsString, equals(expectedResult));
+    }
+  };
+}
+
+final tests = <IsolateTest>[
+  hasStoppedAtBreakpoint,
+  stoppedAtLine(LINE_A),
+  test('code', [
+    'str',
+  ], [
+    ('() { return str.isNullOrEmpty; }()', 'false'),
+    ('str.isNullOrEmpty', 'false'),
+  ]),
+  resumeIsolate,
+  hasStoppedAtBreakpoint,
+  stoppedAtLine(LINE_B),
+  test('code', [
+    'str',
+  ], [
+    ('() { return str.isNullOrEmpty; }()', 'true'),
+    ('str.isNullOrEmpty', 'true'),
+  ]),
+];
+
+void main([args = const <String>[]]) => runIsolateTests(
+      args,
+      tests,
+      'issue_57040_test.dart',
+      testeeConcurrent: code,
+      pauseOnStart: false,
+      pauseOnExit: true,
+    );