From 8f4e5c8194e0a56c63a09971deb47d140c9e0021 Mon Sep 17 00:00:00 2001 From: Anna Gringauze Date: Fri, 11 Aug 2023 16:15:00 +0000 Subject: [PATCH] [ddc] Fix runtime failure on evaluation of expressions that use JS interop and extension types Incremental compiler only runs procedure transformations during expression compilation, as opposed to modular library transformations that are run during initial compilation stage on libraries (including JS interop-related transformations). In this CL: - Add implementation of `performTransformationsOnProcedure` to dev compiler target - runs JS interop-related transformations on a procedure. - Add related expression evaluation tests - expressions using JS interop and extension types. Closes: https://github.com/dart-lang/sdk/issues/53048 Change-Id: I085920db9c3af4c680283c574087d8901c99dfcf Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319585 Commit-Queue: Anna Gringauze Reviewed-by: Srujan Gaddam Reviewed-by: Nicholas Shahan --- pkg/dev_compiler/lib/src/kernel/target.dart | 64 ++++++-- .../expression_compiler_e2e_shared.dart | 138 ++++++++++++++++++ 2 files changed, 188 insertions(+), 14 deletions(-) diff --git a/pkg/dev_compiler/lib/src/kernel/target.dart b/pkg/dev_compiler/lib/src/kernel/target.dart index fad5748efde..6f276b2c9e2 100644 --- a/pkg/dev_compiler/lib/src/kernel/target.dart +++ b/pkg/dev_compiler/lib/src/kernel/target.dart @@ -32,6 +32,8 @@ class DevCompilerTarget extends Target { Map? _nativeClasses; + DiagnosticReporter? _diagnosticReporter; + @override int get enabledLateLowerings => LateLowering.all; @@ -135,6 +137,10 @@ class DevCompilerTarget extends Target { bool _allowedTestLibrary(Uri uri) { // Multi-root scheme used by modular test framework. if (uri.isScheme('dev-dart-app')) return true; + // Test package used by expression evaluation tests. + if (uri.isScheme('package') && uri.path == 'eval_test/test.dart') { + return true; + } return allowedNativeTest(uri); } @@ -173,24 +179,43 @@ class DevCompilerTarget extends Target { {void Function(String msg)? logger, ChangedStructureNotifier? changedStructureNotifier}) { _nativeClasses ??= JsInteropChecks.getNativeClasses(component); - final jsInteropReporter = JsInteropDiagnosticReporter( - diagnosticReporter as DiagnosticReporter); - var jsInteropChecks = JsInteropChecks( + _diagnosticReporter ??= + diagnosticReporter as DiagnosticReporter; + _performTransformations(coreTypes, hierarchy, libraries); + } + + @override + void performTransformationsOnProcedure( + CoreTypes coreTypes, + ClassHierarchy hierarchy, + Procedure procedure, + Map? environmentDefines, + {void Function(String)? logger}) { + _performTransformations(coreTypes, hierarchy, [procedure]); + } + + void _performTransformations( + CoreTypes coreTypes, + ClassHierarchy hierarchy, + List nodes, + ) { + final jsInteropReporter = JsInteropDiagnosticReporter(_diagnosticReporter!); + final jsInteropChecks = JsInteropChecks( coreTypes, hierarchy, jsInteropReporter, _nativeClasses!); - // Process and validate first before doing anything with exports. - for (var library in libraries) { - jsInteropChecks.visitLibrary(library); + for (var node in nodes) { + // Process and validate first before doing anything with exports. + node.accept(jsInteropChecks); } - var exportCreator = ExportCreator(TypeEnvironment(coreTypes, hierarchy), + final exportCreator = ExportCreator(TypeEnvironment(coreTypes, hierarchy), jsInteropReporter, jsInteropChecks.exportChecker); - var jsUtilOptimizer = JsUtilOptimizer(coreTypes, hierarchy); - for (var library in libraries) { - _CovarianceTransformer(library).transform(); + final jsUtilOptimizer = JsUtilOptimizer(coreTypes, hierarchy); + for (var node in nodes) { + _CovarianceTransformer(node).transform(); // Export creator has static checks, so we still visit. - exportCreator.visitLibrary(library); + node.accept(exportCreator); if (!jsInteropReporter.hasJsInteropErrors) { // We can't guarantee calls are well-formed, so don't transform. - jsUtilOptimizer.visitLibrary(library); + node.accept(jsUtilOptimizer); } } } @@ -300,9 +325,20 @@ class _CovarianceTransformer extends RecursiveVisitor { /// aren't in [_checkedMembers]. final _privateFields = []; - final Library _library; + late final Library _library; - _CovarianceTransformer(this._library); + /// Create covariance transformer from a node. + /// + /// The [_node] is expected to be a [Library] in initial compilation + /// and a [Procedure] in the interactive expression compilation. + _CovarianceTransformer(TreeNode node) { + assert( + node is Library || node is Procedure, + 'Unexpected node in _CovarianceTransformer', + ); + if (node is Library) _library = node; + if (node is Procedure) _library = node.enclosingLibrary; + } /// Transforms [_library], eliminating unnecessary checks for private members. /// diff --git a/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_shared.dart b/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_shared.dart index 51357e00d81..51e6b872d4a 100644 --- a/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_shared.dart +++ b/pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_shared.dart @@ -66,6 +66,144 @@ main() { // test support for evaluation in legacy (pre-null safety) code. void runNullSafeSharedTests( SetupCompilerOptions setup, ExpressionEvaluationTestDriver driver) { + group('JS interop with static interop', () { + const interopSource = r''' + @JS() + library debug_static_interop; + + import 'dart:html'; + + import 'dart:_js_annotations' show staticInterop; + import 'dart:js_util'; + import 'dart:js_interop'; + + @JSExport() + class Counter { + int value = 0; + @JSExport('increment') + void renamedIncrement() { + value++; + } + } + + @JS() + @staticInterop + class JSCounter {} + + extension on JSCounter { + external int get value; + external void increment(); + } + + void main() { + var dartCounter = Counter(); + var jsCounter = + createDartExport(dartCounter) as JSCounter; + + dartCounter.renamedIncrement(); + jsCounter.increment(); + + // Breakpoint: bp + print('jsCounter: ${jsCounter.value}'); + } + '''; + + setUpAll(() async { + await driver.initSource(setup, interopSource, + experiments: {'inline-class': true}); + }); + + tearDownAll(() async { + await driver.cleanupTest(); + }); + + test('call extension methods of existing JS object', () async { + await driver.check( + breakpointId: 'bp', + expression: 'dartCounter.value', + expectedResult: '2'); + + await driver.check( + breakpointId: 'bp', + expression: 'jsCounter.value', + expectedResult: '2'); + }); + + test('call extension methods of a new JS object', () async { + await driver.check( + breakpointId: 'bp', + expression: + '(createDartExport(dartCounter) as JSCounter).value', + expectedResult: '2'); + }); + }); + + group('JS interop with extension types', () { + const interopSource = r''' + // @dart=3.2 + + @JS() + library debug_static_interop; + + import 'dart:_js_annotations' show staticInterop; + import 'dart:js_util'; + import 'dart:js_interop'; + + @JSExport() + class Counter { + int value = 0; + @JSExport('increment') + void renamedIncrement() { + value++; + } + } + + extension type JSCounter(JSObject _) { + external int get value; + external void increment(); + } + + void main() { + var dartCounter = Counter(); + var jsCounter = createDartExport(dartCounter) as JSCounter; + + jsCounter.increment(); + dartCounter.renamedIncrement(); + + // Breakpoint: bp + print('JS: ${jsCounter.value}'); // prints '2' + } + '''; + + setUpAll(() async { + await driver.initSource(setup, interopSource, + experiments: {'inline-class': true}); + }); + + tearDownAll(() async { + await driver.cleanupTest(); + }); + + test('call extension getters on existing JS object', () async { + await driver.check( + breakpointId: 'bp', + expression: 'dartCounter.value', + expectedResult: '2'); + + await driver.check( + breakpointId: 'bp', + expression: 'jsCounter.value', + expectedResult: '2'); + }); + + test('call extension getters on a new JS object', () async { + await driver.check( + breakpointId: 'bp', + expression: 'JSCounter(createDartExport(dartCounter)).value', + expectedResult: '2'); + }); + }); + group('Exceptions', () { const exceptionSource = r''' void main() {