Skip to content

Commit 8f9c38b

Browse files
committed
Enable is and as checks on non-ground types
Failures generate a StrongModeError. Fixes #236. Partially addresses #238. Some issues to do: - We're too eager to throw a StrongModeError. E.g., <int>[] is List<String> will throw. - We don't differentiate between implicit and explicit casts. [email protected] Review URL: https://codereview.chromium.org/1298893003 .
1 parent d3bc427 commit 8f9c38b

File tree

11 files changed

+158
-87
lines changed

11 files changed

+158
-87
lines changed

pkg/dev_compiler/lib/runtime/_classes.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ dart_library.library('dart_runtime/_classes', null, /* Imports */[
2424
const copyTheseProperties = dart_utils.copyTheseProperties;
2525
const defineMemoizedGetter = dart_utils.defineMemoizedGetter;
2626
const safeGetOwnProperty = dart_utils.safeGetOwnProperty;
27-
const throwError = dart_utils.throwError;
27+
const throwInternalError = dart_utils.throwInternalError;
2828

2929
const defineProperty = Object.defineProperty;
3030
const getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor;
@@ -110,12 +110,12 @@ dart_library.library('dart_runtime/_classes', null, /* Imports */[
110110
function generic(typeConstructor) {
111111
let length = typeConstructor.length;
112112
if (length < 1) {
113-
throwError('must have at least one generic type argument');
113+
throwInternalError('must have at least one generic type argument');
114114
}
115115
let resultMap = new Map();
116116
function makeGenericType(/*...arguments*/) {
117117
if (arguments.length != length && arguments.length != 0) {
118-
throwError('requires ' + length + ' or 0 type arguments');
118+
throwInternalError('requires ' + length + ' or 0 type arguments');
119119
}
120120
let args = slice.call(arguments);
121121
while (args.length < length) args.push(types.dynamic);
@@ -124,7 +124,7 @@ dart_library.library('dart_runtime/_classes', null, /* Imports */[
124124
for (let i = 0; i < length; i++) {
125125
let arg = args[i];
126126
if (arg == null) {
127-
throwError('type arguments should not be null: '
127+
throwInternalError('type arguments should not be null: '
128128
+ typeConstructor);
129129
}
130130
let map = value;

pkg/dev_compiler/lib/runtime/_errors.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,13 @@ dart_library.library('dart_runtime/_errors', null, /* Imports */[
2929
operations.throw(new core.AssertionError());
3030
}
3131
exports.throwAssertionError = throwAssertionError;
32+
33+
function throwNullValueError() {
34+
// TODO(vsm): Per spec, we should throw an NSM here. Technically, we ought
35+
// to thread through method info, but that uglifies the code and can't
36+
// actually be queried ... it only affects how the error is printed.
37+
operations.throw(new core.NoSuchMethodError(null,
38+
new core.Symbol('<Unexpected Null Value>'), null, null, null));
39+
}
40+
exports.throwNullValueError = throwNullValueError;
3241
});

pkg/dev_compiler/lib/runtime/_operations.js

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -191,26 +191,47 @@ dart_library.library('dart_runtime/_operations', null, /* Imports */[
191191
return false;
192192
}
193193

194-
function instanceOf(obj, type) {
194+
function strongInstanceOf(obj, type) {
195195
return types.isSubtype(rtti.realRuntimeType(obj), type);
196196
}
197-
exports.instanceOf = instanceOf;
197+
exports.strongInstanceOf = strongInstanceOf;
198198

199199
function instanceOfOrNull(obj, type) {
200-
if ((obj == null) || instanceOf(obj, type)) return true;
201-
let actual = rtti.realRuntimeType(obj);
202-
if (_ignoreTypeFailure(actual, type)) return true;
200+
if ((obj == null) || strongInstanceOf(obj, type)) return true;
203201
return false;
204202
}
205-
exports.instanceOfOrNull = instanceOfOrNull;
203+
204+
function instanceOf(obj, type) {
205+
if (strongInstanceOf(obj, type)) return true;
206+
// TODO(vsm): This is perhaps too eager to throw a StrongModeError?
207+
// It will throw on <int>[] is List<String>.
208+
// TODO(vsm): We can statically detect many cases where this
209+
// check is unnecessary.
210+
if (types.isGroundType(type)) return false;
211+
let actual = rtti.realRuntimeType(obj);
212+
dart_utils.throwStrongModeError('Strong mode is check failure: ' +
213+
types.typeName(actual) + ' does not soundly subtype ' +
214+
types.typeName(type));
215+
}
216+
exports.instanceOf = instanceOf;
206217

207218
function cast(obj, type) {
208219
// TODO(vsm): handle non-nullable types
209-
if (obj == null) return obj;
220+
if (instanceOfOrNull(obj, type)) return obj;
210221
let actual = rtti.realRuntimeType(obj);
211-
if (types.isSubtype(actual, type)) return obj;
212-
if (_ignoreTypeFailure(actual, type)) return obj;
213-
errors.throwCastError(actual, type);
222+
if (_ignoreTypeFailure(actual, type)) {
223+
// TODO(vsm): track why this is happening in our async / await tests.
224+
if (types.isGroundType(type)) {
225+
console.error('Should not ignore cast failure from ' +
226+
types.typeName(actual) + ' to ' + types.typeName(type));
227+
}
228+
return obj;
229+
}
230+
if (types.isGroundType(type)) {
231+
errors.throwCastError(actual, type);
232+
}
233+
dart_utils.throwStrongModeError('Strong mode cast failure from ' +
234+
types.typeName(actual) + ' to ' + types.typeName(type));
214235
}
215236
exports.cast = cast;
216237

@@ -230,8 +251,7 @@ dart_library.library('dart_runtime/_operations', null, /* Imports */[
230251

231252
/** Checks that `x` is not null or undefined. */
232253
function notNull(x) {
233-
// TODO(leafp): This is probably not the right error to throw.
234-
if (x == null) throwError('expected not-null value');
254+
if (x == null) errors.throwNullValueError();
235255
return x;
236256
}
237257
exports.notNull = notNull;

pkg/dev_compiler/lib/runtime/dart_library.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ var dart_library =
4848
for (let name of list) {
4949
let lib = libraries[name];
5050
if (!lib) {
51-
dart_utils.throwError('Library not available: ' + name);
51+
dart_utils.throwInternalError('Library not available: ' + name);
5252
}
5353
results.push(handler(lib));
5454
}
@@ -58,7 +58,7 @@ var dart_library =
5858
load(inheritedPendingSet) {
5959
// Check for cycles
6060
if (this._state == LibraryLoader.LOADING) {
61-
dart_utils.throwError('Circular dependence on library: '
61+
dart_utils.throwInternalError('Circular dependence on library: '
6262
+ this._name);
6363
} else if (this._state >= LibraryLoader.LOADED) {
6464
return this._library;
@@ -105,7 +105,9 @@ var dart_library =
105105
function import_(libraryName) {
106106
bootstrap();
107107
let loader = libraries[libraryName];
108-
if (!loader) dart_utils.throwError('Library not found: ' + libraryName);
108+
// TODO(vsm): A user might call this directly from JS (as we do in tests).
109+
// We may want a different error type.
110+
if (!loader) dart_utils.throwInternalError('Library not found: ' + libraryName);
109111
return loader.load();
110112
}
111113
dart_library.import = import_;

pkg/dev_compiler/lib/runtime/dart_runtime.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ dart_library.library('dart_runtime/dart', null, /* Imports */[
8989
'notNull',
9090
'stackPrint',
9191
'stackTrace',
92+
'strongInstanceOf',
9293
'throw',
9394
'toString',
9495
])

pkg/dev_compiler/lib/runtime/dart_utils.js

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,28 @@ var dart_utils =
2121

2222
const slice = [].slice;
2323

24+
class StrongModeError extends Error {
25+
constructor(message) {
26+
super(message);
27+
}
28+
}
29+
30+
/** This error indicates a strong mode specific failure.
31+
*/
32+
function throwStrongModeError(message) {
33+
throw new StrongModeError(message);
34+
}
35+
dart_utils.throwStrongModeError = throwStrongModeError;
2436

2537
/** This error indicates a bug in the runtime or the compiler.
2638
*/
27-
function throwError(message) {
39+
function throwInternalError(message) {
2840
throw Error(message);
2941
}
30-
dart_utils.throwError = throwError;
42+
dart_utils.throwInternalError = throwInternalError;
3143

3244
function assert(condition) {
33-
if (!condition) throwError("The compiler is broken: failed assert");
45+
if (!condition) throwInternalError("The compiler is broken: failed assert");
3446
}
3547
dart_utils.assert = assert;
3648

@@ -60,7 +72,7 @@ var dart_utils =
6072
value = x;
6173
}
6274
function circularInitError() {
63-
throwError('circular initialization for field ' + name);
75+
throwInternalError('circular initialization for field ' + name);
6476
}
6577
function lazyGetter() {
6678
if (init == null) return value;

pkg/dev_compiler/lib/src/checker/checker.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,7 @@ class CodeChecker extends RecursiveAstVisitor {
694694
void _checkRuntimeTypeCheck(AstNode node, TypeName typeName) {
695695
var type = getType(typeName);
696696
if (!rules.isGroundType(type)) {
697-
_recordMessage(new InvalidRuntimeCheckError(node, type));
697+
_recordMessage(new NonGroundTypeCheckInfo(node, type));
698698
}
699699
}
700700

pkg/dev_compiler/lib/src/info.dart

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -464,15 +464,19 @@ class InvalidParameterDeclaration extends StaticError {
464464
@override String get message => 'Type check failed: {0} is not of type {1}';
465465
}
466466

467-
class InvalidRuntimeCheckError extends StaticError {
467+
class NonGroundTypeCheckInfo extends StaticInfo {
468468
final DartType type;
469+
final AstNode node;
469470

470-
InvalidRuntimeCheckError(AstNode node, this.type) : super(node) {
471+
NonGroundTypeCheckInfo(this.node, this.type) {
471472
assert(node is IsExpression || node is AsExpression);
472473
}
473474

474475
@override List<Object> get arguments => [type];
475-
String get message => "Invalid runtime check on non-ground type {0}";
476+
String get message =>
477+
"Runtime check on non-ground type {0} may throw StrongModeError";
478+
479+
toErrorCode() => new HintCode(name, message);
476480
}
477481

478482
// Invalid override of an instance member of a class.

0 commit comments

Comments
 (0)