Skip to content

Commit 300a2ea

Browse files
alexmarkovcommit-bot@chromium.org
authored andcommitted
[cfe] Create normal bodies for redirecting factories
Replace bogus body created for redirecting factories with a normal body which calls target factory or constructor and forwards all arguments. Such a body can be used by back-ends if they choose to treat redirecting factories as ordinary factories. TEST=ci Fixes #41915 Issue #46231 Change-Id: I62c83bcc9005995e85de049d3d929ca86a75297f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/208681 Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent ada4278 commit 300a2ea

File tree

257 files changed

+969
-769
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

257 files changed

+969
-769
lines changed

pkg/front_end/lib/src/api_unstable/dart2js.dart

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import 'package:_fe_analyzer_shared/src/messages/severity.dart' show Severity;
1212

1313
import 'package:_fe_analyzer_shared/src/scanner/scanner.dart' show StringToken;
1414

15-
import 'package:kernel/kernel.dart' show Component, Statement;
15+
import 'package:kernel/kernel.dart' show Component;
1616

1717
import 'package:kernel/ast.dart' as ir;
1818

@@ -37,8 +37,6 @@ import '../fasta/compiler_context.dart' show CompilerContext;
3737

3838
import '../kernel_generator_impl.dart' show generateKernelInternal;
3939

40-
import '../fasta/kernel/redirecting_factory_body.dart' as redirecting;
41-
4240
import 'compiler_state.dart' show InitializedCompilerState;
4341

