languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-10-14 10:18:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

[frontend_server] Make module names debugger-friendly

Browse source 
Module names and import module names generated by the frontend server
are currently breaking debugger's assumptions on module names. This
change fixes it to make debugger works with various compiler
configurations, such as build_runner, frontend server with dwds tests,
and unblocks flutter tools to continue debugger and frontend server
integration.

JavaScriptBundle
  - Change module import names to module names instead of file paths,
    omit leading '/' as it cannot be in a module name in requireJS
  - Module name changes are hidden under "debugger-module-names" flag
    to prevent breaking flutter tools.

FrontendServer
  - Always call exit after listening to commands is finished,
    otherwise process running frontend server does not exit on 'quit'
    command.
  - Call computeCanonicalNames after initial compilation to support
    subsequent module importing in compileExpressioToJs calls
  - Add error handling to usage of kernelToJs compilers

ExpressionCompiler
  - Separate library variables from library fields for loading modules,
    as they can be different, for example
    ```main = require('web/main.dart').web__main```

Closes https://github.com/dart-lang/sdk/issues/40832

Change-Id: Ic103c22932c7181ee325f239193d32361d057ed0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/138010
Commit-Queue: Anna Gringauze <annagrin@google.com>
Reviewed-by: Jonah Williams <jonahwilliams@google.com>
Reviewed-by: Gary Roumanis <grouma@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
This commit is contained in:
Anna Gringauze 2020-03-05 07:39:33 +00:00 committed by commit-bot@chromium.org
parent d1d89c8ce1
commit 7466c1f9da
6 changed files with 121 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
pkg/frontend_server/bin/frontend_server_starter.dart
View file

