[ddc] Update for extra null safety checks in RTI

- In development mode (DDC) the extra null safety errors will be thrown.
- Remove extra code paths that called unsound helpers.
- Fix expectations in weak_null_safety_errors_test.dart.

Change-Id: I107c602b0ae38b13038e501564cba9b8cfc58e70
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329568
Reviewed-by: Mayank Patke <fishythefish@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
This commit is contained in:
Nicholas Shahan 2023-10-19 21:32:15 +00:00 committed by Commit Queue
parent 7da4e0b9e8
commit 8a1e69063e
9 changed files with 68 additions and 225 deletions

View file

@ -2,5 +2,5 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// TODO(nshahan): Provide a DDC implementation.
// Unused in DDC.
void Function(TypeError, StackTrace)? onExtraNullSafetyError;

View file

@ -7238,22 +7238,14 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
return js.call('typeof # == #', [lhs, js.string(typeofName, "'")]);
}
if (_options.newRuntimeTypes) {
// When using the new runtime type system with sound null safety we can
// call to the library directly. In weak mode we call a DDC only method
// that can optionally produce warnings or errors when the check passes
// but would fail with sound null safety.
return _options.soundNullSafety
? js.call('#.#(#)', [
_emitType(type),
_emitMemberName(js_ast.FixedNames.rtiIsField,
memberClass: rtiClass),
lhs
])
: runtimeCall('is(#, #)', [lhs, _emitType(type)]);
}
return js.call('#.is(#)', [_emitType(type), lhs]);
return _options.newRuntimeTypes
? js.call('#.#(#)', [
_emitType(type),
_emitMemberName(js_ast.FixedNames.rtiIsField,
memberClass: rtiClass),
lhs
])
: js.call('#.is(#)', [_emitType(type), lhs]);
}
@override
@ -7331,21 +7323,14 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
js_ast.Expression _emitCast(js_ast.Expression expr, DartType type) {
if (_types.isTop(type)) return expr;
if (_options.newRuntimeTypes) {
// When using the new runtime type system with sound null safety we can
// call to the library directly. In weak mode we call a DDC only method
// that can optionally produce warnings or errors when the cast passes but
// would fail with sound null safety.
return _options.soundNullSafety
? js.call('#.#(#)', [
_emitType(type),
_emitMemberName(js_ast.FixedNames.rtiAsField,
memberClass: rtiClass),
expr
])
: runtimeCall('as(#, #)', [expr, _emitType(type)]);
}
return js.call('#.as(#)', [_emitType(type), expr]);
return _options.newRuntimeTypes
? js.call('#.#(#)', [
_emitType(type),
_emitMemberName(js_ast.FixedNames.rtiAsField,
memberClass: rtiClass),
expr
])
: js.call('#.as(#)', [_emitType(type), expr]);
}
@override

View file

