Skip to content

Commit

Permalink
Remove outer param placeholder and instead of OuterReferenceResolver …
Browse files Browse the repository at this point in the history 
…create the outer param elements.

	Change on 2016/09/30 by kstanger <kstanger@google.com>

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=134802545
  • Loading branch information
@kstanger @tomball
kstanger authored and tomball committed Oct 7, 2016
1 parent 34d2b2c commit da2fd73
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 82 deletions.
23 changes: 9 additions & 14 deletions translator/src/main/java/com/google/devtools/j2objc/translate/InnerClassExtractor.java
View file
Expand Up @@ -162,7 +162,7 @@ protected void addOuterParameters(
AbstractTypeDeclaration typeNode, MethodDeclaration constructor) {
ITypeBinding type = typeNode.getTypeBinding();
TypeElement typeE = typeNode.getTypeElement();
IVariableBinding outerParamBinding = null;
VariableElement outerParam = outerResolver.getOuterParam(typeE);

GeneratedMethodBinding constructorBinding =
new GeneratedMethodBinding(constructor.getMethodBinding().getMethodDeclaration());
Expand All @@ -171,14 +171,9 @@ protected void addOuterParameters(
// Adds the outer and captured parameters to the declaration.
List<SingleVariableDeclaration> captureDecls = constructor.getParameters().subList(0, 0);
List<ITypeBinding> captureTypes = constructorBinding.getParameters().subList(0, 0);
if (outerResolver.needsOuterParam(typeE)) {
ITypeBinding outerType = BindingConverter.unwrapTypeMirrorIntoTypeBinding(
outerResolver.getOuterType(typeE));
GeneratedVariableBinding paramBinding = new GeneratedVariableBinding(
"outer$", Modifier.FINAL, outerType, false, true, type, constructorBinding);
captureDecls.add(new SingleVariableDeclaration(paramBinding));
captureTypes.add(outerType);
outerParamBinding = paramBinding;
if (outerParam != null) {
captureDecls.add(new SingleVariableDeclaration(outerParam));
captureTypes.add(BindingConverter.unwrapTypeMirrorIntoTypeBinding(outerParam.asType()));
}
List<VariableElement> innerFields = outerResolver.getInnerFields(typeE);
List<IVariableBinding> captureParams = Lists.newArrayListWithCapacity(innerFields.size());
Expand Down Expand Up @@ -214,9 +209,9 @@ protected void addOuterParameters(
thisCall.setMethodBinding(newThisBinding);
List<Expression> args = thisCall.getArguments().subList(0, 0);
List<ITypeBinding> params = newThisBinding.getParameters().subList(0, 0);
if (outerParamBinding != null) {
args.add(new SimpleName(outerParamBinding));
params.add(outerParamBinding.getType());
if (outerParam != null) {
args.add(new SimpleName(outerParam));
params.add(BindingConverter.unwrapTypeMirrorIntoTypeBinding(outerParam.asType()));
}
for (IVariableBinding captureParam : captureParams) {
args.add(new SimpleName(captureParam));
Expand All @@ -232,9 +227,9 @@ protected void addOuterParameters(
VariableElement outerField = outerResolver.getOuterField(typeE);
int idx = 0;
if (outerField != null) {
assert outerParamBinding != null;
assert outerParam != null;
statements.add(idx++, new ExpressionStatement(
new Assignment(new SimpleName(outerField), new SimpleName(outerParamBinding))));
new Assignment(new SimpleName(outerField), new SimpleName(outerParam))));
}
for (int i = 0; i < innerFields.size(); i++) {
statements.add(idx++, new ExpressionStatement(new Assignment(
Expand Down
45 changes: 6 additions & 39 deletions translator/src/main/java/com/google/devtools/j2objc/translate/OuterReferenceFixer.java
View file
Expand Up @@ -14,14 +14,11 @@

package com.google.devtools.j2objc.translate;

import com.google.common.collect.Lists;
import com.google.devtools.j2objc.ast.ClassInstanceCreation;
import com.google.devtools.j2objc.ast.Expression;
import com.google.devtools.j2objc.ast.MethodDeclaration;
import com.google.devtools.j2objc.ast.MethodInvocation;
import com.google.devtools.j2objc.ast.Name;
import com.google.devtools.j2objc.ast.SimpleName;
import com.google.devtools.j2objc.ast.SingleVariableDeclaration;
import com.google.devtools.j2objc.ast.SuperConstructorInvocation;
import com.google.devtools.j2objc.ast.SuperMethodInvocation;
import com.google.devtools.j2objc.ast.ThisExpression;
Expand All @@ -32,7 +29,6 @@
import com.google.devtools.j2objc.util.ElementUtil;
import java.util.ArrayList;
import java.util.List;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.TypeElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.TypeMirror;
Expand All @@ -46,31 +42,11 @@
public class OuterReferenceFixer extends TreeVisitor {

private final OuterReferenceResolver outerResolver;
private VariableElement outerParam = null;

public OuterReferenceFixer(OuterReferenceResolver outerResolver) {
this.outerResolver = outerResolver;
}

@Override
public boolean visit(MethodDeclaration node) {
if (node.getExecutableElement().getKind() == ElementKind.CONSTRUCTOR) {
List<SingleVariableDeclaration> params = node.getParameters();
if (params.size() > 0) {
VariableElement firstParam = params.get(0).getVariableElement();
if (firstParam.getSimpleName().toString().equals("outer$")) {
outerParam = firstParam;
}
}
}
return true;
}

@Override
public void endVisit(MethodDeclaration node) {
outerParam = null;
}

@Override
public boolean visit(ClassInstanceCreation node) {
TypeElement newType = (TypeElement) node.getExecutableElement().getEnclosingElement();
Expand Down Expand Up @@ -106,7 +82,7 @@ private Expression getOuterArg(ClassInstanceCreation node, TypeMirror declaringC
}
List<VariableElement> path = outerResolver.getPath(node);
if (path != null) {
return Name.newName(fixPath(path));
return Name.newName(path);
}
return new ThisExpression(declaringClass);
}
Expand All @@ -115,7 +91,7 @@ private Expression getOuterArg(ClassInstanceCreation node, TypeMirror declaringC
public boolean visit(MethodInvocation node) {
List<VariableElement> path = outerResolver.getPath(node);
if (path != null) {
node.setExpression(Name.newName(fixPath(path)));
node.setExpression(Name.newName(path));
}
return true;
}
Expand All @@ -125,7 +101,7 @@ public void endVisit(SuperMethodInvocation node) {
List<VariableElement> path = outerResolver.getPath(node);
if (path != null) {
// We substitute the qualifying type name with the outer variable name.
node.setQualifier(Name.newName(fixPath(path)));
node.setQualifier(Name.newName(path));
} else {
node.setQualifier(null);
}
Expand All @@ -139,7 +115,7 @@ public boolean visit(SimpleName node) {
VariableElement var = path.get(0);
node.replaceWith(TreeUtil.newLiteral(var.getConstantValue(), typeEnv));
} else {
node.replaceWith(Name.newName(fixPath(path)));
node.replaceWith(Name.newName(path));
}
}
return true;
Expand All @@ -149,7 +125,7 @@ public boolean visit(SimpleName node) {
public boolean visit(ThisExpression node) {
List<VariableElement> path = outerResolver.getPath(node);
if (path != null) {
node.replaceWith(Name.newName(fixPath(path)));
node.replaceWith(Name.newName(path));
} else {
node.setQualifier(null);
}
Expand All @@ -168,7 +144,7 @@ public void endVisit(SuperConstructorInvocation node) {
if (outerResolver.needsOuterParam(superType)) {
Expression outerArg = TreeUtil.remove(node.getExpression());
if (outerArg == null) {
outerArg = Name.newName(fixPath(outerResolver.getPath(typeNode)));
outerArg = Name.newName(outerResolver.getPath(typeNode));
}
args.add(outerArg);
parameterTypes.add(outerResolver.getOuterType(superType));
Expand All @@ -188,13 +164,4 @@ public void endVisit(SuperConstructorInvocation node) {
assert element.isVarArgs() || node.getArguments().size() == element.getParameters().size();
}
}

private List<VariableElement> fixPath(List<VariableElement> path) {
if (path.get(0) == OuterReferenceResolver.OUTER_PARAMETER) {
assert outerParam != null;
path = Lists.newArrayList(path);
path.set(0, outerParam);
}
return path;
}
}
82 changes: 54 additions & 28 deletions translator/src/main/java/com/google/devtools/j2objc/translate/OuterReferenceResolver.java
View file
Expand Up @@ -78,16 +78,10 @@
*/
public class OuterReferenceResolver extends TreeVisitor {

// A placeholder variable element that should be replaced with the outer
// parameter in a constructor.
public static final VariableElement OUTER_PARAMETER = new GeneratedVariableElement(
"<placeholder-variable>", null, ElementKind.PARAMETER, null)
.setNonnull(true);

private enum VisitingState { NEEDS_REVISIT, VISITED }

private Map<TypeElement, VariableElement> outerVars = new HashMap<>();
private Set<TypeElement> usesOuterParam = new HashSet<>();
private Map<TypeElement, VariableElement> outerParams = new HashMap<>();
private ListMultimap<TypeElement, Capture> captures = ArrayListMultimap.create();
private Map<TreeNode.Key, List<VariableElement>> outerPaths = new HashMap<>();
private Map<TreeNode.Key, List<List<VariableElement>>> captureArgs = new HashMap<>();
Expand All @@ -105,16 +99,25 @@ public boolean needsOuterReference(TypeElement type) {
}

public boolean needsOuterParam(TypeElement type) {
return outerVars.containsKey(type) || usesOuterParam.contains(type)
|| (ElementUtil.hasOuterContext(type) && !ElementUtil.isLocal(type));
return outerParams.containsKey(type) || automaticOuterParam(type);
}

public VariableElement getOuterParam(TypeElement type) {
return outerParams.get(type);
}

public TypeMirror getOuterType(TypeElement type) {
VariableElement outerField = outerVars.get(type);
if (outerField != null) {
return outerField.asType();
}
return ElementUtil.getDeclaringClass(type).asType();
return getDeclaringType(type);
}

private static TypeMirror getDeclaringType(TypeElement type) {
TypeElement declaringClass = ElementUtil.getDeclaringClass(type);
assert declaringClass != null : "Cannot find declaring class for " + type;
return declaringClass.asType();
}

public VariableElement getOuterField(TypeElement type) {
Expand Down Expand Up @@ -142,6 +145,10 @@ public List<List<VariableElement>> getCaptureArgPaths(TreeNode node) {
return Collections.emptyList();
}

private static boolean automaticOuterParam(TypeElement type) {
return ElementUtil.hasOuterContext(type) && !ElementUtil.isLocal(type);
}

private static class Capture {

private final VariableElement var;
Expand Down Expand Up @@ -229,26 +236,38 @@ private String getCaptureFieldName(VariableElement var, TypeElement type) {
return "val" + (suffix > 0 ? suffix : "") + "$" + var.getSimpleName().toString();
}

private VariableElement getOrCreateOuterField(Scope scope) {
// Check that this isn't a lambda, since we'll always capture the field itself
if (scope.initializingContext && scope == peekScope()) {
usesOuterParam.add(scope.type);
return OUTER_PARAMETER;
private VariableElement getOrCreateOuterParam(TypeElement type) {
VariableElement outerParam = outerParams.get(type);
if (outerParam == null) {
outerParam = new GeneratedVariableElement(
"outer$", getDeclaringType(type), ElementKind.PARAMETER, type)
.setNonnull(true);
outerParams.put(type, outerParam);
}
return outerParam;
}

VariableElement outerField = outerVars.get(scope.type);
private VariableElement getOrCreateOuterField(TypeElement type) {
VariableElement outerField = outerVars.get(type);
if (outerField == null) {
TypeElement declaringClass = ElementUtil.getDeclaringClass(scope.type);
assert declaringClass != null : "Cannot find declaring class for " + scope.type;
outerField = new GeneratedVariableElement(
getOuterFieldName(scope.type), declaringClass.asType(), ElementKind.FIELD, scope.type)
getOuterFieldName(type), getDeclaringType(type), ElementKind.FIELD, type)
.addModifiers(Modifier.PRIVATE, Modifier.FINAL)
.setNonnull(true);
outerVars.put(scope.type, outerField);
outerVars.put(type, outerField);
}
return outerField;
}

private VariableElement getOrCreateOuterVar(Scope scope) {
// Always create the outer param since it is required to initialize the field.
VariableElement outerParam = getOrCreateOuterParam(scope.type);
if (scope.initializingContext && scope == peekScope()) {
return outerParam;
}
return getOrCreateOuterField(scope.type);
}

private VariableElement getOrCreateInnerField(VariableElement var, TypeElement declaringType) {
List<Capture> capturesForType = captures.get(declaringType);
VariableElement innerField = null;
Expand All @@ -272,7 +291,7 @@ private List<VariableElement> getOuterPath(TypeElement type) {
List<VariableElement> path = new ArrayList<>();
for (Scope scope = peekScope(); !type.equals(scope.type); scope = scope.outer) {
if (scope == peekScope() || !ElementUtil.isLambda(scope.type)) {
path.add(getOrCreateOuterField(scope));
path.add(getOrCreateOuterVar(scope));
}
}
return path;
Expand All @@ -282,7 +301,7 @@ private List<VariableElement> getOuterPathInherited(TypeElement type) {
List<VariableElement> path = new ArrayList<>();
for (Scope scope = peekScope(); !scope.inheritedScope.contains(type); scope = scope.outer) {
if (scope == peekScope() || !ElementUtil.isLambda(scope.type)) {
path.add(getOrCreateOuterField(scope));
path.add(getOrCreateOuterVar(scope));
}
}
return path;
Expand Down Expand Up @@ -312,7 +331,7 @@ private List<VariableElement> getPathForLocalVar(VariableElement var) {
while (!(scope = scope.outer).declaredVars.contains(var)) {
// Except for the current scope do not include lambdas in the path.
if (!ElementUtil.isLambda(scope.type)) {
path.add(getOrCreateOuterField(lastScope));
path.add(getOrCreateOuterVar(lastScope));
lastScope = scope;
}
}
Expand All @@ -328,6 +347,9 @@ private void addPath(TreeNode node, List<VariableElement> path) {

private void pushType(TypeElement type) {
topScope = new Scope(topScope, type, typeEnv);
if (automaticOuterParam(type)) {
getOrCreateOuterParam(type);
}
}

private void popType(TreeNode node) {
Expand Down Expand Up @@ -446,12 +468,16 @@ public void endVisit(LambdaExpression node) {
public void endVisit(ExpressionMethodReference node) {
Expression target = node.getExpression();
if (!ElementUtil.isStatic(node.getExecutableElement()) && isValue(target)) {
GeneratedVariableElement targetField = new GeneratedVariableElement(
"target$", target.getTypeMirror(), ElementKind.FIELD, node.getTypeElement())
.addModifiers(Modifier.PRIVATE, Modifier.FINAL)
.setNonnull(true);
TypeElement type = node.getTypeElement();
TypeMirror targetType = target.getTypeMirror();
// Add the target field as an outer field even though it's not really pointing to outer scope.
outerVars.put(node.getTypeElement(), targetField);
outerVars.put(type, new GeneratedVariableElement(
"target$", targetType, ElementKind.FIELD, type)
.addModifiers(Modifier.PRIVATE, Modifier.FINAL)
.setNonnull(true));
outerParams.put(type, new GeneratedVariableElement(
"outer$", targetType, ElementKind.PARAMETER, type)
.setNonnull(true));
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...slator/src/test/java/com/google/devtools/j2objc/translate/OuterReferenceResolverTest.java
View file
Expand Up @@ -84,7 +84,7 @@ public void testInheritedOuterMethod() {
List<VariableElement> bPath = outerResolver.getPath(bNode);
assertNotNull(bPath);
assertEquals(1, bPath.size());
assertEquals(OuterReferenceResolver.OUTER_PARAMETER, bPath.get(0));
assertEquals("outer$", ElementUtil.getName(bPath.get(0)));

// foo() call will need to get to B's scope to call the inherited method.
MethodInvocation fooCall = (MethodInvocation) nodesByType.get(Kind.METHOD_INVOCATION).get(0);
Expand Down

0 comments on commit da2fd73

Please sign in to comment.