Skip to content

Commit

Permalink
[cfe] Report error for missing concrete super target
Browse files Browse the repository at this point in the history
In response to #47406

The error is currently not reported if the mixin declaration is from
an outline dill.

Change-Id: I94a61d6409d0c238614d9f377b5f324153360bc6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249184
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
  • Loading branch information
johnniwinther authored and Commit Bot committed Jun 24, 2022
1 parent 0113970 commit e18977e
Show file tree
Hide file tree
Showing 52 changed files with 8,303 additions and 41 deletions.
95 changes: 95 additions & 0 deletions pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7820,6 +7820,101 @@ const MessageCode messageMissingTypedefParameters = const MessageCode(
problemMessage: r"""A typedef needs an explicit list of parameters.""",
correctionMessage: r"""Try adding a parameter list to the typedef.""");

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Template<
Message Function(
String
name)> templateMixinApplicationNoConcreteGetter = const Template<
Message Function(String name)>(
problemMessageTemplate:
r"""The class doesn't have a concrete implementation of the super-accessed member '#name'.""",
withArguments: _withArgumentsMixinApplicationNoConcreteGetter);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Message Function(String name)> codeMixinApplicationNoConcreteGetter =
const Code<Message Function(String name)>(
"MixinApplicationNoConcreteGetter",
analyzerCodes: <String>[
"MIXIN_APPLICATION_NO_CONCRETE_SUPER_INVOKED_MEMBER"
]);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
Message _withArgumentsMixinApplicationNoConcreteGetter(String name) {
if (name.isEmpty) throw 'No name provided';
name = demangleMixinApplicationName(name);
return new Message(codeMixinApplicationNoConcreteGetter,
problemMessage:
"""The class doesn't have a concrete implementation of the super-accessed member '${name}'.""",
arguments: {'name': name});
}

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Null> codeMixinApplicationNoConcreteMemberContext =
messageMixinApplicationNoConcreteMemberContext;

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const MessageCode messageMixinApplicationNoConcreteMemberContext =
const MessageCode("MixinApplicationNoConcreteMemberContext",
severity: Severity.context,
problemMessage:
r"""This is the super-access that doesn't have a concrete target.""");

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Template<
Message Function(
String
name)> templateMixinApplicationNoConcreteMethod = const Template<
Message Function(String name)>(
problemMessageTemplate:
r"""The class doesn't have a concrete implementation of the super-invoked member '#name'.""",
withArguments: _withArgumentsMixinApplicationNoConcreteMethod);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Message Function(String name)> codeMixinApplicationNoConcreteMethod =
const Code<Message Function(String name)>(
"MixinApplicationNoConcreteMethod",
analyzerCodes: <String>[
"MIXIN_APPLICATION_NO_CONCRETE_SUPER_INVOKED_MEMBER"
]);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
Message _withArgumentsMixinApplicationNoConcreteMethod(String name) {
if (name.isEmpty) throw 'No name provided';
name = demangleMixinApplicationName(name);
return new Message(codeMixinApplicationNoConcreteMethod,
problemMessage:
"""The class doesn't have a concrete implementation of the super-invoked member '${name}'.""",
arguments: {'name': name});
}

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Template<
Message Function(
String
name)> templateMixinApplicationNoConcreteSetter = const Template<
Message Function(String name)>(
problemMessageTemplate:
r"""The class doesn't have a concrete implementation of the super-accessed setter '#name'.""",
withArguments: _withArgumentsMixinApplicationNoConcreteSetter);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Message Function(String name)> codeMixinApplicationNoConcreteSetter =
const Code<Message Function(String name)>(
"MixinApplicationNoConcreteSetter",
analyzerCodes: <String>[
"MIXIN_APPLICATION_NO_CONCRETE_SUPER_INVOKED_MEMBER"
]);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
Message _withArgumentsMixinApplicationNoConcreteSetter(String name) {
if (name.isEmpty) throw 'No name provided';
name = demangleMixinApplicationName(name);
return new Message(codeMixinApplicationNoConcreteSetter,
problemMessage:
"""The class doesn't have a concrete implementation of the super-accessed setter '${name}'.""",
arguments: {'name': name});
}

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Null> codeMixinDeclaresConstructor = messageMixinDeclaresConstructor;

