From 825737dc130343f7d8148979f108b03edf82012f Mon Sep 17 00:00:00 2001 From: John Messerly Date: Wed, 11 May 2016 11:16:00 -0700 Subject: [PATCH] fuse some null checks with type checks, introduce a special bool variant fixes #536 R=sra@google.com Review URL: https://codereview.chromium.org/1964263002 . --- pkg/dev_compiler/lib/runtime/dart_sdk.js | 56 ++++++++++--------- .../lib/src/compiler/code_generator.dart | 42 ++++++++++---- .../test/codegen/expect/notnull.js | 4 +- .../private/ddc_runtime/operations.dart | 49 ++++++++-------- 4 files changed, 85 insertions(+), 66 deletions(-) diff --git a/pkg/dev_compiler/lib/runtime/dart_sdk.js b/pkg/dev_compiler/lib/runtime/dart_sdk.js index 3c01d51cdee0..3ade62b6d439 100644 --- a/pkg/dev_compiler/lib/runtime/dart_sdk.js +++ b/pkg/dev_compiler/lib/runtime/dart_sdk.js @@ -456,22 +456,24 @@ dart_library.library('dart_sdk', null, /* Imports */[ if (obj == null) return obj; let result = dart.strongInstanceOf(obj, type, true); if (result) return obj; + dart._throwCastError(obj, type, result); + }; + dart.test = function(obj) { + if (typeof obj == "boolean") return obj; + dart.throwCastError(dart.getReifiedType(obj), core.bool); + }; + dart._throwCastError = function(obj, type, result) { let actual = dart.getReifiedType(obj); - if (result === false) dart.throwCastError(actual, type); - dart.throwStrongModeError('Strong mode cast failure from ' + dart.typeName(actual) + ' to ' + dart.typeName(type)); + if (result == false) dart.throwCastError(actual, type); + dart.throwStrongModeError('Strong mode cast failure from ' + dart.notNull(dart.as(dart.typeName(actual), core.String)) + ' to ' + dart.notNull(dart.as(dart.typeName(type), core.String))); }; dart.asInt = function(obj) { - if (obj == null) { - return null; - } + if (obj == null) return null; if (Math.floor(obj) != obj) { dart.throwCastError(dart.getReifiedType(obj), core.int); } return obj; }; - dart.arity = function(f) { - return {min: f.length, max: f.length}; - }; dart.equals = function(x, y) { if (x == null || y == null) return x == y; let eq = x['==']; @@ -6976,7 +6978,7 @@ dart_library.library('dart_sdk', null, /* Imports */[ if (this.doneHandlers == null) { this.doneHandlers = []; } - if (dart.notNull(dart.as(dart.dsend(this.doneHandlers, 'contains', responsePort), core.bool))) return; + if (dart.test(dart.dsend(this.doneHandlers, 'contains', responsePort))) return; dart.dsend(this.doneHandlers, 'add', responsePort); } removeDoneListener(responsePort) { @@ -7070,7 +7072,7 @@ dart_library.library('dart_sdk', null, /* Imports */[ _isolate_helper._globalState.currentContext = old; if (old != null) old[_setGlobals](); if (this[_scheduledControlEvents] != null) { - while (dart.notNull(dart.as(dart.dload(this[_scheduledControlEvents], 'isNotEmpty'), core.bool))) { + while (dart.test(dart.dload(this[_scheduledControlEvents], 'isNotEmpty'))) { dart.dcall(dart.dsend(this[_scheduledControlEvents], 'removeFirst')); } } @@ -9289,20 +9291,20 @@ dart_library.library('dart_sdk', null, /* Imports */[ _js_helper.checkBool(isUtc); let jsMonth = dart.dsend(month, '-', 1); let value = null; - if (dart.notNull(dart.as(isUtc, core.bool))) { + if (dart.test(isUtc)) { value = Date.UTC(years, jsMonth, day, hours, minutes, seconds, milliseconds); } else { value = new Date(years, jsMonth, day, hours, minutes, seconds, milliseconds).valueOf(); } - if (dart.notNull(dart.as(dart.dload(value, 'isNaN'), core.bool)) || dart.notNull(dart.as(dart.dsend(value, '<', -MAX_MILLISECONDS_SINCE_EPOCH), core.bool)) || dart.notNull(dart.as(dart.dsend(value, '>', MAX_MILLISECONDS_SINCE_EPOCH), core.bool))) { + if (dart.test(dart.dload(value, 'isNaN')) || dart.test(dart.dsend(value, '<', -MAX_MILLISECONDS_SINCE_EPOCH)) || dart.test(dart.dsend(value, '>', MAX_MILLISECONDS_SINCE_EPOCH))) { return null; } - if (dart.notNull(dart.as(dart.dsend(years, '<=', 0), core.bool)) || dart.notNull(dart.as(dart.dsend(years, '<', 100), core.bool))) return _js_helper.Primitives.patchUpY2K(value, years, isUtc); + if (dart.test(dart.dsend(years, '<=', 0)) || dart.test(dart.dsend(years, '<', 100))) return _js_helper.Primitives.patchUpY2K(value, years, isUtc); return value; } static patchUpY2K(value, years, isUtc) { let date = new Date(value); - if (dart.notNull(dart.as(isUtc, core.bool))) { + if (dart.test(isUtc)) { date.setUTCFullYear(years); } else { date.setFullYear(years); @@ -9410,7 +9412,7 @@ dart_library.library('dart_sdk', null, /* Imports */[ _js_helper.diagnoseIndexError = function(indexable, index) { if (!(typeof index == 'number')) return new core.ArgumentError.value(index, 'index'); let length = dart.as(dart.dload(indexable, 'length'), core.int); - if (dart.notNull(dart.as(dart.dsend(index, '<', 0), core.bool)) || dart.notNull(dart.as(dart.dsend(index, '>=', length), core.bool))) { + if (dart.test(dart.dsend(index, '<', 0)) || dart.test(dart.dsend(index, '>=', length))) { return core.RangeError.index(dart.as(index, core.int), indexable, 'index', null, length); } return new core.RangeError.value(dart.as(index, core.num), 'index'); @@ -9420,14 +9422,14 @@ dart_library.library('dart_sdk', null, /* Imports */[ if (!(typeof start == 'number')) { return new core.ArgumentError.value(start, 'start'); } - if (dart.notNull(dart.as(dart.dsend(start, '<', 0), core.bool)) || dart.notNull(dart.as(dart.dsend(start, '>', length), core.bool))) { + if (dart.test(dart.dsend(start, '<', 0)) || dart.test(dart.dsend(start, '>', length))) { return new core.RangeError.range(dart.as(start, core.num), 0, dart.as(length, core.int), 'start'); } if (end != null) { if (!(typeof end == 'number')) { return new core.ArgumentError.value(end, 'end'); } - if (dart.notNull(dart.as(dart.dsend(end, '<', start), core.bool)) || dart.notNull(dart.as(dart.dsend(end, '>', length), core.bool))) { + if (dart.test(dart.dsend(end, '<', start)) || dart.test(dart.dsend(end, '>', length))) { return new core.RangeError.range(dart.as(end, core.num), dart.as(start, core.int), dart.as(length, core.int), 'end'); } } @@ -28799,7 +28801,7 @@ dart_library.library('dart_sdk', null, /* Imports */[ } get [_errorExplanation]() { dart.assert(this[_hasValue]); - if (dart.notNull(dart.as(dart.dsend(this.invalidValue, '<', 0), core.bool))) { + if (dart.test(dart.dsend(this.invalidValue, '<', 0))) { return ": index must not be negative"; } if (this.length == 0) { @@ -30361,9 +30363,9 @@ dart_library.library('dart_sdk', null, /* Imports */[ } else { result = pathSegments[dartx.map](core.String)(dart.fn(s => core.Uri._uriEncode(core.Uri._pathCharTable, s, convert.UTF8, false), core.String, [core.String]))[dartx.join]("/"); } - if (dart.notNull(dart.as(dart.dload(result, 'isEmpty'), core.bool))) { + if (dart.test(dart.dload(result, 'isEmpty'))) { if (isFile) return "/"; - } else if (ensureLeadingSlash && !dart.notNull(dart.as(dart.dsend(result, 'startsWith', '/'), core.bool))) { + } else if (ensureLeadingSlash && !dart.test(dart.dsend(result, 'startsWith', '/'))) { result = "/" + dart.notNull(dart.as(result, core.String)); } result = core.Uri._normalizePath(dart.as(result, core.String), scheme, hasAuthority); @@ -31250,7 +31252,7 @@ dart_library.library('dart_sdk', null, /* Imports */[ let indices = dart.list([core.UriData._noScheme], core.int); core.UriData._writeUri(dart.as(mimeType, core.String), null, parameters, buffer, indices); indices[dartx.add](buffer.length); - if (dart.notNull(dart.as(percentEncoded, core.bool))) { + if (dart.test(percentEncoded)) { buffer.write(','); core.UriData._uriEncodeBytes(core.UriData._uricTable, bytes, buffer); } else { @@ -33225,13 +33227,13 @@ dart_library.library('dart_sdk', null, /* Imports */[ if (dart.notNull(html_common.isJavaScriptDate(object))) return true; if (dart.is(object, core.List)) { for (let i = 0; i < dart.notNull(object[dartx.length]); i++) { - if (dart.notNull(dart.as(containsDate(object[dartx.get](i)), core.bool))) return true; + if (dart.test(containsDate(object[dartx.get](i)))) return true; } } return false; } dart.fn(containsDate); - if (dart.notNull(dart.as(containsDate(nativeKey), core.bool))) { + if (dart.test(containsDate(nativeKey))) { dart.throw(new core.UnimplementedError('Key containing DateTime')); } return nativeKey; @@ -69613,10 +69615,10 @@ dart_library.library('dart_sdk', null, /* Imports */[ } set height(newHeight) { if (dart.is(newHeight, html$.Dimension)) { - if (dart.notNull(dart.as(dart.dsend(dart.dload(newHeight, 'value'), '<', 0), core.bool))) newHeight = new html$.Dimension.px(0); + if (dart.test(dart.dsend(dart.dload(newHeight, 'value'), '<', 0))) newHeight = new html$.Dimension.px(0); this[_element$][dartx.style][dartx.height] = dart.toString(newHeight); } else if (typeof newHeight == 'number') { - if (dart.notNull(dart.as(dart.dsend(newHeight, '<', 0), core.bool))) newHeight = 0; + if (dart.test(dart.dsend(newHeight, '<', 0))) newHeight = 0; this[_element$][dartx.style][dartx.height] = `${newHeight}px`; } else { dart.throw(new core.ArgumentError("newHeight is not a Dimension or num")); @@ -69624,10 +69626,10 @@ dart_library.library('dart_sdk', null, /* Imports */[ } set width(newWidth) { if (dart.is(newWidth, html$.Dimension)) { - if (dart.notNull(dart.as(dart.dsend(dart.dload(newWidth, 'value'), '<', 0), core.bool))) newWidth = new html$.Dimension.px(0); + if (dart.test(dart.dsend(dart.dload(newWidth, 'value'), '<', 0))) newWidth = new html$.Dimension.px(0); this[_element$][dartx.style][dartx.width] = dart.toString(newWidth); } else if (typeof newWidth == 'number') { - if (dart.notNull(dart.as(dart.dsend(newWidth, '<', 0), core.bool))) newWidth = 0; + if (dart.test(dart.dsend(newWidth, '<', 0))) newWidth = 0; this[_element$][dartx.style][dartx.width] = `${newWidth}px`; } else { dart.throw(new core.ArgumentError("newWidth is not a Dimension or num")); diff --git a/pkg/dev_compiler/lib/src/compiler/code_generator.dart b/pkg/dev_compiler/lib/src/compiler/code_generator.dart index 3d10afd25180..a69a05247f8d 100644 --- a/pkg/dev_compiler/lib/src/compiler/code_generator.dart +++ b/pkg/dev_compiler/lib/src/compiler/code_generator.dart @@ -446,27 +446,44 @@ class CodeGenerator extends GeneralizingAstVisitor } @override - visitAsExpression(AsExpression node) { - var from = getStaticType(node.expression); - var to = node.type.type; - - var fromExpr = _visit(node.expression); + visitAsExpression(AsExpression node) => + _emitCast(node.expression, to: node.type.type); + + /// Emits a cast and/or a null check (i.e. a cast to a non-null type). + JS.Expression _emitCast(Expression fromExpr, + {DartType to, bool checkNull: false}) { + var jsFrom = _visit(fromExpr); + var from = getStaticType(fromExpr); + + JS.Expression maybeCheckNull(JS.Expression jsExpr) { + if (checkNull && isNullable(fromExpr)) { + return js.call('dart.notNull(#)', jsExpr); + } + return jsExpr; + } // Skip the cast if it's not needed. - if (rules.isSubtypeOf(from, to)) return fromExpr; + if (to == null || rules.isSubtypeOf(from, to)) { + return maybeCheckNull(jsFrom); + } // All Dart number types map to a JS double. if (_isNumberInJS(from) && _isNumberInJS(to)) { // Make sure to check when converting to int. if (from != types.intType && to == types.intType) { - return js.call('dart.asInt(#)', [fromExpr]); + // TODO(jmesserly): fuse this with notNull check. + return maybeCheckNull(js.call('dart.asInt(#)', [jsFrom])); } // A no-op in JavaScript. - return fromExpr; + return maybeCheckNull(jsFrom); } - return js.call('dart.as(#, #)', [fromExpr, _emitType(to)]); + if (to == types.boolType && checkNull) { + return js.call('dart.test(#)', _visit(fromExpr)); + } + + return maybeCheckNull(js.call('dart.as(#, #)', [jsFrom, _emitType(to)])); } @override @@ -3061,9 +3078,10 @@ class CodeGenerator extends GeneralizingAstVisitor JS.Expression notNull(Expression expr) { if (expr == null) return null; - var jsExpr = _visit(expr); - if (!isNullable(expr)) return jsExpr; - return js.call('dart.notNull(#)', jsExpr); + if (expr is AsExpression) { + return _emitCast(expr.expression, to: expr.type.type, checkNull: true); + } + return _emitCast(expr, checkNull: true); } @override diff --git a/pkg/dev_compiler/test/codegen/expect/notnull.js b/pkg/dev_compiler/test/codegen/expect/notnull.js index 2bdaa49f4288..7e86acf50366 100644 --- a/pkg/dev_compiler/test/codegen/expect/notnull.js +++ b/pkg/dev_compiler/test/codegen/expect/notnull.js @@ -156,10 +156,10 @@ dart_library.library('notnull', null, /* Imports */[ } f(o) { core.print(1 + dart.notNull(dart.as(this.varField, core.num)) + 2); - while (dart.notNull(dart.as(dart.dsend(this.varField, '<', 10), core.bool))) { + while (dart.test(dart.dsend(this.varField, '<', 10))) { this.varField = dart.dsend(this.varField, '+', 1); } - while (dart.notNull(dart.as(dart.dsend(this.varField, '<', 10), core.bool))) + while (dart.test(dart.dsend(this.varField, '<', 10))) this.varField = dart.dsend(this.varField, '+', 1); core.print(1 + dart.notNull(this.intField) + 2); while (dart.notNull(this.intField) < 10) { diff --git a/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart b/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart index 9f4891f4158a..b177a50eb48f 100644 --- a/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart +++ b/pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart @@ -221,7 +221,7 @@ final _ignoreTypeFailure = JS('', '''(() => { /// and strong mode /// Returns null if [obj] is not an instance of [type] in strong mode /// but might be in spec mode -strongInstanceOf(obj, type, ignoreFromWhiteList) => JS('', '''(() => { +bool strongInstanceOf(obj, type, ignoreFromWhiteList) => JS('', '''(() => { let actual = $getReifiedType($obj); let result = $isSubtype(actual, $type); if (result || actual == $jsobject || @@ -252,36 +252,35 @@ instanceOf(obj, type) => JS('', '''(() => { })()'''); @JSExportName('as') -cast(obj, type) => JS('', '''(() => { - if ($obj == null) return $obj; +cast(obj, type) { + if (obj == null) return obj; - let result = $strongInstanceOf($obj, $type, true); - if (result) return $obj; + bool result = strongInstanceOf(obj, type, true); + if (JS('bool', '#', result)) return obj; + _throwCastError(obj, type, result); +} - let actual = $getReifiedType($obj); +bool test(obj) { + if (JS('bool', 'typeof # == "boolean"', obj)) return JS('bool', '#', obj); + throwCastError(getReifiedType(obj), JS('', '#', bool)); +} - if (result === false) $throwCastError(actual, $type); +void _throwCastError(obj, type, bool result) { + var actual = getReifiedType(obj); + if (result == false) throwCastError(actual, type); - $throwStrongModeError('Strong mode cast failure from ' + - $typeName(actual) + ' to ' + $typeName($type)); -})()'''); + throwStrongModeError('Strong mode cast failure from ' + + typeName(actual) + ' to ' + typeName(type)); +} -asInt(obj) => JS('', '''(() => { - if ($obj == null) { - return null; - } - if (Math.floor($obj) != $obj) { - // Note: null will also be caught by this check - $throwCastError($getReifiedType($obj), $int); - } - return $obj; -})()'''); +asInt(obj) { + if (obj == null) return null; -arity(f) => JS('', '''(() => { - // TODO(jmesserly): need to parse optional params. - // In ES6, length is the number of required arguments. - return { min: $f.length, max: $f.length }; -})()'''); + if (JS('bool', 'Math.floor(#) != #', obj, obj)) { + throwCastError(getReifiedType(obj), JS('', '#', int)); + } + return obj; +} equals(x, y) => JS('', '''(() => { if ($x == null || $y == null) return $x == $y;