Skip to content

Commit 55eb75d

Browse files
[kernel] Delay type finalization when parsing function types.
Currently, the Kernel FE doesn't set up parent pointers correctly for signature function types. This prevents type parameters on generic function types from being finalized correctly. In addition, finalization of generic interface types within a generic function type can crash if they reference not-yet-finalized type parameters of the enclosing function type. This review solves both issues. Several failing tests pass again, although more thorough testing is blocked on Github issue #31213 (nested generic function types crashing in Fasta). Bug: Change-Id: Ib5ee6b2566492e3fd6688fe5a6b6976692562ea1 Reviewed-on: https://dart-review.googlesource.com/16360 Reviewed-by: Régis Crelier <[email protected]>
1 parent f94ffbf commit 55eb75d

File tree

5 files changed

+44
-15
lines changed

5 files changed

+44
-15
lines changed

runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2226,17 +2226,28 @@ void StreamingDartTypeTranslator::BuildInterfaceType(bool simple) {
22262226

22272227
void StreamingDartTypeTranslator::BuildFunctionType(bool simple) {
22282228
Function& signature_function = Function::ZoneHandle(
2229-
Z,
2230-
Function::NewSignatureFunction(*active_class_->klass, Function::Handle(Z),
2231-
TokenPosition::kNoSource));
2229+
Z, Function::NewSignatureFunction(*active_class_->klass,
2230+
active_class_->enclosing != NULL
2231+
? *active_class_->enclosing
2232+
: Function::Handle(Z),
2233+
TokenPosition::kNoSource));
2234+
2235+
// Suspend finalization of types inside this one. They will be finalized after
2236+
// the whole function type is constructed.
2237+
//
2238+
// TODO(31213): Test further when nested generic function types
2239+
// are supported by fasta.
2240+
bool finalize = finalize_;
2241+
finalize_ = false;
22322242

22332243
if (!simple) {
22342244
builder_->LoadAndSetupTypeParameters(active_class_, signature_function,
22352245
builder_->ReadListLength(),
22362246
signature_function);
22372247
}
2248+
22382249
ActiveTypeParametersScope scope(
2239-
active_class_,
2250+
active_class_, &signature_function,
22402251
TypeArguments::Handle(Z, signature_function.type_parameters()), Z);
22412252

22422253
intptr_t required_count;
@@ -2306,6 +2317,8 @@ void StreamingDartTypeTranslator::BuildFunctionType(bool simple) {
23062317
}
23072318
signature_function.set_result_type(result_);
23082319

2320+
finalize_ = finalize;
2321+
23092322
Type& signature_type =
23102323
Type::ZoneHandle(Z, signature_function.SignatureType());
23112324

@@ -8073,7 +8086,11 @@ void StreamingFlowGraphBuilder::LoadAndSetupTypeParameters(
80738086
Function::Cast(set_on).set_type_parameters(type_parameters);
80748087
}
80758088

8076-
ActiveTypeParametersScope(active_class, type_parameters, Z);
8089+
const Function* enclosing = NULL;
8090+
if (!parameterized_function.IsNull()) {
8091+
enclosing = &parameterized_function;
8092+
}
8093+
ActiveTypeParametersScope(active_class, enclosing, type_parameters, Z);
80778094

80788095
// Step b) Fill in the bounds of all [TypeParameter]s.
80798096
for (intptr_t i = 0; i < type_parameter_count; i++) {
@@ -8117,7 +8134,8 @@ void StreamingFlowGraphBuilder::SetupFunctionParameters(
81178134
}
81188135

81198136
ActiveTypeParametersScope scope(
8120-
active_class, TypeArguments::Handle(Z, function.type_parameters()), Z);
8137+
active_class, &function,
8138+
TypeArguments::Handle(Z, function.type_parameters()), Z);
81218139

81228140
function_node_helper->ReadUntilExcluding(
81238141
FunctionNodeHelper::kPositionalParameters);

runtime/vm/compiler/frontend/kernel_to_il.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ ActiveTypeParametersScope::ActiveTypeParametersScope(ActiveClass* active_class,
3030
const Function& innermost,
3131
Zone* Z)
3232
: active_class_(active_class), saved_(*active_class) {
33+
active_class_->enclosing = &innermost;
34+
3335
intptr_t num_params = 0;
3436

3537
Function& f = Function::Handle(Z);
@@ -58,9 +60,12 @@ ActiveTypeParametersScope::ActiveTypeParametersScope(ActiveClass* active_class,
5860

5961
ActiveTypeParametersScope::ActiveTypeParametersScope(
6062
ActiveClass* active_class,
63+
const Function* function,
6164
const TypeArguments& new_params,
6265
Zone* Z)
6366
: active_class_(active_class), saved_(*active_class) {
67+
active_class_->enclosing = function;
68+
6469
if (new_params.IsNull()) return;
6570

6671
const TypeArguments* old_params = active_class->local_type_parameters;

runtime/vm/compiler/frontend/kernel_to_il.h

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,11 @@ typedef ZoneGrowableArray<PushArgumentInstr*>* ArgumentArray;
190190

191191
class ActiveClass {
192192
public:
193-
ActiveClass() : klass(NULL), member(NULL), local_type_parameters(NULL) {}
193+
ActiveClass()
194+
: klass(NULL),
195+
member(NULL),
196+
enclosing(NULL),
197+
local_type_parameters(NULL) {}
194198

195199
bool HasMember() { return member != NULL; }
196200

@@ -221,6 +225,10 @@ class ActiveClass {
221225

222226
const Function* member;
223227

228+
// The innermost enclosing function. This is used for building types, as a
229+
// parent for function types.
230+
const Function* enclosing;
231+
224232
const TypeArguments* local_type_parameters;
225233
};
226234

@@ -258,14 +266,20 @@ class ActiveTypeParametersScope {
258266
// Set the local type parameters of the ActiveClass to be exactly all type
259267
// parameters defined by 'innermost' and any enclosing *closures* (but not
260268
// enclosing methods/top-level functions/classes).
269+
//
270+
// Also, the enclosing function is set to 'innermost'.
261271
ActiveTypeParametersScope(ActiveClass* active_class,
262272
const Function& innermost,
263273
Zone* Z);
264274

265275
// Append the list of the local type parameters to the list in ActiveClass.
276+
//
277+
// Also, the enclosing function is set to 'function'.
266278
ActiveTypeParametersScope(ActiveClass* active_class,
279+
const Function* function,
267280
const TypeArguments& new_params,
268281
Zone* Z);
282+
269283
~ActiveTypeParametersScope() { *active_class_ = saved_; }
270284

271285
private:

tests/corelib_2/corelib_2.status

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,9 +312,6 @@ string_from_environment3_test/02: MissingCompileTimeError
312312
string_from_environment3_test/03: MissingCompileTimeError
313313
string_from_environment3_test/04: MissingCompileTimeError
314314

315-
[ ($compiler == dartk || $compiler == dartkp) && ($runtime == vm || $runtime == dart_precompiled) && $mode == debug]
316-
list_test/*: Crash
317-
318315
[ $compiler == dartkp ]
319316
apply3_test: Crash
320317
from_environment_const_type_test/02: MissingCompileTimeError

tests/language_2/language_2_kernel.status

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -674,11 +674,6 @@ cyclic_type_variable_test/none: Crash
674674
type_parameter_test/04: Crash
675675
type_parameter_test/05: Crash
676676
const_instance_field_test/01: Crash
677-
generic_function_typedef_test/none: Crash
678-
generic_function_typedef_test/01: Crash
679-
generic_local_functions_test: Crash
680-
generic_methods_closure_test: Crash
681-
generic_typedef_test: Crash
682677

683678
[ $compiler == none || $compiler == app_jit || $compiler == dartk || $runtime == dart_precompiled ]
684679
constructor3_test: Fail, OK, Pass

0 commit comments

Comments
 (0)