Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(460)

Unified Diff: lib/compiler/implementation/resolver.dart

Issue 10947024: Made dart2js constructor lookup logic "private"-aware, fixed 4740 bug. (Closed) Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart
Patch Set: Incorporated Peter's comments and feedback. Created 8 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « lib/compiler/implementation/elements/elements.dart ('k') | lib/compiler/implementation/ssa/builder.dart » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/compiler/implementation/resolver.dart
diff --git a/lib/compiler/implementation/resolver.dart b/lib/compiler/implementation/resolver.dart
index 629208757c5f4cbf5be975e972492ce5beddb2f0..8ec37beb8372f76973c6e019b03d1ae6ffbae4de 100644
--- a/lib/compiler/implementation/resolver.dart
+++ b/lib/compiler/implementation/resolver.dart
@@ -87,15 +87,23 @@ class ResolverTask extends CompilerTask {
});
}
+ bool isNamedConstructor(Send node) => node.receiver !== null;
+
SourceString getConstructorName(Send node) {
- if (node.receiver !== null) {
- return node.selector.asIdentifier().source;
- } else {
- return const SourceString('');
- }
+ return node.selector.asIdentifier().source;
}
- FunctionElement resolveConstructorRedirection(FunctionElement constructor) {
+ String constructorNameForDiagnostics(SourceString className,
+ SourceString constructorName) {
+ String classNameString = className.slowToString();
+ String constructorNameString = constructorName.slowToString();
+ return (constructorName === const SourceString(''))
+ ? classNameString
+ : "$classNameString.$constructorNameString";
+ }
+
+ FunctionElement resolveConstructorRedirection(InitializerResolver resolver,
+ FunctionElement constructor) {
if (constructor.isPatched) {
checkMatchingPatchSignatures(constructor, constructor.patch);
constructor = constructor.patch;
@@ -109,10 +117,17 @@ class ResolverTask extends CompilerTask {
if (!initializers.isEmpty() &&
Initializers.isConstructorRedirect(initializers.head)) {
final ClassElement classElement = constructor.getEnclosingClass();
- final SourceString constructorName =
- getConstructorName(initializers.head);
- final SourceString className = classElement.name;
- return classElement.lookupConstructor(className, constructorName);
+ Selector selector;
+ if (isNamedConstructor(initializers.head)) {
+ SourceString constructorName = getConstructorName(initializers.head);
+ selector = new Selector.callConstructor(
+ constructorName,
+ resolver.visitor.enclosingElement.getLibrary());
+ } else {
+ selector = new Selector.callDefaultConstructor(
+ resolver.visitor.enclosingElement.getLibrary());
+ }
+ return classElement.lookupConstructor(selector);
}
return null;
}
@@ -129,7 +144,7 @@ class ResolverTask extends CompilerTask {
return;
}
seen.add(redirection);
- redirection = resolveConstructorRedirection(redirection);
+ redirection = resolveConstructorRedirection(resolver, redirection);
}
}
@@ -290,26 +305,42 @@ class ResolverTask extends CompilerTask {
// [intrface] is an interface, let's say "MyInterface".
// [defaultClass] is a class, let's say "MyClass".
+ Selector selector;
// If the default class implements the interface then we must use the
// default class' name. Otherwise we look for a factory with the name
// of the interface.
- SourceString name;
if (defaultClass.implementsInterface(intrface)) {
- // TODO(ahe): Don't use string replacement here.
- name = new SourceString(constructor.name.slowToString().replaceFirst(
- intrface.name.slowToString(),
- defaultClass.name.slowToString()));
+ var constructorNameString = constructor.name.slowToString();
+ // Create selector based on constructor.name but where interface
+ // is replaced with default class name.
+ // TODO(ahe): Don't use string manipulations here.
+ int classNameSeparatorIndex = constructorNameString.indexOf('\$');
+ if (classNameSeparatorIndex < 0) {
+ selector = new Selector.callDefaultConstructor(
+ defaultClass.getLibrary());
+ } else {
+ selector = new Selector.callConstructor(
+ new SourceString(
+ constructorNameString.substring(classNameSeparatorIndex + 1)),
+ defaultClass.getLibrary());
+ }
+ constructor.defaultImplementation =
+ defaultClass.lookupConstructor(selector);
} else {
- name = constructor.name;
+ selector =
+ new Selector.callConstructor(constructor.name,
+ defaultClass.getLibrary());
+ constructor.defaultImplementation =
+ defaultClass.lookupFactoryConstructor(selector);
}
- constructor.defaultImplementation = defaultClass.lookupConstructor(name);
-
if (constructor.defaultImplementation === null) {
// We failed to find a constructor named either
// "MyInterface.name" or "MyClass.name".
+ // TODO(aprelev@gmail.com): Use constructorNameForDiagnostics in
+ // the error message below.
error(node,
MessageKind.CANNOT_FIND_CONSTRUCTOR2,
- [name, defaultClass.name]);
+ [selector.name, defaultClass.name]);
}
}
@@ -682,6 +713,21 @@ class InitializerResolver {
visitor.visitInStaticContext(init.arguments.head);
}
+ ClassElement getSuperOrThisLookupTarget(FunctionElement constructor,
+ bool isSuperCall,
+ Node diagnosticNode) {
+ ClassElement lookupTarget = constructor.getEnclosingClass();
+ if (isSuperCall) {
+ // Calculate correct lookup target and constructor name.
+ if (lookupTarget === visitor.compiler.objectClass) {
+ error(diagnosticNode, MessageKind.SUPER_INITIALIZER_IN_OBJECT);
+ } else {
+ return lookupTarget.supertype.element;
+ }
+ }
+ return lookupTarget;
+ }
+
Element resolveSuperOrThisForSend(FunctionElement constructor,
FunctionExpression functionNode,
Send call) {
@@ -693,12 +739,39 @@ class InitializerResolver {
});
Selector selector = visitor.mapping.getSelector(call);
bool isSuperCall = Initializers.isSuperConstructorCall(call);
- SourceString constructorName = resolver.getConstructorName(call);
- Element result = resolveSuperOrThis(
- constructor, isSuperCall, false, constructorName, selector, call);
- visitor.useElement(call, result);
- visitor.world.registerStaticUse(result);
- return result;
+
+ ClassElement lookupTarget = getSuperOrThisLookupTarget(constructor,
+ isSuperCall,
+ call);
+ final SourceString className = lookupTarget.name;
+
+ SourceString constructorName;
+ Selector lookupSelector;
+ if (resolver.isNamedConstructor(call)) {
+ constructorName = resolver.getConstructorName(call);
+ lookupSelector = new Selector.callConstructor(
+ constructorName,
+ visitor.enclosingElement.getLibrary());
+ } else {
+ constructorName = const SourceString('');
+ lookupSelector = new Selector.callDefaultConstructor(
+ visitor.enclosingElement.getLibrary());
+ }
+
+ FunctionElement lookedupConstructor =
+ lookupTarget.lookupConstructor(lookupSelector);
+
+ final bool isImplicitSuperCall = false;
+ verifyThatConstructorMatchesCall(lookedupConstructor,
+ selector,
+ isImplicitSuperCall,
+ call,
+ constructorName,
+ className);
+
+ visitor.useElement(call, lookedupConstructor);
+ visitor.world.registerStaticUse(lookedupConstructor);
+ return lookedupConstructor;
}
void resolveImplicitSuperConstructorSend(FunctionElement constructor,
@@ -709,55 +782,57 @@ class InitializerResolver {
if (classElement != visitor.compiler.objectClass) {
assert(superClass !== null);
assert(superClass.resolutionState == STATE_DONE);
- SourceString name = const SourceString('');
- Selector call = new Selector.call(name, classElement.getLibrary(), 0);
- var element = resolveSuperOrThis(constructor, true, true,
- name, call, functionNode);
- visitor.world.registerStaticUse(element);
- }
- }
-
- Element resolveSuperOrThis(FunctionElement constructor,
- bool isSuperCall,
- bool isImplicitSuperCall,
- SourceString constructorName,
- Selector selector,
- Node diagnosticNode) {
- ClassElement lookupTarget = constructor.getEnclosingClass();
- bool validTarget = true;
- FunctionElement result;
- if (isSuperCall) {
- // Calculate correct lookup target and constructor name.
- if (lookupTarget === visitor.compiler.objectClass) {
- error(diagnosticNode, MessageKind.SUPER_INITIALIZER_IN_OBJECT);
- } else {
- lookupTarget = lookupTarget.supertype.element;
- }
- }
-
- // Lookup constructor and try to match it to the selector.
- ResolverTask resolver = visitor.compiler.resolver;
- final SourceString className = lookupTarget.name;
- result = lookupTarget.lookupConstructor(className, constructorName);
- if (result === null || !result.isGenerativeConstructor()) {
- String classNameString = className.slowToString();
- String constructorNameString = constructorName.slowToString();
- String name = (constructorName === const SourceString(''))
- ? classNameString
- : "$classNameString.$constructorNameString";
+ SourceString constructorName = const SourceString('');
+ Selector callToMatch = new Selector.call(
+ constructorName,
+ classElement.getLibrary(),
+ 0);
+
+ final bool isSuperCall = true;
+ ClassElement lookupTarget = getSuperOrThisLookupTarget(constructor,
+ isSuperCall,
+ functionNode);
+ final SourceString className = lookupTarget.name;
+ Element calledConstructor = lookupTarget.lookupConstructor(
+ new Selector.callDefaultConstructor(
+ visitor.enclosingElement.getLibrary()));
+
+ final bool isImplicitSuperCall = true;
+ verifyThatConstructorMatchesCall(calledConstructor,
+ callToMatch,
+ isImplicitSuperCall,
+ functionNode,
+ className,
+ const SourceString(''));
+
+ visitor.world.registerStaticUse(calledConstructor);
+ }
+ }
+
+ void verifyThatConstructorMatchesCall(
+ FunctionElement lookedupConstructor,
+ Selector call,
+ bool isImplicitSuperCall,
+ Node diagnosticNode,
+ SourceString className,
+ SourceString constructorName) {
+ if (lookedupConstructor === null
+ || !lookedupConstructor.isGenerativeConstructor()) {
+ var fullConstructorName =
+ visitor.compiler.resolver.constructorNameForDiagnostics(className,
+ constructorName);
MessageKind kind = isImplicitSuperCall
- ? MessageKind.CANNOT_RESOLVE_CONSTRUCTOR_FOR_IMPLICIT
- : MessageKind.CANNOT_RESOLVE_CONSTRUCTOR;
- error(diagnosticNode, kind, [name]);
+ ? MessageKind.CANNOT_RESOLVE_CONSTRUCTOR_FOR_IMPLICIT
+ : MessageKind.CANNOT_RESOLVE_CONSTRUCTOR;
+ error(diagnosticNode, kind, [fullConstructorName]);
} else {
- if (!selector.applies(result, visitor.compiler)) {
+ if (!call.applies(lookedupConstructor, visitor.compiler)) {
MessageKind kind = isImplicitSuperCall
? MessageKind.NO_MATCHING_CONSTRUCTOR_FOR_IMPLICIT
: MessageKind.NO_MATCHING_CONSTRUCTOR;
error(diagnosticNode, kind);
}
}
- return result;
}
FunctionElement resolveRedirection(FunctionElement constructor,
@@ -2799,22 +2874,33 @@ class ConstructorResolver extends CommonResolverVisitor<Element> {
}
}
+ Selector createConstructorSelector(SourceString constructorName) {
+ return constructorName == const SourceString('')
+ ? new Selector.callDefaultConstructor(
+ resolver.enclosingElement.getLibrary())
+ : new Selector.callConstructor(
+ constructorName,
+ resolver.enclosingElement.getLibrary());
+ }
+
// TODO(ngeoffray): method named lookup should not report errors.
FunctionElement lookupConstructor(ClassElement cls,
Node diagnosticNode,
SourceString constructorName) {
cls.ensureResolved(compiler);
- Element result = cls.lookupConstructor(cls.name, constructorName);
+ Selector selector = createConstructorSelector(constructorName);
+ Element result = cls.lookupConstructor(selector);
if (result === null) {
- String fullConstructorName = cls.name.slowToString();
- if (constructorName !== const SourceString('')) {
- fullConstructorName = '$fullConstructorName'
- '.${constructorName.slowToString()}';
- }
- return failOrReturnErroneousElement(cls, diagnosticNode,
- new SourceString(fullConstructorName),
- MessageKind.CANNOT_FIND_CONSTRUCTOR,
- [fullConstructorName]);
+ String fullConstructorName =
+ resolver.compiler.resolver.constructorNameForDiagnostics(
+ cls.name,
+ constructorName);
+ return failOrReturnErroneousElement(
+ cls,
+ diagnosticNode,
+ new SourceString(fullConstructorName),
+ MessageKind.CANNOT_FIND_CONSTRUCTOR,
+ [fullConstructorName]);
} else if (inConstContext && !result.modifiers.isConst()) {
error(diagnosticNode, MessageKind.CONSTRUCTOR_IS_NOT_CONST);
}
« no previous file with comments | « lib/compiler/implementation/elements/elements.dart ('k') | lib/compiler/implementation/ssa/builder.dart » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698