Skip to content

Commit a79b7c6

Browse files
jensjohaCommit Queue
jensjoha
authored and
Commit Queue
committed
[kernel/CFE/DDC] Remove old DartScopeBuilder; use new one
Change-Id: I4805c47958e2d4fbc19acbdada23cad014c3a747 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/446042 Reviewed-by: Nicholas Shahan <[email protected]> Commit-Queue: Jens Johansen <[email protected]>
1 parent 187c3cb commit a79b7c6

File tree

9 files changed

+314
-477
lines changed

9 files changed

+314
-477
lines changed

‎pkg/dev_compiler/lib/src/kernel/expression_compiler.dart‎

Lines changed: 53 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import 'package:_fe_analyzer_shared/src/messages/codes.dart'
99
import 'package:_fe_analyzer_shared/src/messages/diagnostic_message.dart'
1010
show CfeDiagnosticMessage, DiagnosticMessageHandler;
1111
import 'package:front_end/src/api_unstable/ddc.dart';
12-
import 'package:kernel/ast.dart' show Component, Library;
12+
import 'package:kernel/ast.dart' show Component, Library, TreeNode;
1313
import 'package:kernel/dart_scope_calculator.dart';
1414

1515
import '../compiler/js_names.dart' as js_ast;
@@ -105,6 +105,7 @@ class ExpressionCompiler {
105105
scriptUri == null ? null : Uri.parse(scriptUri),
106106
line,
107107
column,
108+
jsScope,
108109
);
109110
if (dartScope == null) {
110111
_log('Scope not found at $libraryUri:$line:$column');
@@ -323,6 +324,7 @@ class ExpressionCompiler {
323324
Uri? scriptFileUri,
324325
int line,
325326
int column,
327+
Map<String, String> jsScope,
326328
) {
327329
if (line < 0) {
328330
onDiagnostic(
@@ -349,48 +351,65 @@ class ExpressionCompiler {
349351
return null;
350352
}
351353

352-
// TODO(jensj): Eventually make the scriptUri required and always use this,
353-
// but for now use the old mechanism when no script is provided.
354+
// TODO(jensj): Eventually make the scriptUri required.
354355
if (scriptFileUri != null) {
355-
final offset = _component.getOffset(library.fileUri, line, column);
356-
final scope2 = DartScopeBuilder2.findScopeFromOffset(
356+
final offset = _component.getOffset(scriptFileUri, line, column);
357+
final scope = DartScopeBuilder2.findScopeFromOffset(
357358
library,
358359
scriptFileUri,
359360
offset,
360361
);
361-
return scope2;
362+
return scope;
362363
}
363364

364-
var scope = DartScopeBuilder.findScope(_component, library, line, column);
365-
if (scope == null) {
366-
// Fallback mechanism to allow a evaluation of an expression at the
367-
// library level within the Dart SDK.
368-
//
369-
// Currently we lack the full dill and metadata for the Dart SDK module to
370-
// be able to use the same mechanism of expression evaluation as the rest
371-
// of a program. Because of that, expression evaluation at arbitrary
372-
// scopes is not supported in the Dart SDK. However, we can still support
373-
// compiling expressions that will be evaluated at the library level. We
374-
// determine if that's the case by recognizing that all such requests use
375-
// line 1 and column 1.
376-
if (line <= 1 && column <= 1 && library.importUri.isScheme('dart')) {
377-
_log('Fallback: use library scope for the Dart SDK');
378-
scope = DartScope(library, null, null, {}, []);
379-
} else {
380-
onDiagnostic(
381-
_createInternalError(
382-
libraryUri,
383-
line,
384-
column,
385-
'Dart scope not found for location',
386-
),
365+
// No scriptUri provided. Find - and try - all of the possible ones.
366+
final fileUris = [library.fileUri];
367+
for (final part in library.parts) {
368+
try {
369+
final partUriIsh = library.fileUri.resolve(part.partUri);
370+
if (partUriIsh.isScheme('package')) {
371+
for (final source in _component.uriToSource.values) {
372+
if (source.importUri == partUriIsh && source.importUri != null) {
373+
fileUris.add(source.importUri!);
374+
break;
375+
}
376+
}
377+
} else {
378+
fileUris.add(partUriIsh);
379+
}
380+
} catch (e) {
381+
// Bad url.
382+
_log(
383+
"Got exception '$e' when resolving ${part.partUri} "
384+
'against ${library.fileUri}',
387385
);
388-
return null;
389386
}
390387
}
391388

392-
_log('Detected expression compilation scope');
393-
return scope;
389+
final foundScopes = <DartScope>[];
390+
for (final fileUri in fileUris) {
391+
final offset = _component.getOffset(fileUri, line, column);
392+
foundScopes.add(
393+
DartScopeBuilder2.findScopeFromOffset(library, fileUri, offset),
394+
);
395+
}
396+
if (foundScopes.length == 1) {
397+
return foundScopes[0];
398+
}
399+
400+
// Pick the "best" (most likely) scope based on content compare to js.
401+
final jsKeys = jsScope.keys.toSet();
402+
var bestIntersectionCount = -1;
403+
var bestScope = foundScopes[0];
404+
for (final scope in foundScopes) {
405+
final intersection = jsKeys.intersection(scope.variables.keys.toSet());
406+
if (intersection.length > bestIntersectionCount) {
407+
bestIntersectionCount = intersection.length;
408+
bestScope = scope;
409+
}
410+
}
411+
412+
return bestScope;
394413
}
395414

396415
Library? _getLibrary(Uri libraryUri) {
@@ -418,6 +437,8 @@ class ExpressionCompiler {
418437
methodName: methodName,
419438
className: scope.cls?.name,
420439
isStatic: scope.isStatic,
440+
offset: scope.offset ?? TreeNode.noOffset,
441+
scriptUri: scope.fileUri?.toString(),
421442
);
422443

423444
_log('Compiled expression to kernel');

‎pkg/dev_compiler/lib/src/kernel/expression_compiler_worker.dart‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -397,9 +397,9 @@ class ExpressionCompilerWorker {
397397
// Failed to compile component, report compilation errors.
398398
if (expressionCompiler == null) {
399399
return {
400-
'errors': errors,
401-
'warnings': warnings,
402-
'infos': infos,
400+
'errors': errors.toList(),
401+
'warnings': warnings.toList(),
402+
'infos': infos.toList(),
403403
'compiledProcedure': null,
404404
'succeeded': false,
405405
};
@@ -418,9 +418,9 @@ class ExpressionCompilerWorker {
418418

419419
// Return result or expression compilation errors.
420420
return {
421-
'errors': errors,
422-
'warnings': warnings,
423-
'infos': infos,
421+
'errors': errors.toList(),
422+
'warnings': warnings.toList(),
423+
'infos': infos.toList(),
424424
'compiledProcedure': compiledProcedure,
425425
'succeeded': errors.isEmpty,
426426
};

‎pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_shared.dart‎

Lines changed: 133 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ void runExpressionCompilationTests(ExpressionCompilerWorkerTestDriver driver) {
294294
'command': 'CompileExpression',
295295
'expression': 'count',
296296
'line': 9,
297-
'column': 1,
297+
'column': 3,
298298
'jsModules': {},
299299
'jsScope': {'count': 'count'},
300300
'libraryUri': driver.config.getModule('mainModule').libraryUris.first,
@@ -325,8 +325,8 @@ void runExpressionCompilationTests(ExpressionCompilerWorkerTestDriver driver) {
325325
driver.requestController.add({
326326
'command': 'CompileExpression',
327327
'expression': 'ret',
328-
'line': 19,
329-
'column': 1,
328+
'line': 18,
329+
'column': 5,
330330
'jsModules': {},
331331
'jsScope': {'ret': 'ret'},
332332
'libraryUri': driver.config.getModule('mainModule').libraryUris.first,
@@ -446,6 +446,96 @@ void runExpressionCompilationTests(ExpressionCompilerWorkerTestDriver driver) {
446446
);
447447
});
448448

449+
test('can compile expressions in part file', () {
450+
driver.requestController.add({
451+
'command': 'UpdateDeps',
452+
'inputs': driver.inputs,
453+
});
454+
455+
// We're really in the main file - but we're not telling.
456+
driver.requestController.add({
457+
'command': 'CompileExpression',
458+
'expression': 'x.length',
459+
'line': 6,
460+
'column': 3,
461+
'jsModules': {},
462+
'jsScope': {'x': 'x', 'z': 'z'},
463+
'libraryUri': driver.config.getModule('testModule5').libraryUris.last,
464+
'moduleName': driver.config.getModule('testModule5').moduleName,
465+
});
466+
467+
// We're really in the part file - but we're not telling.
468+
driver.requestController.add({
469+
'command': 'CompileExpression',
470+
'expression': 'y + 1',
471+
'line': 6,
472+
'column': 3,
473+
'jsModules': {},
474+
'jsScope': {'y': 'y', 'z': 'z'},
475+
'libraryUri': driver.config.getModule('testModule5').libraryUris.last,
476+
'moduleName': driver.config.getModule('testModule5').moduleName,
477+
});
478+
479+
// We're really in the main file - but we're not telling.
480+
driver.requestController.add({
481+
'command': 'CompileExpression',
482+
'expression': 'z.length',
483+
'line': 6,
484+
'column': 3,
485+
'jsModules': {},
486+
'jsScope': {'x': 'x', 'z': 'z'},
487+
'libraryUri': driver.config.getModule('testModule5').libraryUris.last,
488+
'moduleName': driver.config.getModule('testModule5').moduleName,
489+
});
490+
491+
// We're really in the part file - but we're not telling.
492+
driver.requestController.add({
493+
'command': 'CompileExpression',
494+
'expression': 'z + 1',
495+
'line': 6,
496+
'column': 3,
497+
'jsModules': {},
498+
'jsScope': {'y': 'y', 'z': 'z'},
499+
'libraryUri': driver.config.getModule('testModule5').libraryUris.last,
500+
'moduleName': driver.config.getModule('testModule5').moduleName,
501+
});
502+
503+
expect(
504+
driver.responseController.stream,
505+
emitsInOrder([
506+
equals({'succeeded': true}),
507+
equals({
508+
'succeeded': true,
509+
'errors': isEmpty,
510+
'warnings': isEmpty,
511+
'infos': isEmpty,
512+
'compiledProcedure': contains('return x.length;'),
513+
}),
514+
equals({
515+
'succeeded': true,
516+
'errors': isEmpty,
517+
'warnings': isEmpty,
518+
'infos': isEmpty,
519+
'compiledProcedure': contains('return y + 1;'),
520+
}),
521+
equals({
522+
'succeeded': true,
523+
'errors': isEmpty,
524+
'warnings': isEmpty,
525+
'infos': isEmpty,
526+
'compiledProcedure': contains('return z.length;'),
527+
}),
528+
equals({
529+
'succeeded': true,
530+
'errors': isEmpty,
531+
'warnings': isEmpty,
532+
'infos': isEmpty,
533+
'compiledProcedure': contains('return z + 1;'),
534+
}),
535+
]),
536+
);
537+
});
538+
449539
test('can compile series of expressions in various libraries', () {
450540
driver.requestController.add({
451541
'command': 'UpdateDeps',
@@ -735,6 +825,17 @@ class TestProjectConfiguration {
735825
TestProjectConfiguration(this.rootDirectory, this.moduleFormat);
736826

737827
void initialize() {
828+
final testModule5 = ModuleConfiguration(
829+
root: root,
830+
outputDir: outputDir,
831+
moduleName: 'packages/_testPackage/test_library5',
832+
libraryUris: ['package:_testPackage/test_library8.dart'],
833+
dependencies: [],
834+
jsFileName: 'test_library5.js',
835+
fullDillFileName: 'test_library5.full.dill',
836+
summaryDillFileName: 'test_library5.dill',
837+
);
838+
738839
final testModule4 = ModuleConfiguration(
739840
root: root,
740841
outputDir: outputDir,
@@ -798,6 +899,7 @@ class TestProjectConfiguration {
798899
);
799900

800901
modules = {
902+
'testModule5': testModule5,
801903
'testModule4': testModule4,
802904
'testModule3': testModule3,
803905
'testModule2': testModule2,
@@ -886,8 +988,7 @@ var global = 0;
886988
887989
void main() {
888990
var count = 0;
889-
// line 9
890-
print('Global is: \${++global}');
991+
print('Global is: \${++global}'); // line 9
891992
print('Count is: \${++count}');
892993
893994
B b = new B();
@@ -896,8 +997,7 @@ void main() {
896997
extension NumberParsing on String {
897998
int parseInt() {
898999
var ret = int.parse(this);
899-
// line 19
900-
return ret;
1000+
return ret; // line 18
9011001
}
9021002
}
9031003
@@ -1012,6 +1112,32 @@ import 'package:_testPackage/test_library6.dart';
10121112
class E {
10131113
void foo(D bar) {}
10141114
}
1115+
''');
1116+
1117+
var testLibrary8 = root.resolve('lib/test_library8.dart');
1118+
var testLibrary8Part = root.resolve('lib/test_library8_part.dart');
1119+
File.fromUri(testLibrary8)
1120+
..createSync()
1121+
..writeAsStringSync('''
1122+
part 'test_library8_part.dart';
1123+
void main() {
1124+
String x = "foo";
1125+
String z = "foo";
1126+
// padding
1127+
foo(); // line 6 column 3 offset 101
1128+
print(x);
1129+
}
1130+
''');
1131+
File.fromUri(testLibrary8Part)
1132+
..createSync()
1133+
..writeAsStringSync('''
1134+
part of 'test_library8.dart';
1135+
void foo() {
1136+
int y = 42;
1137+
int z = 42;
1138+
// padding...............
1139+
print(y); // line 6 column 3 offset 101
1140+
}
10151141
''');
10161142
}
10171143
}

0 commit comments

Comments
 (0)