Skip to content

Commit

Permalink
[cfe] Handle tear-offs of patched constructors
Browse files Browse the repository at this point in the history 
Previously the tear-offs created for the origin constructors were not
replaced with the tear-offs created for the patch constructors - as is
normally done for the constructors themselves. This lead the tear-offs
to refer to unlowered code, breaking dart2js.

Closes #48776

Change-Id: I1cb09c07bb2ac7fffb81acd31547ee96e7ecdc64
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/242284
Reviewed-by: Chloe Stefantsova <[email protected]>
Commit-Queue: Johnni Winther <[email protected]>
  • Loading branch information
@johnniwinther
johnniwinther authored and Commit Bot committed Apr 25, 2022
1 parent 0f0f045 commit 6fdaed9
Show file tree
Hide file tree
Showing 46 changed files with 1,606 additions and 117 deletions.
131 changes: 88 additions & 43 deletions pkg/front_end/lib/src/fasta/kernel/constructor_tearoff_lowering.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,22 +159,41 @@ Procedure createTypedefTearOffProcedure(
reference);
}

/// Creates the parameters and body for [tearOff] based on [constructor] in
/// [enclosingClass].
void buildConstructorTearOffProcedure(Procedure tearOff, Member constructor,
Class enclosingClass, SourceLibraryBuilder libraryBuilder) {
/// Creates the parameters and body for [tearOff] based on
/// [declarationConstructor] in [enclosingClass].
///
/// The [declarationConstructor] is the origin constructor and
/// [implementationConstructor] is the patch constructor, if patched, otherwise
/// it is the [declarationConstructor].
void buildConstructorTearOffProcedure(
{required Procedure tearOff,
required Member declarationConstructor,
required Member implementationConstructor,
required Class enclosingClass,
required SourceLibraryBuilder libraryBuilder}) {
assert(
declarationConstructor is Constructor ||
(declarationConstructor is Procedure &&
declarationConstructor.isFactory) ||
(declarationConstructor is Procedure &&
declarationConstructor.isStatic),
"Unexpected constructor tear off target $declarationConstructor "
"(${declarationConstructor.runtimeType}).");
assert(
constructor is Constructor ||
(constructor is Procedure && constructor.isFactory) ||
(constructor is Procedure && constructor.isStatic),
"Unexpected constructor tear off target $constructor "
"(${constructor.runtimeType}).");
declarationConstructor is Constructor ||
(declarationConstructor is Procedure &&
declarationConstructor.isFactory) ||
(declarationConstructor is Procedure &&
declarationConstructor.isStatic),
"Unexpected constructor tear off target $declarationConstructor "
"(${declarationConstructor.runtimeType}).");

FunctionNode function = implementationConstructor.function!;

int fileOffset = tearOff.fileOffset;

FunctionNode function = constructor.function!;
List<TypeParameter> classTypeParameters;
if (constructor is Constructor) {
if (declarationConstructor is Constructor) {
// Generative constructors implicitly have the type parameters of the
// enclosing class.
classTypeParameters = enclosingClass.typeParameters;
Expand All @@ -189,37 +208,54 @@ void buildConstructorTearOffProcedure(Procedure tearOff, Member constructor,

List<DartType> typeArguments = freshTypeParameters.freshTypeArguments;
Substitution substitution = freshTypeParameters.substitution;
_createParameters(tearOff, constructor, substitution, libraryBuilder);
_createParameters(tearOff, implementationConstructor, function, substitution,
libraryBuilder);
Arguments arguments = _createArguments(tearOff, typeArguments, fileOffset);
_createTearOffBody(tearOff, constructor, arguments);
_createTearOffBody(tearOff, declarationConstructor, arguments);
tearOff.function.fileOffset = tearOff.fileOffset;
tearOff.function.fileEndOffset = tearOff.fileOffset;
updatePrivateMemberName(tearOff, libraryBuilder);
}

/// Creates the parameters and body for [tearOff] for a typedef tearoff of
/// [constructor] in [enclosingClass] with [typeParameters] as the typedef
/// parameters and [typeArguments] as the arguments passed to the
/// [declarationConstructor] in [enclosingClass] with [typeParameters] as the
/// typedef parameters and [typeArguments] as the arguments passed to the
/// [enclosingClass].
///
/// The [declarationConstructor] is the origin constructor and
/// [implementationConstructor] is the patch constructor, if patched, otherwise
/// it is the [declarationConstructor].
void buildTypedefTearOffProcedure(
Procedure tearOff,
Member constructor,
Class enclosingClass,
List<TypeParameter> typeParameters,
List<DartType> typeArguments,
SourceLibraryBuilder libraryBuilder) {
{required Procedure tearOff,
required Member declarationConstructor,
required Member implementationConstructor,
required Class enclosingClass,
required List<TypeParameter> typeParameters,
required List<DartType> typeArguments,
required SourceLibraryBuilder libraryBuilder}) {
assert(
constructor is Constructor ||
(constructor is Procedure && constructor.isFactory) ||
(constructor is Procedure && constructor.isStatic),
"Unexpected constructor tear off target $constructor "
"(${constructor.runtimeType}).");
declarationConstructor is Constructor ||
(declarationConstructor is Procedure &&
declarationConstructor.isFactory) ||
(declarationConstructor is Procedure &&
declarationConstructor.isStatic),
"Unexpected constructor tear off target $declarationConstructor "
"(${declarationConstructor.runtimeType}).");
assert(
implementationConstructor is Constructor ||
(implementationConstructor is Procedure &&
implementationConstructor.isFactory) ||
(implementationConstructor is Procedure &&
implementationConstructor.isStatic),
"Unexpected constructor tear off target $implementationConstructor "
"(${declarationConstructor.runtimeType}).");

FunctionNode function = implementationConstructor.function!;

int fileOffset = tearOff.fileOffset;

FunctionNode function = constructor.function!;
List<TypeParameter> classTypeParameters;
if (constructor is Constructor) {
if (declarationConstructor is Constructor) {
// Generative constructors implicitly have the type parameters of the
// enclosing class.
classTypeParameters = enclosingClass.typeParameters;
Expand All @@ -242,37 +278,43 @@ void buildTypedefTearOffProcedure(
}
_createParameters(
tearOff,
constructor,
implementationConstructor,
function,
Substitution.fromPairs(classTypeParameters, typeArguments),
libraryBuilder);
Arguments arguments = _createArguments(tearOff, typeArguments, fileOffset);
_createTearOffBody(tearOff, constructor, arguments);
_createTearOffBody(tearOff, declarationConstructor, arguments);
tearOff.function.fileOffset = tearOff.fileOffset;
tearOff.function.fileEndOffset = tearOff.fileOffset;
updatePrivateMemberName(tearOff, libraryBuilder);
}

/// Creates the parameters for the redirecting factory [tearOff] based on the
/// [redirectingConstructor] declaration.
/// [declarationConstructor] declaration.
///
/// The [declarationConstructor] is the [Procedure] for the origin constructor
/// and [implementationConstructorFunctionNode] is the [FunctionNode] for the
/// implementation constructor. If the constructor is patched, these are not
/// connected until [Builder.finishPatch].
FreshTypeParameters buildRedirectingFactoryTearOffProcedureParameters(
Procedure tearOff,
Procedure redirectingConstructor,
SourceLibraryBuilder libraryBuilder) {
assert(redirectingConstructor.isRedirectingFactory);
FunctionNode function = redirectingConstructor.function;
{required Procedure tearOff,
required Procedure implementationConstructor,
required SourceLibraryBuilder libraryBuilder}) {
assert(implementationConstructor.isRedirectingFactory);
FunctionNode function = implementationConstructor.function;
FreshTypeParameters freshTypeParameters =
_createFreshTypeParameters(function.typeParameters, tearOff.function);
Substitution substitution = freshTypeParameters.substitution;
_createParameters(
tearOff, redirectingConstructor, substitution, libraryBuilder);
_createParameters(tearOff, implementationConstructor, function, substitution,
libraryBuilder);
tearOff.function.fileOffset = tearOff.fileOffset;
tearOff.function.fileEndOffset = tearOff.fileOffset;
updatePrivateMemberName(tearOff, libraryBuilder);
return freshTypeParameters;
}

/// Creates the body for the redirecting factory [tearOff] with the target
/// [constructor] and [typeArguments].
/// Creates the body for the redirecting factory [tearOff] with the [target]
/// constructor and [typeArguments].
///
/// Returns the [DelayedDefaultValueCloner] object need to perform default value
/// computation.
Expand Down Expand Up @@ -355,9 +397,12 @@ FreshTypeParameters _createFreshTypeParameters(
/// Creates the parameters for the [tearOff] lowering based of the parameters
/// in [constructor] and using the [substitution] to compute the parameter and
/// return types.
void _createParameters(Procedure tearOff, Member constructor,
Substitution substitution, SourceLibraryBuilder libraryBuilder) {
FunctionNode function = constructor.function!;
void _createParameters(
Procedure tearOff,
Member constructor,
FunctionNode function,
Substitution substitution,
SourceLibraryBuilder libraryBuilder) {
for (VariableDeclaration constructorParameter
in function.positionalParameters) {
VariableDeclaration tearOffParameter = new VariableDeclaration(
Expand Down
36 changes: 36 additions & 0 deletions pkg/front_end/lib/src/fasta/kernel/kernel_helper.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,3 +300,39 @@ class TypeDependency {
_hasBeenInferred = true;
}
}

/// Copies properties, function parameters and body from the [patch] constructor
/// to its [origin].
void finishConstructorPatch(Constructor origin, Constructor patch) {
// TODO(ahe): restore file-offset once we track both origin and patch file
// URIs. See https://github.com/dart-lang/sdk/issues/31579
origin.fileUri = patch.fileUri;
origin.startFileOffset = patch.startFileOffset;
origin.fileOffset = patch.fileOffset;
origin.fileEndOffset = patch.fileEndOffset;
origin.annotations.forEach((m) => m.fileOffset = patch.fileOffset);

origin.isExternal = patch.isExternal;
origin.function = patch.function;
origin.function.parent = origin;
origin.initializers = patch.initializers;
setParents(origin.initializers, origin);
}

/// Copies properties, function parameters and body from the [patch] procedure
/// to its [origin].
void finishProcedurePatch(Procedure origin, Procedure patch) {
// TODO(ahe): restore file-offset once we track both origin and patch file
// URIs. See https://github.com/dart-lang/sdk/issues/31579
origin.fileUri = patch.fileUri;
origin.startFileOffset = patch.startFileOffset;
origin.fileOffset = patch.fileOffset;
origin.fileEndOffset = patch.fileEndOffset;
origin.annotations.forEach((m) => m.fileOffset = patch.fileOffset);

origin.isAbstract = patch.isAbstract;
origin.isExternal = patch.isExternal;
origin.function = patch.function;
origin.function.parent = origin;
origin.isRedirectingFactory = patch.isRedirectingFactory;
}
16 changes: 12 additions & 4 deletions pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1092,8 +1092,12 @@ class KernelTarget extends TargetImplementation {
forAbstractClassOrEnum: classBuilder.isAbstract);

if (constructorTearOff != null) {
buildConstructorTearOffProcedure(constructorTearOff, constructor,
classBuilder.cls, classBuilder.libraryBuilder);
buildConstructorTearOffProcedure(
tearOff: constructorTearOff,
declarationConstructor: constructor,
implementationConstructor: constructor,
enclosingClass: classBuilder.cls,
libraryBuilder: classBuilder.libraryBuilder);
}
SyntheticSourceConstructorBuilder constructorBuilder =
new SyntheticSourceConstructorBuilder(
Expand Down Expand Up @@ -1170,8 +1174,12 @@ class KernelTarget extends TargetImplementation {
forAbstractClassOrEnum:
enclosingClass.isAbstract || enclosingClass.isEnum);
if (constructorTearOff != null) {
buildConstructorTearOffProcedure(constructorTearOff, constructor,
classBuilder.cls, classBuilder.libraryBuilder);
buildConstructorTearOffProcedure(
tearOff: constructorTearOff,
declarationConstructor: constructor,
implementationConstructor: constructor,
enclosingClass: classBuilder.cls,
libraryBuilder: classBuilder.libraryBuilder);
}
return new SyntheticSourceConstructorBuilder(
classBuilder, constructor, constructorTearOff);
Expand Down
33 changes: 16 additions & 17 deletions pkg/front_end/lib/src/fasta/source/source_constructor_builder.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ import '../kernel/constructor_tearoff_lowering.dart';
import '../kernel/expression_generator_helper.dart';
import '../kernel/hierarchy/class_member.dart' show ClassMember;
import '../kernel/kernel_helper.dart'
show DelayedDefaultValueCloner, TypeDependency;
show
DelayedDefaultValueCloner,
TypeDependency,
finishConstructorPatch,
finishProcedurePatch;
import '../kernel/utils.dart'
show isRedirectingGenerativeConstructorImplementation;
import '../messages.dart'
Expand Down Expand Up @@ -202,8 +206,12 @@ class DeclaredSourceConstructorBuilder extends SourceFunctionBuilderImpl
updatePrivateMemberName(_constructor, libraryBuilder);

if (_constructorTearOff != null) {
buildConstructorTearOffProcedure(_constructorTearOff!, _constructor,
classBuilder.cls, libraryBuilder);
buildConstructorTearOffProcedure(
tearOff: _constructorTearOff!,
declarationConstructor: constructor,
implementationConstructor: _constructor,
enclosingClass: classBuilder.cls,
libraryBuilder: libraryBuilder);
}

_hasBeenBuilt = true;
Expand Down Expand Up @@ -660,20 +668,11 @@ class DeclaredSourceConstructorBuilder extends SourceFunctionBuilderImpl
}

void _finishPatch() {
// TODO(ahe): restore file-offset once we track both origin and patch file
// URIs. See https://github.com/dart-lang/sdk/issues/31579
origin.constructor.fileUri = fileUri;
origin.constructor.startFileOffset = _constructor.startFileOffset;
origin.constructor.fileOffset = _constructor.fileOffset;
origin.constructor.fileEndOffset = _constructor.fileEndOffset;
origin.constructor.annotations
.forEach((m) => m.fileOffset = _constructor.fileOffset);

origin.constructor.isExternal = _constructor.isExternal;
origin.constructor.function = _constructor.function;
origin.constructor.function.parent = origin.constructor;
origin.constructor.initializers = _constructor.initializers;
setParents(origin.constructor.initializers, origin.constructor);
finishConstructorPatch(origin.constructor, _constructor);

if (_constructorTearOff != null) {
finishProcedurePatch(origin._constructorTearOff!, _constructorTearOff!);
}
}

@override
Expand Down
32 changes: 14 additions & 18 deletions pkg/front_end/lib/src/fasta/source/source_factory_builder.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,12 @@ class SourceFactoryBuilder extends SourceFunctionBuilderImpl {
_procedureInternal.isStatic = isStatic;

if (_factoryTearOff != null) {
buildConstructorTearOffProcedure(_factoryTearOff!, _procedureInternal,
classBuilder!.cls, libraryBuilder);
buildConstructorTearOffProcedure(
tearOff: _factoryTearOff!,
declarationConstructor: _procedure,
implementationConstructor: _procedureInternal,
enclosingClass: classBuilder!.cls,
libraryBuilder: libraryBuilder);
}
return _procedureInternal;
}
Expand Down Expand Up @@ -224,21 +228,11 @@ class SourceFactoryBuilder extends SourceFunctionBuilderImpl {
}

void _finishPatch() {
// TODO(ahe): restore file-offset once we track both origin and patch file
// URIs. See https://github.com/dart-lang/sdk/issues/31579
origin._procedure.fileUri = fileUri;
origin._procedure.startFileOffset = _procedureInternal.startFileOffset;
origin._procedure.fileOffset = _procedureInternal.fileOffset;
origin._procedure.fileEndOffset = _procedureInternal.fileEndOffset;
origin._procedure.annotations
.forEach((m) => m.fileOffset = _procedureInternal.fileOffset);

origin._procedure.isAbstract = _procedureInternal.isAbstract;
origin._procedure.isExternal = _procedureInternal.isExternal;
origin._procedure.function = _procedureInternal.function;
origin._procedure.function.parent = origin._procedure;
origin._procedure.isRedirectingFactory =
_procedureInternal.isRedirectingFactory;
finishProcedurePatch(origin._procedure, _procedureInternal);

if (_factoryTearOff != null) {
finishProcedurePatch(origin._factoryTearOff!, _factoryTearOff!);
}
}

@override
Expand Down Expand Up @@ -373,7 +367,9 @@ class RedirectingFactoryBuilder extends SourceFactoryBuilder {
if (_factoryTearOff != null) {
_tearOffTypeParameters =
buildRedirectingFactoryTearOffProcedureParameters(
_factoryTearOff!, _procedureInternal, libraryBuilder);
tearOff: _factoryTearOff!,
implementationConstructor: _procedureInternal,
libraryBuilder: libraryBuilder);
}
return _procedureInternal;
}
Expand Down
Loading

0 comments on commit 6fdaed9

Please sign in to comment.