remove private fields and modules calculations from expression compiler

- Remove collecting private fields from scope calculation
  We only need to collect the fields from the evaluated
  expression and they are already collected separately.

- Correct private fields calculation
  Currently, library names are assumed to be the same as the module,
  which creates redefinitions of library variable in produced Javascript
  code, and exceptions in chrome for modules that contain more than one
  library.
  The actual fix will come from making DDC incremental by storing more
  information, for now make a best effort to compute correct names for
  library variables from the module data that expression compiler
  receives from chrome. The following code will be produced to calculate
  private symbols _pf1 (from lib1, module1) and _pf2
  (from lib2 module1):

  let lib1 = require('module1.dart').lib1;
  let lib2 = require('module1.dart').lib2;

  let _pf1 = dart.privateNames(lib1, '_pf1');
  let _pf2 = dart.privateNames(lib2, '_pf2');

  Note that this seems to work even if the current breakpoint is inside
  the define statement for lib1, and the lib1 symbol is not exported
  yet.

Closes: https://github.com/dart-lang/sdk/issues/40272
Closes: https://github.com/dart-lang/sdk/issues/41585
Change-Id: I141719d2d5d5c08dd3c0ef5f0406756dce5575ab
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/145307
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Anna Gringauze <annagrin@google.com>
This commit is contained in:
Anna Gringauze 2020-05-05 17:41:17 +00:00 committed by commit-bot@chromium.org
parent 8061d2d6cc
commit 6f16174d56
2 changed files with 60 additions and 237 deletions

View file

