Skip to content

Commit

Permalink
[VM/runtime] Allow generic function types as type arguments in the VM.
Browse files Browse the repository at this point in the history
Type parameter scopes were not set properly when loading bounds with generic function types from kernel file.
The trail to avoid infinite recursion while comparing type parameter bounds was not used properly.

Fixes #45313
Fixes #45322
Fixes #44930

TEST=regression tests added

Change-Id: I096b7d8c22aa5b3a3a9e12a59b1417388ff5171f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/191465
Commit-Queue: Régis Crelier <[email protected]>
Reviewed-by: Siva Annamalai <[email protected]>
Reviewed-by: Alexander Markov <[email protected]>
  • Loading branch information
crelier authored and [email protected] committed Mar 17, 2021
1 parent 1a7d39b commit 33cabf4
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 16 deletions.
52 changes: 40 additions & 12 deletions runtime/vm/compiler/frontend/kernel_translation_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3054,22 +3054,26 @@ void TypeTranslator::BuildFunctionType(bool simple) {

// Suspend finalization of types inside this one. They will be finalized after
// the whole function type is constructed.
//
// TODO(31213): Test further when nested generic function types
// are supported by fasta.
bool finalize = finalize_;
finalize_ = false;
intptr_t type_parameter_count = 0;

if (!simple) {
type_parameter_count = helper_->ReadListLength();
LoadAndSetupTypeParameters(
active_class_, Object::null_function(), Object::null_class(), signature,
helper_->ReadListLength(), active_class_->klass->nnbd_mode());
type_parameter_count, active_class_->klass->nnbd_mode());
}

ActiveTypeParametersScope scope(
active_class_, &signature,
TypeArguments::Handle(Z, signature.type_parameters()), Z);

if (!simple) {
LoadAndSetupBounds(active_class_, Object::null_function(),
Object::null_class(), signature, type_parameter_count);
}

intptr_t required_count;
intptr_t all_count;
intptr_t positional_count;
Expand Down Expand Up @@ -3324,7 +3328,6 @@ void TypeTranslator::LoadAndSetupTypeParameters(
? Nullability::kNonNullable
: Nullability::kLegacy;

// Step a)
// - Create array of [TypeParameter] objects (without bound).
// - Create array of [String] objects.
type_parameters = TypeArguments::New(type_parameter_count);
Expand Down Expand Up @@ -3354,20 +3357,35 @@ void TypeTranslator::LoadAndSetupTypeParameters(
} else {
name = H.DartIdentifier(lib, helper.name_index_).ptr();
}
// Bounds are filled later in LoadAndSetupBounds as bound types may
// reference type parameters which are not created yet.
parameter = TypeParameter::New(
parameterized_class, offset, offset + i, name, null_bound,
helper.IsGenericCovariantImpl(), nullability);
type_parameters.SetTypeAt(i, parameter);
}
}
}

