Skip to content

Commit

Permalink
[cfe] Perform checks on factories of extension type declarations
Browse files Browse the repository at this point in the history
Closes #53209
Closes #53140
Part of #49731

Change-Id: Ia94b1e85d6775efc23bf732441fa66d4de1de515
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/332403
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
  • Loading branch information
chloestefantsova authored and Commit Queue committed Nov 1, 2023
1 parent dd3705d commit 8b01baa
Show file tree
Hide file tree
Showing 33 changed files with 1,228 additions and 69 deletions.
3 changes: 2 additions & 1 deletion pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,8 @@ class KernelTarget extends TargetImplementation {

benchmarker
?.enterPhase(BenchmarkPhases.outline_checkRedirectingFactories);
loader.checkRedirectingFactories(sortedSourceClassBuilders);
loader.checkRedirectingFactories(
sortedSourceClassBuilders, sortedSourceExtensionTypeBuilders);

benchmarker
?.enterPhase(BenchmarkPhases.outline_finishSynthesizedParameters);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import '../util/helpers.dart';
import 'class_declaration.dart';
import 'source_builder_mixins.dart';
import 'source_constructor_builder.dart';
import 'source_factory_builder.dart';
import 'source_field_builder.dart';
import 'source_library_builder.dart';
import 'source_member_builder.dart';
Expand Down Expand Up @@ -484,6 +485,15 @@ class SourceExtensionTypeDeclarationBuilder
}
}

void checkRedirectingFactories(TypeEnvironment typeEnvironment) {
Iterator<SourceFactoryBuilder> iterator =
constructorScope.filteredIterator<SourceFactoryBuilder>(
parent: this, includeDuplicates: true, includeAugmentations: true);
while (iterator.moveNext()) {
iterator.current.checkRedirectingFactories(typeEnvironment);
}
}

