Skip to content

Commit

Permalink
[dart2js] Don't rely on == null promoting to Null in rti.dart.
Browse files Browse the repository at this point in the history
Although the true branch of `x != null` and the false branch of `x ==
null` promote `x` to non-nullable, the opposing branches do not promote
`x` to `Null` in the current flow analysis spec, which applies to the
CFE.

When switching from the dart2js static type computation to the CFE
static type computation, attempting to use `x` in these branches may
generate an unintended type check, which can overflow the stack if this
occurs in code responsible for performing type checks. The fix is fairly
straightforward - we can simply use an unchecked cast (via `JS`) instead.

See dart-lang/language#1505 for discussion on
why the expected promotion does not (yet) occur.

Change-Id: Ia9cca4e1aa8e9c67b42c60189f0d3811afb61360
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/289061
Reviewed-by: Stephen Adams <sra@google.com>
  • Loading branch information
fishythefish authored and Commit Queue committed Mar 17, 2023
1 parent 6f45c85 commit 62e3c47
Showing 1 changed file with 12 additions and 12 deletions.
24 changes: 12 additions & 12 deletions sdk/lib/_internal/js_shared/lib/rti.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1183,8 +1183,7 @@ class _Error extends Error {
final String _message;
_Error(this._message);

static String compose(
Object? object, String checkedTypeDescription) {
static String compose(Object? object, String checkedTypeDescription) {
String objectDescription = Error.safeToString(object);
Rti objectRti = _structuralTypeOf(object);
String objectTypeDescription = _rtiToString(objectRti, null);
Expand Down Expand Up @@ -1265,7 +1264,7 @@ bool _asBool(Object? object) {
bool? _asBoolS(dynamic object) {
if (true == object) return true;
if (false == object) return false;
if (object == null) return object;
if (object == null) return _Utils.asNull(object);
throw _TypeError.forType(object, 'bool');
}

Expand All @@ -1274,7 +1273,7 @@ bool? _asBoolS(dynamic object) {
bool? _asBoolQ(dynamic object) {
if (true == object) return true;
if (false == object) return false;
if (object == null) return object;
if (object == null) return _Utils.asNull(object);
throw _TypeError.forType(object, 'bool?');
}

Expand All @@ -1289,15 +1288,15 @@ double _asDouble(Object? object) {
/// Called from generated code.
double? _asDoubleS(dynamic object) {
if (_isNum(object)) return _Utils.asDouble(object);
if (object == null) return object;
if (object == null) return _Utils.asNull(object);
throw _TypeError.forType(object, 'double');
}

/// Specialization for 'as double?'.
/// Called from generated code.
double? _asDoubleQ(dynamic object) {
if (_isNum(object)) return _Utils.asDouble(object);
if (object == null) return object;
if (object == null) return _Utils.asNull(object);
throw _TypeError.forType(object, 'double?');
}

Expand All @@ -1319,15 +1318,15 @@ int _asInt(Object? object) {
/// Called from generated code.
int? _asIntS(dynamic object) {
if (_isInt(object)) return _Utils.asInt(object);
if (object == null) return object;
if (object == null) return _Utils.asNull(object);
throw _TypeError.forType(object, 'int');
}

/// Specialization for 'as int?'.
/// Called from generated code.
int? _asIntQ(dynamic object) {
if (_isInt(object)) return _Utils.asInt(object);
if (object == null) return object;
if (object == null) return _Utils.asNull(object);
throw _TypeError.forType(object, 'int?');
}

Expand All @@ -1348,15 +1347,15 @@ num _asNum(Object? object) {
/// Called from generated code.
num? _asNumS(dynamic object) {
if (_isNum(object)) return _Utils.asNum(object);
if (object == null) return object;
if (object == null) return _Utils.asNull(object);
throw _TypeError.forType(object, 'num');
}

/// Specialization for 'as num?'.
/// Called from generated code.
num? _asNumQ(dynamic object) {
if (_isNum(object)) return _Utils.asNum(object);
if (object == null) return object;
if (object == null) return _Utils.asNull(object);
throw _TypeError.forType(object, 'num?');
}

Expand All @@ -1377,15 +1376,15 @@ String _asString(Object? object) {
/// Called from generated code.
String? _asStringS(dynamic object) {
if (_isString(object)) return _Utils.asString(object);
if (object == null) return object;
if (object == null) return _Utils.asNull(object);
throw _TypeError.forType(object, 'String');
}

/// Specialization for 'as String?'.
/// Called from generated code.
String? _asStringQ(dynamic object) {
if (_isString(object)) return _Utils.asString(object);
if (object == null) return object;
if (object == null) return _Utils.asNull(object);
throw _TypeError.forType(object, 'String?');
}

Expand Down Expand Up @@ -3394,6 +3393,7 @@ bool isJsFunctionType(Rti t) =>
bool isRecordInterfaceType(Rti t) => _Utils.isIdentical(t, TYPE_REF<Record>());

class _Utils {
static Null asNull(Object? o) => JS('Null', '#', o);
static bool asBool(Object? o) => JS('bool', '#', o);
static double asDouble(Object? o) => JS('double', '#', o);
static int asInt(Object? o) => JS('int', '#', o);
Expand Down

0 comments on commit 62e3c47

Please sign in to comment.