Do not add variables to scope that appear after the breakpoint
During expression evaluation, we are collecting all variables in
all scopes that contain the current breakpoint line, and adding them
as available in current scope, which makes variables declared below
the current breakpoint line declared but undefined in JavaScript.
There is one exception to this seen so far - a variable
declared on the current breakpoint line might appear as not declared
in JavaScript and cause expression evaluation not to work due to
JS compilation errors.
This change fixes the issue by not collecting variables that are
declared on or after the current breakpoint line, making them undeclared
in dart (which also is correct according to dart scoping rules).
Closes: https://github.com/flutter/flutter/issues/72094
Change-Id: I113b69531171e0348d44edb8db6dd08a599c9db3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/177760
Commit-Queue: Anna Gringauze <annagrin@google.com>
Reviewed-by: Mark Zhou <markzipan@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
diff --git a/pkg/dev_compiler/lib/src/kernel/expression_compiler.dart b/pkg/dev_compiler/lib/src/kernel/expression_compiler.dart
index 6af9fa0..8c47904 100644
--- a/pkg/dev_compiler/lib/src/kernel/expression_compiler.dart
+++ b/pkg/dev_compiler/lib/src/kernel/expression_compiler.dart
@@ -163,8 +163,14 @@
@override
void visitVariableDeclaration(VariableDeclaration decl) {
- // collect locals and formals
- _definitions[decl.name] = decl.type;
+ // Collect locals and formals appearing before current breakpoint.
+ // Note that we include variables with no offset because the offset
+ // is not set in many cases in generated code, so omitting them would
+ // make expression evaluation fail in too many cases.
+ // Issue: https://github.com/dart-lang/sdk/issues/43966
+ if (decl.fileOffset < 0 || decl.fileOffset < _offset) {
+ _definitions[decl.name] = decl.type;
+ }
super.visitVariableDeclaration(decl);
}
diff --git a/pkg/dev_compiler/test/expression_compiler/expression_compiler_test.dart b/pkg/dev_compiler/test/expression_compiler/expression_compiler_test.dart
index d6983f9..108bc1e 100644
--- a/pkg/dev_compiler/test/expression_compiler/expression_compiler_test.dart
+++ b/pkg/dev_compiler/test/expression_compiler/expression_compiler_test.dart
@@ -292,6 +292,129 @@
group('Unsound null safety:', () {
var options = SetupCompilerOptions(false);
+ group('Expression compiler scope collection tests', () {
+ var source = '''
+ ${options.dartLangComment}
+
+ class C {
+ C(int this.field);
+
+ int methodFieldAccess(int x) {
+ var inScope = 1;
+ {
+ var innerInScope = global + staticField + field;
+ /* evaluation placeholder */
+ print(innerInScope);
+ var innerNotInScope = 2;
+ }
+ var notInScope = 3;
+ }
+
+ static int staticField = 0;
+ int field;
+ }
+
+ int global = 42;
+ main() => 0;
+ ''';
+
+ TestDriver driver;
+
+ setUp(() {
+ driver = TestDriver(options, source);
+ });
+
+ tearDown(() {
+ driver.delete();
+ });
+
+ test('local in scope', () async {
+ await driver.check(
+ scope: <String, String>{'inScope': '1', 'innerInScope': '0'},
+ expression: 'inScope',
+ expectedResult: '''
+ (function(inScope, innerInScope) {
+ return inScope;
+ }.bind(this)(
+ 1,
+ 0
+ ))
+ ''');
+ });
+
+ test('local in inner scope', () async {
+ await driver.check(
+ scope: <String, String>{'inScope': '1', 'innerInScope': '0'},
+ expression: 'innerInScope',
+ expectedResult: '''
+ (function(inScope, innerInScope) {
+ return innerInScope;
+ }.bind(this)(
+ 1,
+ 0
+ ))
+ ''');
+ });
+
+ test('global in scope', () async {
+ await driver.check(
+ scope: <String, String>{'inScope': '1', 'innerInScope': '0'},
+ expression: 'global',
+ expectedResult: '''
+ (function(inScope, innerInScope) {
+ return foo.global;
+ }.bind(this)(
+ 1,
+ 0
+ ))
+ ''');
+ });
+
+ test('static field in scope', () async {
+ await driver.check(
+ scope: <String, String>{'inScope': '1', 'innerInScope': '0'},
+ expression: 'staticField',
+ expectedResult: '''
+ (function(inScope, innerInScope) {
+ return foo.C.staticField;
+ }.bind(this)(
+ 1,
+ 0
+ ))
+ ''');
+ });
+
+ test('field in scope', () async {
+ await driver.check(
+ scope: <String, String>{'inScope': '1', 'innerInScope': '0'},
+ expression: 'field',
+ expectedResult: '''
+ (function(inScope, innerInScope) {
+ return this.field;
+ }.bind(this)(
+ 1,
+ 0
+ ))
+ ''');
+ });
+
+ test('local not in scope', () async {
+ await driver.check(
+ scope: <String, String>{'inScope': '1', 'innerInScope': '0'},
+ expression: 'notInScope',
+ expectedError:
+ "Error: The getter 'notInScope' isn't defined for the class 'C'.");
+ });
+
+ test('local not in inner scope', () async {
+ await driver.check(
+ scope: <String, String>{'inScope': '1', 'innerInScope': '0'},
+ expression: 'innerNotInScope',
+ expectedError:
+ "Error: The getter 'innerNotInScope' isn't defined for the class 'C'.");
+ });
+ });
+
group('Expression compiler tests in extension method:', () {
var source = '''
${options.dartLangComment}
@@ -2024,6 +2147,129 @@
group('Sound null safety:', () {
var options = SetupCompilerOptions(true);
+ group('Expression compiler scope collection tests', () {
+ var source = '''
+ ${options.dartLangComment}
+
+ class C {
+ C(int this.field);
+
+ int methodFieldAccess(int x) {
+ var inScope = 1;
+ {
+ var innerInScope = global + staticField + field;
+ /* evaluation placeholder */
+ print(innerInScope);
+ var innerNotInScope = 2;
+ }
+ var notInScope = 3;
+ }
+
+ static int staticField = 0;
+ int field;
+ }
+
+ int global = 42;
+ main() => 0;
+ ''';
+
+ TestDriver driver;
+
+ setUp(() {
+ driver = TestDriver(options, source);
+ });
+
+ tearDown(() {
+ driver.delete();
+ });
+
+ test('local in scope', () async {
+ await driver.check(
+ scope: <String, String>{'inScope': '1', 'innerInScope': '0'},
+ expression: 'inScope',
+ expectedResult: '''
+ (function(inScope, innerInScope) {
+ return inScope;
+ }.bind(this)(
+ 1,
+ 0
+ ))
+ ''');
+ });
+
+ test('local in inner scope', () async {
+ await driver.check(
+ scope: <String, String>{'inScope': '1', 'innerInScope': '0'},
+ expression: 'innerInScope',
+ expectedResult: '''
+ (function(inScope, innerInScope) {
+ return innerInScope;
+ }.bind(this)(
+ 1,
+ 0
+ ))
+ ''');
+ });
+
+ test('global in scope', () async {
+ await driver.check(
+ scope: <String, String>{'inScope': '1', 'innerInScope': '0'},
+ expression: 'global',
+ expectedResult: '''
+ (function(inScope, innerInScope) {
+ return foo.global;
+ }.bind(this)(
+ 1,
+ 0
+ ))
+ ''');
+ });
+
+ test('static field in scope', () async {
+ await driver.check(
+ scope: <String, String>{'inScope': '1', 'innerInScope': '0'},
+ expression: 'staticField',
+ expectedResult: '''
+ (function(inScope, innerInScope) {
+ return foo.C.staticField;
+ }.bind(this)(
+ 1,
+ 0
+ ))
+ ''');
+ });
+
+ test('field in scope', () async {
+ await driver.check(
+ scope: <String, String>{'inScope': '1', 'innerInScope': '0'},
+ expression: 'field',
+ expectedResult: '''
+ (function(inScope, innerInScope) {
+ return this.field;
+ }.bind(this)(
+ 1,
+ 0
+ ))
+ ''');
+ });
+
+ test('local not in scope', () async {
+ await driver.check(
+ scope: <String, String>{'inScope': '1', 'innerInScope': '0'},
+ expression: 'notInScope',
+ expectedError:
+ "Error: The getter 'notInScope' isn't defined for the class 'C'.");
+ });
+
+ test('local not in inner scope', () async {
+ await driver.check(
+ scope: <String, String>{'inScope': '1', 'innerInScope': '0'},
+ expression: 'innerNotInScope',
+ expectedError:
+ "Error: The getter 'innerNotInScope' isn't defined for the class 'C'.");
+ });
+ });
+
group('Expression compiler tests in extension method:', () {
var source = '''
${options.dartLangComment}
diff --git a/pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_test.dart b/pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_test.dart
index 68357f8..a8af750 100644
--- a/pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_test.dart
+++ b/pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_test.dart
@@ -119,7 +119,7 @@
extension NumberParsing on String {
int parseInt() {
var ret = int.parse(this);
- // line 17
+ // line 18
return ret;
}
}
@@ -336,7 +336,7 @@
requestController.add({
'command': 'CompileExpression',
'expression': 'ret',
- 'line': 17,
+ 'line': 18,
'column': 1,
'jsModules': {},
'jsScope': {'ret': 'ret'},
