Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 134bb88

Browse files
johnniwinthercommit-bot@chromium.org
authored andcommitted
Handle generic types in native behavior
Previous implementation didn't include subtypes of the specified type. For instance `JS('Rectangle', ...)` should have triggered the inclusion of `_DomRect` but the use of this-type in the subtype test, `_DomRect <: Rectangle<T>`, prohibited this. This fix is to use the raw type instead of the this-type. The subtype test will therefore be `_DomRect <: Rectangle<dynamic>` which correctly includes `_DomRect`. The change has the effect that `JS('List', ...)` now actually includes subtypes of `List`. For this reason uses of `List` have been updated to use `JSArray` or explicitly use `returns:...` to avoid unintended inclusion of native lists such as the native typed arrays. Change-Id: I06ab55d9bf694829596875d9c3a0a6c954d396b7 Reviewed-on: https://dart-review.googlesource.com/73903 Reviewed-by: Stephen Adams <sra@google.com> Commit-Queue: Johnni Winther <johnniwinther@google.com>
1 parent d66572d commit 134bb88

File tree

11 files changed

+71
-55
lines changed

11 files changed

+71
-55
lines changed

pkg/compiler/lib/src/native/enqueue.dart

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,8 @@ abstract class NativeEnqueuerBase implements NativeEnqueuer {
121121
matchingClasses
122122
.addAll(_findUnusedClassesMatching((ClassEntity nativeClass) {
123123
InterfaceType nativeType =
124-
_elementEnvironment.getThisType(nativeClass);
125-
InterfaceType specType =
126-
_elementEnvironment.getThisType(type.element);
124+
_elementEnvironment.getRawType(nativeClass);
125+
InterfaceType specType = _elementEnvironment.getRawType(type.element);
127126
return _dartTypes.isSubtype(nativeType, specType);
128127
}));
129128
} else if (type.isDynamic) {

pkg/compiler/lib/src/resolution/registry.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ class ResolutionWorldImpactBuilder extends WorldImpactBuilderImpl
131131

132132
String toString() {
133133
StringBuffer sb = new StringBuffer();
134-
sb.write('_ResolutionWorldImpact($name)');
134+
sb.write('ResolutionWorldImpactBuilder($name)');
135135
WorldImpact.printOn(sb, this);
136136
if (_features != null) {
137137
sb.write('\n features:');

pkg/compiler/lib/src/ssa/kernel_impact.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class KernelImpactBuilder extends ir.Visitor {
5151
KernelImpactBuilder(
5252
this.elementMap, this.currentMember, this.reporter, this._options)
5353
: this.impactBuilder =
54-
new ResolutionWorldImpactBuilder('${currentMember.name}') {
54+
new ResolutionWorldImpactBuilder('${currentMember}') {
5555
this.classEnsurer = new _ClassEnsurer(this, this.elementMap.types);
5656
}
5757

pkg/compiler/lib/src/ssa/types.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ class AbstractValueFactory {
6464

6565
AbstractValue result =
6666
abstractValueDomain.unionOfMany(typesReturned.map(fromNativeType));
67-
assert(!abstractValueDomain.isEmpty(result));
67+
assert(!abstractValueDomain.isEmpty(result),
68+
"Unexpected empty return value for $nativeBehavior.");
6869
return result;
6970
}
7071
}

sdk/lib/_internal/js_runtime/lib/js_number.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ class JSNumber extends Interceptor implements double {
217217
static String _handleIEtoString(String result) {
218218
// Result is probably IE's untraditional format for large numbers,
219219
// e.g., "8.0000000000008(e+15)" for 0x8000000000000800.toString(16).
220-
var match = JS('List|Null',
220+
var match = JS('JSArray|Null',
221221
r'/^([\da-z]+)(?:\.([\da-z]+))?\(e\+(\d+)\)$/.exec(#)', result);
222222
if (match == null) {
223223
// Then we don't know how to handle it at all.

sdk/lib/html/html_common/conversions.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ abstract class _StructuredClone {
128128
// non-native properties or methods from interceptors and such, e.g.
129129
// an immutability marker. So we had to stop doing that.
130130
var slot = findSlot(e);
131-
var copy = JS('List|Null', '#', readSlot(slot));
131+
var copy = JS('returns:List|Null;creates:;', '#', readSlot(slot));
132132
if (copy != null) return copy;
133133
copy = copyList(e, slot);
134134
return copy;
@@ -237,9 +237,9 @@ abstract class _AcceptStructuredClone {
237237
}
238238

239239
if (isJavaScriptArray(e)) {
240-
var l = JS('List', '#', e);
240+
var l = JS('returns:List;creates:;', '#', e);
241241
var slot = findSlot(l);
242-
var copy = JS('List|Null', '#', readSlot(slot));
242+
var copy = JS('returns:List|Null;creates:;', '#', readSlot(slot));
243243
if (copy != null) return copy;
244244

245245
int length = l.length;

tests/co19_2/co19_2-dart2js.status

Lines changed: 14 additions & 42 deletions
Large diffs are not rendered by default.

tests/compiler/dart2js/equivalence/id_equivalence_helper.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ abstract class DataComputer {
8484
/// Called before testing to setup flags needed for data collection.
8585
void setup() {}
8686

87+
/// Called before testing to setup flags needed for data collection.
88+
void onCompilation(Compiler compiler) {}
89+
8790
/// Function that computes a data mapping for [member].
8891
///
8992
/// Fills [actualMap] with the data and [sourceSpanMap] with the source spans
@@ -149,6 +152,7 @@ Future<CompiledData> computeData(Uri entryPoint,
149152
Expect.isTrue(result.isSuccess, "Unexpected compilation error.");
150153
}
151154
Compiler compiler = result.compiler;
155+
dataComputer.onCompilation(compiler);
152156
dynamic closedWorld = testFrontend
153157
? compiler.resolutionWorldBuilder.closedWorldForTesting
154158
: compiler.backendClosedWorldForTesting;

tests/compiler/dart2js/equivalence/show_helper.dart

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,17 @@ show(ArgResults argResults, DataComputer dataComputer,
6666
} else {
6767
SourceFileProvider provider = data.compiler.provider;
6868
for (Uri uri in data.actualMaps.keys) {
69-
if (show != null && !show.any((f) => '$uri'.endsWith(f))) {
69+
Uri fileUri = uri;
70+
if (fileUri.scheme == 'org-dartlang-sdk') {
71+
fileUri = Uri.base.resolve(fileUri.path.substring(1));
72+
}
73+
if (show != null && !show.any((f) => '$fileUri'.endsWith(f))) {
7074
continue;
7175
}
72-
SourceFile sourceFile = await provider.autoReadFromFile(uri);
76+
SourceFile sourceFile = await provider.autoReadFromFile(fileUri);
7377
String sourceCode = sourceFile?.slowText();
7478
if (sourceCode == null) {
75-
sourceCode = new File.fromUri(uri).readAsStringSync();
79+
sourceCode = new File.fromUri(fileUri).readAsStringSync();
7680
}
7781
if (sourceCode == null) {
7882
print('--source code missing for $uri--------------------------------');
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// ignore: IMPORT_INTERNAL_LIBRARY
6+
import 'dart:_foreign_helper' as foreign show JS;
7+
import 'dart:html';
8+
9+
/*element: main:[null]*/
10+
main() {
11+
createElement();
12+
createRectangle();
13+
}
14+
15+
/*element: createElement:[null|subclass=Element]*/
16+
Element createElement()
17+
// ignore: NATIVE_FUNCTION_BODY_IN_NON_SDK_CODE
18+
native;
19+
20+
/*element: createRectangle:[subclass=DomRectReadOnly]*/
21+
createRectangle() => foreign.JS('Rectangle', "#", null);

0 commit comments

Comments
 (0)