Skip to content

Commit

Permalink
[dart2js] Clean up handling of missing super methods.
Browse files Browse the repository at this point in the history 
Issue: #47406
Issue: #48820
Change-Id: I19de399c4670f9866cffceae3bc3ce19201d1ed3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/352963
Reviewed-by: Johnni Winther <[email protected]>
Commit-Queue: Mayank Patke <[email protected]>
  • Loading branch information
@fishythefish
fishythefish authored and Commit Queue committed Feb 20, 2024
1 parent 3722837 commit b0a96bf
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 249 deletions.
63 changes: 10 additions & 53 deletions pkg/compiler/lib/src/inferrer/builder.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,16 +243,6 @@ class KernelTypeGraphBuilder extends ir.VisitorDefault<TypeInformation?>
return visit(_analyzedNode)!;
}

bool isIncompatibleInvoke(FunctionEntity function, ArgumentsTypes arguments) {
ParameterStructure parameterStructure = function.parameterStructure;

return arguments.positional.length <
parameterStructure.requiredPositionalParameters ||
arguments.positional.length > parameterStructure.positionalParameters ||
arguments.named.keys
.any((name) => !parameterStructure.namedParameters.contains(name));
}

void recordReturnType(TypeInformation type) {
final analyzedMethod = _analyzedMember as FunctionEntity;
_returnType = _inferrer.addReturnTypeForMethod(analyzedMethod, type);
Expand Down Expand Up @@ -940,10 +930,6 @@ class KernelTypeGraphBuilder extends ir.VisitorDefault<TypeInformation?>
ClosureRepresentationInfo info =
_closureDataLookup.getClosureInfo(function);
final callMethod = info.callMethod!;
if (isIncompatibleInvoke(callMethod, argumentsTypes)) {
return _types.dynamicType;
}

TypeInformation type =
handleStaticInvoke(node, selector, callMethod, argumentsTypes);
FunctionType functionType =
Expand Down Expand Up @@ -2153,15 +2139,6 @@ class KernelTypeGraphBuilder extends ir.VisitorDefault<TypeInformation?>
return _types.nonNullEmpty();
}

TypeInformation handleSuperNoSuchMethod(
ir.Node node, Selector selector, ArgumentsTypes? arguments) {
// Ensure we create a node, to make explicit the call to the
// `noSuchMethod` handler.
FunctionEntity noSuchMethod =
_elementMap.getSuperNoSuchMethod(_analyzedMember.enclosingClass!);
return handleStaticInvoke(node, selector, noSuchMethod, arguments);
}