const FunctionType* enclosing = NULL;
if (!parameterized_signature.IsNull()) {
enclosing = &parameterized_signature;
void TypeTranslator::LoadAndSetupBounds(
ActiveClass* active_class,
const Function& function,
const Class& parameterized_class,
const FunctionType& parameterized_signature,
intptr_t type_parameter_count) {
ASSERT(parameterized_class.IsNull() != parameterized_signature.IsNull());
ASSERT(type_parameter_count >= 0);
if (type_parameter_count == 0) {
return;
}
ActiveTypeParametersScope scope(active_class, enclosing, type_parameters, Z);

// Step b) Fill in the bounds and default arguments of all [TypeParameter]s.
const TypeArguments& type_parameters =
TypeArguments::Handle(Z, !parameterized_class.IsNull()
? parameterized_class.type_parameters()
: parameterized_signature.type_parameters());
TypeParameter& parameter = TypeParameter::Handle(Z);

// Fill in the bounds and default arguments of all [TypeParameter]s.
for (intptr_t i = 0; i < type_parameter_count; i++) {
TypeParameterHelper helper(helper_);
helper.ReadUntilExcludingAndSetJustRead(TypeParameterHelper::kBound);
Expand All @@ -3389,6 +3407,9 @@ void TypeTranslator::LoadAndSetupTypeParameters(

// Fix bounds and default arguments in all derived type parameters (with
// different nullabilities).
const intptr_t offset = !parameterized_signature.IsNull()
? parameterized_signature.NumParentTypeArguments()
: 0;
if (active_class->derived_type_parameters != nullptr) {
auto& derived = TypeParameter::Handle(Z);
auto& type = AbstractType::Handle(Z);
Expand Down Expand Up @@ -3535,15 +3556,22 @@ void TypeTranslator::SetupFunctionParameters(

const FunctionType& signature = FunctionType::Handle(Z, function.signature());
ASSERT(!signature.IsNull());
intptr_t type_parameter_count = 0;
if (!is_factory) {
type_parameter_count = helper_->ReadListLength();
LoadAndSetupTypeParameters(active_class_, function, Class::Handle(Z),
signature, helper_->ReadListLength(),
signature, type_parameter_count,
function.nnbd_mode());
function_node_helper->SetJustRead(FunctionNodeHelper::kTypeParameters);
}

ActiveTypeParametersScope scope(active_class_, function, &signature, Z);

if (!is_factory) {
LoadAndSetupBounds(active_class_, function, Class::Handle(Z), signature,
type_parameter_count);
function_node_helper->SetJustRead(FunctionNodeHelper::kTypeParameters);
}

function_node_helper->ReadUntilExcluding(
FunctionNodeHelper::kPositionalParameters);

Expand Down
6 changes: 6 additions & 0 deletions runtime/vm/compiler/frontend/kernel_translation_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -1478,6 +1478,12 @@ class TypeTranslator {
intptr_t type_parameter_count,
const NNBDMode nnbd_mode);

void LoadAndSetupBounds(ActiveClass* active_class,
const Function& function,
const Class& parameterized_class,
const FunctionType& parameterized_signature,
intptr_t type_parameter_count);

const Type& ReceiverType(const Class& klass);

void SetupFunctionParameters(const Class& klass,
Expand Down
3 changes: 3 additions & 0 deletions runtime/vm/kernel_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1424,6 +1424,9 @@ void KernelLoader::LoadPreliminaryClass(ClassHelper* class_helper,
Object::null_function_type(),
type_parameter_count, klass->nnbd_mode());

T.LoadAndSetupBounds(&active_class_, Object::null_function(), *klass,
Object::null_function_type(), type_parameter_count);

// Set super type. Some classes (e.g., Object) do not have one.
Tag type_tag = helper_.ReadTag(); // read super class type (part 1).
if (type_tag == kSomething) {
Expand Down
12 changes: 8 additions & 4 deletions runtime/vm/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21117,10 +21117,12 @@ bool TypeParameter::IsEquivalent(const Instance& other,
return false;
}
// Compare bounds.
if (TestAndAddBuddyToTrail(&trail, other_type_param)) {
return true;
}
AbstractType& type = AbstractType::Handle(bound());
AbstractType& other_type = AbstractType::Handle(other_type_param.bound());
if (!TestAndAddBuddyToTrail(&trail, other_type_param) &&
!type.IsEquivalent(other_type, kind, trail)) {
if (!type.IsEquivalent(other_type, kind, trail)) {
return false;
}
if (kind == TypeEquality::kCanonical) {
Expand Down Expand Up @@ -21164,11 +21166,13 @@ bool TypeParameter::IsEquivalent(const Instance& other,
}
}
// Compare bounds.
if (TestAndAddBuddyToTrail(&trail, other_type_param)) {
return true;
}
AbstractType& upper_bound = AbstractType::Handle(bound());
AbstractType& other_type_param_upper_bound =
AbstractType::Handle(other_type_param.bound());
if (!TestAndAddBuddyToTrail(&trail, other_type_param) &&
!upper_bound.IsEquivalent(other_type_param_upper_bound, kind, trail)) {
if (!upper_bound.IsEquivalent(other_type_param_upper_bound, kind, trail)) {
return false;
}
}
Expand Down
17 changes: 17 additions & 0 deletions tests/language/regress/regress45313_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) 2021, 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.

// SharedOptions=--enable-experiment=generic-metadata

import "package:expect/expect.dart";

typedef TEST_TYPEDEF<TT extends T Function<T>(T)> = void
Function<TTT extends TT>();
void testme<TT extends T Function<T>(T)>() {}

main() {
Expect.isTrue(testme is TEST_TYPEDEF);
TEST_TYPEDEF ttttt = testme;
Expect.isTrue(ttttt is TEST_TYPEDEF);
}

0 comments on commit 33cabf4

Please sign in to comment.