From 6f060282f4c98fbc7ce77cc9bdbdc6553e8d42e4 Mon Sep 17 00:00:00 2001 From: Asger Feldthaus Date: Thu, 28 Apr 2016 12:28:32 +0200 Subject: [PATCH] [kernel] Support external constant factories such as String.fromEnvironment. Also fixes a bug in translation of factory calls. --- pkg/kernel/binary.md | 2 +- .../lib/analyzer/ast_from_analyzer.dart | 50 ++++++++++++------- pkg/kernel/lib/analyzer/loader.dart | 3 +- pkg/kernel/lib/ast.dart | 26 +++++++--- 4 files changed, 54 insertions(+), 27 deletions(-) diff --git a/pkg/kernel/binary.md b/pkg/kernel/binary.md index 40109fbd860..a9083de3345 100644 --- a/pkg/kernel/binary.md +++ b/pkg/kernel/binary.md @@ -225,7 +225,7 @@ enum ProcedureKind { type Procedure extends Member { Byte tag = 6; Byte kind; // Index into the ProcedureKind enum above. - Byte flags (isStatic, isAbstract, isExternal); + Byte flags (isStatic, isAbstract, isExternal, isConst); Name name; // Can only be absent if abstract, but tag is there anyway. Option function; diff --git a/pkg/kernel/lib/analyzer/ast_from_analyzer.dart b/pkg/kernel/lib/analyzer/ast_from_analyzer.dart index e58683dd902..a8ba188e09f 100644 --- a/pkg/kernel/lib/analyzer/ast_from_analyzer.dart +++ b/pkg/kernel/lib/analyzer/ast_from_analyzer.dart @@ -122,7 +122,7 @@ class ReferenceScope { } bool supportsConstructorCall(Element element) { - return element is ConstructorElement; + return element is ConstructorElement && !element.isFactory; } Element desynthesizeGetter(Element element) { @@ -1123,7 +1123,7 @@ class ExpressionBuilder } } - ast.Constructor resolveEffectiveTarget(ConstructorElement element) { + ConstructorElement resolveEffectiveTarget(ConstructorElement element) { ConstructorElement anchor = null; int anchorLifetime = 1; while (true) { @@ -1134,7 +1134,7 @@ class ExpressionBuilder return null; } if (node.redirectedConstructor == null) { - return scope.resolveConstructor(node.element); + return node.element; } element = node.redirectedConstructor.staticElement; if (element == null) return null; // Unresolved. @@ -1158,20 +1158,32 @@ class ExpressionBuilder var arguments = buildArguments(node.argumentList, explicitTypeArguments: type.typeArguments, inferTypeArguments: inferTypeArguments); - var target = node.staticElement; - if (target is ConstructorElement && target.isFactory) { + var element = node.staticElement; + if (element is ConstructorElement && element.isFactory) { if (node.isConst) { - ast.Constructor constructor = resolveEffectiveTarget(target); - if (constructor == null || - !constructor.isConst || - target.enclosingElement.isAbstract) { - // TODO: Preserve enough information to throw the right exception. + // Constant factory calls are resolved to their effective targets. + element = resolveEffectiveTarget(element); + // TODO: Preserve enough information to throw the right exception. + if (element == null) { + return new ast.InvalidExpression(); + } + if (element.isExternal && element.isConst && element.isFactory) { + // TODO: Keep track of the fact that the call site was a 'const' call. + ast.Member target = scope.resolveMethod(element); + return target is ast.Procedure + ? new ast.StaticInvocation(target, arguments) + : new ast.InvalidExpression(); + } else if (element.isConst && !element.enclosingElement.isAbstract) { + ast.Constructor target = scope.resolveConstructor(element); + return target != null + ? new ast.ConstructorInvocation(target, arguments, isConst: true) + : new ast.InvalidExpression(); + } else { return new ast.InvalidExpression(); } - return new ast.ConstructorInvocation(constructor, arguments, - isConst: true); } else { - var procedure = scope.resolveMethod(target); + // Non-constant call to factory procedure. + var procedure = scope.resolveMethod(element); if (procedure == null) { // TODO: Preserve enough information to throw the right exception. return new ast.InvalidExpression(); @@ -1179,10 +1191,11 @@ class ExpressionBuilder return new ast.StaticInvocation(procedure, arguments); } } else { + // Ordinary constructor call. var constructor = scope.resolveConstructor(node.staticElement); if (constructor == null || (node.isConst && !constructor.isConst) || - target.enclosingElement.isAbstract) { + element.enclosingElement.isAbstract) { // TODO: Preserve enough information to throw the right exception. return new ast.InvalidExpression(); } @@ -1707,9 +1720,9 @@ class ClassBodyBuilder extends GeneralizingAstVisitor { // separate mixin classes. bool isRootClass = node.element.supertype == null; if (!isRootClass) { - ast.DartType superClass = scope - .buildOptionalTypeAnnotation(node.extendsClause?.superclass) ?? - new ast.InterfaceType(scope.getRootClassReference()); + ast.DartType superClass = + scope.buildOptionalTypeAnnotation(node.extendsClause?.superclass) ?? + new ast.InterfaceType(scope.getRootClassReference()); if (superClass is! ast.InterfaceType) { // TODO: Handle the error case where the super class is InvalidType. log.warning('Unresolved type super type ' @@ -1800,7 +1813,8 @@ class ClassBodyBuilder extends GeneralizingAstVisitor { addTypeParameterBounds(node.typeParameters); var baseType = scope.buildTypeAnnotation(node.superclass); var mixins = node.withClause.mixinTypes.map(scope.buildTypeAnnotation); - classNode.superType = buildMixinType(baseType, mixins.take(mixins.length - 1)); + classNode.superType = + buildMixinType(baseType, mixins.take(mixins.length - 1)); classNode.mixedInType = mixins.last; addImplementedClasses(node.implementsClause); ClassElement element = node.element; diff --git a/pkg/kernel/lib/analyzer/loader.dart b/pkg/kernel/lib/analyzer/loader.dart index 7dd4b2ffbcc..327e8b1683b 100644 --- a/pkg/kernel/lib/analyzer/loader.dart +++ b/pkg/kernel/lib/analyzer/loader.dart @@ -164,7 +164,8 @@ class AnalyzerLoader implements ReferenceLevelLoader { _nameOfMember(constructor), ast.ProcedureKind.Factory, null, isAbstract: false, isStatic: true, - isExternal: constructor.isExternal); + isExternal: constructor.isExternal, + isConst: constructor.isConst); } return new ast.Constructor(null, name: _nameOfMember(element), diff --git a/pkg/kernel/lib/ast.dart b/pkg/kernel/lib/ast.dart index 2126460b00f..86f7c37281d 100644 --- a/pkg/kernel/lib/ast.dart +++ b/pkg/kernel/lib/ast.dart @@ -517,7 +517,9 @@ class Field extends Member { class Constructor extends Member { int flags = 0; - /// Cosmetic name of the constructor. + /// Name of the constructor. + /// + /// For non-external constructors, the name is cosmetic. /// /// For unnamed constructors, this is the empty string (in a [Name]). Name name; @@ -597,24 +599,30 @@ class Procedure extends Member { FunctionNode function; // Body is null if and only if abstract or external. Procedure(this.name, this.kind, this.function, - {bool isAbstract: false, bool isStatic: false, bool isExternal: false}) { + {bool isAbstract: false, + bool isStatic: false, + bool isExternal: false, + bool isConst: false}) { function?.parent = this; this.isAbstract = isAbstract; this.isStatic = isStatic; this.isExternal = isExternal; + this.isConst = isConst; } - Procedure.abstract_(this.name, this.kind) - : function = null, - flags = FlagAbstract; static const int FlagStatic = 1 << 0; // Must match serialized bit positions. static const int FlagAbstract = 1 << 1; static const int FlagExternal = 1 << 2; + static const int FlagConst = 1 << 3; // Only for external const factories. bool get isStatic => flags & FlagStatic != 0; bool get isAbstract => flags & FlagAbstract != 0; bool get isExternal => flags & FlagExternal != 0; + /// True if this has the `const` modifier. This is only possible for external + /// constant factories, such as `String.fromEnvironment`. + bool get isConst => flags & FlagConst != 0; + void set isStatic(bool value) { flags = value ? (flags | FlagStatic) : (flags & ~FlagStatic); } @@ -627,6 +635,10 @@ class Procedure extends Member { flags = value ? (flags | FlagExternal) : (flags & ~FlagExternal); } + void set isConst(bool value) { + flags = value ? (flags | FlagConst) : (flags & ~FlagConst); + } + accept(MemberVisitor v) => v.visitProcedure(this); acceptReference(MemberReferenceVisitor v) => v.visitProcedureReference(this); @@ -1103,7 +1115,7 @@ abstract class InvocationExpression extends Expression { /// Name of the invoked method. /// - /// May be `null` if the target is synthetic static member without a name. + /// May be `null` if the target is a synthetic static member without a name. Name get name; } @@ -1172,7 +1184,7 @@ class SuperMethodInvocation extends InvocationExpression { /// /// The provided arguments might not match the parameters of the target. class StaticInvocation extends InvocationExpression { - Procedure target; // Static method. + Procedure target; Arguments arguments; Name get name => target?.name;