@override
TypeInformation visitSuperPropertyGet(ir.SuperPropertyGet node) {
// TODO(herhut): We could do better here if we knew what we
Expand All @@ -2170,13 +2147,6 @@ class KernelTypeGraphBuilder extends ir.VisitorDefault<TypeInformation?>

final target = getEffectiveSuperTarget(node.interfaceTarget);
Selector selector = Selector.getter(_elementMap.getName(node.name));
if (target == null) {
// TODO(johnniwinther): Remove this when the CFE checks for missing
// concrete super targets.
// TODO(48820): If this path is infeasible, update types on
// getEffectiveSuperTarget.
return handleSuperNoSuchMethod(node, selector, null);
}
MemberEntity member = _elementMap.getMember(target);
TypeInformation type = handleStaticInvoke(node, selector, member, null);
if (member.isGetter) {
Expand Down Expand Up @@ -2208,11 +2178,6 @@ class KernelTypeGraphBuilder extends ir.VisitorDefault<TypeInformation?>
final target = getEffectiveSuperTarget(node.interfaceTarget);
Selector selector = Selector.setter(_elementMap.getName(node.name));
ArgumentsTypes arguments = ArgumentsTypes([rhsType], null);
if (target == null) {
// TODO(johnniwinther): Remove this when the CFE checks for missing
// concrete super targets.
return handleSuperNoSuchMethod(node, selector, arguments);
}
final member = _elementMap.getMember(target);
handleStaticInvoke(node, selector, member, arguments);
return rhsType;
Expand All @@ -2227,27 +2192,19 @@ class KernelTypeGraphBuilder extends ir.VisitorDefault<TypeInformation?>
final target = getEffectiveSuperTarget(node.interfaceTarget);
ArgumentsTypes arguments = analyzeArguments(node.arguments);
Selector selector = _elementMap.getSelector(node);
if (target == null) {
// TODO(johnniwinther): Remove this when the CFE checks for missing
// concrete super targets.
return handleSuperNoSuchMethod(node, selector, arguments);
}
MemberEntity member = _elementMap.getMember(target);
assert(member.isFunction, "Unexpected super invocation target: $member");
if (isIncompatibleInvoke(member as FunctionEntity, arguments)) {
return handleSuperNoSuchMethod(node, selector, arguments);
} else {
TypeInformation type =
handleStaticInvoke(node, selector, member, arguments);
FunctionType functionType =
_elementMap.elementEnvironment.getFunctionType(member);
if (functionType.returnType.containsFreeTypeVariables) {
// The return type varies with the call site so we narrow the static
// return type.
type = _types.narrowType(type, _getStaticType(node));
}
return type;
member as FunctionEntity;
TypeInformation type =
handleStaticInvoke(node, selector, member, arguments);
FunctionType functionType =
_elementMap.elementEnvironment.getFunctionType(member);
if (functionType.returnType.containsFreeTypeVariables) {
// The return type varies with the call site so we narrow the static
// return type.
type = _types.narrowType(type, _getStaticType(node));
}
return type;
}

@override
Expand Down
6 changes: 3 additions & 3 deletions pkg/compiler/lib/src/ir/impact.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,12 @@ abstract class ImpactRegistry {

void registerInstanceSet(ir.DartType receiverType, ir.Member target);

void registerSuperInvocation(ir.Member? target, int positionalArguments,
void registerSuperInvocation(ir.Member target, int positionalArguments,
List<String> namedArguments, List<ir.DartType> typeArguments);

void registerSuperGet(ir.Member? target);
void registerSuperGet(ir.Member target);

void registerSuperSet(ir.Member? target);
void registerSuperSet(ir.Member target);

void registerSuperInitializer(
ir.Constructor source,
Expand Down
18 changes: 9 additions & 9 deletions pkg/compiler/lib/src/ir/impact_data.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ class ImpactBuilder extends ir.RecursiveVisitor implements ImpactRegistry {
@override
void visitSuperMethodInvocation(ir.SuperMethodInvocation node) {
registerSuperInvocation(
getEffectiveSuperTarget(node.interfaceTarget)!,
getEffectiveSuperTarget(node.interfaceTarget),
node.arguments.positional.length,
_getNamedArguments(node.arguments),
node.arguments.types);
Expand All @@ -688,12 +688,12 @@ class ImpactBuilder extends ir.RecursiveVisitor implements ImpactRegistry {

@override
void visitSuperPropertyGet(ir.SuperPropertyGet node) {
registerSuperGet(getEffectiveSuperTarget(node.interfaceTarget)!);
registerSuperGet(getEffectiveSuperTarget(node.interfaceTarget));
}

@override
void visitSuperPropertySet(ir.SuperPropertySet node) {
registerSuperSet(getEffectiveSuperTarget(node.interfaceTarget)!);
registerSuperSet(getEffectiveSuperTarget(node.interfaceTarget));
node.value.accept(this);
}

Expand Down Expand Up @@ -788,19 +788,19 @@ class ImpactBuilder extends ir.RecursiveVisitor implements ImpactRegistry {
}

@override
void registerSuperSet(ir.Member? target) {
(_data._superSets ??= []).add(target!);
void registerSuperSet(ir.Member target) {
(_data._superSets ??= []).add(target);
}

@override
void registerSuperGet(ir.Member? target) {
(_data._superGets ??= []).add(target!);
void registerSuperGet(ir.Member target) {
(_data._superGets ??= []).add(target);
}

@override
void registerSuperInvocation(ir.Member? target, int positionalArguments,
void registerSuperInvocation(ir.Member target, int positionalArguments,
List<String> namedArguments, List<ir.DartType> typeArguments) {
(_data._superInvocations ??= []).add(_SuperInvocation(target!,
(_data._superInvocations ??= []).add(_SuperInvocation(target,
_CallStructure(positionalArguments, namedArguments, typeArguments)));
}

Expand Down
9 changes: 2 additions & 7 deletions pkg/compiler/lib/src/ir/util.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,15 +315,10 @@ bool memberEntityIsInWebLibrary(MemberEntity entity) {
///
/// See [ir.ProcedureStubKind.ConcreteMixinStub] for why concrete mixin stubs
/// are inserted in the first place.
ir.Member? getEffectiveSuperTarget(ir.Member? target) {
ir.Member getEffectiveSuperTarget(ir.Member target) {
if (target is ir.Procedure) {
if (target.stubKind == ir.ProcedureStubKind.ConcreteMixinStub) {
return getEffectiveSuperTarget(target.stubTarget);
}
// TODO(johnniwinther): Remove this when the CFE reports an error on
// missing concrete super targets.
if (target.isAbstract) {
return null;
return getEffectiveSuperTarget(target.stubTarget!);
}
}
return target;
Expand Down
18 changes: 0 additions & 18 deletions pkg/compiler/lib/src/js_backend/backend_impact.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,18 +201,6 @@ class BackendImpacts {
],
);

late final BackendImpact superNoSuchMethod = BackendImpact(
staticUses: [
_commonElements.createInvocationMirror,
_commonElements.objectNoSuchMethod!
],
otherImpacts: [
_needsInt('Needed to encode the invocation kind of super.noSuchMethod.'),
_needsList('Needed to encode the arguments of super.noSuchMethod.'),
_needsString('Needed to encode the name of super.noSuchMethod.')
],
);

late final BackendImpact constantMapLiteral = BackendImpact(
instantiatedClasses: [
_commonElements.constantMapClass,
Expand All @@ -239,12 +227,6 @@ class BackendImpacts {
return intValues;
}

/// Helper for registering that `List` is needed.
BackendImpact _needsList(String reason) {
// TODO(johnniwinther): Register [reason] for use in dump-info.
return listValues;
}

/// Helper for registering that `String` is needed.
BackendImpact _needsString(String reason) {
// TODO(johnniwinther): Register [reason] for use in dump-info.
Expand Down
4 changes: 0 additions & 4 deletions pkg/compiler/lib/src/js_model/element_map.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,6 @@ abstract class JsToElementMap {
/// Returns the [ClassEntity] corresponding to the class [node].
ClassEntity getClass(ir.Class node);

/// Returns the `noSuchMethod` [FunctionEntity] call from a
/// `super.noSuchMethod` invocation within [cls].
FunctionEntity getSuperNoSuchMethod(ClassEntity cls);

/// Returns the [Name] corresponding to [name].
Name getName(ir.Name name);

Expand Down
24 changes: 0 additions & 24 deletions pkg/compiler/lib/src/js_model/element_map_impl.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1336,30 +1336,6 @@ class JsKernelToElementMap implements JsToElementMap, IrToElementMap {
return ConstructedConstantValue(_commonElements.requiredSentinelType, {});
}

@override
FunctionEntity getSuperNoSuchMethod(ClassEntity cls) {
while (true) {
ClassEntity? superclass = elementEnvironment.getSuperClass(cls);
if (superclass == null) break;
MemberEntity? member = elementEnvironment.lookupLocalClassMember(
superclass, Names.noSuchMethod_);
if (member != null && !member.isAbstract) {
if (member.isFunction) {
final function = member as FunctionEntity;
if (function.parameterStructure.positionalParameters >= 1) {
return function;
}
}
// If [member] is not a valid `noSuchMethod` the target is
// `Object.superNoSuchMethod`.
break;
}
cls = superclass;
}
return elementEnvironment.lookupLocalClassMember(
commonElements.objectClass, Names.noSuchMethod_)! as FunctionEntity;
}

JTypeVariable createTypeVariable(
Entity typeDeclaration, String name, int index) {
return JTypeVariable(typeDeclaration, name, index);
Expand Down
67 changes: 20 additions & 47 deletions pkg/compiler/lib/src/kernel/kernel_impact.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,63 +474,36 @@ class KernelImpactConverter implements ImpactRegistry {
}

@override
void registerSuperInvocation(ir.Member? target, int positionalArguments,
void registerSuperInvocation(ir.Member target, int positionalArguments,
List<String> namedArguments, List<ir.DartType> typeArguments) {
if (target != null) {
FunctionEntity method = elementMap.getMember(target) as FunctionEntity;
List<DartType>? dartTypeArguments = _getTypeArguments(typeArguments);
impactBuilder.registerStaticUse(StaticUse.superInvoke(
method,
CallStructure(positionalArguments + namedArguments.length,
namedArguments, typeArguments.length),
dartTypeArguments));
} else {
// TODO(johnniwinther): Remove this when the CFE checks for missing
// concrete super targets.
impactBuilder.registerStaticUse(StaticUse.superInvoke(
elementMap.getSuperNoSuchMethod(currentMember.enclosingClass!),
CallStructure.ONE_ARG));
registerBackendImpact(_impacts.superNoSuchMethod);
}
FunctionEntity method = elementMap.getMember(target) as FunctionEntity;
List<DartType>? dartTypeArguments = _getTypeArguments(typeArguments);
impactBuilder.registerStaticUse(StaticUse.superInvoke(
method,
CallStructure(positionalArguments + namedArguments.length,
namedArguments, typeArguments.length),
dartTypeArguments));
}

@override
void registerSuperGet(ir.Member? target) {
if (target != null) {
MemberEntity member = elementMap.getMember(target);
if (member.isFunction) {
impactBuilder.registerStaticUse(
StaticUse.superTearOff(member as FunctionEntity));
} else {
impactBuilder.registerStaticUse(StaticUse.superGet(member));
}
void registerSuperGet(ir.Member target) {
MemberEntity member = elementMap.getMember(target);
if (member.isFunction) {
impactBuilder
.registerStaticUse(StaticUse.superTearOff(member as FunctionEntity));
} else {
// TODO(johnniwinther): Remove this when the CFE checks for missing
// concrete super targets.
impactBuilder.registerStaticUse(StaticUse.superInvoke(
elementMap.getSuperNoSuchMethod(currentMember.enclosingClass!),
CallStructure.ONE_ARG));
registerBackendImpact(_impacts.superNoSuchMethod);
impactBuilder.registerStaticUse(StaticUse.superGet(member));
}
}

@override
void registerSuperSet(ir.Member? target) {
if (target != null) {
MemberEntity member = elementMap.getMember(target);
if (member is FieldEntity) {
impactBuilder.registerStaticUse(StaticUse.superFieldSet(member));
} else {
impactBuilder.registerStaticUse(
StaticUse.superSetterSet(member as FunctionEntity));
}
void registerSuperSet(ir.Member target) {
MemberEntity member = elementMap.getMember(target);
if (member is FieldEntity) {
impactBuilder.registerStaticUse(StaticUse.superFieldSet(member));
} else {
// TODO(johnniwinther): Remove this when the CFE checks for missing
// concrete super targets.
impactBuilder.registerStaticUse(StaticUse.superInvoke(
elementMap.getSuperNoSuchMethod(currentMember.enclosingClass!),
CallStructure.ONE_ARG));
registerBackendImpact(_impacts.superNoSuchMethod);
impactBuilder.registerStaticUse(
StaticUse.superSetterSet(member as FunctionEntity));
}
}

Expand Down
Loading

0 comments on commit b0a96bf

Please sign in to comment.