Skip to content

Commit 0496569

Browse files
author
Jenny Messerly
committed
implement super mixins in dartdevc and fix a few issues in Analyzer
Fixes #34167. This implements the Dart 2 mixin proposal (https://goo.gl/KEKQyv) for DDC. When the mixin is applied, a class is created for the application that extends the correct superclass and has all of the instance members, so `super` works correctly. This also fixes a few minor issues in Analyzer's (mostly complete) implementation: - InterfaceType.isObject now returns false for Dart 2 mixins. - Least upper bound calculation recognizes mixins are not Object. - Interface of the mixin now implements its superclass constraints. - Mixin superclass constraints are checked against the superclass and all previously applied mixins (if any); this keeps it working with the subtype fix above, and also prevents a not-yet-applied mixin from satisfying the constraint The language_2/mixin_declaration tests were updated with a few minor fixes now that we can run Analyzer/dartdevc to test them. This change implements super mixins for DDC's Kernel backend (DDK) too. This will be enabled once Kernel adds a flag to recognize which Class nodes are mixins (vs normal classes). Change-Id: Ib3c4fcb12de9988345e52d92931196828d8227c3 Reviewed-on: https://dart-review.googlesource.com/74965 Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Vijay Menon <[email protected]>
1 parent 0471d7e commit 0496569

24 files changed

+363
-144
lines changed

