From 7dda27b543e3c0911ecc962c63946dc6d0082309 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Thu, 7 Dec 2023 23:30:06 +0000 Subject: [PATCH] inference-update-3: use unpromoted type as context for local variable assignments. This change implements one of the features of experimental feature `inference-update-3`: with the experimental feature enabled, assignments to local variables use the declared (or inferred) type of the local variable as the context for evaluating the RHS of the assignment, regardless of whether the local variable is promoted. With the experimental feature disabled, assignments to local variables continue to use the promoted type of the local variable as the context for evaluating the RHS of the assignment. This eliminates one of the scenarios in which the context type of an assignment is "aspirational" (i.e., not required to be met in order to prevent a compile time error). Once all aspirational context types have been removed from the language, we will be able to re-work coercions to be based on context type, which fixes a number of usability footguns in the language. See https://github.com/dart-lang/language/issues/3471 for details. Bug: https://github.com/dart-lang/language/issues/3471 Change-Id: Ic07ac1810b641a9208c168846cd5fd912088d62b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/338802 Reviewed-by: Bob Nystrom Reviewed-by: Konstantin Shcheglov Reviewed-by: Johnni Winther Commit-Queue: Paul Berry --- .../assignment_expression_resolver.dart | 5 +- .../type_inference/inference_visitor.dart | 3 +- ...ocal_assignment_context_disabled_test.dart | 164 ++++++++++++++++++ ...romoted_local_assignment_context_test.dart | 84 +++++++++ tests/language/static_type_helper.dart | 2 +- 5 files changed, 255 insertions(+), 3 deletions(-) create mode 100644 tests/language/inference_update_3/promoted_local_assignment_context_disabled_test.dart create mode 100644 tests/language/inference_update_3/promoted_local_assignment_context_test.dart diff --git a/pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart index 6955f6038b5..aae52748a75 100644 --- a/pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'package:_fe_analyzer_shared/src/flow_analysis/flow_analysis.dart'; +import 'package:analyzer/dart/analysis/features.dart'; import 'package:analyzer/dart/ast/token.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; @@ -86,7 +87,9 @@ class AssignmentExpressionResolver { DartType? rhsContext; { var leftType = node.writeType; - if (writeElement is VariableElement) { + if (writeElement is VariableElement && + !_resolver.definingLibrary.featureSet + .isEnabled(Feature.inference_update_3)) { leftType = _resolver.localVariableTypeProvider .getType(left as SimpleIdentifier, isRead: false); } diff --git a/pkg/front_end/lib/src/fasta/type_inference/inference_visitor.dart b/pkg/front_end/lib/src/fasta/type_inference/inference_visitor.dart index c5b29166c9b..396b9294d2a 100644 --- a/pkg/front_end/lib/src/fasta/type_inference/inference_visitor.dart +++ b/pkg/front_end/lib/src/fasta/type_inference/inference_visitor.dart @@ -8596,7 +8596,8 @@ class InferenceVisitorImpl extends InferenceVisitorBase } DartType declaredOrInferredType = variable.lateType ?? variable.type; DartType? promotedType; - if (isNonNullableByDefault) { + if (isNonNullableByDefault && + !libraryBuilder.libraryFeatures.inferenceUpdate3.isEnabled) { promotedType = flowAnalysis.promotedType(variable); } ExpressionInferenceResult rhsResult = inferExpression( diff --git a/tests/language/inference_update_3/promoted_local_assignment_context_disabled_test.dart b/tests/language/inference_update_3/promoted_local_assignment_context_disabled_test.dart new file mode 100644 index 00000000000..19b6d026f1f --- /dev/null +++ b/tests/language/inference_update_3/promoted_local_assignment_context_disabled_test.dart @@ -0,0 +1,164 @@ +// Copyright (c) 2023, 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 that when `inference-update-3` is disabled, and an assignment is made +// to a local variable (or parameter), the unpromoted type of the variable is +// used as the context for the RHS of the assignment. + +// @dart=3.3 + +import 'package:expect/expect.dart'; + +import '../static_type_helper.dart'; + +void testNonDemotingAssignmentOfParameter(num? x, num? y) { + if (x != null) { + x = contextType(1)..expectStaticType>(); + } + if (y is int) { + y = contextType(1)..expectStaticType>(); + } +} + +void testNonDemotingAssignmentOfExplicitlyTypedLocal(num? x) { + num? y = x; + if (y != null) { + y = contextType(1)..expectStaticType>(); + } + y = x; + if (y is int) { + y = contextType(1)..expectStaticType>(); + } +} + +void testNonDemotingAssignmentOfImplicitlyTypedLocal(num? x) { + var y = x; + if (y != null) { + y = contextType(1)..expectStaticType>(); + } + y = x; + if (y is int) { + y = contextType(1)..expectStaticType>(); + } +} + +void testDemotingAssignmentOfParameter(num? x, num? y) { + if (x != null) { + // A type error is expected at runtime, because type inference will fill in + // a type of `num` for the type argument of `contextType`, and `contextType` + // tries to cast its input to its type argument. + // + // We can't use `Expect.throwsTypeError` to check for this, because then the + // assignment would be happening inside a function expression, blocking type + // promotion. + bool errorOccurred = false; + try { + x = contextType(null)..expectStaticType>(); + } on TypeError { + errorOccurred = true; + } + Expect.equals(hasSoundNullSafety, errorOccurred); + } + if (y is int) { + // A type error is expected at runtime, because type inference will fill in + // a type of `int` for the type argument of `contextType`, and `contextType` + // tries to cast its input to its type argument. + // + // We can't use `Expect.throwsTypeError` to check for this, because then the + // assignment would be happening inside a function expression, blocking type + // promotion. + bool errorOccurred = false; + try { + y = contextType(1.5)..expectStaticType>(); + } on TypeError { + errorOccurred = true; + } + Expect.isTrue(errorOccurred); + } +} + +void testDemotingAssignmentOfExplicitlyTypedLocal(num? x) { + num? y = x; + if (y != null) { + // A type error is expected at runtime, because type inference will fill in + // a type of `num` for the type argument of `contextType`, and `contextType` + // tries to cast its input to its type argument. + // + // We can't use `Expect.throwsTypeError` to check for this, because then the + // assignment would be happening inside a function expression, blocking type + // promotion. + bool errorOccurred = false; + try { + y = contextType(null)..expectStaticType>(); + } on TypeError { + errorOccurred = true; + } + Expect.equals(hasSoundNullSafety, errorOccurred); + } + y = x; + if (y is int) { + // A type error is expected at runtime, because type inference will fill in + // a type of `int` for the type argument of `contextType`, and `contextType` + // tries to cast its input to its type argument. + // + // We can't use `Expect.throwsTypeError` to check for this, because then the + // assignment would be happening inside a function expression, blocking type + // promotion. + bool errorOccurred = false; + try { + y = contextType(1.5)..expectStaticType>(); + } on TypeError { + errorOccurred = true; + } + Expect.isTrue(errorOccurred); + } +} + +void testDemotingAssignmentOfImplicitlyTypedLocal(num? x) { + var y = x; + if (y != null) { + // A type error is expected at runtime, because type inference will fill in + // a type of `num` for the type argument of `contextType`, and `contextType` + // tries to cast its input to its type argument. + // + // We can't use `Expect.throwsTypeError` to check for this, because then the + // assignment would be happening inside a function expression, blocking type + // promotion. + bool errorOccurred = false; + try { + y = contextType(null)..expectStaticType>(); + } on TypeError { + errorOccurred = true; + } + Expect.equals(hasSoundNullSafety, errorOccurred); + } + y = x; + if (y is int) { + // A type error is expected at runtime, because type inference will fill in + // a type of `int` for the type argument of `contextType`, and `contextType` + // tries to cast its input to its type argument. + // + // We can't use `Expect.throwsTypeError` to check for this, because then the + // assignment would be happening inside a function expression, blocking type + // promotion. + bool errorOccurred = false; + try { + y = contextType(1.5)..expectStaticType>(); + } on TypeError { + errorOccurred = true; + } + Expect.isTrue(errorOccurred); + } +} + +main() { + for (var x in [null, 0, 0.5]) { + testNonDemotingAssignmentOfParameter(x, x); + testNonDemotingAssignmentOfExplicitlyTypedLocal(x); + testNonDemotingAssignmentOfImplicitlyTypedLocal(x); + testDemotingAssignmentOfParameter(x, x); + testDemotingAssignmentOfExplicitlyTypedLocal(x); + testDemotingAssignmentOfImplicitlyTypedLocal(x); + } +} diff --git a/tests/language/inference_update_3/promoted_local_assignment_context_test.dart b/tests/language/inference_update_3/promoted_local_assignment_context_test.dart new file mode 100644 index 00000000000..2e59c013023 --- /dev/null +++ b/tests/language/inference_update_3/promoted_local_assignment_context_test.dart @@ -0,0 +1,84 @@ +// Copyright (c) 2023, 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 that when `inference-update-3` is enabled, and an assignment is made to +// a local variable (or parameter), the unpromoted type of the variable is used +// as the context for the RHS of the assignment. + +// SharedOptions=--enable-experiment=inference-update-3 + +import '../static_type_helper.dart'; + +void testNonDemotingAssignmentOfParameter(num? x, num? y) { + if (x != null) { + x = contextType(1)..expectStaticType>(); + } + if (y is int) { + y = contextType(1)..expectStaticType>(); + } +} + +void testNonDemotingAssignmentOfExplicitlyTypedLocal(num? x) { + num? y = x; + if (y != null) { + y = contextType(1)..expectStaticType>(); + } + y = x; + if (y is int) { + y = contextType(1)..expectStaticType>(); + } +} + +void testNonDemotingAssignmentOfImplicitlyTypedLocal(num? x) { + var y = x; + if (y != null) { + y = contextType(1)..expectStaticType>(); + } + y = x; + if (y is int) { + y = contextType(1)..expectStaticType>(); + } +} + +void testDemotingAssignmentOfParameter(num? x, num? y) { + if (x != null) { + x = contextType(null)..expectStaticType>(); + } + if (y is int) { + y = contextType(null)..expectStaticType>(); + } +} + +void testDemotingAssignmentOfExplicitlyTypedLocal(num? x) { + num? y = x; + if (y != null) { + y = contextType(null)..expectStaticType>(); + } + y = x; + if (y is int) { + y = contextType(null)..expectStaticType>(); + } +} + +void testDemotingAssignmentOfImplicitlyTypedLocal(num? x) { + var y = x; + if (y != null) { + y = contextType(null)..expectStaticType>(); + } + y = x; + if (y is int) { + y = contextType(null)..expectStaticType>(); + } +} + +main() { + for (var x in [null, 0, 0.5]) { + testNonDemotingAssignmentOfParameter(x, x); + testNonDemotingAssignmentOfExplicitlyTypedLocal(x); + testNonDemotingAssignmentOfImplicitlyTypedLocal(x); + testDemotingAssignmentOfParameter(x, x); + testDemotingAssignmentOfExplicitlyTypedLocal(x); + testDemotingAssignmentOfImplicitlyTypedLocal(x); + } +} diff --git a/tests/language/static_type_helper.dart b/tests/language/static_type_helper.dart index 14acd637f2b..cb81a52c563 100644 --- a/tests/language/static_type_helper.dart +++ b/tests/language/static_type_helper.dart @@ -14,7 +14,7 @@ Object? context(T x) => x; /// ```dart /// int x = contextType(1 /* valid value */)..expectStaticType>; /// ``` -T contextType(Object result) => result as T; +T contextType(Object? result) => result as T; extension StaticType on T { /// Check the static type.