Skip to content

Commit

Permalink
[cfe] Use effective target in redirecting factory tear off lowering
Browse files Browse the repository at this point in the history
The tear-off lowering for a redirecting factory called the
immediate target (even when this was also a redirecting factory)
instead of the effective target. This caused problem in backends
that don't support redirecting factories directly.

Closes #47916

Change-Id: Iafb11c42c1e99e70ed44b0835473a8c69e995b01
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/235780
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
  • Loading branch information
johnniwinther authored and Commit Bot committed Mar 8, 2022
1 parent e1718d5 commit 5090974
Show file tree
Hide file tree
Showing 61 changed files with 1,966 additions and 15 deletions.
2 changes: 1 addition & 1 deletion pkg/front_end/lib/src/fasta/scope.dart
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ mixin ErroneousMemberBuilderMixin implements SourceMemberBuilder {
}

@override
LibraryBuilder get library {
SourceLibraryBuilder get library {
throw new UnsupportedError('AmbiguousMemberBuilder.library');
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/front_end/lib/src/fasta/source/source_class_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2808,6 +2808,9 @@ class _RedirectingConstructorsFieldBuilder extends DillFieldBuilder
_RedirectingConstructorsFieldBuilder(Field field, SourceClassBuilder parent)
: super(field, parent);

@override
SourceLibraryBuilder get library => super.library as SourceLibraryBuilder;

@override
void buildOutlineExpressions(
SourceLibraryBuilder library,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,6 @@ class DeclaredSourceConstructorBuilder extends SourceFunctionBuilderImpl
super(metadata, modifiers, returnType, name, typeVariables, formals,
compilationUnit, charOffset, nativeMethodName);

@override
SourceLibraryBuilder get library => super.library as SourceLibraryBuilder;

@override
SourceClassBuilder get classBuilder =>
super.classBuilder as SourceClassBuilder;
Expand Down Expand Up @@ -783,6 +780,9 @@ class SyntheticSourceConstructorBuilder extends DillConstructorBuilder
_synthesizedFunctionNode = synthesizedFunctionNode,
super(constructor, constructorTearOff, parent);

@override
SourceLibraryBuilder get library => super.library as SourceLibraryBuilder;

// TODO(johnniwinther,cstefantsova): Rename [actualOrigin] to avoid the
// confusion with patches.
MemberBuilder? get actualOrigin {
Expand Down
41 changes: 33 additions & 8 deletions pkg/front_end/lib/src/fasta/source/source_factory_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,11 @@ class RedirectingFactoryBuilder extends SourceFactoryBuilder {
inferrer.helper = library.loader.createBodyBuilderForOutlineExpression(
library, classBuilder, this, classBuilder!.scope, fileUri);
Builder? targetBuilder = redirectionTarget.target;
if (targetBuilder is SourceMemberBuilder) {
// Ensure that target has been built.
targetBuilder.buildOutlineExpressions(targetBuilder.library,
classHierarchy, delayedActionPerformers, synthesizedFunctionNodes);
}
if (targetBuilder is FunctionBuilder) {
target = targetBuilder.member;
} else if (targetBuilder is DillMemberBuilder) {
Expand Down Expand Up @@ -457,14 +462,34 @@ class RedirectingFactoryBuilder extends SourceFactoryBuilder {
new RedirectingFactoryBody(target, typeArguments, function);
function.body!.parent = function;
}
if (_factoryTearOff != null &&
(target is Constructor || target is Procedure && target.isFactory)) {
synthesizedFunctionNodes.add(buildRedirectingFactoryTearOffBody(
_factoryTearOff!,
target!,
typeArguments ?? [],
_tearOffTypeParameters!,
library));
if (_factoryTearOff != null) {
Set<Procedure> seenTargets = {};
while (target is Procedure && target.isRedirectingFactory) {
if (!seenTargets.add(target)) {
// Cyclic dependency.
target = null;
break;
}
RedirectingFactoryBody body =
target.function.body as RedirectingFactoryBody;
if (typeArguments != null) {
Substitution substitution = Substitution.fromPairs(
target.function.typeParameters, typeArguments);
typeArguments =
body.typeArguments?.map(substitution.substituteType).toList();
} else {
typeArguments = body.typeArguments;
}
target = body.target;
}
if (target is Constructor || target is Procedure && target.isFactory) {
synthesizedFunctionNodes.add(buildRedirectingFactoryTearOffBody(
_factoryTearOff!,
target!,
typeArguments ?? [],
_tearOffTypeParameters!,
library));
}
}
if (isConst && isPatch) {
_finishPatch();
Expand Down
3 changes: 0 additions & 3 deletions pkg/front_end/lib/src/fasta/source/source_field_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -282,9 +282,6 @@ class SourceFieldBuilder extends SourceMemberBuilderImpl
_typeEnsured = true;
}

@override
SourceLibraryBuilder get library => super.library as SourceLibraryBuilder;

@override
Member get member => _fieldEncoding.field;

Expand Down
6 changes: 6 additions & 0 deletions pkg/front_end/lib/src/fasta/source/source_member_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ import 'source_class_builder.dart';
abstract class SourceMemberBuilder implements MemberBuilder {
MemberDataForTesting? get dataForTesting;

@override
SourceLibraryBuilder get library;

/// Builds the core AST structures for this member as needed for the outline.
void buildMembers(
SourceLibraryBuilder library, void Function(Member, BuiltMemberKind) f);
Expand Down Expand Up @@ -87,6 +90,9 @@ abstract class SourceMemberBuilderImpl extends MemberBuilderImpl
retainDataForTesting ? new MemberDataForTesting() : null,
super(parent, charOffset, fileUri);

@override
SourceLibraryBuilder get library => super.library as SourceLibraryBuilder;

bool get isRedirectingGenerativeConstructor => false;

@override
Expand Down
19 changes: 19 additions & 0 deletions pkg/front_end/testcases/dart2js/issue47916.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright (c) 2022, 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.

abstract class A {
const factory A() = B;
}

abstract class B implements A {
const factory B() = C;
}

class C implements B {
const C();
}

main() {
A.new;
}
34 changes: 34 additions & 0 deletions pkg/front_end/testcases/dart2js/issue47916.dart.strong.expect
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;

abstract class A extends core::Object {
static final field dynamic _redirecting# = <dynamic>[#C1]/*isLegacy*/;
static factory •() → self::A
return self::B::•();
static method _#new#tearOff() → self::A
return new self::C::•();
}
abstract class B extends core::Object implements self::A {
static final field dynamic _redirecting# = <dynamic>[#C2]/*isLegacy*/;
static factory •() → self::B
return new self::C::•();
static method _#new#tearOff() → self::B
return new self::C::•();
}
class C extends core::Object implements self::B /*hasConstConstructor*/ {
const constructor •() → self::C
: super core::Object::•()
;
static method _#new#tearOff() → self::C
return new self::C::•();
}
static method main() → dynamic {
#C3;
}

constants {
#C1 = constructor-tearoff self::A::•
#C2 = constructor-tearoff self::B::•
#C3 = static-tearoff self::A::_#new#tearOff
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;

abstract class A extends core::Object {
static final field dynamic _redirecting# = <dynamic>[#C1]/*isLegacy*/;
static factory •() → self::A
return self::B::•();
static method _#new#tearOff() → self::A
return new self::C::•();
}
abstract class B extends core::Object implements self::A {
static final field dynamic _redirecting# = <dynamic>[#C2]/*isLegacy*/;
static factory •() → self::B
return new self::C::•();
static method _#new#tearOff() → self::B
return new self::C::•();
}
class C extends core::Object implements self::B /*hasConstConstructor*/ {
const constructor •() → self::C
: super core::Object::•()
;
static method _#new#tearOff() → self::C
return new self::C::•();
}
static method main() → dynamic {
#C3;
}

constants {
#C1 = constructor-tearoff self::A::•
#C2 = constructor-tearoff self::B::•
#C3 = static-tearoff self::A::_#new#tearOff
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
abstract class A {
const factory A() = B;
}

abstract class B implements A {
const factory B() = C;
}

class C implements B {
const C();
}

main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
abstract class A {
const factory A() = B;
}

abstract class B implements A {
const factory B() = C;
}

class C implements B {
const C();
}

main() {}
34 changes: 34 additions & 0 deletions pkg/front_end/testcases/dart2js/issue47916.dart.weak.expect
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;

abstract class A extends core::Object {
static final field dynamic _redirecting# = <dynamic>[#C1]/*isLegacy*/;
static factory •() → self::A
return self::B::•();
static method _#new#tearOff() → self::A
return new self::C::•();
}
abstract class B extends core::Object implements self::A {
static final field dynamic _redirecting# = <dynamic>[#C2]/*isLegacy*/;
static factory •() → self::B
return new self::C::•();
static method _#new#tearOff() → self::B
return new self::C::•();
}
class C extends core::Object implements self::B /*hasConstConstructor*/ {
const constructor •() → self::C
: super core::Object::•()
;
static method _#new#tearOff() → self::C
return new self::C::•();
}
static method main() → dynamic {
#C3;
}

constants {
#C1 = constructor-tearoff self::A::•
#C2 = constructor-tearoff self::B::•
#C3 = static-tearoff self::A::_#new#tearOff
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;

abstract class A extends core::Object {
static final field dynamic _redirecting# = <dynamic>[#C1]/*isLegacy*/;
static factory •() → self::A
return self::B::•();
static method _#new#tearOff() → self::A
return new self::C::•();
}
abstract class B extends core::Object implements self::A {
static final field dynamic _redirecting# = <dynamic>[#C2]/*isLegacy*/;
static factory •() → self::B
return new self::C::•();
static method _#new#tearOff() → self::B
return new self::C::•();
}
class C extends core::Object implements self::B /*hasConstConstructor*/ {
const constructor •() → self::C
: super core::Object::•()
;
static method _#new#tearOff() → self::C
return new self::C::•();
}
static method main() → dynamic {
#C3;
}

constants {
#C1 = constructor-tearoff self::A::•
#C2 = constructor-tearoff self::B::•
#C3 = static-tearoff self::A::_#new#tearOff
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;

abstract class A extends core::Object {
static final field dynamic _redirecting# = <dynamic>[self::A::•]/*isLegacy*/;
static factory •() → self::A
return self::B::•();
static method _#new#tearOff() → self::A
return new self::C::•();
}
abstract class B extends core::Object implements self::A {
static final field dynamic _redirecting# = <dynamic>[self::B::•]/*isLegacy*/;
static factory •() → self::B
return new self::C::•();
static method _#new#tearOff() → self::B
return new self::C::•();
}
class C extends core::Object implements self::B /*hasConstConstructor*/ {
const constructor •() → self::C
: super core::Object::•()
;
static method _#new#tearOff() → self::C
return new self::C::•();
}
static method main() → dynamic
;


Extra constant evaluation status:
Evaluated: ConstructorTearOff @ org-dartlang-testcase:///issue47916.dart:5:16 -> ConstructorTearOffConstant(A.)
Evaluated: ConstructorTearOff @ org-dartlang-testcase:///issue47916.dart:9:16 -> ConstructorTearOffConstant(B.)
Extra constant evaluation: evaluated: 9, effectively constant: 2
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
library /*isNonNullableByDefault*/;
import self as self;
import "dart:core" as core;

abstract class A extends core::Object {
static final field dynamic _redirecting# = <dynamic>[#C1]/*isLegacy*/;
static factory •() → self::A
return self::B::•();
static method _#new#tearOff() → self::A
return new self::C::•();
}
abstract class B extends core::Object implements self::A {
static final field dynamic _redirecting# = <dynamic>[#C2]/*isLegacy*/;
static factory •() → self::B
return new self::C::•();
static method _#new#tearOff() → self::B
return new self::C::•();
}
class C extends core::Object implements self::B /*hasConstConstructor*/ {
const constructor •() → self::C
: super core::Object::•()
;
static method _#new#tearOff() → self::C
return new self::C::•();
}
static method main() → dynamic {
#C3;
}

constants {
#C1 = constructor-tearoff self::A::•
#C2 = constructor-tearoff self::B::•
#C3 = static-tearoff self::A::_#new#tearOff
}
Loading

0 comments on commit 5090974

Please sign in to comment.