@ -34,9 +34,8 @@ import 'package:kernel/ast.dart'
VariableDeclaration,
Visitor;
// TODO(annagrin): remove private fields
// See [issue 40272](https://github.com/dart-lang/sdk/issues/40272)
/// Dart scope
///
/// Provides information about symbols available inside a dart scope.
class DartScope {
final Library library;
@ -44,21 +43,21 @@ class DartScope {
final Procedure procedure;
final Map<String, DartType> definitions;
final List<TypeParameter> typeParameters;
final Set<String> privateFields;
DartScope(this.library, this.cls, this.procedure, this.definitions,
this.typeParameters, this.privateFields);
this.typeParameters);
@override
String toString() {
return '''DartScope {
Library = ${library.importUri},
Class ${cls?.name},
Procedure $procedure,
isStatic ${procedure.isStatic},
Library: ${library.importUri},
Class: ${cls?.name},
Procedure: $procedure,
isStatic: ${procedure.isStatic},
Scope: $definitions,
typeParameters: $typeParameters,
privateFields: $privateFields''';
typeParameters: $typeParameters
}
''';
}
}
@ -77,7 +76,6 @@ class DartScopeBuilder extends Visitor<void> {
final int _line;
final int _column;
int _offset;
final Set<String> _privateFields = {};
final Map<String, DartType> _definitions = {};
final List<TypeParameter> _typeParameters = [];
@ -86,8 +84,7 @@ class DartScopeBuilder extends Visitor<void> {
DartScope build() {
if (_library == null || _procedure == null) return null;
return DartScope(_library, _cls, _procedure, _definitions, _typeParameters,
_privateFields);
return DartScope(_library, _cls, _procedure, _definitions, _typeParameters);
}
@override
@ -113,20 +110,6 @@ class DartScopeBuilder extends Visitor<void> {
}
}
@override
void visitField(Field node) {
if (node.name.isPrivate) {
_privateFields.add(node.name.name);
}
}
@override
void visitFieldReference(Field node) {
if (node.name.isPrivate) {
_privateFields.add(node.name.name);
}
}
@override
void visitProcedure(Procedure p) {
if (_scopeContainsOffset(p.fileOffset, p.fileEndOffset, _offset)) {
@ -184,35 +167,38 @@ class DartScopeBuilder extends Visitor<void> {
}
class PrivateFieldsVisitor extends Visitor<void> {
final Set<String> _privateFields = {};
Set<String> getPrivateFields() {
return _privateFields;
}
final Map<String, String> privateFields = {};
@override
void defaultNode(Node node) {
node.visitChildren(this);
}
@override
void visitFieldReference(Field node) {
if (node.name.isPrivate) {
privateFields[node.name.name] = node.name.library.importUri.toString();
}
}
@override
void visitField(Field node) {
if (node.name.isPrivate) {
_privateFields.add(node.name.name);
privateFields[node.name.name] = node.name.library.importUri.toString();
}
}
@override
void visitPropertyGet(PropertyGet node) {
if (node.name.isPrivate) {
_privateFields.add(node.name.name);
privateFields[node.name.name] = node.name.library.importUri.toString();
}
}
@override
void visitPropertySet(PropertySet node) {
if (node.name.isPrivate) {
_privateFields.add(node.name.name);
privateFields[node.name.name] = node.name.library.importUri.toString();
}
}
}
@ -428,8 +414,6 @@ error.name + ": " + error.message;
Map<String, String> modules,
String currentModule,
String expression) async {
var currentModuleVariable = currentModule.split('/').last;
// 1. Compile expression to kernel AST
var procedure = await _compiler.compileExpression(
@ -459,57 +443,51 @@ error.name + ": " + error.message;
var jsFun = _kernel2jsCompiler.emitFunction(
procedure.function, '$debugProcedureName');
// 3. apply (hopefully temporary) workarounds for what ideally
// need to be done in FE
// Unused symbols are not captured inside functions, for example,
// core.print is not available inside a top-level function foo()
// if foo does not use anything from core.
// 3. apply temporary workarounds for what ideally
// needs to be done in the compiler
// get private fields accessed by the evaluated expression
var fieldsCollector = PrivateFieldsVisitor();
procedure.accept(fieldsCollector);
var privateFields = fieldsCollector.getPrivateFields();
privateFields
.removeWhere((String name) => scope.privateFields.contains(name));
var privateFields = fieldsCollector.privateFields;
// collect libraries where private fields are defined
var currentLibraries = <String, String>{};
var currentModules = <String, String>{};
for (var variable in modules.keys) {
var module = modules[variable];
for (var field in privateFields.keys) {
var library = privateFields[field];
var libraryVariable =
library.replaceAll('.dart', '').replaceAll('/', '__');
if (libraryVariable.endsWith(variable)) {
if (currentLibraries[field] != null) {
onDiagnostic(_createInternalError(
scope.library.importUri,
0,
0,
'ExpressionCompiler: $field defined in more than one library: '
'${currentLibraries[field]}, $variable'));
return null;
}
currentLibraries[field] = variable;
currentModules[variable] = module;
}
}
}
_log('ExpressionCompiler: privateFields: $privateFields');
_log('ExpressionCompiler: currentLibraries: $currentLibraries');
_log('ExpressionCompiler: currentModules: $currentModules');
var body = js_ast.Block([
// require dart, core, self and other modules
...modules.keys.map((String variable) {
var module = modules[variable];
_log('ExpressionCompiler: '
'module: $module, '
'variable: $variable, '
'currentModule: $currentModule');
return _createRequireModuleStatement(
module,
// Inside a module, the library variable compiler creates is the
// file name without extension (currentModuleVariable).
//
// Inside a non-package module, the statement we need to produce
// to reload the library is
// 'main = require('web/main.dart').web__main'
//
// This is the only special case where currentModuleVariable
// ('main') is different from variable and field ('web_name')
//
// In all other cases, the variable, currentModuleVariable and
// the field are the same:
// 'library = require('packages/_test/library.dart').library'
//
// TODO(annagrin): save metadata decribing the variables and fields
// to use during compilation and use this information here to make
// expression compilation resilient to name convention changes
// See [issue 891](https://github.com/dart-lang/webdev/issues/891)
module == currentModule ? currentModuleVariable : variable,
variable);
}),
// require modules used in evaluated expression
...currentModules.keys.map((String variable) =>
_createRequireModuleStatement(
currentModules[variable], variable, variable)),
// re-create private field accessors
...scope.privateFields
.map((String v) => _createPrivateField(v, currentModuleVariable)),
...privateFields
.map((String v) => _createPrivateField(v, currentModuleVariable)),
...currentLibraries.keys
.map((String k) => _createPrivateField(k, currentLibraries[k])),
// statements generated by the FE
...jsFun.body.statements
]);

View file

@ -286,9 +286,6 @@ int main() {
expression: 'ret',
expectedResult: '''
(function(ret) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
return ret;
}(
1234
@ -359,11 +356,6 @@ int main() {
expression: 'x',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return x;
}.bind(this)(
1
@ -377,11 +369,6 @@ int main() {
expression: 'this',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return this;
}.bind(this)(
1
@ -395,11 +382,6 @@ int main() {
expression: 'x + 1',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return dart.dsend(x, '+', [1]);
}.bind(this)(
1
@ -413,11 +395,6 @@ int main() {
expression: 'x + staticField',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return dart.dsend(x, '+', [foo.C.staticField]);
}.bind(this)(
1
@ -432,9 +409,6 @@ int main() {
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return dart.dsend(x, '+', [foo.C._staticField]);
}.bind(this)(
@ -449,11 +423,6 @@ int main() {
expression: 'x + field',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return dart.dsend(x, '+', [this.field]);
}.bind(this)(
1
@ -468,10 +437,7 @@ int main() {
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return dart.dsend(x, '+', [this[_field]]);
}.bind(this)(
1
@ -485,11 +451,6 @@ int main() {
expression: 'x + global',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return dart.dsend(x, '+', [foo.global]);
}.bind(this)(
1
@ -503,11 +464,6 @@ int main() {
expression: 'methodFieldAccess(2)',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return this.methodFieldAccess(2);
}.bind(this)(
1
@ -521,11 +477,6 @@ int main() {
expression: 'asyncMethod(2)',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return this.asyncMethod(2);
}.bind(this)(
1
@ -539,11 +490,6 @@ int main() {
expression: '"1234".parseInt()',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return foo['NumberParsing|parseInt']("1234");
}.bind(this)(
1
@ -558,10 +504,7 @@ int main() {
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return this[_field] = 2;
}.bind(this)(
1
@ -575,11 +518,6 @@ int main() {
expression: 'field = 2',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return this.field = 2;
}.bind(this)(
1
@ -594,9 +532,6 @@ int main() {
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return foo.C._staticField = 2;
}.bind(this)(
@ -611,11 +546,6 @@ int main() {
expression: 'staticField = 2',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return foo.C.staticField = 2;
}.bind(this)(
1
@ -678,11 +608,6 @@ int main() {
expression: 'x + staticField',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return dart.dsend(x, '+', [foo.C.staticField]);
}.bind(this)(
1
@ -697,9 +622,6 @@ int main() {
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return dart.dsend(x, '+', [foo.C._staticField]);
}.bind(this)(
@ -714,11 +636,6 @@ int main() {
expression: 'x + field',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return dart.dsend(x, '+', [this.field]);
}.bind(this)(
1
@ -733,10 +650,7 @@ int main() {
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return dart.dsend(x, '+', [this[_field]]);
}.bind(this)(
1
@ -751,10 +665,7 @@ int main() {
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return this[_field] = 2;
}.bind(this)(
1
@ -768,11 +679,6 @@ int main() {
expression: 'field = 2',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return this.field = 2;
}.bind(this)(
1
@ -787,9 +693,6 @@ int main() {
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return foo.C._staticField = 2;
}.bind(this)(
@ -804,11 +707,6 @@ int main() {
expression: 'staticField = 2',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
let _staticField = dart.privateName(foo, "_staticField");
return foo.C.staticField = 2;
}.bind(this)(
1
@ -856,10 +754,6 @@ int main() {
expression: 'x',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
return x;
}.bind(this)(
1
@ -873,10 +767,6 @@ int main() {
expression: 'this',
expectedResult: '''
(function(x) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
return this;
}.bind(this)(
1
@ -946,9 +836,6 @@ int main() {
expression: 'x',
expectedResult: '''
(function(x, c) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
return x;
}(
1, null
@ -962,9 +849,6 @@ int main() {
expression: 'c',
expectedResult: '''
(function(x, c) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
return c;
}(
1, null
@ -978,9 +862,6 @@ int main() {
expression: 'C(1,3)',
expectedResult: '''
(function(x, c) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
return new foo.C.new(1, 3);
}(
1, null
@ -995,8 +876,6 @@ int main() {
expectedResult: '''
(function(x, c) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
return new foo.C.new(1, 3)[_field];
}(
@ -1011,9 +890,6 @@ int main() {
expression: 'C.staticField',
expectedResult: '''
(function(x, c) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
return foo.C.staticField;
}(
1, null
@ -1034,9 +910,6 @@ int main() {
expression: 'c.field',
expectedResult: '''
(function(x, c) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
return dart.dloadRepl(c, 'field');
}(
1, null
@ -1051,8 +924,6 @@ int main() {
expectedResult: '''
(function(x, c) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
return dart.dloadRepl(c, _field);
}(
@ -1067,9 +938,6 @@ int main() {
expression: 'c.methodFieldAccess(2)',
expectedResult: '''
(function(x, c) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
return dart.dsendRepl(c, 'methodFieldAccess', [2]);
}(
1, null
@ -1083,9 +951,6 @@ int main() {
expression: 'c.asyncMethod(2)',
expectedResult: '''
(function(x, c) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
return dart.dsendRepl(c, 'asyncMethod', [2]);
}(
1, null
@ -1099,9 +964,6 @@ int main() {
expression: '"1234".parseInt()',
expectedResult: '''
(function(x, c) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
return foo['NumberParsing|parseInt']("1234");
}(
1, null
@ -1116,8 +978,6 @@ int main() {
expectedResult: '''
(function(x, c) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
let _field = dart.privateName(foo, "_field");
return dart.dputRepl(c, _field, 2);
}(
@ -1132,9 +992,6 @@ int main() {
expression: 'c.field = 2',
expectedResult: '''
(function(x, c) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
return dart.dputRepl(c, 'field', 2);
}(
1, null
@ -1155,9 +1012,6 @@ int main() {
expression: 'C.staticField = 2',
expectedResult: '''
(function(x, c) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
return foo.C.staticField = 2;
}(
1, null
@ -1171,9 +1025,6 @@ int main() {
expression: 'print(x)',
expectedResult: '''
(function(x, c) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
return core.print(x);
}(
1, null
@ -1225,9 +1076,6 @@ int main() {
expression: r"'$x+$y+$z'",
expectedResult: '''
(function(x, c, y, z) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
return dart.str(x) + "+" + dart.str(y) + "+" + dart.str(z);
}(
1, null, 3, 0
@ -1241,9 +1089,6 @@ int main() {
expression: r"'$y+$z'",
expectedResult: '''
(function(x, c, y, z) {
let foo = require('foo.dart').foo;
let dart = require('dart_sdk').dart;
let core = require('dart_sdk').core;
return dart.str(y) + "+" + dart.str(z);
}(
1, null, 3, 0