Skip to content

Commit 5855d91

Browse files
natebiggsCommit Queue
authored andcommitted
[ddc] Correctly extend type parameters for factory constructors.
RtiTypeEnvironment currently does not support being extended. However it's used for factory constructors which themselves can have generic functions defined in their bodies. This means we have to allow the ability to extend RtiTypeEnvironments as well. Bug: flutter/flutter#160338 Change-Id: I9b4e79b44f503a4a987e0c38da86fdce81361e4c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/406344 Reviewed-by: Mark Zhou <[email protected]> Reviewed-by: Nicholas Shahan <[email protected]> Commit-Queue: Nate Biggs <[email protected]>
1 parent fc2c46a commit 5855d91

File tree

5 files changed

+65
-31
lines changed

5 files changed

+65
-31
lines changed

pkg/dev_compiler/lib/src/kernel/compiler.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2479,11 +2479,12 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
24792479
/// own type parameters, this will need to be changed to call
24802480
/// [_emitFunction] instead.
24812481
var name = node.name.text;
2482-
var savedTypeEnvironment = _currentTypeEnvironment;
2482+
final savedTypeEnvironment = _currentTypeEnvironment;
24832483
_currentTypeEnvironment = RtiTypeEnvironment([
24842484
...function.typeParameters,
24852485
..._currentTypeEnvironment.classTypeParameters
24862486
]);
2487+
24872488
var jsBody = js_ast.Block(_withCurrentFunction(function, () {
24882489
var block = _emitArgumentInitializers(function, name);
24892490
block.add(_emitFunctionScopedBody(function));
@@ -3408,7 +3409,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
34083409
var env =
34093410
js.call('#.instanceType(this)', [_emitLibraryName(_rtiLibrary)]);
34103411
return emitRtiEval(env, recipe);
3411-
case ExtendedClassTypeEnvironment():
3412+
case ExtendedTypeEnvironment():
34123413
// Class type environments are already constructed and attached to the
34133414
// instance of a generic class, but function type parameters need to
34143415
// be bound.

pkg/dev_compiler/lib/src/kernel/compiler_new.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2660,11 +2660,12 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
26602660
/// own type parameters, this will need to be changed to call
26612661
/// [_emitFunction] instead.
26622662
var name = node.name.text;
2663-
var savedTypeEnvironment = _currentTypeEnvironment;
2663+
final savedTypeEnvironment = _currentTypeEnvironment;
26642664
_currentTypeEnvironment = RtiTypeEnvironment([
26652665
...function.typeParameters,
26662666
..._currentTypeEnvironment.classTypeParameters
26672667
]);
2668+
26682669
var jsBody = js_ast.Block(_withCurrentFunction(function, () {
26692670
var block = _emitArgumentInitializers(function, name);
26702671
block.add(_emitFunctionScopedBody(function));
@@ -3813,7 +3814,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
38133814
var env =
38143815
js.call('#.instanceType(this)', [_emitLibraryName(_rtiLibrary)]);
38153816
return emitRtiEval(env, recipe);
3816-
case ExtendedClassTypeEnvironment():
3817+
case ExtendedTypeEnvironment():
38173818
// Class type environments are already constructed and attached to the
38183819
// instance of a generic class, but function type parameters need to
38193820
// be bound.

pkg/dev_compiler/lib/src/kernel/type_environment.dart

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ class EmptyTypeEnvironment implements DDCTypeEnvironment {
8585

8686
/// A type environment introduced by a class with one or more generic type
8787
/// parameters.
88-
class ClassTypeEnvironment implements DDCTypeEnvironment {
88+
class ClassTypeEnvironment
89+
implements DDCTypeEnvironment, ExtendableTypeEnvironment {
90+
@override
8991
final List<TypeParameter> _typeParameters;
9092

9193
ClassTypeEnvironment(this._typeParameters);
@@ -94,7 +96,7 @@ class ClassTypeEnvironment implements DDCTypeEnvironment {
9496
DDCTypeEnvironment extend(List<TypeParameter> parameters) {
9597
return parameters.isEmpty
9698
? this
97-
: ExtendedClassTypeEnvironment(this, [...parameters]);
99+
: ExtendedTypeEnvironment<ClassTypeEnvironment>(this, [...parameters]);
98100
}
99101

100102
@override
@@ -125,17 +127,19 @@ class ClassTypeEnvironment implements DDCTypeEnvironment {
125127

126128
/// A type environment introduced by a subroutine, wherein an RTI object is
127129
/// explicitly provided.
128-
class RtiTypeEnvironment implements DDCTypeEnvironment {
130+
class RtiTypeEnvironment
131+
implements DDCTypeEnvironment, ExtendableTypeEnvironment {
132+
@override
129133
final List<TypeParameter> _typeParameters;
130134

131135
RtiTypeEnvironment(this._typeParameters);
132136

133137
@override
134138
DDCTypeEnvironment extend(List<TypeParameter> parameters) {
135-
/// RtiTypeEnvironments are only used for constructors, factories, and type
136-
/// signatures - none of which can accept generic arguments.
137-
throw Exception(
138-
'RtiTypeEnvironments should not receive extended type parameters.');
139+
/// RtiTypeEnvironments are only used for factories and type signatures. Of
140+
/// these factories can have generic functions defined in their bodies that
141+
/// require extending the environment.
142+
return ExtendedTypeEnvironment<RtiTypeEnvironment>(this, [...parameters]);
139143
}
140144

141145
@override
@@ -210,19 +214,25 @@ class BindingTypeEnvironment implements DDCTypeEnvironment {
210214
bool get isSingleTypeParameter => _typeParameters.length == 1;
211215
}
212216

213-
/// A type environment based on one introduced by a generic class but extended
214-
/// with additional parameters from methods with generic type parameters.
215-
class ExtendedClassTypeEnvironment implements DDCTypeEnvironment {
216-
final ClassTypeEnvironment _classEnvironment;
217+
abstract class ExtendableTypeEnvironment extends DDCTypeEnvironment {
218+
List<TypeParameter> get _typeParameters;
219+
}
220+
221+
/// A type environment based on one introduced by a generic environment but
222+
/// extended with additional parameters from methods/functions with generic type
223+
/// parameters.
224+
class ExtendedTypeEnvironment<T extends ExtendableTypeEnvironment>
225+
implements DDCTypeEnvironment {
226+
final T _baseTypeEnvironment;
217227
final List<TypeParameter> _typeParameters;
218228

219-
ExtendedClassTypeEnvironment(this._classEnvironment, this._typeParameters);
229+
ExtendedTypeEnvironment(this._baseTypeEnvironment, this._typeParameters);
220230

221231
@override
222232
DDCTypeEnvironment extend(List<TypeParameter> parameters) {
223233
return parameters.isEmpty
224234
? this
225-
: ExtendedClassTypeEnvironment(_classEnvironment, [
235+
: ExtendedTypeEnvironment(_baseTypeEnvironment, [
226236
// Place new parameters first so they can effectively shadow
227237
// parameters already in the environment.
228238
...parameters, ..._typeParameters
@@ -231,36 +241,36 @@ class ExtendedClassTypeEnvironment implements DDCTypeEnvironment {
231241

232242
@override
233243
DDCTypeEnvironment prune(Iterable<TypeParameter> requiredParameters) {
234-
var classEnvironmentNeeded =
235-
requiredParameters.any(_classEnvironment._typeParameters.contains);
244+
var baseEnvironmentNeeded =
245+
requiredParameters.any(_baseTypeEnvironment._typeParameters.contains);
236246
var additionalParameters =
237247
requiredParameters.where(_typeParameters.contains);
238248
if (additionalParameters.isEmpty) {
239-
return classEnvironmentNeeded
240-
// Simply using the class environment has a compact representation
249+
return baseEnvironmentNeeded
250+
// Simply using the base environment has a compact representation
241251
// and is already constructed.
242-
? _classEnvironment
252+
? _baseTypeEnvironment
243253
// No type parameters are needed from this environment.
244254
: const EmptyTypeEnvironment();
245255
}
246256
// This is already the exact environment needed.
247257
if (additionalParameters.length == _typeParameters.length) return this;
248-
// An extended class environment with fewer additional parameters is needed.
249-
return ExtendedClassTypeEnvironment(
250-
_classEnvironment, additionalParameters.toList());
258+
// An extended environment with fewer additional parameters is needed.
259+
return ExtendedTypeEnvironment(
260+
_baseTypeEnvironment, additionalParameters.toList());
251261
}
252262

253263
@override
254264
int recipeIndexOf(TypeParameter parameter) {
255265
// Search in the extended type parameters first. They can shadow parameters
256-
// from the class.
266+
// from the base environment.
257267
var i = _typeParameters.indexOf(parameter);
258268
// Type parameter was found and should be a one based index. Zero refers to
259-
// the original class rti that is the basis on this environment.
269+
// the original rti that is the basis of this environment.
260270
if (i >= 0) return i + 1;
261-
i = _classEnvironment.recipeIndexOf(parameter);
271+
i = _baseTypeEnvironment.recipeIndexOf(parameter);
262272
// The type parameter bindings with the closest scope have the lowest
263-
// indices. Since the parameter was found in the class binding the index
273+
// indices. Since the parameter was found in the base binding the index
264274
// must be offset by the number of extended parameters.
265275
if (i > 0) return i + _typeParameters.length;
266276
// Type parameter not found.
@@ -269,7 +279,7 @@ class ExtendedClassTypeEnvironment implements DDCTypeEnvironment {
269279

270280
@override
271281
List<TypeParameter> get classTypeParameters =>
272-
UnmodifiableListView(_classEnvironment.classTypeParameters);
282+
UnmodifiableListView(_baseTypeEnvironment.classTypeParameters);
273283

274284
@override
275285
List<TypeParameter> get functionTypeParameters =>

pkg/dev_compiler/lib/src/kernel/type_recipe_generator.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ class _TypeRecipeVisitor extends DartTypeVisitor<String> {
482482
// with indices.
483483
var typeParamInBindingPosition = _typeEnvironment
484484
is BindingTypeEnvironment ||
485-
(_typeEnvironment is ExtendedClassTypeEnvironment &&
485+
(_typeEnvironment is ExtendedTypeEnvironment &&
486486
_typeEnvironment.functionTypeParameters.contains(node.parameter));
487487
if (!typeParamInBindingPosition) {
488488
// We don't emit RTI rules for anonymous mixins since 1) their
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
class A<U> {
6+
A();
7+
8+
factory A.foo(U j) {
9+
void func<T>(T i, U j) {
10+
print(i);
11+
print(j);
12+
}
13+
14+
func(3, j);
15+
16+
return A();
17+
}
18+
}
19+
20+
void main() {
21+
A.foo('hi');
22+
}

0 commit comments

Comments
 (0)