Expand Down
2 changes: 1 addition & 1 deletion pkg/compiler/lib/src/js_model/element_map_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ class JsKernelToElementMap implements JsToElementMap, IrToElementMap {
//
// so we need get the superclasses from the on-clause, A, B, and C,
// through [superclassConstraints].
for (ir.Supertype constraint in node.superclassConstraints()) {
for (ir.Supertype constraint in node.onClause) {
interfaces.add(processSupertype(constraint));
}
// Set superclass to `Object`.
Expand Down
2 changes: 1 addition & 1 deletion pkg/compiler/lib/src/kernel/element_map_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ class KernelToElementMap implements IrToElementMap {
//
// so we need get the superclasses from the on-clause, A, B, and C,
// through [superclassConstraints].
for (ir.Supertype constraint in node.superclassConstraints()) {
for (ir.Supertype constraint in node.onClause) {
interfaces.add(processSupertype(constraint));
}
// Set superclass to `Object`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,10 @@ class _CloneMixinMethodsWithSuper {
ensureExistingProcedureMaps();
Procedure? existingGetter = existingNonSetters[field.name];
Procedure? existingSetter = existingSetters[field.name];
cls.addField(cloneVisitor.cloneField(
field, null, existingGetter?.reference, existingSetter?.reference));
Field clone = cloneVisitor.cloneField(
field, null, existingGetter?.reference, existingSetter?.reference);
cls.addField(clone);
clone.transformerFlags = field.transformerFlags;
if (existingGetter != null) {
cls.procedures.remove(existingGetter);
}
Expand All @@ -84,8 +86,10 @@ class _CloneMixinMethodsWithSuper {
if (existingProcedure != null) {
cls.procedures.remove(existingProcedure);
}
cls.addProcedure(cloneVisitor.cloneProcedure(
procedure, existingProcedure?.reference));
Procedure clone = cloneVisitor.cloneProcedure(
procedure, existingProcedure?.reference);
cls.addProcedure(clone);
clone.transformerFlags = procedure.transformerFlags;
continue;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,9 @@ class LateLowering {
fileUri: fileUri, reference: field.getterReference)
..fileOffset = fileOffset
..isNonNullableByDefault = true;
// The initializer is copied from [field] to [getter] so we copy the
// transformer flags to reflect whether the getter contains super calls.
getter.transformerFlags = field.transformerFlags;
enclosingClass.addProcedure(getter);

VariableDeclaration setterValue = VariableDeclaration('value', type: type)
Expand Down
3 changes: 1 addition & 2 deletions pkg/dev_compiler/lib/src/kernel/compiler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1397,8 +1397,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
var savedClass = _classEmittingSignatures;
_classEmittingSignatures = c;

var interfaces = c.implementedTypes.toList()
..addAll(c.superclassConstraints());
var interfaces = c.implementedTypes.toList()..addAll(c.onClause);
if (interfaces.isNotEmpty) {
body.add(js.statement('#[#] = () => [#];', [
className,
Expand Down
1 change: 1 addition & 0 deletions pkg/front_end/lib/src/fasta/kernel/benchmarker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ enum BenchmarkPhases {
outline_installAllComponentProblems,

body_buildBodies,
body_checkMixinSuperAccesses,
body_finishSynthesizedParameters,
body_finishDeferredLoadTearoffs,
body_finishNoSuchMethodForwarders,
Expand Down
3 changes: 3 additions & 0 deletions pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,9 @@ class KernelTarget extends TargetImplementation {
benchmarker?.enterPhase(BenchmarkPhases.body_buildBodies);
await loader.buildBodies(loader.sourceLibraryBuilders);

benchmarker?.enterPhase(BenchmarkPhases.body_checkMixinSuperAccesses);
loader.checkMixinSuperAccesses();

benchmarker?.enterPhase(BenchmarkPhases.body_finishSynthesizedParameters);
finishSynthesizedParameters();

Expand Down
4 changes: 1 addition & 3 deletions pkg/front_end/lib/src/fasta/kernel/verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import 'package:_fe_analyzer_shared/src/messages/severity.dart' show Severity;
import 'package:kernel/ast.dart';
import 'package:kernel/target/targets.dart';

import 'package:kernel/transformations/flags.dart' show TransformerFlag;

import 'package:kernel/type_environment.dart' show TypeEnvironment;

import 'package:kernel/verifier.dart'
Expand Down Expand Up @@ -170,7 +168,7 @@ class FastaVerifyingVisitor extends VerifyingVisitor {
if (containingMember == null) {
problem(node, 'Super call outside of any member');
} else {
if (containingMember.transformerFlags & TransformerFlag.superCalls == 0) {
if (!containingMember.containsSuperCalls) {
problem(
node, 'Super call in a member lacking TransformerFlag.superCalls');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,7 @@ class SourceClassBuilder extends ClassBuilderImpl
// the declaration's superclass constraints.
InterfaceType supertype = cls.supertype!.asInterfaceType;
Substitution substitution = Substitution.fromSupertype(cls.mixedInType!);
for (Supertype constraint in cls.mixedInClass!.superclassConstraints()) {
for (Supertype constraint in cls.mixedInClass!.onClause) {
InterfaceType requiredInterface =
substitution.substituteSupertype(constraint).asInterfaceType;
InterfaceType? implementedInterface = hierarchy.getTypeAsInstanceOf(
Expand Down
25 changes: 7 additions & 18 deletions pkg/front_end/lib/src/fasta/source/source_field_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,6 @@ class SourceFieldBuilder extends SourceMemberBuilderImpl
ClassHierarchy classHierarchy,
List<DelayedActionPerformer> delayedActionPerformers,
List<DelayedDefaultValueCloner> delayedDefaultValueCloners) {
_fieldEncoding.completeSignature(classHierarchy.coreTypes);

for (Annotatable annotatable in _fieldEncoding.annotatables) {
MetadataBuilder.buildAnnotations(
annotatable,
Expand Down Expand Up @@ -593,10 +591,6 @@ abstract class FieldEncoding {
/// Returns a list of the setters created by this field encoding.
List<ClassMember> getLocalSetters(SourceFieldBuilder fieldBuilder);

/// Ensures that the signatures all members created by this field encoding
/// are fully typed.
void completeSignature(CoreTypes coreTypes);

/// Returns `true` if this encoding is a late lowering.
bool get isLateLowering;
}
Expand Down Expand Up @@ -657,9 +651,6 @@ class RegularFieldEncoding implements FieldEncoding {
_field.type = value;
}

@override
void completeSignature(CoreTypes coreTypes) {}

@override
void createBodies(CoreTypes coreTypes, Expression? initializer) {
if (initializer != null) {
Expand Down Expand Up @@ -899,11 +890,6 @@ abstract class AbstractLateFieldEncoding implements FieldEncoding {
late_lowering.computeIsSetEncoding(_type!, _isSetStrategy);
}

@override
void completeSignature(CoreTypes coreTypes) {
_lateIsSetField?.type = coreTypes.boolRawType(Nullability.nonNullable);
}

@override
void createBodies(CoreTypes coreTypes, Expression? initializer) {
assert(_type != null, "Type has not been computed for field $name.");
Expand All @@ -924,6 +910,10 @@ abstract class AbstractLateFieldEncoding implements FieldEncoding {
}
_lateGetter.function.body = _createGetterBody(coreTypes, name, initializer)
..parent = _lateGetter.function;
// The initializer is copied from [_field] to [_lateGetter] so we copy the
// transformer flags to reflect whether the getter contains super calls.
_lateGetter.transformerFlags = _field.transformerFlags;

if (_lateSetter != null) {
_lateSetter!.function.body = _createSetterBody(
coreTypes, name, _lateSetter!.function.positionalParameters.first)
Expand Down Expand Up @@ -1099,7 +1089,9 @@ abstract class AbstractLateFieldEncoding implements FieldEncoding {
_lateIsSetField!
..isStatic = !isInstanceMember
..isStatic = _field.isStatic
..isExtensionMember = isExtensionMember;
..isExtensionMember = isExtensionMember
..type = libraryBuilder.loader
.createCoreType('bool', Nullability.nonNullable);
updatePrivateMemberName(_lateIsSetField!, libraryBuilder);
}
_lateGetter
Expand Down Expand Up @@ -1721,9 +1713,6 @@ class AbstractOrExternalFieldEncoding implements FieldEncoding {
}
}

@override
void completeSignature(CoreTypes coreTypes) {}

@override
void createBodies(CoreTypes coreTypes, Expression? initializer) {
//assert(initializer != null);
Expand Down
40 changes: 35 additions & 5 deletions pkg/front_end/lib/src/fasta/source/source_library_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1300,6 +1300,7 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
.addAll(part.library.problemsAsJson!);
}
part.collectInferableTypes(_inferableTypes!);
part.takeMixinApplications(_mixinApplications!);
if (library != part.library) {
// Mark the part library as synthetic as it's not an actual library
// (anymore).
Expand Down Expand Up @@ -1863,12 +1864,12 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
if (declaration.declaresConstConstructor) {
modifiers |= declaresConstConstructorMask;
}
ClassBuilder classBuilder = new SourceClassBuilder(
SourceClassBuilder classBuilder = new SourceClassBuilder(
metadata,
modifiers,
className,
typeVariables,
applyMixins(supertype, mixins, startOffset, nameOffset, endOffset,
_applyMixins(supertype, mixins, startOffset, nameOffset, endOffset,
className, isMixinDeclaration,
typeVariables: typeVariables,
isMacro: false,
Expand Down Expand Up @@ -2137,7 +2138,7 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
getterReference: referenceFrom?.reference);
}

TypeBuilder? applyMixins(
TypeBuilder? _applyMixins(
TypeBuilder? supertype,
MixinApplicationBuilder? mixinApplications,
int startCharOffset,
Expand Down Expand Up @@ -2397,13 +2398,42 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
applicationTypeArguments, charOffset,
instanceTypeVariableAccess:
InstanceTypeVariableAccessState.Allowed);
registerMixinApplication(application, mixin);
}
return supertype;
} else {
return supertype;
}
}

Map<SourceClassBuilder, TypeBuilder>? _mixinApplications = {};

/// Registers that [mixinApplication] is a mixin application introduced by
/// the [mixedInType] in a with-clause.
///
/// This is used to check that super access in mixin declarations have a
/// concrete target.
void registerMixinApplication(
SourceClassBuilder mixinApplication, TypeBuilder mixedInType) {
assert(
_mixinApplications != null, "Late registration of mixin application.");
_mixinApplications![mixinApplication] = mixedInType;
}

void takeMixinApplications(
Map<SourceClassBuilder, TypeBuilder> mixinApplications) {
assert(_mixinApplications != null,
"Mixin applications have already been processed.");
mixinApplications.addAll(_mixinApplications!);
_mixinApplications = null;
Iterable<SourceLibraryBuilder>? patchLibraries = this.patchLibraries;
if (patchLibraries != null) {
for (SourceLibraryBuilder patchLibrary in patchLibraries) {
patchLibrary.takeMixinApplications(mixinApplications);
}
}
}

void addNamedMixinApplication(
List<MetadataBuilder>? metadata,
String name,
Expand All @@ -2420,7 +2450,7 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
// Nested declaration began in `OutlineBuilder.beginNamedMixinApplication`.
endNestedDeclaration(TypeParameterScopeKind.namedMixinApplication, name)
.resolveNamedTypes(typeVariables, this);
supertype = applyMixins(supertype, mixinApplication, startCharOffset,
supertype = _applyMixins(supertype, mixinApplication, startCharOffset,
charOffset, charEndOffset, name, false,
metadata: metadata,
name: name,
Expand Down Expand Up @@ -2876,7 +2906,7 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
metadata,
name,
typeVariables,
applyMixins(
_applyMixins(
loader.target.underscoreEnumType,
supertypeBuilder,
startCharOffset,
Expand Down
Loading

0 comments on commit e18977e

Please sign in to comment.