@ -6,7 +6,7 @@ import 'dart:io';
import '../lib/frontend_server.dart';
Future<Null> main(List<String> args) async {
final int exitCode = await starter(args);
exitCode = await starter(args);
if (exitCode != 0) {
exit(exitCode);
}

39
pkg/frontend_server/lib/frontend_server.dart
View file

@ -19,7 +19,7 @@ import 'package:dev_compiler/dev_compiler.dart' show DevCompilerTarget;
import 'package:front_end/src/api_prototype/compiler_options.dart'
show CompilerOptions, parseExperimentalFlags;
import 'package:front_end/src/api_unstable/vm.dart';
import 'package:kernel/ast.dart';
import 'package:kernel/ast.dart' show Library, Procedure, LibraryDependency;
import 'package:kernel/binary/ast_to_binary.dart';
import 'package:kernel/kernel.dart'
show Component, loadComponentSourceFromBytes;
@ -160,7 +160,9 @@ ArgParser argParser = ArgParser(allowTrailingOptions: true)
help: 'Name of the subdirectory of //data for output files')
..addOption('far-manifest', help: 'Path to output Fuchsia package manifest')
..addOption('libraries-spec',
help: 'A path or uri to the libraries specification JSON file');
help: 'A path or uri to the libraries specification JSON file')
..addFlag('debugger-module-names',
help: 'Use debugger-friendly modules names', defaultsTo: false);
String usage = '''
Usage: server [options] [input.dart]
@ -306,7 +308,8 @@ class FrontendCompiler implements CompilerInterface {
{this.printerFactory,
this.transformer,
this.unsafePackageSerialization,
this.incrementalSerialization: true}) {
this.incrementalSerialization: true,
this.useDebuggerModuleNames: false}) {
_outputStream ??= stdout;
printerFactory ??= new BinaryPrinterFactory();
}
@ -315,6 +318,7 @@ class FrontendCompiler implements CompilerInterface {
BinaryPrinterFactory printerFactory;
bool unsafePackageSerialization;
bool incrementalSerialization;
bool useDebuggerModuleNames;
CompilerOptions _compilerOptions;
BytecodeOptions _bytecodeOptions;
@ -492,6 +496,7 @@ class FrontendCompiler implements CompilerInterface {
incrementalSerializer = _generator.incrementalSerializer;
_component = component;
_component.computeCanonicalNames();
} else {
if (options['link-platform']) {
// TODO(aam): Remove linkedDependencies once platform is directly embedded
@ -603,7 +608,8 @@ class FrontendCompiler implements CompilerInterface {
sourceFile.parent.createSync(recursive: true);
}
_bundler = JavaScriptBundler(
component, strongComponents, fileSystemScheme, packageConfig);
component, strongComponents, fileSystemScheme, packageConfig,
useDebuggerModuleNames: useDebuggerModuleNames);
final sourceFileSink = sourceFile.openWrite();
final manifestFileSink = manifestFile.openWrite();
final sourceMapsFileSink = sourceMapsFile.openWrite();
@ -904,6 +910,11 @@ class FrontendCompiler implements CompilerInterface {
if (_bundler != null) {
var kernel2jsCompiler = _bundler.compilers[moduleName];
if (kernel2jsCompiler == null) {
throw Exception('Cannot find kernel2js compiler for $moduleName. '
'Compilers are avaiable for modules: '
'\n\t${_bundler.compilers.keys.toString()}');
}
assert(kernel2jsCompiler != null);
var evaluator = new ExpressionCompiler(
@ -914,7 +925,7 @@ class FrontendCompiler implements CompilerInterface {
var procedure = await evaluator.compileExpressionToJs(libraryUri, line,
column, jsModules, jsFrameValues, moduleName, expression);
var result = procedure ?? errors[0];
var result = errors.length > 0 ? errors[0] : procedure;
// TODO(annagrin): kernelBinaryFilename is too specific
// rename to _outputFileName?
@ -1117,15 +1128,15 @@ class _CompileExpressionToJsRequest {
/// Listens for the compilation commands on [input] stream.
/// This supports "interactive" recompilation mode of execution.
void listenAndCompile(CompilerInterface compiler, Stream<List<int>> input,
ArgResults options, Completer<int> completer,
StreamSubscription<String> listenAndCompile(CompilerInterface compiler,
Stream<List<int>> input, ArgResults options, Completer<int> completer,
{IncrementalCompiler generator}) {
_State state = _State.READY_FOR_INSTRUCTION;
_CompileExpressionRequest compileExpressionRequest;
_CompileExpressionToJsRequest compileExpressionToJsRequest;
String boundaryKey;
String recompileEntryPoint;
input
return input
.transform(utf8.decoder)
.transform(const LineSplitter())
.listen((String string) async {
@ -1195,7 +1206,9 @@ void listenAndCompile(CompilerInterface compiler, Stream<List<int>> input,
} else if (string == 'reset') {
compiler.resetIncrementalCompiler();
} else if (string == 'quit') {
completer.complete(0);
if (!completer.isCompleted) {
completer.complete(0);
}
}
break;
case _State.RECOMPILE_LIST:
@ -1362,7 +1375,8 @@ Future<int> starter(
compiler ??= FrontendCompiler(output,
printerFactory: binaryPrinterFactory,
unsafePackageSerialization: options["unsafe-package-serialization"],
incrementalSerialization: options["incremental-serialization"]);
incrementalSerialization: options["incremental-serialization"],
useDebuggerModuleNames: options['debugger-module-names']);
if (options.rest.isNotEmpty) {
return await compiler.compile(options.rest[0], options,
@ -1372,7 +1386,8 @@ Future<int> starter(
}
Completer<int> completer = Completer<int>();
listenAndCompile(compiler, input ?? stdin, options, completer,
var subscription = listenAndCompile(
compiler, input ?? stdin, options, completer,
generator: generator);
return completer.future;
return completer.future..then((value) => subscription.cancel());
}

75
pkg/frontend_server/lib/src/expression_compiler.dart
View file

@ -3,6 +3,7 @@
// found in the LICENSE file.
import 'dart:async';
import 'dart:io';
import 'package:_fe_analyzer_shared/src/messages/diagnostic_message.dart'
show DiagnosticMessage, DiagnosticMessageHandler;
@ -232,7 +233,10 @@ class ExpressionCompiler {
void _log(String message) {
if (verbose) {
print(message);
// writing to stdout breaks communication to
// frontend server, which is done on stdin/stdout,
// so we use stderr here instead
stderr.writeln(message);
}
}
@ -250,14 +254,14 @@ class ExpressionCompiler {
///
/// Returns expression compiled to JavaScript or null on error.
/// Errors are reported using onDiagnostic function
/// [moduleName] is of the form '/packages/hello_world_main.dart'
/// [moduleName] is of the form 'packages/hello_world_main.dart'
/// [jsFrameValues] is a map from js variable name to its primitive value
/// or another variable name, for example
/// { 'x': '1', 'y': 'y', 'o': 'null' }
/// [jsModules] is a map from variable name to the module name, where
/// variable name is the name originally used in JavaScript to contain the
/// module object, for example:
/// { 'dart':'dart_sdk', 'main': '/packages/hello_world_main.dart' }
/// { 'dart':'dart_sdk', 'main': 'packages/hello_world_main.dart' }
Future<String> compileExpressionToJs(
String libraryUri,
int line,
@ -270,8 +274,6 @@ class ExpressionCompiler {
_log('ExpressionCompiler: compiling: $expression in $moduleName');
var moduleVariable = moduleName.split('/').last;
var dartScope = await _findScopeAt(Uri.parse(libraryUri), line, column);
if (dartScope == null) {
_log('ExpressionCompiler: scope not found at $libraryUri:$line:$column');
@ -299,8 +301,8 @@ class ExpressionCompiler {
// 3. compile dart expression to JS
var jsExpression = await _compileExpression(
dartScope, jsModules, moduleVariable, expression);
var jsExpression =
await _compileExpression(dartScope, jsModules, moduleName, expression);
if (jsExpression == null) {
_log('ExpressionCompiler: failed to compile $expression, $jsExpression');
@ -370,10 +372,13 @@ error.name + ": " + error.message;
Future<Library> _getLibrary(Uri libraryUri) async {
return await _compiler.context.runInContext((_) async {
var builder = _compiler.userCode.loader.builders[libraryUri];
var library =
_compiler.userCode.loader.read(libraryUri, -1, accessor: builder);
if (builder != null) {
var library =
_compiler.userCode.loader.read(libraryUri, -1, accessor: builder);
return library.library;
return library.library;
}
return null;
});
}
@ -381,12 +386,12 @@ error.name + ": " + error.message;
/// example:
/// let dart = require('dart_sdk').dart;
js_ast.Statement _createRequireModuleStatement(
String moduleName, String moduleVariable) {
String moduleName, String moduleVariable, String fieldName) {
var variableName = moduleVariable.replaceFirst('.dart', '');
var rhs = js_ast.PropertyAccess.field(
js_ast.Call(js_ast.Identifier('require'),
[js_ast.LiteralExpression('\'$moduleName\'')]),
'$variableName');
'$fieldName');
return rhs.toVariableDeclaration(js_ast.Identifier('$variableName'));
}
@ -416,16 +421,18 @@ error.name + ": " + error.message;
/// [scope] current dart scope information
/// [modules] map from module variable names to module names in JavaScript
/// code. For example,
/// { 'dart':'dart_sdk', 'main': '/packages/hello_world_main.dart' }
/// { 'dart':'dart_sdk', 'main': 'packages/hello_world_main.dart' }
/// [currentModule] current js module name.
/// For example, in library package:hello_world/main.dart:
/// '/packages/hello_world/main.dart'
/// 'packages/hello_world/main.dart'
/// [expression] expression to compile in given [scope].
Future<String> _compileExpression(
DartScope scope,
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(
@ -437,6 +444,10 @@ error.name + ": " + error.message;
scope.cls?.name,
scope.procedure.isStatic);
// TODO(annagrin): The condition below seems to be always false.
// Errors are still correctly reported in the frontent_server,
// but we end up doing unnesessary work below.
// Add communication of error state from compiler here.
if (_compiler.context.errors.length > 0) {
return null;
}
@ -468,18 +479,46 @@ error.name + ": " + error.message;
var body = js_ast.Block([
// require dart, core, self and other modules
...modules.keys.map((String variable) {
return _createRequireModuleStatement(modules[variable], 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);
}),
// re-create private field accessors
...scope.privateFields
.map((String v) => _createPrivateField(v, currentModule)),
...privateFields.map((String v) => _createPrivateField(v, currentModule)),
.map((String v) => _createPrivateField(v, currentModuleVariable)),
...privateFields
.map((String v) => _createPrivateField(v, currentModuleVariable)),
// statements generated by the FE
...jsFun.body.statements
]);
var jsFunModified = js_ast.Fun(jsFun.params, body);
_log('ExpressionCompiler: JS AST: ${jsFunModified.toString()}');
_log('ExpressionCompiler: JS AST: $jsFunModified');
// 4. print JS ast to string for evaluation

36
pkg/frontend_server/lib/src/javascript_bundle.dart
View file

@ -24,11 +24,13 @@ import 'strong_components.dart';
/// only the updated libraries.
class JavaScriptBundler {
JavaScriptBundler(this._originalComponent, this._strongComponents,
this._fileSystemScheme, this._packageConfig)
this._fileSystemScheme, this._packageConfig,
{this.useDebuggerModuleNames = false})
: compilers = <String, ProgramCompiler>{} {
_summaries = <Component>[];
_summaryUris = <Uri>[];
_moduleImportForSummary = <Uri, String>{};
_moduleImportNameForSummary = <Uri, String>{};
_uriToComponent = <Uri, Component>{};
for (Uri uri in _strongComponents.modules.keys) {
final List<Library> libraries = _strongComponents.modules[uri].toList();
@ -39,7 +41,13 @@ class JavaScriptBundler {
);
_summaries.add(summaryComponent);
_summaryUris.add(uri);
_moduleImportForSummary[uri] = '${urlForComponentUri(uri)}.lib.js';
var baseName = urlForComponentUri(uri);
_moduleImportForSummary[uri] = '$baseName.lib.js';
if (useDebuggerModuleNames) {
_moduleImportNameForSummary[uri] = makeDebuggerModuleName(baseName);
}
_uriToComponent[uri] = summaryComponent;
}
}
@ -48,11 +56,13 @@ class JavaScriptBundler {
final Component _originalComponent;
final String _fileSystemScheme;
final PackageConfig _packageConfig;
final bool useDebuggerModuleNames;
final Map<String, ProgramCompiler> compilers;
List<Component> _summaries;
List<Uri> _summaryUris;
Map<Uri, String> _moduleImportForSummary;
Map<Uri, String> _moduleImportNameForSummary;
Map<Uri, Component> _uriToComponent;
/// Compile each component into a single JavaScript module.
@ -72,7 +82,10 @@ class JavaScriptBundler {
final summaryToModule = Map<Component, String>.identity();
for (var i = 0; i < _summaries.length; i++) {
var summary = _summaries[i];
var moduleImport = _moduleImportForSummary[_summaryUris[i]];
var moduleImport = useDebuggerModuleNames
// debugger loads modules by modules names, not paths
? _moduleImportNameForSummary[_summaryUris[i]]
: _moduleImportForSummary[_summaryUris[i]];
for (var l in summary.libraries) {
assert(!importToSummary.containsKey(l));
importToSummary[l] = summary;
@ -95,10 +108,15 @@ class JavaScriptBundler {
final summaryComponent = _uriToComponent[moduleUri];
// module name to use in trackLibraries
// use full path for tracking if module uri is not a package uri
final String moduleName = moduleUri.scheme == 'package'
? '/packages/${moduleUri.path}'
: moduleUri.path;
// use full path for tracking if module uri is not a package uri.
String moduleName = urlForComponentUri(moduleUri);
if (useDebuggerModuleNames) {
// Skip the leading '/' as module names are used to require
// modules using module paths mape in RequireJS, which treats
// names with leading '/' or '.js' extensions specially
// and tries to load them without mapping.
moduleName = makeDebuggerModuleName(moduleName);
}
var compiler = ProgramCompiler(
_originalComponent,
@ -160,3 +178,7 @@ class JavaScriptBundler {
String urlForComponentUri(Uri componentUri) => componentUri.scheme == 'package'
? '/packages/${componentUri.path}'
: componentUri.path;
String makeDebuggerModuleName(String name) {
return name.startsWith('/') ? name.substring(1) : name;
}

8
pkg/frontend_server/test/frontend_server_test.dart
View file

@ -548,7 +548,7 @@ void main() async {
];
var library = 'package:hello/foo.dart';
var module = '/packages/hello/foo.dart';
var module = 'packages/hello/foo.dart';
final StreamController<List<int>> streamController =
StreamController<List<int>>();
@ -1470,7 +1470,7 @@ true
..writeAsStringSync("hello:${tempDir.uri}\n");
var library = 'package:hello/foo.dart';
var module = '/packages/hello/foo.dart';
var module = 'packages/hello/foo.dart';
var dillFile = File('${tempDir.path}/foo.dart.dill');
var sourceFile = File('${dillFile.path}.sources');
@ -1486,6 +1486,7 @@ true
'--output-dill=${dillFile.path}',
'--target=dartdevc',
'--packages=${tempDir.path}/.packages',
'--debugger-module-names'
];
final StreamController<List<int>> streamController =
@ -1577,7 +1578,7 @@ true
..writeAsStringSync("hello:${tempDir.uri}\n");
var library = 'package:hello/foo.dart';
var module = '/packages/hello/foo.dart';
var module = 'packages/hello/foo.dart';
var dillFile = File('${tempDir.path}/foo.dart.dill');
var sourceFile = File('${dillFile.path}.sources');
@ -1593,6 +1594,7 @@ true
'--output-dill=${dillFile.path}',
'--target=dartdevc',
'--packages=${tempDir.path}/.packages',
'--debugger-module-names'
];
final StreamController<List<int>> streamController =

6
pkg/frontend_server/test/src/expression_compiler_test.dart
View file

@ -137,7 +137,8 @@ class TestCompiler {
// create expression compiler
var evaluator = new ExpressionCompiler(
compiler, kernel2jsCompiler, component,
verbose: false, onDiagnostic: setup.options.onDiagnostic);
verbose: setup.options.verbose,
onDiagnostic: setup.options.onDiagnostic);
// collect all module names and paths
Map<Uri, Module> moduleInfo = _collectModules(component);
@ -164,9 +165,6 @@ class TestCompiler {
return TestCompilationResult(jsExpression, false);
}
// use to generate expected test results
//print("\njsExpression: ${jsExpression}");
return TestCompilationResult(jsExpression, true);
}