From a09c5857721ae9d7768b123b2826ef9396e47004 Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Fri, 7 Jan 2022 14:22:26 +0000 Subject: [PATCH] [vm/aot] Keep static type of a conditional expression when removing it in tree shaker In order to make sure static types in kernel AST are still correct after tree shaking, tree shaker needs to insert no-op unsafeCast calls when removing conditional expressions, as ConditionalExpression nodes have explicit 'staticType' which may be different from static types of sub-expressions. Otherwise we might end up with an untyped function call (FunctionInvocation with FunctionAccessKind.Function) performed from a receiver with a known function type. That would violate VM's assumptions about static type of receiver being checked by the front-end and trigger assertion in pkg/vm/lib/transformations/call_site_annotator.dart. TEST=runtime/tests/vm/dart/untyped_function_invocation_with_known_function_type_test.dart TEST=pkg/vm/testcases/transformations/type_flow/transformer/null_test_elimination2_nnbd_strong.dart Change-Id: I6bf201a979d1b71eb48ed04f154adf2b62dac922 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/226741 Reviewed-by: Johnni Winther Reviewed-by: Slava Egorov Commit-Queue: Alexander Markov --- .../type_flow/transformer.dart | 14 ++++++++-- .../null_test_elimination2_nnbd_strong.dart | 20 ++++++++++++++ ..._test_elimination2_nnbd_strong.dart.expect | 19 +++++++++++++ ...ocation_with_known_function_type_test.dart | 27 +++++++++++++++++++ 4 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 pkg/vm/testcases/transformations/type_flow/transformer/null_test_elimination2_nnbd_strong.dart create mode 100644 pkg/vm/testcases/transformations/type_flow/transformer/null_test_elimination2_nnbd_strong.dart.expect create mode 100644 runtime/tests/vm/dart/untyped_function_invocation_with_known_function_type_test.dart diff --git a/pkg/vm/lib/transformations/type_flow/transformer.dart b/pkg/vm/lib/transformations/type_flow/transformer.dart index d82925ad6c1..6533e548588 100644 --- a/pkg/vm/lib/transformations/type_flow/transformer.dart +++ b/pkg/vm/lib/transformations/type_flow/transformer.dart @@ -1583,12 +1583,22 @@ class _TreeShakerPass1 extends RemovingTransformer { if (_isExtendedBoolLiteral(condition)) { final bool value = _getExtendedBoolLiteralValue(condition); final Expression expr = transform(value ? node.then : node.otherwise); + Expression result; if (condition is BlockExpression) { condition.value = expr; expr.parent = condition; - return condition; + result = condition; } else { - return expr; + result = expr; + } + if (node.staticType != result.getStaticType(staticTypeContext)) { + return StaticInvocation( + unsafeCast, + Arguments([result], + types: [visitDartType(node.staticType, cannotRemoveSentinel)])) + ..fileOffset = node.fileOffset; + } else { + return result; } } node.condition = condition..parent = node; diff --git a/pkg/vm/testcases/transformations/type_flow/transformer/null_test_elimination2_nnbd_strong.dart b/pkg/vm/testcases/transformations/type_flow/transformer/null_test_elimination2_nnbd_strong.dart new file mode 100644 index 00000000000..47a6a15e0b2 --- /dev/null +++ b/pkg/vm/testcases/transformations/type_flow/transformer/null_test_elimination2_nnbd_strong.dart @@ -0,0 +1,20 @@ +// 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. + +// Tests elimination of null test in a conditional expression of +// a different static type. + +bool _defaultCheck([dynamic _]) => true; + +void testStaticTypeOfConditional(bool Function(T error)? check, Object e) { + // Verify that null test elimination leaves unsafeCast here to + // keep static type of 'check ?? _defaultCheck' expression. + if (e is T && (check ?? _defaultCheck)(e)) { + print('ok'); + } +} + +void main() { + testStaticTypeOfConditional((_) => true, 'hi'); +} diff --git a/pkg/vm/testcases/transformations/type_flow/transformer/null_test_elimination2_nnbd_strong.dart.expect b/pkg/vm/testcases/transformations/type_flow/transformer/null_test_elimination2_nnbd_strong.dart.expect new file mode 100644 index 00000000000..de2490beb0c --- /dev/null +++ b/pkg/vm/testcases/transformations/type_flow/transformer/null_test_elimination2_nnbd_strong.dart.expect @@ -0,0 +1,19 @@ +library #lib /*isNonNullableByDefault*/; +import self as self; +import "dart:core" as core; +import "dart:_internal" as _in; + +static method _defaultCheck([dynamic _ = #C1]) → core::bool + return true; +static method testStaticTypeOfConditional([@vm.inferred-type.metadata=!] (self::testStaticTypeOfConditional::T%) →? core::bool check) → void { + if(#C2 is{ForNonNullableByDefault} self::testStaticTypeOfConditional::T% && (let final (self::testStaticTypeOfConditional::T%) →? core::bool #t1 = check in _in::unsafeCast(#t1{(self::testStaticTypeOfConditional::T%) → core::bool}))(#C2) as{TypeError,ForDynamic,ForNonNullableByDefault} core::bool) { + core::print("ok"); + } +} +static method main() → void { + self::testStaticTypeOfConditional((core::String _) → core::bool => true); +} +constants { + #C1 = null + #C2 = "hi" +} diff --git a/runtime/tests/vm/dart/untyped_function_invocation_with_known_function_type_test.dart b/runtime/tests/vm/dart/untyped_function_invocation_with_known_function_type_test.dart new file mode 100644 index 00000000000..9c1650152e3 --- /dev/null +++ b/runtime/tests/vm/dart/untyped_function_invocation_with_known_function_type_test.dart @@ -0,0 +1,27 @@ +// 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. + +// Verifies that compiler doesn't crash when it sees untyped function +// invocation (FunctionAccessKind.Function) but receiver has a known +// function type. + +import "package:expect/expect.dart"; + +bool ok = false; +bool _defaultCheck([dynamic _]) => true; + +void foo(bool Function(T error)? check, Object e) { + // Function call on the result of 'check ?? _defaultCheck' is untyped + // (assumes static type Function). However, AOT compiler can eliminate + // null test and might be able to reduce the expression to 'check' with + // known function type. + if (e is T && (check ?? _defaultCheck)(e)) { + ok = true; + } +} + +void main() { + foo((_) => true, 'hi'); + Expect.isTrue(ok); +}