Skip to content

Commit

Permalink
[cfe] Remove coercions for super parameters
Browse files Browse the repository at this point in the history
Part of #47525

Change-Id: Iadb2f0125318edd73ab84e5b1a9cafae098ec618
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/239081
Reviewed-by: Johnni Winther <[email protected]>
Commit-Queue: Chloe Stefantsova <[email protected]>
  • Loading branch information
chloestefantsova authored and Commit Bot committed Mar 29, 2022
1 parent 95d1ae9 commit 61465fb
Show file tree
Hide file tree
Showing 13 changed files with 1,400 additions and 8 deletions.
14 changes: 12 additions & 2 deletions pkg/front_end/lib/src/fasta/kernel/body_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1854,6 +1854,7 @@ class BodyBuilder extends StackListenerImpl

List<Expression>? positionalSuperParametersAsArguments;
List<NamedExpression>? namedSuperParametersAsArguments;
Set<String>? namedSuperParameterNames;
if (formals != null) {
for (FormalParameterBuilder formal in formals) {
if (formal.isSuperInitializingFormal) {
Expand All @@ -1865,6 +1866,7 @@ class BodyBuilder extends StackListenerImpl
forNullGuardedAccess: false)
..fileOffset = formal.charOffset)
..fileOffset = formal.charOffset);
(namedSuperParameterNames ??= <String>{}).add(formal.name);
} else {
(positionalSuperParametersAsArguments ??= <Expression>[]).add(
new VariableGetImpl(formal.variable!,
Expand All @@ -1886,7 +1888,7 @@ class BodyBuilder extends StackListenerImpl
superInitializer.fileOffset, noLength))
..parent = constructor;
} else if (libraryBuilder.enableSuperParametersInLibrary) {
Arguments arguments = superInitializer.arguments;
ArgumentsImpl arguments = superInitializer.arguments as ArgumentsImpl;

if (positionalSuperParametersAsArguments != null) {
if (arguments.positional.isNotEmpty) {
Expand All @@ -1904,12 +1906,14 @@ class BodyBuilder extends StackListenerImpl
} else {
arguments.positional.addAll(positionalSuperParametersAsArguments);
setParents(positionalSuperParametersAsArguments, arguments);
arguments.positionalAreSuperParameters = true;
}
}
if (namedSuperParametersAsArguments != null) {
// TODO(cstefantsova): Report name conflicts.
arguments.named.addAll(namedSuperParametersAsArguments);
setParents(namedSuperParametersAsArguments, arguments);
arguments.namedSuperParameterNames = namedSuperParameterNames;
}
}
} else if (initializers.last is RedirectingInitializer) {
Expand Down Expand Up @@ -1958,7 +1962,7 @@ class BodyBuilder extends StackListenerImpl
/// >unless the enclosing class is class Object.
Constructor? superTarget = lookupConstructor(emptyName, isSuper: true);
Initializer initializer;
Arguments arguments;
ArgumentsImpl arguments;
List<Expression>? positionalArguments;
List<NamedExpression>? namedArguments;
if (libraryBuilder.enableSuperParametersInLibrary) {
Expand All @@ -1976,13 +1980,19 @@ class BodyBuilder extends StackListenerImpl
forNullGuardedAccess: false)
]);
}

if (positionalArguments != null || namedArguments != null) {
arguments = forest.createArguments(
noLocation, positionalArguments ?? <Expression>[],
named: namedArguments);
} else {
arguments = forest.createArgumentsEmpty(noLocation);
}

arguments.positionalAreSuperParameters =
positionalSuperParametersAsArguments != null;
arguments.namedSuperParameterNames = namedSuperParameterNames;

