Skip to content

Commit 8f4e5c8

Browse files
Anna GringauzeCommit Queue
Anna Gringauze
authored and
Commit Queue
committed
[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: #53048 Change-Id: I085920db9c3af4c680283c574087d8901c99dfcf Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319585 Commit-Queue: Anna Gringauze <[email protected]> Reviewed-by: Srujan Gaddam <[email protected]> Reviewed-by: Nicholas Shahan <[email protected]>
1 parent 410c394 commit 8f4e5c8

File tree

2 files changed

+188
-14
lines changed

2 files changed

+188
-14
lines changed

pkg/dev_compiler/lib/src/kernel/target.dart

Lines changed: 50 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ class DevCompilerTarget extends Target {
3232

3333
Map<String, Class>? _nativeClasses;
3434

35+
DiagnosticReporter<Message, LocatedMessage>? _diagnosticReporter;
36+
3537
@override
3638
int get enabledLateLowerings => LateLowering.all;
3739

@@ -135,6 +137,10 @@ class DevCompilerTarget extends Target {
135137
bool _allowedTestLibrary(Uri uri) {
136138
// Multi-root scheme used by modular test framework.
137139
if (uri.isScheme('dev-dart-app')) return true;
140+
// Test package used by expression evaluation tests.
141+
if (uri.isScheme('package') && uri.path == 'eval_test/test.dart') {
142+
return true;
143+
}
138144
return allowedNativeTest(uri);
139145
}
140146

@@ -173,24 +179,43 @@ class DevCompilerTarget extends Target {
173179
{void Function(String msg)? logger,
174180
ChangedStructureNotifier? changedStructureNotifier}) {
175181
_nativeClasses ??= JsInteropChecks.getNativeClasses(component);
176-
final jsInteropReporter = JsInteropDiagnosticReporter(
177-
diagnosticReporter as DiagnosticReporter<Message, LocatedMessage>);
178-
var jsInteropChecks = JsInteropChecks(
182+
_diagnosticReporter ??=
183+
diagnosticReporter as DiagnosticReporter<Message, LocatedMessage>;
184+
_performTransformations(coreTypes, hierarchy, libraries);
185+
}
186+
187+
@override
188+
void performTransformationsOnProcedure(
189+
CoreTypes coreTypes,
190+
ClassHierarchy hierarchy,
191+
Procedure procedure,
192+
Map<String, String>? environmentDefines,
193+
{void Function(String)? logger}) {
194+
_performTransformations(coreTypes, hierarchy, [procedure]);
195+
}
196+
197+
void _performTransformations(
198+
CoreTypes coreTypes,
199+
ClassHierarchy hierarchy,
200+
List<TreeNode> nodes,
201+
) {
202+
final jsInteropReporter = JsInteropDiagnosticReporter(_diagnosticReporter!);
203+
final jsInteropChecks = JsInteropChecks(
179204
coreTypes, hierarchy, jsInteropReporter, _nativeClasses!);
180-
// Process and validate first before doing anything with exports.
181-
for (var library in libraries) {
182-
jsInteropChecks.visitLibrary(library);
205+
for (var node in nodes) {
206+
// Process and validate first before doing anything with exports.
207+
node.accept(jsInteropChecks);
183208
}
184-
var exportCreator = ExportCreator(TypeEnvironment(coreTypes, hierarchy),
209+
final exportCreator = ExportCreator(TypeEnvironment(coreTypes, hierarchy),
185210
jsInteropReporter, jsInteropChecks.exportChecker);
186-
var jsUtilOptimizer = JsUtilOptimizer(coreTypes, hierarchy);
187-
for (var library in libraries) {
188-
_CovarianceTransformer(library).transform();
211+
final jsUtilOptimizer = JsUtilOptimizer(coreTypes, hierarchy);
212+
for (var node in nodes) {
213+
_CovarianceTransformer(node).transform();
189214
// Export creator has static checks, so we still visit.
190-
exportCreator.visitLibrary(library);
215+
node.accept(exportCreator);
191216
if (!jsInteropReporter.hasJsInteropErrors) {
192217
// We can't guarantee calls are well-formed, so don't transform.
193-
jsUtilOptimizer.visitLibrary(library);
218+
node.accept(jsUtilOptimizer);
194219
}
195220
}
196221
}
@@ -300,9 +325,20 @@ class _CovarianceTransformer extends RecursiveVisitor {
300325
/// aren't in [_checkedMembers].
301326
final _privateFields = <Field>[];
302327

303-
final Library _library;
328+
late final Library _library;
304329

305-
_CovarianceTransformer(this._library);
330+
/// Create covariance transformer from a node.
331+
///
332+
/// The [_node] is expected to be a [Library] in initial compilation
333+
/// and a [Procedure] in the interactive expression compilation.
334+
_CovarianceTransformer(TreeNode node) {
335+
assert(
336+
node is Library || node is Procedure,
337+
'Unexpected node in _CovarianceTransformer',
338+
);
339+
if (node is Library) _library = node;
340+
if (node is Procedure) _library = node.enclosingLibrary;
341+
}
306342

307343
/// Transforms [_library], eliminating unnecessary checks for private members.
308344
///

pkg/dev_compiler/test/expression_compiler/expression_compiler_e2e_shared.dart

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,144 @@ main() {
6666
// test support for evaluation in legacy (pre-null safety) code.
6767
void runNullSafeSharedTests(
6868
SetupCompilerOptions setup, ExpressionEvaluationTestDriver driver) {
69+
group('JS interop with static interop', () {
70+
const interopSource = r'''
71+
@JS()
72+
library debug_static_interop;
73+
74+
import 'dart:html';
75+
76+
import 'dart:_js_annotations' show staticInterop;
77+
import 'dart:js_util';
78+
import 'dart:js_interop';
79+
80+
@JSExport()
81+
class Counter {
82+
int value = 0;
83+
@JSExport('increment')
84+
void renamedIncrement() {
85+
value++;
86+
}
87+
}
88+
89+
@JS()
90+
@staticInterop
91+
class JSCounter {}
92+
93+
extension on JSCounter {
94+
external int get value;
95+
external void increment();
96+
}
97+
98+
void main() {
99+
var dartCounter = Counter();
100+
var jsCounter =
101+
createDartExport<Counter>(dartCounter) as JSCounter;
102+
103+
dartCounter.renamedIncrement();
104+
jsCounter.increment();
105+
106+
// Breakpoint: bp
107+
print('jsCounter: ${jsCounter.value}');
108+
}
109+
''';
110+
111+
setUpAll(() async {
112+
await driver.initSource(setup, interopSource,
113+
experiments: {'inline-class': true});
114+
});
115+
116+
tearDownAll(() async {
117+
await driver.cleanupTest();
118+
});
119+
120+
test('call extension methods of existing JS object', () async {
121+
await driver.check(
122+
breakpointId: 'bp',
123+
expression: 'dartCounter.value',
124+
expectedResult: '2');
125+
126+
await driver.check(
127+
breakpointId: 'bp',
128+
expression: 'jsCounter.value',
129+
expectedResult: '2');
130+
});
131+
132+
test('call extension methods of a new JS object', () async {
133+
await driver.check(
134+
breakpointId: 'bp',
135+
expression:
136+
'(createDartExport<Counter>(dartCounter) as JSCounter).value',
137+
expectedResult: '2');
138+
});
139+
});
140+
141+
group('JS interop with extension types', () {
142+
const interopSource = r'''
143+
// @dart=3.2
144+
145+
@JS()
146+
library debug_static_interop;
147+
148+
import 'dart:_js_annotations' show staticInterop;
149+
import 'dart:js_util';
150+
import 'dart:js_interop';
151+
152+
@JSExport()
153+
class Counter {
154+
int value = 0;
155+
@JSExport('increment')
156+
void renamedIncrement() {
157+
value++;
158+
}
159+
}
160+
161+
extension type JSCounter(JSObject _) {
162+
external int get value;
163+
external void increment();
164+
}
165+
166+
void main() {
167+
var dartCounter = Counter();
168+
var jsCounter = createDartExport<Counter>(dartCounter) as JSCounter;
169+
170+
jsCounter.increment();
171+
dartCounter.renamedIncrement();
172+
173+
// Breakpoint: bp
174+
print('JS: ${jsCounter.value}'); // prints '2'
175+
}
176+
''';
177+
178+
setUpAll(() async {
179+
await driver.initSource(setup, interopSource,
180+
experiments: {'inline-class': true});
181+
});
182+
183+
tearDownAll(() async {
184+
await driver.cleanupTest();
185+
});
186+
187+
test('call extension getters on existing JS object', () async {
188+
await driver.check(
189+
breakpointId: 'bp',
190+
expression: 'dartCounter.value',
191+
expectedResult: '2');
192+
193+
await driver.check(
194+
breakpointId: 'bp',
195+
expression: 'jsCounter.value',
196+
expectedResult: '2');
197+
});
198+
199+
test('call extension getters on a new JS object', () async {
200+
await driver.check(
201+
breakpointId: 'bp',
202+
expression: 'JSCounter(createDartExport<Counter>(dartCounter)).value',
203+
expectedResult: '2');
204+
});
205+
});
206+
69207
group('Exceptions', () {
70208
const exceptionSource = r'''
71209
void main() {

0 commit comments

Comments
 (0)