@override
void buildOutlineExpressions(
ClassHierarchy classHierarchy,
Expand Down
15 changes: 15 additions & 0 deletions pkg/front_end/lib/src/fasta/source/source_factory_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,8 @@ class RedirectingFactoryBuilder extends SourceFactoryBuilder {

FreshTypeParameters? _tearOffTypeParameters;

bool _hasBeenCheckedAsRedirectingFactory = false;

RedirectingFactoryBuilder(
List<MetadataBuilder>? metadata,
int modifiers,
Expand Down Expand Up @@ -737,6 +739,9 @@ class RedirectingFactoryBuilder extends SourceFactoryBuilder {

@override
void _checkRedirectingFactory(TypeEnvironment typeEnvironment) {
if (_hasBeenCheckedAsRedirectingFactory) return;
_hasBeenCheckedAsRedirectingFactory = true;

// Check that factory declaration is not cyclic.
if (_isCyclicRedirectingFactory(this)) {
libraryBuilder.addProblemForRedirectingFactory(
Expand Down Expand Up @@ -788,6 +793,16 @@ class RedirectingFactoryBuilder extends SourceFactoryBuilder {
return;
}

Builder? redirectionTargetBuilder = redirectionTarget.target;
if (redirectionTargetBuilder is RedirectingFactoryBuilder) {
redirectionTargetBuilder._checkRedirectingFactory(typeEnvironment);
String? errorMessage = redirectionTargetBuilder
.function.redirectingFactoryTarget?.errorMessage;
if (errorMessage != null) {
setRedirectingFactoryError(errorMessage);
}
}

// Redirection to generative enum constructors is forbidden and is reported
// as an error elsewhere.
if (!((classBuilder?.cls.isEnum ?? false) &&
Expand Down
12 changes: 11 additions & 1 deletion pkg/front_end/lib/src/fasta/source/source_loader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2774,14 +2774,24 @@ severity: $severity
.logMs("Updated ${changedClasses.length} classes in kernel hierarchy");
}

void checkRedirectingFactories(List<SourceClassBuilder> sourceClasses) {
void checkRedirectingFactories(
List<SourceClassBuilder> sourceClasses,
List<SourceExtensionTypeDeclarationBuilder>
sourceExtensionTypeDeclarationBuilders) {
// TODO(ahe): Move this to [ClassHierarchyBuilder].
for (SourceClassBuilder builder in sourceClasses) {
if (builder.libraryBuilder.loader == this && !builder.isPatch) {
builder.checkRedirectingFactories(
typeInferenceEngine.typeSchemaEnvironment);
}
}
for (SourceExtensionTypeDeclarationBuilder builder
in sourceExtensionTypeDeclarationBuilders) {
if (builder.libraryBuilder.loader == this && !builder.isPatch) {
builder.checkRedirectingFactories(
typeInferenceEngine.typeSchemaEnvironment);
}
}
ticker.logMs("Checked redirecting factories");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ class TypeInferrerImpl implements TypeInferrer {
staticTarget: target);
visitor.checkCleanState();
DartType resultType = result.inferredType;
if (resultType is InterfaceType) {
if (resultType is TypeDeclarationType) {
return resultType.typeArguments;
} else {
return null;
Expand Down
1 change: 1 addition & 0 deletions pkg/front_end/test/spell_checking_list_tests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ gave
gc
gcd
gclient
ge
gesture
gi
gm
Expand Down
11 changes: 8 additions & 3 deletions pkg/front_end/testcases/dart2js/issue47916e.dart.strong.expect
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ import self as self;
import "dart:core" as core;

abstract class A extends core::Object {
static factory •() → self::A /* redirection-target: self::B::• */
return self::B::•();
static method _#new#tearOff() → self::A;
static factory •() → self::A
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916e.dart:10:23: Error: Redirection constructor target not found: 'C.named'
const factory B() = C.named;
^";
static method _#new#tearOff() → self::A
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916e.dart:10:23: Error: Redirection constructor target not found: 'C.named'
const factory B() = C.named;
^";
}
abstract class B extends core::Object implements self::A {
static factory •() → self::B
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ import self as self;
import "dart:core" as core;

abstract class A extends core::Object {
static factory •() → self::A /* redirection-target: self::B::• */
return self::B::•();
static method _#new#tearOff() → self::A;
static factory •() → self::A
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916e.dart:10:23: Error: Redirection constructor target not found: 'C.named'
const factory B() = C.named;
^";
static method _#new#tearOff() → self::A
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916e.dart:10:23: Error: Redirection constructor target not found: 'C.named'
const factory B() = C.named;
^";
}
abstract class B extends core::Object implements self::A {
static factory •() → self::B
Expand Down
11 changes: 8 additions & 3 deletions pkg/front_end/testcases/dart2js/issue47916e.dart.weak.expect
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ import self as self;
import "dart:core" as core;

abstract class A extends core::Object {
static factory •() → self::A /* redirection-target: self::B::• */
return self::B::•();
static method _#new#tearOff() → self::A;
static factory •() → self::A
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916e.dart:10:23: Error: Redirection constructor target not found: 'C.named'
const factory B() = C.named;
^";
static method _#new#tearOff() → self::A
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916e.dart:10:23: Error: Redirection constructor target not found: 'C.named'
const factory B() = C.named;
^";
}
abstract class B extends core::Object implements self::A {
static factory •() → self::B
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ import self as self;
import "dart:core" as core;

abstract class A extends core::Object {
static factory •() → self::A /* redirection-target: self::B::• */
return self::B::•();
static method _#new#tearOff() → self::A;
static factory •() → self::A
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916e.dart:10:23: Error: Redirection constructor target not found: 'C.named'
const factory B() = C.named;
^";
static method _#new#tearOff() → self::A
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916e.dart:10:23: Error: Redirection constructor target not found: 'C.named'
const factory B() = C.named;
^";
}
abstract class B extends core::Object implements self::A {
static factory •() → self::B
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ import self as self;
import "dart:core" as core;

abstract class A extends core::Object {
static factory •() → self::A /* redirection-target: self::B::• */
return self::B::•();
static method _#new#tearOff() → self::A;
static factory •() → self::A
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916e.dart:10:23: Error: Redirection constructor target not found: 'C.named'
const factory B() = C.named;
^";
static method _#new#tearOff() → self::A
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916e.dart:10:23: Error: Redirection constructor target not found: 'C.named'
const factory B() = C.named;
^";
}
abstract class B extends core::Object implements self::A {
static factory •() → self::B
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ import self as self;
import "dart:core" as core;

abstract class A extends core::Object {
static factory •() → self::A /* redirection-target: self::B::• */
return self::B::•();
static method _#new#tearOff() → self::A;
static factory •() → self::A
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916e.dart:10:23: Error: Redirection constructor target not found: 'C.named'
const factory B() = C.named;
^";
static method _#new#tearOff() → self::A
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916e.dart:10:23: Error: Redirection constructor target not found: 'C.named'
const factory B() = C.named;
^";
}
abstract class B extends core::Object implements self::A {
static factory •() → self::B
Expand Down
11 changes: 8 additions & 3 deletions pkg/front_end/testcases/dart2js/issue47916f.dart.strong.expect
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,14 @@ import self as self;
import "dart:core" as core;

abstract class A<T extends core::Object? = dynamic> extends core::Object {
static factory •<T extends core::Object? = dynamic>() → self::A<self::A::•::T%> /* redirection-target: self::B::•<self::A::•::T%>*/
return self::B::•<self::A::•::T%>();
static method _#new#tearOff<T extends core::Object? = dynamic>() → self::A<self::A::_#new#tearOff::T%>;
static factory •<T extends core::Object? = dynamic>() → self::A<self::A::•::T%>
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916f.dart:10:17: Error: Cyclic definition of factory 'B'.
const factory B() = C;
^";
static method _#new#tearOff<T extends core::Object? = dynamic>() → self::A<self::A::_#new#tearOff::T%>
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916f.dart:10:17: Error: Cyclic definition of factory 'B'.
const factory B() = C;
^";
}
abstract class B<T extends core::Object? = dynamic> extends core::Object implements self::A<self::B::T%> {
static factory •<T extends core::Object? = dynamic>() → self::B<self::B::•::T%>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,14 @@ import self as self;
import "dart:core" as core;

abstract class A<T extends core::Object? = dynamic> extends core::Object {
static factory •<T extends core::Object? = dynamic>() → self::A<self::A::•::T%> /* redirection-target: self::B::•<self::A::•::T%>*/
return self::B::•<self::A::•::T%>();
static method _#new#tearOff<T extends core::Object? = dynamic>() → self::A<self::A::_#new#tearOff::T%>;
static factory •<T extends core::Object? = dynamic>() → self::A<self::A::•::T%>
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916f.dart:10:17: Error: Cyclic definition of factory 'B'.
const factory B() = C;
^";
static method _#new#tearOff<T extends core::Object? = dynamic>() → self::A<self::A::_#new#tearOff::T%>
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916f.dart:10:17: Error: Cyclic definition of factory 'B'.
const factory B() = C;
^";
}
abstract class B<T extends core::Object? = dynamic> extends core::Object implements self::A<self::B::T%> {
static factory •<T extends core::Object? = dynamic>() → self::B<self::B::•::T%>
Expand Down
11 changes: 8 additions & 3 deletions pkg/front_end/testcases/dart2js/issue47916f.dart.weak.expect
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,14 @@ import self as self;
import "dart:core" as core;

abstract class A<T extends core::Object? = dynamic> extends core::Object {
static factory •<T extends core::Object? = dynamic>() → self::A<self::A::•::T%> /* redirection-target: self::B::•<self::A::•::T%>*/
return self::B::•<self::A::•::T%>();
static method _#new#tearOff<T extends core::Object? = dynamic>() → self::A<self::A::_#new#tearOff::T%>;
static factory •<T extends core::Object? = dynamic>() → self::A<self::A::•::T%>
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916f.dart:10:17: Error: Cyclic definition of factory 'B'.
const factory B() = C;
^";
static method _#new#tearOff<T extends core::Object? = dynamic>() → self::A<self::A::_#new#tearOff::T%>
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916f.dart:10:17: Error: Cyclic definition of factory 'B'.
const factory B() = C;
^";
}
abstract class B<T extends core::Object? = dynamic> extends core::Object implements self::A<self::B::T%> {
static factory •<T extends core::Object? = dynamic>() → self::B<self::B::•::T%>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,14 @@ import self as self;
import "dart:core" as core;

abstract class A<T extends core::Object? = dynamic> extends core::Object {
static factory •<T extends core::Object? = dynamic>() → self::A<self::A::•::T%> /* redirection-target: self::B::•<self::A::•::T%>*/
return self::B::•<self::A::•::T%>();
static method _#new#tearOff<T extends core::Object? = dynamic>() → self::A<self::A::_#new#tearOff::T%>;
static factory •<T extends core::Object? = dynamic>() → self::A<self::A::•::T%>
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916f.dart:10:17: Error: Cyclic definition of factory 'B'.
const factory B() = C;
^";
static method _#new#tearOff<T extends core::Object? = dynamic>() → self::A<self::A::_#new#tearOff::T%>
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916f.dart:10:17: Error: Cyclic definition of factory 'B'.
const factory B() = C;
^";
}
abstract class B<T extends core::Object? = dynamic> extends core::Object implements self::A<self::B::T%> {
static factory •<T extends core::Object? = dynamic>() → self::B<self::B::•::T%>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,14 @@ import self as self;
import "dart:core" as core;

abstract class A<T extends core::Object? = dynamic> extends core::Object {
static factory •<T extends core::Object? = dynamic>() → self::A<self::A::•::T%> /* redirection-target: self::B::•<self::A::•::T%>*/
return self::B::•<self::A::•::T%>();
static method _#new#tearOff<T extends core::Object? = dynamic>() → self::A<self::A::_#new#tearOff::T%>;
static factory •<T extends core::Object? = dynamic>() → self::A<self::A::•::T%>
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916f.dart:10:17: Error: Cyclic definition of factory 'B'.
const factory B() = C;
^";
static method _#new#tearOff<T extends core::Object? = dynamic>() → self::A<self::A::_#new#tearOff::T%>
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916f.dart:10:17: Error: Cyclic definition of factory 'B'.
const factory B() = C;
^";
}
abstract class B<T extends core::Object? = dynamic> extends core::Object implements self::A<self::B::T%> {
static factory •<T extends core::Object? = dynamic>() → self::B<self::B::•::T%>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,14 @@ import self as self;
import "dart:core" as core;

abstract class A<T extends core::Object? = dynamic> extends core::Object {
static factory •<T extends core::Object? = dynamic>() → self::A<self::A::•::T%> /* redirection-target: self::B::•<self::A::•::T%>*/
return self::B::•<self::A::•::T%>();
static method _#new#tearOff<T extends core::Object? = dynamic>() → self::A<self::A::_#new#tearOff::T%>;
static factory •<T extends core::Object? = dynamic>() → self::A<self::A::•::T%>
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916f.dart:10:17: Error: Cyclic definition of factory 'B'.
const factory B() = C;
^";
static method _#new#tearOff<T extends core::Object? = dynamic>() → self::A<self::A::_#new#tearOff::T%>
return invalid-expression "pkg/front_end/testcases/dart2js/issue47916f.dart:10:17: Error: Cyclic definition of factory 'B'.
const factory B() = C;
^";
}
abstract class B<T extends core::Object? = dynamic> extends core::Object implements self::A<self::B::T%> {
static factory •<T extends core::Object? = dynamic>() → self::B<self::B::•::T%>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ static inline-class-member method ET3|constructor#c1<T extends core::num>() →
}
static inline-class-member method ET3|constructor#_#c1#tearOff<T extends core::num>() → self::ET3<self::ET3|constructor#_#c1#tearOff::T> /* = core::int */
return self::ET3|constructor#c1<self::ET3|constructor#_#c1#tearOff::T>();
static inline-class-member method ET3|constructor#f1<T extends core::num>() → self::ET3<self::ET3|constructor#f1::T> /* = core::int */ /* redirection-target: self::ET3|constructor#c1<dynamic>*/
return self::ET3|constructor#c1<dynamic>();
static inline-class-member method ET3|constructor#f1<T extends core::num>() → self::ET3<self::ET3|constructor#f1::T> /* = core::int */ /* redirection-target: self::ET3|constructor#c1<self::ET3|constructor#f1::T>*/
return self::ET3|constructor#c1<self::ET3|constructor#f1::T>();
static inline-class-member method ET3|constructor#_#f1#tearOff<T extends core::num>() → self::ET3<self::ET3|constructor#_#f1#tearOff::T> /* = core::int */
return self::ET3|constructor#c1<dynamic>();
return self::ET3|constructor#c1<self::ET3|constructor#_#f1#tearOff::T>();
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,7 @@ static inline-class-member method ET3|constructor#c1<T extends core::num>() →
}
static inline-class-member method ET3|constructor#_#c1#tearOff<T extends core::num>() → self::ET3<self::ET3|constructor#_#c1#tearOff::T> /* = core::int */
return self::ET3|constructor#c1<self::ET3|constructor#_#c1#tearOff::T>();
static inline-class-member method ET3|constructor#f1<T extends core::num>() → self::ET3<self::ET3|constructor#f1::T> /* = core::int */ /* redirection-target: self::ET3|constructor#c1<dynamic>*/
return self::ET3|constructor#c1<dynamic>();
static inline-class-member method ET3|constructor#f1<T extends core::num>() → self::ET3<self::ET3|constructor#f1::T> /* = core::int */ /* redirection-target: self::ET3|constructor#c1<self::ET3|constructor#f1::T>*/
return self::ET3|constructor#c1<self::ET3|constructor#f1::T>();
static inline-class-member method ET3|constructor#_#f1#tearOff<T extends core::num>() → self::ET3<self::ET3|constructor#_#f1#tearOff::T> /* = core::int */
return self::ET3|constructor#c1<dynamic>();


Extra constant evaluation status:
Evaluated: StaticInvocation @ org-dartlang-testcase:///generic_factory.dart:7:17 -> IntConstant(0)
Evaluated: StaticInvocation @ org-dartlang-testcase:///generic_factory.dart:7:11 -> IntConstant(0)
Extra constant evaluation: evaluated: 10, effectively constant: 2
return self::ET3|constructor#c1<self::ET3|constructor#_#f1#tearOff::T>();
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ static inline-class-member method ET3|constructor#c1<T extends core::num>() →
}
static inline-class-member method ET3|constructor#_#c1#tearOff<T extends core::num>() → self::ET3<self::ET3|constructor#_#c1#tearOff::T> /* = core::int */
return self::ET3|constructor#c1<self::ET3|constructor#_#c1#tearOff::T>();
static inline-class-member method ET3|constructor#f1<T extends core::num>() → self::ET3<self::ET3|constructor#f1::T> /* = core::int */ /* redirection-target: self::ET3|constructor#c1<dynamic>*/
return self::ET3|constructor#c1<dynamic>();
static inline-class-member method ET3|constructor#f1<T extends core::num>() → self::ET3<self::ET3|constructor#f1::T> /* = core::int */ /* redirection-target: self::ET3|constructor#c1<self::ET3|constructor#f1::T>*/
return self::ET3|constructor#c1<self::ET3|constructor#f1::T>();
static inline-class-member method ET3|constructor#_#f1#tearOff<T extends core::num>() → self::ET3<self::ET3|constructor#_#f1#tearOff::T> /* = core::int */
return self::ET3|constructor#c1<dynamic>();
return self::ET3|constructor#c1<self::ET3|constructor#_#f1#tearOff::T>();
Loading

0 comments on commit 8b01baa

Please sign in to comment.