if (superTarget == null ||
checkArgumentsForFunction(superTarget.function, arguments,
builder.charOffset, const <TypeParameter>[]) !=
Expand Down
11 changes: 11 additions & 0 deletions pkg/front_end/lib/src/fasta/kernel/internal_ast.dart
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,17 @@ class ArgumentsImpl extends Arguments {

List<Object?>? argumentsOriginalOrder;

/// True if the arguments are passed to the super-constructor in a
/// super-initializer, and the positional parameters are super-initializer
/// parameters. It is true that either all of the positional parameters are
/// super-initializer parameters or none of them, so a simple boolean
/// accurately reflects the state.
bool positionalAreSuperParameters = false;

/// Names of the named positional parameters. If none of the parameters are
/// super-positional, the field is null.
Set<String>? namedSuperParameterNames;

ArgumentsImpl.internal(
{required List<Expression> positional,
required List<DartType>? types,
Expand Down
30 changes: 24 additions & 6 deletions pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@ class TypeInferrerImpl implements TypeInferrer {
DartType? declaredContextType,
DartType? runtimeCheckedType,
bool isVoidAllowed: false,
bool coerceExpression: true,
Template<Message Function(DartType, DartType, bool)>? errorTemplate,
Template<Message Function(DartType, DartType, bool)>?
nullabilityErrorTemplate,
Expand All @@ -509,6 +510,7 @@ class TypeInferrerImpl implements TypeInferrer {
declaredContextType: declaredContextType,
runtimeCheckedType: runtimeCheckedType,
isVoidAllowed: isVoidAllowed,
coerceExpression: coerceExpression,
errorTemplate: errorTemplate,
nullabilityErrorTemplate: nullabilityErrorTemplate,
nullabilityNullErrorTemplate: nullabilityNullErrorTemplate,
Expand All @@ -527,6 +529,7 @@ class TypeInferrerImpl implements TypeInferrer {
DartType? declaredContextType,
DartType? runtimeCheckedType,
bool isVoidAllowed: false,
bool coerceExpression: true,
Template<Message Function(DartType, DartType, bool)>? errorTemplate,
Template<Message Function(DartType, DartType, bool)>?
nullabilityErrorTemplate,
Expand Down Expand Up @@ -578,7 +581,8 @@ class TypeInferrerImpl implements TypeInferrer {
contextType, inferenceResult.inferredType,
isNonNullableByDefault: isNonNullableByDefault,
isVoidAllowed: isVoidAllowed,
isExpressionTypePrecise: preciseTypeErrorTemplate != null);
isExpressionTypePrecise: preciseTypeErrorTemplate != null,
coerceExpression: coerceExpression);

if (assignabilityResult.needsTearOff) {
TypedTearoff typedTearoff = _tearOffCall(inferenceResult.expression,
Expand Down Expand Up @@ -782,7 +786,8 @@ class TypeInferrerImpl implements TypeInferrer {
DartType contextType, DartType expressionType,
{required bool isNonNullableByDefault,
required bool isVoidAllowed,
required bool isExpressionTypePrecise}) {
required bool isExpressionTypePrecise,
required bool coerceExpression}) {
// ignore: unnecessary_null_comparison
assert(isNonNullableByDefault != null);
// ignore: unnecessary_null_comparison
Expand All @@ -794,7 +799,7 @@ class TypeInferrerImpl implements TypeInferrer {
// should tear off `.call`.
// TODO(paulberry): use resolveTypeParameter. See findInterfaceMember.
bool needsTearoff = false;
if (expressionType is InterfaceType) {
if (coerceExpression && expressionType is InterfaceType) {
Class classNode = expressionType.classNode;
Member? callMember =
classHierarchy.getInterfaceMember(classNode, callName);
Expand All @@ -813,7 +818,7 @@ class TypeInferrerImpl implements TypeInferrer {
}
}
ImplicitInstantiation? implicitInstantiation;
if (libraryBuilder.enableConstructorTearOffsInLibrary) {
if (coerceExpression && libraryBuilder.enableConstructorTearOffsInLibrary) {
implicitInstantiation =
computeImplicitInstantiation(expressionType, contextType);
if (implicitInstantiation != null) {
Expand Down Expand Up @@ -867,8 +872,15 @@ class TypeInferrerImpl implements TypeInferrer {
return const AssignabilityResult(AssignabilityKind.unassignablePrecise,
needsTearOff: false);
}
// Insert an implicit downcast.
return new AssignabilityResult(AssignabilityKind.assignableCast,

if (coerceExpression) {
// Insert an implicit downcast.
return new AssignabilityResult(AssignabilityKind.assignableCast,
needsTearOff: needsTearoff,
implicitInstantiation: implicitInstantiation);
}

return new AssignabilityResult(AssignabilityKind.unassignable,
needsTearOff: needsTearoff,
implicitInstantiation: implicitInstantiation);
}
Expand Down Expand Up @@ -2618,17 +2630,23 @@ class TypeInferrerImpl implements TypeInferrer {
DartType actualType = actualTypes![i];
Expression expression;
NamedExpression? namedExpression;
bool coerceExpression;
if (i < numPositionalArgs) {
expression = arguments.positional[positionalShift + i];
positionalArgumentTypes.add(actualType);
coerceExpression = !arguments.positionalAreSuperParameters;
} else {
namedExpression = arguments.named[i - numPositionalArgs];
expression = namedExpression.value;
namedArgumentTypes
.add(new NamedType(namedExpression.name, actualType));
coerceExpression = !(arguments.namedSuperParameterNames
?.contains(namedExpression.name) ??
false);
}
expression = ensureAssignable(expectedType, actualType, expression,
isVoidAllowed: expectedType is VoidType,
coerceExpression: coerceExpression,
// TODO(johnniwinther): Specialize message for operator
// invocations.
errorTemplate: templateArgumentTypeNotAssignable,
Expand Down
2 changes: 2 additions & 0 deletions pkg/front_end/test/spell_checking_list_common.txt
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ account
accounted
accumulate
accurate
accurately
achieve
act
acting
Expand Down Expand Up @@ -489,6 +490,7 @@ closure
closures
clue
code
coerce
coincides
coinductively
collapses
Expand Down
82 changes: 82 additions & 0 deletions pkg/front_end/testcases/super_parameters/no_coercions.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// 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.

class A1 {
A1(int x);
}

class B1 extends A1 {
B1.one(dynamic super.x); // Error.
B1.two(dynamic super.x) : super(); // Error.
}

class A2 {
A2({required String x});
}

class B2 extends A2 {
B2.one({required dynamic super.x}); // Error.
B2.two({required dynamic super.x}) : super(); // Error.
}

class A3 {
A3(num Function(double) f);
}

class B3 extends A3 {
B3.one(X Function<X>(double) super.f); // Error.
B3.two(X Function<X>(double) super.f) : super(); // Error.
}

class A4 {
A4({required num Function(double) f});
}

class B4 extends A4 {
B4.one({required X Function<X>(double) super.f}); // Error.
B4.two({required X Function<X>(double) super.f}) : super(); // Error.
}

abstract class C5 {
String call(int x, num y);
}

class A5 {
A5(String Function(int, num) f);
}

class B5 extends A5 {
B5.one(C5 super.f); // Error.
B5.two(C5 super.f) : super(); // Error.
}

class A6 {
A6({required String Function(int, num) f});
}

class B6 extends A6 {
B6.one({required C5 super.f}); // Error.
B6.two({required C5 super.f}) : super(); // Error.
}

class A7 {
A7({required int x1,
required int x2,
required bool Function(Object) f1,
required bool Function(Object) f2,
required void Function(dynamic) g1,
required void Function(dynamic) g2});
}

class B7 extends A7 {
B7({required dynamic super.x1, // Error.
required dynamic x2,
required X Function<X>(Object) super.f1, // Error.
required X Function<X>(Object) f2,
required void Function<X>(X) super.g1, // Error.
required void Function<X>(X) g2}) :
super(x2: x2, f2: f2, g2: g2); // Ok.
}

main() {}
Loading

0 comments on commit 61465fb

Please sign in to comment.