From 1e723f68d1bf0bf0c5a52d5832f36cd4a5e8b322 Mon Sep 17 00:00:00 2001 From: "sigurdm@google.com" Date: Thu, 30 Apr 2015 11:38:54 +0000 Subject: [PATCH] Allow use of deferred type-literals in non-constant contexts. BUG= dartbug.com/22893 R=johnniwinther@google.com Review URL: https://codereview.chromium.org//1109393012 git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@45459 260f80e4-7a28-3924-810f-c04153c831b5 --- .../lib/src/compile_time_constants.dart | 89 +++++++++------ .../lib/src/constants/expressions.dart | 37 ++++++ .../lib/src/dart_backend/backend.dart | 6 +- .../src/dart_backend/backend_ast_emitter.dart | 6 + pkg/compiler/lib/src/deferred_load.dart | 106 +++++++++++------- .../constant_handler_javascript.dart | 14 ++- .../src/js_backend/type_variable_handler.dart | 4 +- pkg/compiler/lib/src/resolution/members.dart | 12 +- pkg/compiler/lib/src/ssa/builder.dart | 1 + pkg/compiler/lib/src/ssa/nodes.dart | 2 + tests/language/language_dart2js.status | 2 - 11 files changed, 183 insertions(+), 96 deletions(-) diff --git a/pkg/compiler/lib/src/compile_time_constants.dart b/pkg/compiler/lib/src/compile_time_constants.dart index c16a09ceba8..a320b23f83c 100644 --- a/pkg/compiler/lib/src/compile_time_constants.dart +++ b/pkg/compiler/lib/src/compile_time_constants.dart @@ -41,14 +41,21 @@ abstract class ConstantCompiler extends ConstantEnvironment { /// if possible. void compileVariable(VariableElement element); - /// Compiles the compile-time constant for [node], or reports an error if - /// [node] is not a compile-time constant. + /// Compiles the constant for [node]. + /// + /// Reports an error if [node] is not a compile-time constant and + /// [enforceConst]. + /// + /// If `!enforceConst`, then if [node] is a "runtime constant" (for example + /// a reference to a deferred constant) it will be returned - otherwise null + /// is returned. /// /// Depending on implementation, the constant compiler might also compute - /// the compile-time constant for the backend interpretation of constants. + /// the constant for the backend interpretation of constants. /// /// The returned constant is always of the frontend interpretation. - ConstantExpression compileNode(Node node, TreeElements elements); + ConstantExpression compileNode(Node node, TreeElements elements, + {bool enforceConst: true}); /// Compiles the compile-time constant for the value [metadata], or reports an /// error if the value is not a compile-time constant. @@ -213,8 +220,9 @@ abstract class ConstantCompilerBase implements ConstantCompiler { return constant != null ? constant.expression : null; } - ConstantExpression compileNode(Node node, TreeElements elements) { - return compileNodeWithDefinitions(node, elements); + ConstantExpression compileNode(Node node, TreeElements elements, + {bool enforceConst: true}) { + return compileNodeWithDefinitions(node, elements, isConst: enforceConst); } ConstantExpression compileMetadata(MetadataAnnotation metadata, @@ -469,13 +477,11 @@ class CompileTimeConstantEvaluator extends Visitor { context, node, new SymbolConstantExpression(constant.value, text)); } - AstConstant makeTypeConstant(Node node, DartType elementType) { + ConstantExpression makeTypeConstant(DartType elementType) { DartType constantType = compiler.backend.typeImplementation.computeType(compiler); - return new AstConstant( - context, node, new TypeConstantExpression( - new TypeConstantValue(elementType, constantType), - elementType)); + return new TypeConstantExpression( + new TypeConstantValue(elementType, constantType), elementType); } /// Returns true if the prefix of the send resolves to a deferred import @@ -491,7 +497,7 @@ class CompileTimeConstantEvaluator extends Visitor { if (Elements.isClass(element) || Elements.isTypedef(element)) { TypeDeclarationElement typeDeclarationElement = element; DartType type = typeDeclarationElement.rawType; - return makeTypeConstant(node, type); + return new AstConstant(element, node, makeTypeConstant(type)); } return signalNotCompileTimeConstant(node); } @@ -500,32 +506,27 @@ class CompileTimeConstantEvaluator extends Visitor { AstConstant visitSend(Send send) { Element element = elements[send]; if (send.isPropertyAccess) { - if (isDeferredUse(send)) { - return signalNotCompileTimeConstant(send, - message: MessageKind.DEFERRED_COMPILE_TIME_CONSTANT); - } + ConstantExpression result; + if (Elements.isStaticOrTopLevelFunction(element)) { FunctionElementX function = element; function.computeType(compiler); - return new AstConstant( - context, send, new FunctionConstantExpression( - new FunctionConstantValue(function), - function)); + result = new FunctionConstantExpression( + new FunctionConstantValue(function), function); } else if (Elements.isStaticOrTopLevelField(element)) { - ConstantExpression result; + ConstantExpression elementExpression; if (element.isConst) { - result = handler.compileConstant(element); + elementExpression = handler.compileConstant(element); } else if (element.isFinal && !isEvaluatingConstant) { - result = handler.compileVariable(element); + elementExpression = handler.compileVariable(element); } - if (result != null) { - return new AstConstant( - context, send, - new VariableConstantExpression(result.value, element)); + if (elementExpression != null) { + result = + new VariableConstantExpression(elementExpression.value, element); } } else if (Elements.isClass(element) || Elements.isTypedef(element)) { assert(elements.isTypeLiteral(send)); - return makeTypeConstant(send, elements.getTypeLiteralType(send)); + result = makeTypeConstant(elements.getTypeLiteralType(send)); } else if (send.receiver != null) { if (send.selector.asIdentifier().source == "length") { AstConstant left = evaluate(send.receiver); @@ -533,22 +534,37 @@ class CompileTimeConstantEvaluator extends Visitor { StringConstantValue stringConstantValue = left.value; DartString string = stringConstantValue.primitiveValue; IntConstantValue length = constantSystem.createInt(string.length); - return new AstConstant( - context, send, new VariableConstantExpression(length, element)); + result = new VariableConstantExpression(length, element); } } // Fall through to error handling. } else if (!Elements.isUnresolved(element) && element.isVariable && element.isConst) { - ConstantExpression result = handler.compileConstant(element); - if (result != null) { - return new AstConstant( - context, send, - new VariableConstantExpression(result.value, element)); + ConstantExpression variableExpression = + handler.compileConstant(element); + if (variableExpression != null) { + result = new VariableConstantExpression(variableExpression.value, + element); } } - return signalNotCompileTimeConstant(send); + if (result == null) { + return signalNotCompileTimeConstant(send); + } + if (isDeferredUse(send)) { + if (isEvaluatingConstant) { + error(send, MessageKind.DEFERRED_COMPILE_TIME_CONSTANT); + } + PrefixElement prefix = compiler.deferredLoadTask + .deferredPrefixElement(send, elements); + result = new DeferredConstantExpression( + new DeferredConstantValue(result.value, prefix), + result, + prefix); + compiler.deferredLoadTask + .registerConstantDeferredUse(result.value, prefix); + } + return new AstConstant(context, send, result); } else if (send.isCall) { if (element == compiler.identicalFunction && send.argumentCount() == 2) { @@ -743,7 +759,6 @@ class CompileTimeConstantEvaluator extends Visitor { } concreteArgumentMap[argument] = evaluateConstant(argument); } - List normalizedArguments = evaluateArgumentsToConstructor( node, callStructure, send.arguments, constructor.implementation, diff --git a/pkg/compiler/lib/src/constants/expressions.dart b/pkg/compiler/lib/src/constants/expressions.dart index 1263ada4303..c45497581c8 100644 --- a/pkg/compiler/lib/src/constants/expressions.dart +++ b/pkg/compiler/lib/src/constants/expressions.dart @@ -10,6 +10,7 @@ import '../elements/elements.dart' show ConstructorElement, Element, FunctionElement, + PrefixElement, VariableElement; import '../resolution/operators.dart'; import '../universe/universe.dart' show CallStructure; @@ -21,6 +22,7 @@ enum ConstantExpressionKind { CONCATENATE, CONDITIONAL, CONSTRUCTED, + DEFERRED, DOUBLE, ERRONEOUS, FUNCTION, @@ -606,6 +608,33 @@ class ConditionalConstantExpression extends ConstantExpression { } } +/// A constant expression referenced with a deferred prefix. +/// For example `lib.C`. +class DeferredConstantExpression extends ConstantExpression { + final ConstantValue value; + final ConstantExpression expression; + final PrefixElement prefix; + + DeferredConstantExpression(this.value, this.expression, this.prefix); + + ConstantExpressionKind get kind => ConstantExpressionKind.DEFERRED; + + @override + int _computeHashCode() { + return 13 * expression.hashCode; + } + + @override + bool _equals(DeferredConstantExpression other) { + return expression == other.expression; + } + + @override + accept(ConstantExpressionVisitor visitor, [context]) { + return visitor.visitDeferred(this, context); + } +} + abstract class ConstantExpressionVisitor { const ConstantExpressionVisitor(); @@ -626,6 +655,7 @@ abstract class ConstantExpressionVisitor { R visitIdentical(IdenticalConstantExpression exp, C context); R visitUnary(UnaryConstantExpression exp, C context); R visitConditional(ConditionalConstantExpression exp, C context); + R visitDeferred(DeferredConstantExpression exp, C context); } /// Represents the declaration of a constant [element] with value [expression]. @@ -808,5 +838,12 @@ class ConstExpPrinter extends ConstantExpressionVisitor { write(exp, exp.falseExp); } + @override + visitDeferred(DeferredConstantExpression exp, context) { + sb.write(exp.prefix.deferredImport.prefix.source); + sb.write('.'); + write(exp, exp.expression); + } + String toString() => sb.toString(); } \ No newline at end of file diff --git a/pkg/compiler/lib/src/dart_backend/backend.dart b/pkg/compiler/lib/src/dart_backend/backend.dart index 763e2e3c95f..90ae82eebdc 100644 --- a/pkg/compiler/lib/src/dart_backend/backend.dart +++ b/pkg/compiler/lib/src/dart_backend/backend.dart @@ -521,9 +521,11 @@ class DartConstantTask extends ConstantCompilerTask }); } - ConstantExpression compileNode(Node node, TreeElements elements) { + ConstantExpression compileNode(Node node, TreeElements elements, + {bool enforceConst: true}) { return measure(() { - return constantCompiler.compileNodeWithDefinitions(node, elements); + return constantCompiler.compileNodeWithDefinitions(node, elements, + isConst: enforceConst); }); } diff --git a/pkg/compiler/lib/src/dart_backend/backend_ast_emitter.dart b/pkg/compiler/lib/src/dart_backend/backend_ast_emitter.dart index beb05de1da9..a578d2efb1f 100644 --- a/pkg/compiler/lib/src/dart_backend/backend_ast_emitter.dart +++ b/pkg/compiler/lib/src/dart_backend/backend_ast_emitter.dart @@ -1279,6 +1279,12 @@ class ConstantEmitter BuilderContext context) { return handlePrimitiveConstant(exp.value); } + + @override + Expression visitDeferred(DeferredConstantExpression exp, + BuilderContext context) { + return exp.expression.accept(this); + } } /// Moves function parameters into a separate variable if one of its uses is diff --git a/pkg/compiler/lib/src/deferred_load.dart b/pkg/compiler/lib/src/deferred_load.dart index b0192972f82..b4ef154f4a0 100644 --- a/pkg/compiler/lib/src/deferred_load.dart +++ b/pkg/compiler/lib/src/deferred_load.dart @@ -194,7 +194,6 @@ class DeferredLoadTask extends CompilerTask { /// Returns the [OutputUnit] where [constant] belongs. OutputUnit outputUnitForConstant(ConstantValue constant) { if (!isProgramSplit) return mainOutputUnit; - return _constantToOutputUnit[constant]; } @@ -249,16 +248,6 @@ class DeferredLoadTask extends CompilerTask { Set constants, isMirrorUsage) { - /// Recursively add the constant and its dependencies to [constants]. - void addConstants(ConstantValue constant) { - if (constants.contains(constant)) return; - constants.add(constant); - if (constant is ConstructedConstantValue) { - elements.add(constant.type.element); - } - constant.getDependencies().forEach(addConstants); - } - /// Collects all direct dependencies of [element]. /// /// The collected dependent elements and constants are are added to @@ -289,7 +278,7 @@ class DeferredLoadTask extends CompilerTask { } treeElements.forEachConstantNode((Node node, _) { // Explicitly depend on the backend constants. - addConstants( + constants.add( backend.constants.getConstantForNode(node, treeElements).value); }); elements.addAll(treeElements.otherDependencies); @@ -300,7 +289,7 @@ class DeferredLoadTask extends CompilerTask { ConstantExpression constant = backend.constants.getConstantForMetadata(metadata); if (constant != null) { - addConstants(constant.value); + constants.add(constant.value); } } @@ -401,40 +390,70 @@ class DeferredLoadTask extends CompilerTask { return result; } - /// Recursively traverses the graph of dependencies from [element], mapping - /// deferred imports to each dependency it needs in the sets - /// [_importedDeferredBy] and [_constantsDeferredBy]. - void _mapDependencies(Element element, Import import, - {isMirrorUsage: false}) { - Set elements = _importedDeferredBy.putIfAbsent(import, - () => new Set()); + /// Add all dependencies of [constant] to the mapping of [import]. + void _mapConstantDependencies(ConstantValue constant, Import import) { Set constants = _constantsDeferredBy.putIfAbsent(import, () => new Set()); + if (constants.contains(constant)) return; + constants.add(constant); + if (constant is ConstructedConstantValue) { + _mapDependencies(element: constant.type.element, import: import); + } + constant.getDependencies().forEach((ConstantValue dependency) { + _mapConstantDependencies(dependency, import); + }); + } - // Only process elements once, unless we are doing dependencies due to - // mirrors, which are added in additional traversals. - if (!isMirrorUsage && elements.contains(element)) return; - // Anything used directly by main will be loaded from the start - // We do not need to traverse it again. - if (import != _fakeMainImport && _mainElements.contains(element)) return; + /// Recursively traverses the graph of dependencies from one of [element] + /// or [constant], mapping deferred imports to each dependency it needs in the + /// sets [_importedDeferredBy] and [_constantsDeferredBy]. + /// Only one of [element] and [constant] should be given. + void _mapDependencies({Element element, + Import import, + isMirrorUsage: false}) { + + Set elements = _importedDeferredBy.putIfAbsent(import, + () => new Set()); - // Here we modify [_importedDeferredBy]. - elements.add(element); Set dependentElements = new Set(); + Set dependentConstants = new Set(); - // This call can modify [_importedDeferredBy] and [_constantsDeferredBy]. - _collectAllElementsAndConstantsResolvedFrom( - element, dependentElements, constants, isMirrorUsage); + LibraryElement library; + + if (element != null) { + // Only process elements once, unless we are doing dependencies due to + // mirrors, which are added in additional traversals. + if (!isMirrorUsage && elements.contains(element)) return; + // Anything used directly by main will be loaded from the start + // We do not need to traverse it again. + if (import != _fakeMainImport && _mainElements.contains(element)) return; + elements.add(element); + + + // This call can modify [dependentElements] and [dependentConstants]. + _collectAllElementsAndConstantsResolvedFrom( + element, dependentElements, dependentConstants, isMirrorUsage); + + library = element.library; + } - LibraryElement library = element.library; for (Element dependency in dependentElements) { if (_isExplicitlyDeferred(dependency, library)) { for (Import deferredImport in _getImports(dependency, library)) { - _mapDependencies(dependency, deferredImport); - }; + _mapDependencies(element: dependency, import: deferredImport); + } } else { - _mapDependencies(dependency, import); + _mapDependencies(element: dependency, import: import); + } + } + + for (ConstantValue dependency in dependentConstants) { + if (dependency is DeferredConstantValue) { + _mapConstantDependencies(dependency, + dependency.prefix.deferredImport); + } else { + _mapConstantDependencies(dependency, import); } } } @@ -451,7 +470,8 @@ class DeferredLoadTask extends CompilerTask { // So we have to filter them out here. if (element is AnalyzableElementX && !element.hasTreeElements) return; if (compiler.backend.isAccessibleByReflection(element)) { - _mapDependencies(element, deferredImport, isMirrorUsage: true); + _mapDependencies(element: element, import: deferredImport, + isMirrorUsage: true); } } @@ -466,8 +486,8 @@ class DeferredLoadTask extends CompilerTask { ConstantExpression constant = backend.constants.getConstantForMetadata(metadata); if (constant != null) { - _mapDependencies(constant.value.getType(compiler.coreTypes).element, - deferredImport); + _mapConstantDependencies(constant.value, + deferredImport); } } for (LibraryTag tag in library.tags) { @@ -475,8 +495,8 @@ class DeferredLoadTask extends CompilerTask { ConstantExpression constant = backend.constants.getConstantForMetadata(metadata); if (constant != null) { - _mapDependencies(constant.value.getType(compiler.coreTypes).element, - deferredImport); + _mapConstantDependencies(constant.value, + deferredImport); } } } @@ -580,14 +600,14 @@ class DeferredLoadTask extends CompilerTask { measureElement(mainLibrary, () { // Starting from main, traverse the program and find all dependencies. - _mapDependencies(compiler.mainFunction, _fakeMainImport); + _mapDependencies(element: compiler.mainFunction, import: _fakeMainImport); // Also add "global" dependencies to the main OutputUnit. These are // things that the backend need but cannot associate with a particular // element, for example, startRootIsolate. This set also contains // elements for which we lack precise information. for (Element element in compiler.globalDependencies.otherDependencies) { - _mapDependencies(element, _fakeMainImport); + _mapDependencies(element: element, import: _fakeMainImport); } // Now check to see if we have to add more elements due to mirrors. @@ -615,6 +635,8 @@ class DeferredLoadTask extends CompilerTask { .imports.add(import); } } + } + for (Import import in _constantsDeferredBy.keys) { for (ConstantValue constant in _constantsDeferredBy[import]) { // Only one file should be loaded when the program starts, so make // sure that only one OutputUnit is created for [fakeMainImport]. diff --git a/pkg/compiler/lib/src/js_backend/constant_handler_javascript.dart b/pkg/compiler/lib/src/js_backend/constant_handler_javascript.dart index 4771a5bc6e4..0f66c0d7a96 100644 --- a/pkg/compiler/lib/src/js_backend/constant_handler_javascript.dart +++ b/pkg/compiler/lib/src/js_backend/constant_handler_javascript.dart @@ -41,11 +41,14 @@ class JavaScriptConstantTask extends ConstantCompilerTask { }); } - ConstantExpression compileNode(Node node, TreeElements elements) { + ConstantExpression compileNode(Node node, TreeElements elements, + {bool enforceConst: true}) { return measure(() { ConstantExpression result = - dartConstantCompiler.compileNode(node, elements); - jsConstantCompiler.compileNode(node, elements); + dartConstantCompiler.compileNode(node, elements, + enforceConst: enforceConst); + jsConstantCompiler.compileNode(node, elements, + enforceConst: enforceConst); return result; }); } @@ -163,8 +166,9 @@ class JavaScriptConstantCompiler extends ConstantCompilerBase return initialValue; } - ConstantExpression compileNode(Node node, TreeElements elements) { - return compileNodeWithDefinitions(node, elements); + ConstantExpression compileNode(Node node, TreeElements elements, + {bool enforceConst: true}) { + return compileNodeWithDefinitions(node, elements, isConst: enforceConst); } ConstantExpression compileNodeWithDefinitions(Node node, diff --git a/pkg/compiler/lib/src/js_backend/type_variable_handler.dart b/pkg/compiler/lib/src/js_backend/type_variable_handler.dart index 2f7fa2903dc..a49e7e4312b 100644 --- a/pkg/compiler/lib/src/js_backend/type_variable_handler.dart +++ b/pkg/compiler/lib/src/js_backend/type_variable_handler.dart @@ -75,8 +75,8 @@ class TypeVariableHandler { AstConstant wrapConstant(ConstantExpression constant) { return new AstConstant(typeVariableElement, - typeVariableElement.node, - constant); + typeVariableElement.node, + constant); } ConstantExpression name = new StringConstantExpression( diff --git a/pkg/compiler/lib/src/resolution/members.dart b/pkg/compiler/lib/src/resolution/members.dart index 22b8461ef34..986663e59f2 100644 --- a/pkg/compiler/lib/src/resolution/members.dart +++ b/pkg/compiler/lib/src/resolution/members.dart @@ -2868,7 +2868,7 @@ class ResolverVisitor extends MappingVisitor { // Don't try to make constants of calls to type literals. if (!node.isCall) { - analyzeConstantDeferred(node); + analyzeConstantDeferred(node, enforceConst: false); } else { // The node itself is not a constant but we register the selector (the // identifier that refers to the class/typedef) as a constant. @@ -2878,7 +2878,7 @@ class ResolverVisitor extends MappingVisitor { // the type literal from the selector. registry.useElement(node.selector, target); } - analyzeConstantDeferred(node.selector); + analyzeConstantDeferred(node.selector, enforceConst: false); } } if (isPotentiallyMutableTarget(target)) { @@ -3441,10 +3441,10 @@ class ResolverVisitor extends MappingVisitor { } } - void analyzeConstant(Node node) { + void analyzeConstant(Node node, {enforceConst: true}) { ConstantExpression constant = compiler.resolver.constantCompiler.compileNode( - node, registry.mapping); + node, registry.mapping, enforceConst: enforceConst); if (constant == null) { assert(invariant(node, compiler.compilationFailed)); @@ -3476,9 +3476,9 @@ class ResolverVisitor extends MappingVisitor { } } - void analyzeConstantDeferred(Node node) { + void analyzeConstantDeferred(Node node, {bool enforceConst: true}) { addDeferredAction(enclosingElement, () { - analyzeConstant(node); + analyzeConstant(node, enforceConst: enforceConst); }); } diff --git a/pkg/compiler/lib/src/ssa/builder.dart b/pkg/compiler/lib/src/ssa/builder.dart index 723e88bf8a5..e4b5c8bfb39 100644 --- a/pkg/compiler/lib/src/ssa/builder.dart +++ b/pkg/compiler/lib/src/ssa/builder.dart @@ -5116,6 +5116,7 @@ class SsaBuilder extends NewResolvedVisitor { /// Generate the constant value for a constant type literal. void generateConstantTypeLiteral(ast.Send node) { + generateIsDeferredLoadedCheckIfNeeded(node); // TODO(karlklose): add type representation if (node.isCall) { // The node itself is not a constant but we register the selector (the diff --git a/pkg/compiler/lib/src/ssa/nodes.dart b/pkg/compiler/lib/src/ssa/nodes.dart index d57a37649e9..e642edeae71 100644 --- a/pkg/compiler/lib/src/ssa/nodes.dart +++ b/pkg/compiler/lib/src/ssa/nodes.dart @@ -188,6 +188,8 @@ class HGraph { HConstant addDeferredConstant(ConstantValue constant, PrefixElement prefix, Compiler compiler) { + // TODO(sigurdm,johnniwinter): These deferred constants should be created + // by the constant evaluator. ConstantValue wrapper = new DeferredConstantValue(constant, prefix); compiler.deferredLoadTask.registerConstantDeferredUse(wrapper, prefix); return addConstant(wrapper, compiler); diff --git a/tests/language/language_dart2js.status b/tests/language/language_dart2js.status index 84d92d3267b..459dcb98fe5 100644 --- a/tests/language/language_dart2js.status +++ b/tests/language/language_dart2js.status @@ -14,8 +14,6 @@ async_star_test/05: RuntimeError, Timeout try_catch_on_syntax_test/10: Fail # Issue 19823 try_catch_on_syntax_test/11: Fail # Issue 19823 -deferred_load_constants_test/none: CompileTimeError # issue 22893 - [ $compiler == dart2js && $runtime == jsshell ] await_for_test: Skip # Jsshell does not provide periodic timers, Issue 7728 async_star_test: RuntimeError # Jsshell does not provide non-zero timers, Issue 7728