4442
import 'util.dart' show equalLists, equalMaps, equalSets;
@@ -242,18 +240,4 @@ Iterable<String> getSupportedLibraryNames(
242240
/// constructor.
243241
// TODO(sigmund): Delete this API once `member.isRedirectingFactory`
244242
// is implemented correctly for patch files (Issue #33495).
245-
bool isRedirectingFactory(ir.Procedure member) {
246-
if (member.kind == ir.ProcedureKind.Factory) {
247-
Statement? body = member.function.body;
248-
if (body is redirecting.RedirectingFactoryBody) return true;
249-
if (body is ir.ExpressionStatement) {
250-
ir.Expression expression = body.expression;
251-
if (expression is ir.Let) {
252-
if (expression.variable.name == redirecting.letName) {
253-
return true;
254-
}
255-
}
256-
}
257-
}
258-
return false;
259-
}
243+
bool isRedirectingFactory(ir.Procedure member) => member.isRedirectingFactory;

pkg/front_end/lib/src/fasta/builder/class_builder.dart

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ import '../fasta_codes.dart';
4444

4545
import '../kernel/kernel_helper.dart';
4646

47+
import '../kernel/redirecting_factory_body.dart' show RedirectingFactoryBody;
48+
4749
import '../loader.dart';
4850

4951
import '../modifier.dart';
@@ -921,7 +923,8 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
921923
targetNode.computeFunctionType(library.nonNullable);
922924
if (typeArguments != null &&
923925
targetFunctionType.typeParameters.length != typeArguments.length) {
924-
addProblem(
926+
addProblemForRedirectingFactory(
927+
factory,
925928
templateTypeArgumentMismatch
926929
.withArguments(targetFunctionType.typeParameters.length),
927930
redirectionTarget.charOffset,
@@ -946,7 +949,8 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
946949
Loader loader = library.loader;
947950
if (!typeEnvironment.isSubtypeOf(typeArgument, typeParameterBound,
948951
SubtypeCheckMode.ignoringNullabilities)) {
949-
addProblem(
952+
addProblemForRedirectingFactory(
953+
factory,
950954
templateRedirectingFactoryIncompatibleTypeArgument.withArguments(
951955
typeArgument,
952956
typeParameterBound,
@@ -957,7 +961,8 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
957961
} else if (library.isNonNullableByDefault && loader is SourceLoader) {
958962
if (!typeEnvironment.isSubtypeOf(typeArgument, typeParameterBound,
959963
SubtypeCheckMode.withNullabilities)) {
960-
addProblem(
964+
addProblemForRedirectingFactory(
965+
factory,
961966
templateRedirectingFactoryIncompatibleTypeArgument
962967
.withArguments(typeArgument, typeParameterBound,
963968
library.isNonNullableByDefault),
@@ -1002,9 +1007,65 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
10021007
}
10031008
}
10041009

1010+
bool _isCyclicRedirectingFactory(RedirectingFactoryBuilder factory) {
1011+
// We use the [tortoise and hare algorithm]
1012+
// (https://en.wikipedia.org/wiki/Cycle_detection#Tortoise_and_hare) to
1013+
// handle cycles.
1014+
Builder? tortoise = factory;
1015+
Builder? hare = factory.redirectionTarget.target;
1016+
if (hare == factory) {
1017+
return true;
1018+
}
1019+
while (tortoise != hare) {
1020+
// Hare moves 2 steps forward.
1021+
if (hare is! RedirectingFactoryBuilder) {
1022+
return false;
1023+
}
1024+
hare = hare.redirectionTarget.target;
1025+
if (hare == factory) {
1026+
return true;
1027+
}
1028+
if (hare is! RedirectingFactoryBuilder) {
1029+
return false;
1030+
}
1031+
hare = hare.redirectionTarget.target;
1032+
if (hare == factory) {
1033+
return true;
1034+
}
1035+
// Tortoise moves one step forward. No need to test type of tortoise
1036+
// as it follows hare which already checked types.
1037+
tortoise =
1038+
(tortoise as RedirectingFactoryBuilder).redirectionTarget.target;
1039+
}
1040+
// Cycle found, but original factory doesn't belong to a cycle.
1041+
return false;
1042+
}
1043+
1044+
void addProblemForRedirectingFactory(RedirectingFactoryBuilder factory,
1045+
Message message, int charOffset, int length) {
1046+
addProblem(message, charOffset, length);
1047+
String text = library.loader.target.context
1048+
.format(
1049+
message.withLocation(fileUri, charOffset, length), Severity.error)
1050+
.plain;
1051+
factory.body = new RedirectingFactoryBody.error(text);
1052+
}
1053+
10051054
@override
10061055
void checkRedirectingFactory(
10071056
RedirectingFactoryBuilder factory, TypeEnvironment typeEnvironment) {
1057+
// Check that factory declaration is not cyclic.
1058+
if (_isCyclicRedirectingFactory(factory)) {
1059+
addProblemForRedirectingFactory(
1060+
factory,
1061+
templateCyclicRedirectingFactoryConstructors
1062+
.withArguments("${factory.member.enclosingClass!.name}"
1063+
"${factory.name == '' ? '' : '.${factory.name}'}"),
1064+
factory.charOffset,
1065+
noLength);
1066+
return;
1067+
}
1068+
10081069
// The factory type cannot contain any type parameters other than those of
10091070
// its enclosing class, because constructors cannot specify type parameters
10101071
// of their own.
@@ -1016,21 +1077,25 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
10161077

10171078
// TODO(hillerstrom): It would be preferable to know whether a failure
10181079
// happened during [_computeRedirecteeType].
1019-
if (redirecteeType == null) return;
1080+
if (redirecteeType == null) {
1081+
return;
1082+
}
10201083

10211084
// Check whether [redirecteeType] <: [factoryType].
10221085
Loader loader = library.loader;
10231086
if (!typeEnvironment.isSubtypeOf(
10241087
redirecteeType, factoryType, SubtypeCheckMode.ignoringNullabilities)) {
1025-
addProblem(
1088+
addProblemForRedirectingFactory(
1089+
factory,
10261090
templateIncompatibleRedirecteeFunctionType.withArguments(
10271091
redirecteeType, factoryType, library.isNonNullableByDefault),
10281092
factory.redirectionTarget.charOffset,
10291093
noLength);
10301094
} else if (library.isNonNullableByDefault && loader is SourceLoader) {
10311095
if (!typeEnvironment.isSubtypeOf(
10321096
redirecteeType, factoryType, SubtypeCheckMode.withNullabilities)) {
1033-
addProblem(
1097+
addProblemForRedirectingFactory(
1098+
factory,
10341099
templateIncompatibleRedirecteeFunctionType.withArguments(
10351100
redirecteeType, factoryType, library.isNonNullableByDefault),
10361101
factory.redirectionTarget.charOffset,

pkg/front_end/lib/src/fasta/builder/factory_builder.dart

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ class SourceFactoryBuilder extends FunctionBuilderImpl {
207207
if (bodyInternal != null) {
208208
unexpected("null", "${bodyInternal.runtimeType}", charOffset, fileUri);
209209
}
210-
bodyInternal = new RedirectingFactoryBody(target, typeArguments);
210+
bodyInternal = new RedirectingFactoryBody(target, typeArguments, function);
211211
function.body = bodyInternal;
212212
bodyInternal?.parent = function;
213213
if (isPatch) {
@@ -307,7 +307,7 @@ class RedirectingFactoryBuilder extends SourceFactoryBuilder {
307307
noLength, fileUri);
308308
}
309309

310-
bodyInternal = new RedirectingFactoryBody(target, typeArguments);
310+
bodyInternal = new RedirectingFactoryBody(target, typeArguments, function);
311311
function.body = bodyInternal;
312312
bodyInternal?.parent = function;
313313
_procedure.isRedirectingFactory = true;
@@ -436,7 +436,10 @@ class RedirectingFactoryBuilder extends SourceFactoryBuilder {
436436
target.enclosingClass!.typeParameters.length, const DynamicType(),
437437
growable: true);
438438
}
439-
member.function!.body = new RedirectingFactoryBody(target, typeArguments);
439+
440+
function.body =
441+
new RedirectingFactoryBody(target, typeArguments, function);
442+
function.body!.parent = function;
440443
}
441444
if (_factoryTearOff != null &&
442445
(target is Constructor || target is Procedure && target.isFactory)) {

pkg/front_end/lib/src/fasta/kernel/body_builder.dart

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,13 +1247,10 @@ class BodyBuilder extends ScopeListener<JumpTarget>
12471247
if (redirectingFactoryBody != null) {
12481248
// If the redirection target is itself a redirecting factory, it means
12491249
// that it is unresolved.
1250-
assert(redirectingFactoryBody.isUnresolved);
1251-
String errorName = redirectingFactoryBody.unresolvedName!;
1252-
replacementNode = buildProblem(
1253-
fasta.templateMethodNotFound.withArguments(errorName),
1254-
fileOffset,
1255-
noLength,
1256-
suppressMessage: true);
1250+
assert(redirectingFactoryBody.isError);
1251+
String errorMessage = redirectingFactoryBody.errorMessage!;
1252+
replacementNode = new InvalidExpression(errorMessage)
1253+
..fileOffset = fileOffset;
12571254
} else {
12581255
Substitution substitution = Substitution.fromPairs(
12591256
initialTarget.function.typeParameters, arguments.types);
@@ -2373,10 +2370,8 @@ class BodyBuilder extends ScopeListener<JumpTarget>
23732370
push(forest.createStringLiteral(offsetForToken(token), value));
23742371
} else {
23752372
int count = 1 + interpolationCount * 2;
2376-
List<Object>? parts = const FixedNullableList<Object>().popNonNullable(
2377-
stack,
2378-
count,
2379-
/* dummyValue = */ 0);
2373+
List<Object>? parts = const FixedNullableList<Object>()
2374+
.popNonNullable(stack, count, /* dummyValue = */ 0);
23802375
if (parts == null) {
23812376
push(new ParserRecovery(endToken.charOffset));
23822377
return;
@@ -6427,12 +6422,8 @@ class BodyBuilder extends ScopeListener<JumpTarget>
64276422
allowPotentiallyConstantType: allowPotentiallyConstantType);
64286423
if (message == null) return unresolved;
64296424
return new UnresolvedType(
6430-
new NamedTypeBuilder(
6431-
typeParameter.name!,
6432-
builder.nullabilityBuilder,
6433-
/* arguments = */ null,
6434-
unresolved.fileUri,
6435-
unresolved.charOffset)
6425+
new NamedTypeBuilder(typeParameter.name!, builder.nullabilityBuilder,
6426+
/* arguments = */ null, unresolved.fileUri, unresolved.charOffset)
64366427
..bind(new InvalidTypeDeclarationBuilder(
64376428
typeParameter.name!, message)),
64386429
unresolved.charOffset,

0 commit comments

Comments
 (0)