‎pkg/analyzer/lib/src/dart/element/element.dart

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -626,8 +626,8 @@ class ClassElementImpl extends AbstractClassElementImpl
626626
return false;
627627
}
628628
if (supertype == null) {
629-
// Should never happen, since Object is the only class that has no
630-
// supertype, and it should have been caught by the test above.
629+
// Should never happen, since Object and mixins are the only classes that
630+
// have no supertype, and they should have been caught by the test above.
631631
assert(false);
632632
return false;
633633
}
@@ -647,8 +647,8 @@ class ClassElementImpl extends AbstractClassElementImpl
647647
}
648648
classesSeen.add(nearestNonMixinClass);
649649
if (nearestNonMixinClass.supertype == null) {
650-
// Should never happen, since Object is the only class that has no
651-
// supertype, and it is not a mixin application.
650+
// Should never happen, since Object and mixins are the only classes that
651+
// have no supertype, and they are not mixin applications.
652652
assert(false);
653653
return false;
654654
}
@@ -1106,9 +1106,9 @@ class ClassElementImpl extends AbstractClassElementImpl
11061106
// forwarded to this class.
11071107
Iterable<ConstructorElement> constructorsToForward;
11081108
if (supertype == null) {
1109-
// Shouldn't ever happen, since the only class with no supertype is
1110-
// Object, and it isn't a mixin application. But for safety's sake just
1111-
// assume an empty list.
1109+
// Shouldn't ever happen, since the only classes with no supertype are
1110+
// Object and mixins, and they aren't a mixin application. But for
1111+
// safety's sake just assume an empty list.
11121112
assert(false);
11131113
constructorsToForward = <ConstructorElement>[];
11141114
} else if (!supertype.element.isMixinApplication) {

‎pkg/analyzer/lib/src/dart/element/type.dart

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1339,7 +1339,7 @@ class InterfaceTypeImpl extends TypeImpl implements InterfaceType {
13391339
}
13401340

13411341
@override
1342-
bool get isObject => element.supertype == null;
1342+
bool get isObject => element.supertype == null && !element.isMixin;
13431343

13441344
@override
13451345
List<MethodElement> get methods {
@@ -1495,10 +1495,9 @@ class InterfaceTypeImpl extends TypeImpl implements InterfaceType {
14951495
ClassElement jElement = j.element;
14961496
InterfaceType supertype = jElement.supertype;
14971497
//
1498-
// If J has no direct supertype then it is Object, and Object has no direct
1499-
// supertypes.
1498+
// If J is Object, then it has no direct supertypes.
15001499
//
1501-
if (supertype == null) {
1500+
if (j.isObject) {
15021501
return false;
15031502
}
15041503
//
@@ -2212,7 +2211,7 @@ class InterfaceTypeImpl extends TypeImpl implements InterfaceType {
22122211
InterfaceType type, int depth, HashSet<ClassElement> visitedTypes) {
22132212
ClassElement classElement = type.element;
22142213
// Object case
2215-
if (classElement.supertype == null || visitedTypes.contains(classElement)) {
2214+
if (type.isObject || visitedTypes.contains(classElement)) {
22162215
return depth;
22172216
}
22182217
int longestPath = 1;
@@ -2244,10 +2243,12 @@ class InterfaceTypeImpl extends TypeImpl implements InterfaceType {
22442243
// TODO(brianwilkerson) Does this also need to add in the number of mixin
22452244
// classes?
22462245
InterfaceType supertype = classElement.supertype;
2247-
pathLength = _computeLongestInheritancePathToObject(
2248-
supertype, depth + 1, visitedTypes);
2249-
if (pathLength > longestPath) {
2250-
longestPath = pathLength;
2246+
if (supertype != null) {
2247+
pathLength = _computeLongestInheritancePathToObject(
2248+
supertype, depth + 1, visitedTypes);
2249+
if (pathLength > longestPath) {
2250+
longestPath = pathLength;
2251+
}
22512252
}
22522253
} finally {
22532254
visitedTypes.remove(classElement);

‎pkg/analyzer/lib/src/dart/resolver/inheritance_manager.dart

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ class InheritanceManager {
283283
}
284284
InterfaceType supertype = classElt.supertype;
285285
if (supertype == null) {
286-
// classElt is Object
286+
// classElt is Object or a mixin
287287
_classLookup[classElt] = resultMap;
288288
return resultMap;
289289
}
@@ -380,9 +380,8 @@ class InheritanceManager {
380380
// functionality in InheritanceManagerTest
381381
chain.add(currentType);
382382
ClassElement classElt = currentType.element;
383-
InterfaceType supertype = classElt.supertype;
384383
// Base case- reached Object
385-
if (supertype == null) {
384+
if (currentType.isObject) {
386385
// Looked up the chain all the way to Object, return null.
387386
// This should never happen.
388387
return;
@@ -411,8 +410,9 @@ class InheritanceManager {
411410
}
412411
}
413412
// Superclass
414-
ClassElement superclassElt = supertype.element;
415-
if (lookupMember(superclassElt, memberName) != null) {
413+
InterfaceType supertype = classElt.supertype;
414+
if (supertype != null &&
415+
lookupMember(supertype.element, memberName) != null) {
416416
_computeInheritancePath(chain, supertype, memberName);
417417
return;
418418
}

‎pkg/analyzer/lib/src/generated/element_resolver.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -719,8 +719,9 @@ class ElementResolver extends SimpleAstVisitor<Object> {
719719
// Generate the type name.
720720
// The error code will never be generated via type propagation
721721
DartType getSuperType(DartType type) {
722-
if (type is InterfaceType && !type.isObject) {
723-
return type.superclass;
722+
if (type is InterfaceType) {
723+
InterfaceType superclass = type.superclass;
724+
if (superclass != null) return superclass;
724725
}
725726
return type;
726727
}

‎pkg/analyzer/lib/src/generated/error_verifier.dart

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1910,7 +1910,9 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
19101910
return false;
19111911
}
19121912
bool problemReported = false;
1913-
for (TypeName mixinName in withClause.mixinTypes) {
1913+
List<TypeName> mixinTypes = withClause.mixinTypes;
1914+
for (int i = 0; i < mixinTypes.length; i++) {
1915+
TypeName mixinName = mixinTypes[i];
19141916
DartType mixinType = mixinName.type;
19151917
if (mixinType is InterfaceType) {
19161918
if (_checkForExtendsOrImplementsDisallowedClass(
@@ -1923,7 +1925,8 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
19231925
problemReported = true;
19241926
}
19251927
if (mixinElement.isMixin) {
1926-
if (_checkForMixinSuperclassConstraints(mixinName)) {
1928+
if (_checkForMixinSuperclassConstraints(
1929+
mixinName, mixinTypes.take(i))) {
19271930
problemReported = true;
19281931
}
19291932
if (_checkForMixinSuperInvokedMembers(mixinName, mixinElement)) {
@@ -4256,12 +4259,14 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
42564259
}
42574260

42584261
/// Check that superclass constrains for the mixin type of [mixinName]
4259-
/// are satisfied by the [_enclosingClass].
4260-
bool _checkForMixinSuperclassConstraints(TypeName mixinName) {
4261-
InterfaceType enclosingType = _enclosingClass.type;
4262+
/// are satisfied by the superclass and/or any previous mixin applications.
4263+
bool _checkForMixinSuperclassConstraints(
4264+
TypeName mixinName, Iterable<TypeName> previousMixins) {
4265+
List<InterfaceType> supertypes = [_enclosingClass.supertype];
4266+
supertypes.addAll(previousMixins.map((t) => t.type));
42624267
InterfaceType mixinType = mixinName.type;
42634268
for (var constraint in mixinType.superclassConstraints) {
4264-
if (!_typeSystem.isSubtypeOf(enclosingType, constraint)) {
4269+
if (!supertypes.any((s) => _typeSystem.isSubtypeOf(s, constraint))) {
42654270
_errorReporter.reportErrorForNode(
42664271
CompileTimeErrorCode.MIXIN_APPLICATION_NOT_IMPLEMENTED_INTERFACE,
42674272
mixinName.name,

‎pkg/analyzer/lib/src/generated/type_system.dart

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1582,7 +1582,9 @@ class StrongTypeSystemImpl extends TypeSystem {
15821582
visitedTypes ??= new HashSet<ClassElement>();
15831583
if (!visitedTypes.add(i1Element)) return false;
15841584

1585-
if (_isInterfaceSubtypeOf(i1.superclass, i2, visitedTypes)) {
1585+
InterfaceType superclass = i1.superclass;
1586+
if (superclass != null &&
1587+
_isInterfaceSubtypeOf(superclass, i2, visitedTypes)) {
15861588
return true;
15871589
}
15881590

@@ -1598,6 +1600,14 @@ class StrongTypeSystemImpl extends TypeSystem {
15981600
}
15991601
}
16001602

1603+
if (i1Element.isMixin) {
1604+
for (final parent in i1.superclassConstraints) {
1605+
if (_isInterfaceSubtypeOf(parent, i2, visitedTypes)) {
1606+
return true;
1607+
}
1608+
}
1609+
}
1610+
16011611
return false;
16021612
}
16031613

‎pkg/dev_compiler/lib/src/analyzer/code_generator.dart

Lines changed: 95 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -971,7 +971,9 @@ class CodeGenerator extends Object
971971
_emitVirtualFieldSymbols(classElem, body);
972972
_emitClassSignature(classElem, className, memberMap, body);
973973
_initExtensionSymbols(classElem);
974-
_defineExtensionMembers(className, body);
974+
if (!classElem.isMixin) {
975+
_defineExtensionMembers(className, body);
976+
}
975977
_emitClassMetadata(classNode.metadata, className, body);
976978

977979
var classDef = JS.Statement.from(body);
@@ -1233,7 +1235,7 @@ class CodeGenerator extends Object
12331235

12341236
@override
12351237
JS.Statement visitMixinDeclaration(MixinDeclaration node) {
1236-
throw new UnimplementedError();
1238+
return _emitClassDeclaration(node, node.declaredElement, node.members);
12371239
}
12381240

12391241
/// Wraps a possibly generic class in its type arguments.
@@ -1275,6 +1277,70 @@ class CodeGenerator extends Object
12751277
return js.statement('# = #;', [className, classExpr]);
12761278
}
12771279

1280+
/// Like [_emitClassStatement] but emits a Dart 2.1 mixin represented by
1281+
/// [classElem].
1282+
///
1283+
/// Mixins work similar to normal classes, but their instance methods close
1284+
/// over the actual superclass. Given a Dart class like:
1285+
///
1286+
/// mixin M on C {
1287+
/// foo() => super.foo() + 42;
1288+
/// }
1289+
///
1290+
/// We generate a JS class like this:
1291+
///
1292+
/// lib.M = class M extends core.Object {}
1293+
/// lib.M[dart.mixinOn] = (C) => class M extends C {
1294+
/// foo() {
1295+
/// return super.foo() + 42;
1296+
/// }
1297+
/// };
1298+
///
1299+
/// The special `dart.mixinOn` symbolized property is used by the runtime
1300+
/// helper `dart.applyMixin`. The helper calls the function with the actual
1301+
/// base class, and then copies the resulting members to the destination
1302+
/// class.
1303+
///
1304+
/// In the long run we may be able to improve this so we do not have the
1305+
/// unnecessary class, but for now, this lets us get the right semantics with
1306+
/// minimal compiler and runtime changes.
1307+
void _emitMixinStatement(
1308+
ClassElement classElem,
1309+
JS.Expression className,
1310+
JS.Expression heritage,
1311+
List<JS.Method> methods,
1312+
List<JS.Statement> body) {
1313+
assert(classElem.isMixin);
1314+
1315+
var staticMethods = methods.where((m) => m.isStatic).toList();
1316+
var instanceMethods = methods.where((m) => !m.isStatic).toList();
1317+
body.add(
1318+
_emitClassStatement(classElem, className, heritage, staticMethods));
1319+
1320+
var superclassId = JS.TemporaryId(
1321+
classElem.superclassConstraints.map((t) => t.name).join('_'));
1322+
var classId =
1323+
className is JS.Identifier ? className : JS.TemporaryId(classElem.name);
1324+
1325+
var mixinMemberClass =
1326+
JS.ClassExpression(classId, superclassId, instanceMethods);
1327+
1328+
JS.Node arrowFnBody = mixinMemberClass;
1329+
var extensionInit = <JS.Statement>[];
1330+
_defineExtensionMembers(classId, extensionInit);
1331+
if (extensionInit.isNotEmpty) {
1332+
extensionInit.insert(0, mixinMemberClass.toStatement());
1333+
extensionInit.add(classId.toReturn());
1334+
arrowFnBody = JS.Block(extensionInit);
1335+
}
1336+
1337+
body.add(js.statement('#[#.mixinOn] = #', [
1338+
className,
1339+
runtimeModule,
1340+
JS.ArrowFun([superclassId], arrowFnBody)
1341+
]));
1342+
}
1343+
12781344
void _defineClass(
12791345
ClassElement classElem,
12801346
JS.Expression className,
@@ -1305,7 +1371,9 @@ class CodeGenerator extends Object
13051371
if (t.typeArguments.any(defer)) return true;
13061372
if (t is InterfaceType) {
13071373
var e = t.element;
1308-
return e.mixins.any(defer) || defer(e.supertype);
1374+
if (e.mixins.any(defer)) return true;
1375+
var supertype = e.supertype;
1376+
return supertype != null && defer(supertype);
13091377
}
13101378
}
13111379
return false;
@@ -1328,7 +1396,7 @@ class CodeGenerator extends Object
13281396
return base;
13291397
}
13301398

1331-
var supertype = classElem.supertype;
1399+
var supertype = classElem.isMixin ? types.objectType : classElem.supertype;
13321400
var hasUnnamedSuper = _hasUnnamedConstructor(supertype.element);
13331401

13341402
void emitMixinConstructors(JS.Expression className, [InterfaceType mixin]) {
@@ -1385,7 +1453,7 @@ class CodeGenerator extends Object
13851453
var classExpr = deferMixin ? getBaseClass(0) : className;
13861454

13871455
mixinBody
1388-
.add(runtimeStatement('mixinMembers(#, #)', [classExpr, mixinClass]));
1456+
.add(runtimeStatement('applyMixin(#, #)', [classExpr, mixinClass]));
13891457

13901458
_topLevelClass = savedTopLevel;
13911459

@@ -1395,8 +1463,8 @@ class CodeGenerator extends Object
13951463
//
13961464
// We do this with the following pattern:
13971465
//
1398-
// mixinMembers(C, class C$ extends M { <methods> });
1399-
mixinBody.add(runtimeStatement('mixinMembers(#, #)', [
1466+
// applyMixin(C, class C$ extends M { <methods> });
1467+
mixinBody.add(runtimeStatement('applyMixin(#, #)', [
14001468
classExpr,
14011469
JS.ClassExpression(
14021470
JS.TemporaryId(classElem.name), mixinClass, methods)
@@ -1428,19 +1496,26 @@ class CodeGenerator extends Object
14281496
hasUnnamedSuper = hasUnnamedSuper || _hasUnnamedConstructor(m.element);
14291497

14301498
if (shouldDefer(m)) {
1431-
deferredSupertypes.add(runtimeStatement('mixinMembers(#, #)',
1499+
deferredSupertypes.add(runtimeStatement('applyMixin(#, #)',
14321500
[getBaseClass(mixinLength - i), emitDeferredType(m)]));
14331501
} else {
14341502
body.add(
1435-
runtimeStatement('mixinMembers(#, #)', [mixinId, emitClassRef(m)]));
1503+
runtimeStatement('applyMixin(#, #)', [mixinId, emitClassRef(m)]));
14361504
}
14371505

14381506
baseClass = mixinId;
14391507
}
14401508

14411509
_topLevelClass = savedTopLevel;
14421510

1443-
body.add(_emitClassStatement(classElem, className, baseClass, methods));
1511+
if (classElem.isMixin) {
1512+
// TODO(jmesserly): we could make this more efficient, as this creates
1513+
// an extra unnecessary class. But it's the easiest way to handle the
1514+
// current system.
1515+
_emitMixinStatement(classElem, className, baseClass, methods, body);
1516+
} else {
1517+
body.add(_emitClassStatement(classElem, className, baseClass, methods));
1518+
}
14441519

14451520
if (classElem.isMixinApplication) emitMixinConstructors(className);
14461521
}
@@ -2069,9 +2144,13 @@ class CodeGenerator extends Object
20692144
/// Emit the signature on the class recording the runtime type information
20702145
void _emitClassSignature(ClassElement classElem, JS.Expression className,
20712146
Map<Element, Declaration> annotatedMembers, List<JS.Statement> body) {
2072-
if (classElem.interfaces.isNotEmpty) {
2147+
if (classElem.interfaces.isNotEmpty ||
2148+
classElem.superclassConstraints.isNotEmpty) {
2149+
var interfaces = classElem.interfaces.toList()
2150+
..addAll(classElem.superclassConstraints);
2151+
20732152
body.add(js.statement('#[#.implements] = () => [#];',
2074-
[className, runtimeModule, classElem.interfaces.map(_emitType)]));
2153+
[className, runtimeModule, interfaces.map(_emitType)]));
20752154
}
20762155

20772156
void emitSignature(String name, List<JS.Property> elements) {
@@ -2363,7 +2442,9 @@ class CodeGenerator extends Object
23632442
// Get the supertype's unnamed constructor.
23642443
superCtor ??= element.supertype?.element?.unnamedConstructor;
23652444
if (superCtor == null) {
2366-
assert(element.type.isObject || options.unsafeForceCompile);
2445+
assert(element.type.isObject ||
2446+
element.isMixin ||
2447+
options.unsafeForceCompile);
23672448
return null;
23682449
}
23692450

@@ -2384,6 +2465,7 @@ class CodeGenerator extends Object
23842465

23852466
bool _hasUnnamedSuperConstructor(ClassElement e) {
23862467
var supertype = e.supertype;
2468+
// Object or mixin declaration.
23872469
if (supertype == null) return false;
23882470
if (_hasUnnamedConstructor(supertype.element)) return true;
23892471
for (var mixin in e.mixins) {

‎pkg/dev_compiler/lib/src/analyzer/element_helpers.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ List<ClassElement> getSuperclasses(ClassElement cls) {
139139
if (mixin != null) result.add(mixin);
140140
}
141141
var supertype = cls.supertype;
142+
// Object or mixin declaration.
142143
if (supertype == null) break;
143144

144145
cls = supertype.element;

0 commit comments

Comments
 (0)