Skip to content

Commit

Permalink
[stable] [vm/aot/tfa] Fix handling of type parameter nullability in f…
Browse files Browse the repository at this point in the history
…actory constructors

TFA represents type parameters inside factory constructors as
additional parameters. In a summary, type parameter type is represented
simply as a reference to a parameter. This approach ignores
nullability of a type parameter type, which is not correct.

This change add a new ApplyNullability operation to a summary in order
to apply any extra nullability ('?' or '*') on top of the type argument
passed to a factory constructor.

TEST=pkg/vm/testcases/transformations/type_flow/transformer/regress_50392_nnbd_strong.dart
Fixes #50392

Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/268381
Change-Id: I74080813663fbb7176ad30c0daf9f75de087506b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/268500
Reviewed-by: Siva Annamalai <[email protected]>
  • Loading branch information
alexmarkov committed Nov 19, 2022
1 parent 657d2c9 commit 203d14e
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 18 deletions.
46 changes: 29 additions & 17 deletions pkg/vm/lib/transformations/type_flow/summary.dart
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class StatementVisitor {
void visitUse(Use expr) => visitDefault(expr);
void visitCall(Call expr) => visitDefault(expr);
void visitExtract(Extract expr) => visitDefault(expr);
void visitApplyNullability(ApplyNullability expr) => visitDefault(expr);
void visitCreateConcreteType(CreateConcreteType expr) => visitDefault(expr);
void visitCreateRuntimeType(CreateRuntimeType expr) => visitDefault(expr);
void visitTypeCheck(TypeCheck expr) => visitDefault(expr);
Expand Down Expand Up @@ -405,23 +406,7 @@ class Extract extends Statement {
final typeArg = typeArgs[interfaceOffset + paramIndex];
Type extracted = typeArg;
if (typeArg is RuntimeType) {
final argNullability = typeArg.nullability;
if (argNullability != nullability) {
// Apply nullability of type parameter type.
Nullability result;
if (argNullability == Nullability.nullable ||
nullability == Nullability.nullable) {
result = Nullability.nullable;
} else if (argNullability == Nullability.legacy ||
nullability == Nullability.legacy) {
result = Nullability.legacy;
} else {
result = Nullability.nonNullable;
}
if (argNullability != result) {
extracted = typeArg.withNullability(result);
}
}
extracted = typeArg.applyNullability(nullability);
} else {
assert(typeArg is UnknownType);
}
Expand All @@ -444,6 +429,33 @@ class Extract extends Statement {
}
}

// Applies nullability to a given type argument.
class ApplyNullability extends Statement {
TypeExpr arg;

final Nullability nullability;

ApplyNullability(this.arg, this.nullability);

@override
void accept(StatementVisitor visitor) => visitor.visitApplyNullability(this);

@override
String dump() => "$label = _ApplyNullability ($arg, ${nullability.suffix})";

@override
Type apply(List<Type?> computedTypes, TypeHierarchy typeHierarchy,
CallHandler callHandler) {
Type argType = arg.getComputedType(computedTypes);
if (argType is RuntimeType) {
return argType.applyNullability(nullability);
} else {
assert(argType is UnknownType);
}
return argType;
}
}

// Instantiate a concrete type with type arguments. For example, used to fill in
// "T = int" in "C<T>" to create "C<int>".
//
Expand Down
16 changes: 15 additions & 1 deletion pkg/vm/lib/transformations/type_flow/summary_collector.dart
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,11 @@ class _SummaryNormalizer extends StatementVisitor {
void visitExtract(Extract expr) {
expr.arg = _normalizeExpr(expr.arg, true);
}

@override
void visitApplyNullability(ApplyNullability expr) {
expr.arg = _normalizeExpr(expr.arg, true);
}
}

/// Detects whether the control flow can pass through the function body and
Expand Down Expand Up @@ -2445,7 +2450,16 @@ class RuntimeTypeTranslatorImpl extends DartTypeVisitor<TypeExpr>
final functionTypeVariables = this.functionTypeVariables;
if (functionTypeVariables != null) {
final result = functionTypeVariables[type.parameter];
if (result != null) return result;
if (result != null) {
final nullability = type.nullability;
if (nullability != Nullability.nonNullable &&
nullability != Nullability.undetermined) {
final applyNullability = ApplyNullability(result, nullability);
summary!.add(applyNullability);
return applyNullability;
}
return result;
}
}
if (type.parameter.parent is! Class) return const UnknownType();
final interfaceClass = type.parameter.parent as Class;
Expand Down
20 changes: 20 additions & 0 deletions pkg/vm/lib/transformations/type_flow/types.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,26 @@ class RuntimeType extends Type {
RuntimeType withNullability(Nullability n) =>
RuntimeType(_type.withDeclaredNullability(n), typeArgs);

RuntimeType applyNullability(Nullability nullability) {
final thisNullability = this.nullability;
if (thisNullability != nullability) {
Nullability result;
if (thisNullability == Nullability.nullable ||
nullability == Nullability.nullable) {
result = Nullability.nullable;
} else if (thisNullability == Nullability.legacy ||
nullability == Nullability.legacy) {
result = Nullability.legacy;
} else {
result = Nullability.nonNullable;
}
if (thisNullability != result) {
return withNullability(result);
}
}
return this;
}

DartType get representedTypeRaw => _type;

DartType get representedType {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
// 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.

// Regression test for https://github.com/dart-lang/sdk/issues/50392

void main(List<String> arguments) {
final model = Model();

print(model.value); // null
print(model.value == null); // true
}

class Model<T extends num> {
final T? value;

Model._(this.value);

factory Model() {
Object? value = (int.parse('1') == 1) ? null : 42;
return Model._(value as T?);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
library #lib /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;
import "dart:_internal" as _in;

class Model<T extends core::num> extends core::Object {
[@vm.inferred-type.metadata=dart.core::_Smi?] [@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasNonThisUses:false,hasTearOffUses:false,getterSelectorId:1] final field self::Model::T? value;
constructor _([@vm.inferred-type.metadata=dart.core::_Smi?] self::Model::T? value) → self::Model<self::Model::T>
: self::Model::value = value, super core::Object::•()
;
static factory •<T extends core::num>() → self::Model<self::Model::•::T> {
core::Object? value = [@vm.direct-call.metadata=dart.core::_IntegerImplementation.==] [@vm.inferred-type.metadata=dart.core::bool (skip check)] [@vm.inferred-type.metadata=int] core::int::parse("1") =={core::num::==}{(core::Object) → core::bool} 1 ?{core::int?} null : 42;
return new self::Model::_<self::Model::•::T>(_in::unsafeCast<self::Model::•::T?>(value));
}
}
static method main(core::List<core::String> arguments) → void {
final self::Model<core::num> model = [@vm.inferred-type.metadata=#lib::Model<dart.core::num>] self::Model::•<core::num>();
core::print([@vm.direct-call.metadata=#lib::Model.value] [@vm.inferred-type.metadata=dart.core::_Smi?] model.{self::Model::value}{core::num?});
core::print([@vm.direct-call.metadata=#lib::Model.value] [@vm.inferred-type.metadata=dart.core::_Smi?] model.{self::Model::value}{core::num?} == null);
}

0 comments on commit 203d14e

Please sign in to comment.