From 6b12473f75788fbf7cdca52c68b7bd99609ba221 Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Tue, 19 Dec 2023 16:14:39 +0000 Subject: [PATCH] [vm] Adjust return value of async/async*/sync* after the language spec change Calculation of element type for the value returned from async/async*/sync* functions was recently adjusted in the spec to address soundness issue (https://github.com/dart-lang/language/pull/3218). This change adjusts Dart VM to conform to the new spec. TEST=language/async/regression_54311_test TEST=co19/Language/Functions/element_type_* Fixes https://github.com/dart-lang/sdk/issues/54316 Issue https://github.com/dart-lang/sdk/issues/54311 Issue https://github.com/dart-lang/sdk/issues/54159 Change-Id: I4c51e7cba704d034350519375210bdb2086c5432 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342082 Reviewed-by: Alexander Aprelev Reviewed-by: Erik Ernst Commit-Queue: Alexander Markov --- .../frontend/kernel_binary_flowgraph.cc | 71 ++++++++++--------- .../frontend/kernel_binary_flowgraph.h | 3 +- runtime/vm/compiler/frontend/scope_builder.cc | 5 ++ .../language/async/regression_54311_test.dart | 29 ++++++++ 4 files changed, 73 insertions(+), 35 deletions(-) create mode 100644 tests/language/async/regression_54311_test.dart diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc index ee5c2bef2fe0..606e6e37d8e2 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc @@ -559,33 +559,25 @@ Fragment StreamingFlowGraphBuilder::SetupCapturedParameters( } Fragment StreamingFlowGraphBuilder::InitSuspendableFunction( - const Function& dart_function) { + const Function& dart_function, + const AbstractType* emitted_value_type) { Fragment body; if (dart_function.IsAsyncFunction()) { - const auto& result_type = - AbstractType::Handle(Z, dart_function.result_type()); - auto& type_args = TypeArguments::ZoneHandle(Z); - if (result_type.IsType() && - (Class::Handle(Z, result_type.type_class()).IsFutureClass() || - result_type.IsFutureOrType())) { - ASSERT(result_type.IsFinalized()); - type_args = Type::Cast(result_type).GetInstanceTypeArguments(H.thread()); - } - + ASSERT(emitted_value_type != nullptr); + auto& type_args = TypeArguments::ZoneHandle(Z, TypeArguments::New(1)); + type_args.SetTypeAt(0, *emitted_value_type); + type_args = Class::Handle(Z, IG->object_store()->future_class()) + .GetInstanceTypeArguments(H.thread(), type_args); body += TranslateInstantiatedTypeArguments(type_args); body += B->Call1ArgStub(TokenPosition::kNoSource, Call1ArgStubInstr::StubId::kInitAsync); body += Drop(); } else if (dart_function.IsAsyncGenerator()) { - const auto& result_type = - AbstractType::Handle(Z, dart_function.result_type()); - auto& type_args = TypeArguments::ZoneHandle(Z); - if (result_type.IsType() && - (result_type.type_class() == IG->object_store()->stream_class())) { - ASSERT(result_type.IsFinalized()); - type_args = Type::Cast(result_type).GetInstanceTypeArguments(H.thread()); - } - + ASSERT(emitted_value_type != nullptr); + auto& type_args = TypeArguments::ZoneHandle(Z, TypeArguments::New(1)); + type_args.SetTypeAt(0, *emitted_value_type); + type_args = Class::Handle(Z, IG->object_store()->stream_class()) + .GetInstanceTypeArguments(H.thread(), type_args); body += TranslateInstantiatedTypeArguments(type_args); body += B->Call1ArgStub(TokenPosition::kNoSource, Call1ArgStubInstr::StubId::kInitAsyncStar); @@ -595,15 +587,11 @@ Fragment StreamingFlowGraphBuilder::InitSuspendableFunction( SuspendInstr::StubId::kYieldAsyncStar); body += Drop(); } else if (dart_function.IsSyncGenerator()) { - const auto& result_type = - AbstractType::Handle(Z, dart_function.result_type()); - auto& type_args = TypeArguments::ZoneHandle(Z); - if (result_type.IsType() && - (result_type.type_class() == IG->object_store()->iterable_class())) { - ASSERT(result_type.IsFinalized()); - type_args = Type::Cast(result_type).GetInstanceTypeArguments(H.thread()); - } - + ASSERT(emitted_value_type != nullptr); + auto& type_args = TypeArguments::ZoneHandle(Z, TypeArguments::New(1)); + type_args.SetTypeAt(0, *emitted_value_type); + type_args = Class::Handle(Z, IG->object_store()->iterable_class()) + .GetInstanceTypeArguments(H.thread(), type_args); body += TranslateInstantiatedTypeArguments(type_args); body += B->Call1ArgStub(TokenPosition::kNoSource, Call1ArgStubInstr::StubId::kInitSyncStar); @@ -618,6 +606,8 @@ Fragment StreamingFlowGraphBuilder::InitSuspendableFunction( if (scope->num_context_variables() > 0) { body += CloneContext(scope->context_slots()); } + } else { + ASSERT(emitted_value_type == nullptr); } return body; } @@ -784,17 +774,30 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfFunction( LocalVariable* first_parameter = nullptr; TokenPosition token_position = TokenPosition::kNoSource; + const AbstractType* emitted_value_type = nullptr; { AlternativeReadingScope alt(&reader_); FunctionNodeHelper function_node_helper(this); function_node_helper.ReadUntilExcluding( FunctionNodeHelper::kPositionalParameters); - intptr_t list_length = ReadListLength(); // read number of positionals. - if (list_length > 0) { - intptr_t first_parameter_offset = ReaderOffset() + data_program_offset_; - first_parameter = LookupVariable(first_parameter_offset); + { + AlternativeReadingScope alt2(&reader_); + intptr_t list_length = ReadListLength(); // read number of positionals. + if (list_length > 0) { + intptr_t first_parameter_offset = ReaderOffset() + data_program_offset_; + first_parameter = LookupVariable(first_parameter_offset); + } } token_position = function_node_helper.position_; + if (dart_function.IsSuspendableFunction()) { + function_node_helper.ReadUntilExcluding( + FunctionNodeHelper::kEmittedValueType); + if (ReadTag() == kSomething) { + emitted_value_type = &T.BuildType(); // read emitted value type. + } else { + UNREACHABLE(); + } + } } auto graph_entry = flow_graph_builder_->graph_entry_ = @@ -833,7 +836,7 @@ FlowGraph* StreamingFlowGraphBuilder::BuildGraphOfFunction( // objects than necessary during GC. const Fragment body = ClearRawParameters(dart_function) + B->BuildNullAssertions() + - InitSuspendableFunction(dart_function) + + InitSuspendableFunction(dart_function, emitted_value_type) + BuildFunctionBody(dart_function, first_parameter, is_constructor); auto extra_entry_point_style = diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h index 475d660b3541..cd69b6a04577 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h @@ -85,7 +85,8 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper { TokenPosition position); Fragment CheckStackOverflowInPrologue(const Function& dart_function); Fragment SetupCapturedParameters(const Function& dart_function); - Fragment InitSuspendableFunction(const Function& dart_function); + Fragment InitSuspendableFunction(const Function& dart_function, + const AbstractType* emitted_value_type); Fragment ShortcutForUserDefinedEquals(const Function& dart_function, LocalVariable* first_parameter); Fragment TypeArgumentsHandling(const Function& dart_function); diff --git a/runtime/vm/compiler/frontend/scope_builder.cc b/runtime/vm/compiler/frontend/scope_builder.cc index 435d92fb80d0..2a1b3a508e19 100644 --- a/runtime/vm/compiler/frontend/scope_builder.cc +++ b/runtime/vm/compiler/frontend/scope_builder.cc @@ -243,6 +243,11 @@ ScopeBuildingResult* ScopeBuilder::BuildScopes() { // async/async*/sync* function. It may reference receiver or type // arguments of the enclosing function which need to be captured. VisitDartType(); + + // Visit optional future value type. + if (helper_.ReadTag() == kSomething) { + VisitDartType(); + } } // We generate a synthetic body for implicit closure functions - which diff --git a/tests/language/async/regression_54311_test.dart b/tests/language/async/regression_54311_test.dart new file mode 100644 index 000000000000..a73a0b63cae0 --- /dev/null +++ b/tests/language/async/regression_54311_test.dart @@ -0,0 +1,29 @@ +// 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. + +// Verifies that futureValueType(FutureOr) = Object. +// +// This is a special case because NORM(FutureOr) = Object, +// and futureValueType(Object) = Object?, so futureValueType cannot be +// applied to a normalized type. +// +// Regression test for https://github.com/dart-lang/sdk/issues/54311. + +import 'dart:async'; + +import "package:expect/expect.dart"; + +FutureOr fn1() async { + return Future.value(42); +} + +FutureOr fn2() async => 42; + +void main() async { + final value1 = await fn1(); + Expect.equals(42, value1); + + final value2 = await fn2(); + Expect.equals(42, value2); +}