@ -14,10 +14,13 @@ argumentError(value) {
}
/// Only used during the development of the new runtime type system in branches
/// that should never be executed.
/// that should never be executed in the active type system.
// TODO(48585): Remove after switching to the new runtime type system.
Never throwUnimplementedInOldRti() => throw UnimplementedError(
'This code path is not supported with the old runtime type system.');
Never throwUnimplementedInCurrentRti() {
var systemVersion = compileTimeFlag('newRuntimeTypes') ? 'new' : 'old';
throw UnimplementedError('This code path is not supported with the '
'$systemVersion runtime type system.');
}
throwUnimplementedError(String message) {
throw UnimplementedError(message);

View file

@ -187,16 +187,10 @@ dput(obj, field, value) {
if (f != null) {
var setterType = getSetterType(getType(obj), f);
if (setterType != null) {
if (JS_GET_FLAG('NEW_RUNTIME_TYPES')) {
return compileTimeFlag('soundNullSafety')
// Call the 'as' method directly in sound mode.
? JS('', '#[#] = #.#(#)', obj, f, setterType,
JS_GET_NAME(JsGetName.RTI_FIELD_AS), value)
// Call `cast` in weak mode to provide optional warnings or errors.
: JS('', '#[#] = #', obj, f, cast(value, setterType));
} else {
return JS('', '#[#] = #.as(#)', obj, f, setterType, value);
}
return JS_GET_FLAG('NEW_RUNTIME_TYPES')
? JS('', '#[#] = #.#(#)', obj, f, setterType,
JS_GET_NAME(JsGetName.RTI_FIELD_AS), value)
: JS('', '#[#] = #.as(#)', obj, f, setterType, value);
}
// Always allow for JS interop objects.
if (isJsInterop(obj)) return JS('', '#[#] = #', obj, f, value);
@ -269,44 +263,26 @@ String? _argumentErrors(Object type, @notNull List actuals, namedActuals) {
}
}
// Now that we know the signature matches, we can perform type checks.
if (compileTimeFlag('soundNullSafety')) {
// Call the 'as' method directly in sound mode.
for (var i = 0; i < requiredCount; ++i) {
var requiredRti = JS<rti.Rti>('!', '#[#]', requiredPositional, i);
var passedValue = JS('', '#[#]', actuals, i);
JS('', '#.#(#)', requiredRti, JS_GET_NAME(JsGetName.RTI_FIELD_AS),
for (var i = 0; i < requiredCount; ++i) {
var requiredRti = JS<rti.Rti>('!', '#[#]', requiredPositional, i);
var passedValue = JS('', '#[#]', actuals, i);
JS('', '#.#(#)', requiredRti, JS_GET_NAME(JsGetName.RTI_FIELD_AS),
passedValue);
}
for (var i = 0; i < extras; ++i) {
var optionalRti = JS<rti.Rti>('!', '#[#]', optionalPositional, i);
var passedValue = JS('', '#[#]', actuals, i + requiredCount);
JS('', '#.#(#)', optionalRti, JS_GET_NAME(JsGetName.RTI_FIELD_AS),
passedValue);
}
if (names != null) {
for (var name in names) {
var namedRti = JS<rti.Rti>(
'!', '#[#] || #[#]', requiredNamed, name, optionalNamed, name);
var passedValue = JS('', '#[#]', namedActuals, name);
JS('', '#.#(#)', namedRti, JS_GET_NAME(JsGetName.RTI_FIELD_AS),
passedValue);
}
for (var i = 0; i < extras; ++i) {
var optionalRti = JS<rti.Rti>('!', '#[#]', optionalPositional, i);
var passedValue = JS('', '#[#]', actuals, i + requiredCount);
JS('', '#.#(#)', optionalRti, JS_GET_NAME(JsGetName.RTI_FIELD_AS),
passedValue);
}
if (names != null) {
for (var name in names) {
var namedRti = JS<rti.Rti>(
'!', '#[#] || #[#]', requiredNamed, name, optionalNamed, name);
var passedValue = JS('', '#[#]', namedActuals, name);
JS('', '#.#(#)', namedRti, JS_GET_NAME(JsGetName.RTI_FIELD_AS),
passedValue);
}
}
} else {
// Call `cast` in weak mode to provide optional warnings or errors.
for (var i = 0; i < requiredCount; ++i) {
cast(JS('', '#[#]', actuals, i), JS('', '#[#]', requiredPositional, i));
}
for (var i = 0; i < extras; ++i) {
cast(JS('', '#[#]', actuals, i + requiredCount),
JS('', '#[#]', optionalPositional, i));
}
if (names != null) {
for (var name in names) {
cast(JS('', '#[#]', namedActuals, name),
JS('', '#[#] || #[#]', requiredNamed, name, optionalNamed, name));
}
}
}
return null;
} else {
@ -517,10 +493,10 @@ _checkAndCall(f, ftype, obj, typeArgs, args, named, displayName) {
// there is no longer any warnings/errors in weak null safety mode.
if (bound != typeArg) {
var instantiatedBound = rti.substitute(bound, typeArgs);
var validSubtype = compileTimeFlag('soundNullSafety')
? rti.isSubtype(JS_EMBEDDED_GLOBAL('', RTI_UNIVERSE), typeArg,
instantiatedBound)
: _isSubtypeWithWarning(typeArg, instantiatedBound);
var validSubtype = rti.isSubtype(
JS_EMBEDDED_GLOBAL('', RTI_UNIVERSE),
typeArg,
instantiatedBound);
if (!validSubtype) {
throwTypeError("The type '${rti.rtiToString(typeArg)}' "
"is not a subtype of the type variable bound "
@ -720,38 +696,7 @@ dsetindex(obj, index, value) =>
@JSExportName('is')
bool instanceOf(obj, type) {
if (JS_GET_FLAG('NEW_RUNTIME_TYPES')) {
// When running without sound null safety is type tests are dispatched here
// to issue optional warnings/errors when the result is true but would be
// false with sound null safety.
var testRti = JS<rti.Rti>('!', '#', type);
// TODO(nshahan): Move to isSubtype in dart:rti once all fast path checks
// have been updated to be aware of
// `JS_GET_FLAG('EXTRA_NULL_SAFETY_CHECKS')`.
rti.Rti legacyErasedType;
if (JS_GET_FLAG('PRINT_LEGACY_STARS')) {
// When preserving the legacy stars in the runtime type, avoid caching
// the version with erased types on the Rti.
var legacyErasedRecipe = rti.Rti.getLegacyErasedRecipe(testRti);
legacyErasedType = rti.findType(legacyErasedRecipe);
} else {
legacyErasedType = rti.getLegacyErasedRti(testRti);
}
extraNullSafetyChecks = true;
legacyTypeChecks = false;
var result = JS<bool>('bool', '#.#(#)', legacyErasedType,
JS_GET_NAME(JsGetName.RTI_FIELD_IS), obj);
extraNullSafetyChecks = false;
legacyTypeChecks = true;
if (result) return true;
result =
JS('bool', '#.#(#)', testRti, JS_GET_NAME(JsGetName.RTI_FIELD_IS), obj);
if (result) {
// Type test returned true but would be false with sound null safety.
var t1 = runtimeType(obj);
var t2 = rti.createRuntimeType(testRti);
_nullWarn('$t1 is not a subtype of $t2.');
}
return result;
throwUnimplementedInCurrentRti();
} else {
if (obj == null) {
return _equalType(type, Null) ||
@ -770,41 +715,7 @@ bool instanceOf(obj, type) {
@JSExportName('as')
cast(obj, type) {
if (JS_GET_FLAG('NEW_RUNTIME_TYPES')) {
// When running without sound null safety casts are dispatched here to issue
// optional warnings/errors when casts pass but would fail with sound null
// safety.
var testRti = JS<rti.Rti>('!', '#', type);
if (obj == null && !rti.isNullable(testRti)) {
// Allow cast to pass but warn that it would fail in sound null safety.
_nullWarnOnType(type);
return obj;
}
// TODO(nshahan): Move to isSubtype in dart:rti once all fast path checks
// have been updated to be aware of
// `JS_GET_FLAG('EXTRA_NULL_SAFETY_CHECKS')`.
rti.Rti legacyErasedType;
if (JS_GET_FLAG('PRINT_LEGACY_STARS')) {
// When preserving the legacy stars in the runtime type, avoid caching
// the version with erased types on the Rti.
var legacyErasedRecipe = rti.Rti.getLegacyErasedRecipe(testRti);
legacyErasedType = rti.findType(legacyErasedRecipe);
} else {
legacyErasedType = rti.getLegacyErasedRti(testRti);
}
extraNullSafetyChecks = true;
legacyTypeChecks = false;
var result = JS<bool>('!', '#.#(#)', legacyErasedType,
JS_GET_NAME(JsGetName.RTI_FIELD_IS), obj);
extraNullSafetyChecks = false;
legacyTypeChecks = true;
if (result) return obj;
// Perform the actual cast and allow the error to be thrown if it fails.
JS('', '#.#(#)', testRti, JS_GET_NAME(JsGetName.RTI_FIELD_AS), obj);
// Cast succeeds but would fail with sound null safety.
var t1 = runtimeType(obj);
var t2 = rti.createRuntimeType(testRti);
_nullWarn('$t1 is not a subtype of $t2.');
return obj;
throwUnimplementedInCurrentRti();
} else {
// We hoist the common case where null is checked against another type here
// for better performance.
@ -819,54 +730,6 @@ cast(obj, type) {
}
}
/// Returns `true` if [t1] is a subtype of [t2].
///
/// This method should only be called when running with weak null safety.
///
/// Will produce a warning/error (if enabled) when the subtype passes but would
/// fail in sound null safety.
// TODO(48585) Revise argument types after removing old type representation.
@notNull
bool _isSubtypeWithWarning(@notNull t1, @notNull t2) {
// Avoid classes from the rti library appearing unless they are used.
if (JS_GET_FLAG('NEW_RUNTIME_TYPES')) {
var t1Rti = JS<rti.Rti>('!', '#', t1);
var t2Rti = JS<rti.Rti>('!', '#', t2);
// TODO(nshahan): Move to isSubtype in dart:rti once all fast path checks
// have been updated to be aware of
// `JS_GET_FLAG('EXTRA_NULL_SAFETY_CHECKS')`.
rti.Rti legacyErasedType;
if (JS_GET_FLAG('PRINT_LEGACY_STARS')) {
// When preserving the legacy stars in the runtime type, avoid caching
// the version with erased types on the Rti.
var legacyErasedRecipe = rti.Rti.getLegacyErasedRecipe(t2Rti);
legacyErasedType = rti.findType(legacyErasedRecipe);
} else {
legacyErasedType = rti.getLegacyErasedRti(t2Rti);
}
extraNullSafetyChecks = true;
legacyTypeChecks = false;
var validSubtype = rti.isSubtype(
JS_EMBEDDED_GLOBAL('', RTI_UNIVERSE), t1Rti, legacyErasedType);
extraNullSafetyChecks = false;
legacyTypeChecks = true;
if (validSubtype) return true;
validSubtype =
rti.isSubtype(JS_EMBEDDED_GLOBAL('', RTI_UNIVERSE), t1Rti, t2Rti);
if (validSubtype) {
// Subtype check passes but would fail with sound null safety.
_nullWarn('${rti.createRuntimeType(t1Rti)} '
'is not a subtype of '
'${rti.createRuntimeType(t2Rti)}.');
}
return validSubtype;
} else {
// Should never be reached because this method isn't called when using old
// runtime type representation.
throwUnimplementedInOldRti();
}
}
bool test(bool? obj) {
if (obj == null) throw BooleanConversionAssertionError();
return obj;

View file

@ -74,6 +74,7 @@ void weakNullSafetyErrors(bool showErrors) {
}
_weakNullSafetyErrors = showErrors;
extraNullSafetyChecks = showErrors;
}
@notNull
@ -1221,17 +1222,10 @@ bool isType(obj) => JS('', '#[#] === #', obj, _runtimeType, Type);
void checkTypeBound(
@notNull Object type, @notNull Object bound, @notNull String name) {
bool validSubtype;
if (JS_GET_FLAG('NEW_RUNTIME_TYPES')) {
validSubtype = compileTimeFlag('soundNullSafety')
// Check subtype directly in sound mode.
? rti.isSubtype(JS_EMBEDDED_GLOBAL('', RTI_UNIVERSE),
JS<rti.Rti>('!', '#', type), JS<rti.Rti>('!', '#', bound))
// Check subtype but issue warnings/errors in weak mode.
: _isSubtypeWithWarning(type, bound);
} else {
validSubtype = isSubtypeOf(type, bound);
}
var validSubtype = JS_GET_FLAG('NEW_RUNTIME_TYPES')
? rti.isSubtype(JS_EMBEDDED_GLOBAL('', RTI_UNIVERSE),
JS<rti.Rti>('!', '#', type), JS<rti.Rti>('!', '#', bound))
: isSubtypeOf(type, bound);
if (!validSubtype) {
throwTypeError('type `$type` does not extend `$bound` of `$name`.');
}

View file

@ -50,7 +50,7 @@ class JSArray<E> extends JavaScriptObject
/// `JS_EMBEDDED_GLOBAL('', ARRAY_RTI_PROPERTY)`.
Object get arrayRti => JS_GET_FLAG('NEW_RUNTIME_TYPES')
? dart.typeRep<JSArray<E>>()
: throw dart.throwUnimplementedInOldRti();
: throw dart.throwUnimplementedInCurrentRti();
/// Unsupported action, only provided here to help diagnosis of an accidental
/// attempt to set the value manually.

View file

@ -887,7 +887,7 @@ Object? createRecordTypePredicate(String partialShapeTag, JSArray fieldRtis) {
rti.pairwiseIsTest(fieldRtis, JS<JSArray>('!', '#.values', obj));
};
} else {
dart.throwUnimplementedInOldRti();
dart.throwUnimplementedInCurrentRti();
}
}
@ -911,7 +911,7 @@ rti.Rti getRtiForRecord(Object? record) {
return rti.evaluateRtiForRecord(recipeBuffer.toString(), recordObj.values);
} else {
dart.throwUnimplementedInOldRti();
dart.throwUnimplementedInCurrentRti();
}
}

View file

@ -36,9 +36,11 @@ bool _reportingExtraNullSafetyError = false;
@pragma('dart2js:as:trust')
void _onExtraNullSafetyError(TypeError error, StackTrace trace) {
// If [onExtraNullSafetyError] itself produces an extra null safety error,
// this avoids blowing the stack.
if (!_reportingExtraNullSafetyError) {
if (JS_GET_FLAG('DEV_COMPILER')) {
throw error;
} else if (!_reportingExtraNullSafetyError) {
// If [onExtraNullSafetyError] itself produces an extra null safety error,
// this avoids blowing the stack.
_reportingExtraNullSafetyError = true;
if (onExtraNullSafetyError != null) {
(onExtraNullSafetyError as OnExtraNullSafetyError)(error, trace);

View file

@ -13,8 +13,8 @@ import 'package:expect/expect.dart';
/// `--weak-null-safety-errors`.
void fn(StringBuffer arg) {}
void testArg<T>(T t) => t is T;
void testReturn<T>(T Function() f) => throw 'do not call';
void testArg<T>(T t) => throw 'do not call';
T testReturn<T>() => throw 'do not call';
const c = C<Duration>();
@ -28,10 +28,7 @@ void main() {
dynamic dynamicNull = null;
Expect.throwsTypeError(() => fn(dynamicNull));
var l = [Duration(days: 1), null];
Expect.throwsTypeError(() => l as List<Duration>);
Expect.throwsTypeError(() => (testReturn<Duration?>) as Duration Function());
Expect.throwsTypeError(() => [Duration(days: 1), null] as List<Duration>);
// Constants get legacy types introduced in their type arguments.
C<Duration?> c2 = c;
@ -44,6 +41,5 @@ void main() {
// subtype of `void Function(Duration?)`. In sound null safety the signature
// is `void Function(Duration)` which should fail in the cast.
Expect.throwsTypeError(() => (testArg<Duration>) as void Function(Duration?));
Expect.throwsTypeError(
() => (testReturn<Duration>) as void Function(Duration? Function()));
Expect.throwsTypeError(() => (testReturn<Duration?>) as Duration Function());
}