Skip to content

Commit

Permalink
Fixes a local class that inherits from a local class.
Browse files Browse the repository at this point in the history 
Eliminates an unnecessary outer field when a local class has an enclosed construction of itself.
Moves addition of outer and capture args on SuperConstructorInvocation into OuterReferenceFixer.

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

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=133281450
  • Loading branch information
@kstanger @tomball
kstanger authored and tomball committed Sep 16, 2016
1 parent ed37efd commit 7d35ec5
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 72 deletions.
23 changes: 0 additions & 23 deletions translator/src/main/java/com/google/devtools/j2objc/translate/InnerClassExtractor.java
View file
Expand Up @@ -28,7 +28,6 @@
import com.google.devtools.j2objc.ast.ExpressionStatement; import com.google.devtools.j2objc.ast.ExpressionStatement;
import com.google.devtools.j2objc.ast.FieldDeclaration; import com.google.devtools.j2objc.ast.FieldDeclaration;
import com.google.devtools.j2objc.ast.MethodDeclaration; import com.google.devtools.j2objc.ast.MethodDeclaration;
import com.google.devtools.j2objc.ast.Name;
import com.google.devtools.j2objc.ast.SimpleName; import com.google.devtools.j2objc.ast.SimpleName;
import com.google.devtools.j2objc.ast.SingleVariableDeclaration; import com.google.devtools.j2objc.ast.SingleVariableDeclaration;
import com.google.devtools.j2objc.ast.Statement; import com.google.devtools.j2objc.ast.Statement;
Expand Down Expand Up @@ -229,7 +228,6 @@ protected void addOuterParameters(
TranslationUtil.findDefaultConstructorBinding(superType, typeEnv)); TranslationUtil.findDefaultConstructorBinding(superType, typeEnv));
statements.add(0, superCall); statements.add(0, superCall);
} }
passOuterParamToSuper(typeNode, superCall, superType, outerParamBinding);
VariableElement outerField = outerResolver.getOuterField(typeE); VariableElement outerField = outerResolver.getOuterField(typeE);
int idx = 0; int idx = 0;
if (outerField != null) { if (outerField != null) {
Expand All @@ -245,25 +243,4 @@ protected void addOuterParameters(
assert constructor.getParameters().size() assert constructor.getParameters().size()
== constructor.getMethodBinding().getParameterTypes().length; == constructor.getMethodBinding().getParameterTypes().length;
} }

private void passOuterParamToSuper(
AbstractTypeDeclaration typeNode, SuperConstructorInvocation superCall,
ITypeBinding superType, IVariableBinding outerParamBinding) {
if (!BindingUtil.hasOuterContext(superType) || superCall.getExpression() != null) {
return;
}
assert outerParamBinding != null;
GeneratedMethodBinding superCallBinding =
new GeneratedMethodBinding(superCall.getMethodBinding().getMethodDeclaration());
superCall.setMethodBinding(superCallBinding);

List<VariableElement> path = outerResolver.getPath(typeNode);
assert path != null && path.size() > 0;
path = Lists.newArrayList(path);
path.set(0, (VariableElement) BindingConverter.getElement(outerParamBinding));
Name superOuterArg = Name.newName(path);

superCall.addArgument(0, superOuterArg);
superCallBinding.addParameter(0, superType.getDeclaringClass());
}
} }
1 change: 0 additions & 1 deletion translator/src/main/java/com/google/devtools/j2objc/translate/LambdaRewriter.java
View file
Expand Up @@ -91,7 +91,6 @@ public void endVisit(LambdaExpression node) {
ClassInstanceCreation creation = new ClassInstanceCreation( ClassInstanceCreation creation = new ClassInstanceCreation(
constructorBinding, Type.newType(lambdaType.asType())); constructorBinding, Type.newType(lambdaType.asType()));


typeDecl.setKey(node.getKey());
creation.setKey(node.getKey()); creation.setKey(node.getKey());


removeCastExpression(node); removeCastExpression(node);
Expand Down
59 changes: 38 additions & 21 deletions translator/src/main/java/com/google/devtools/j2objc/translate/OuterReferenceFixer.java
View file
Expand Up @@ -25,6 +25,7 @@
import com.google.devtools.j2objc.ast.SuperConstructorInvocation; import com.google.devtools.j2objc.ast.SuperConstructorInvocation;
import com.google.devtools.j2objc.ast.SuperMethodInvocation; import com.google.devtools.j2objc.ast.SuperMethodInvocation;
import com.google.devtools.j2objc.ast.ThisExpression; import com.google.devtools.j2objc.ast.ThisExpression;
import com.google.devtools.j2objc.ast.TreeNode;
import com.google.devtools.j2objc.ast.TreeUtil; import com.google.devtools.j2objc.ast.TreeUtil;
import com.google.devtools.j2objc.ast.TreeVisitor; import com.google.devtools.j2objc.ast.TreeVisitor;
import com.google.devtools.j2objc.types.GeneratedExecutableElement; import com.google.devtools.j2objc.types.GeneratedExecutableElement;
Expand Down Expand Up @@ -74,28 +75,26 @@ public void endVisit(MethodDeclaration node) {
public boolean visit(ClassInstanceCreation node) { public boolean visit(ClassInstanceCreation node) {
TypeElement newType = (TypeElement) node.getExecutableElement().getEnclosingElement(); TypeElement newType = (TypeElement) node.getExecutableElement().getEnclosingElement();
TypeElement declaringClass = ElementUtil.getDeclaringClass(newType); TypeElement declaringClass = ElementUtil.getDeclaringClass(newType);
if (newType.getModifiers().contains(javax.lang.model.element.Modifier.STATIC)
|| declaringClass == null) {
return true;
}
List<TypeMirror> parameterTypes = new ArrayList<>(); List<TypeMirror> parameterTypes = new ArrayList<>();
GeneratedExecutableElement element =
new GeneratedExecutableElement(node.getExecutableElement());

List<Expression> captureArgs = node.getArguments().subList(0, 0); List<Expression> captureArgs = node.getArguments().subList(0, 0);

if (outerResolver.needsOuterParam(newType)) { if (outerResolver.needsOuterParam(newType)) {
captureArgs.add(getOuterArg(node, declaringClass.asType())); captureArgs.add(getOuterArg(node, declaringClass.asType()));
parameterTypes.add(declaringClass.asType()); parameterTypes.add(declaringClass.asType());
} }


for (List<VariableElement> captureArgPath : outerResolver.getCaptureArgPaths(node)) { for (List<VariableElement> captureArgPath : outerResolver.getCaptureArgPaths(node)) {
captureArgPath = fixPath(captureArgPath);
captureArgs.add(Name.newName(captureArgPath)); captureArgs.add(Name.newName(captureArgPath));
parameterTypes.add(captureArgPath.get(captureArgPath.size() - 1).asType()); parameterTypes.add(captureArgPath.get(captureArgPath.size() - 1).asType());
} }
element.addParametersPlaceholderFront(parameterTypes);
node.setExecutableElement(element); if (!parameterTypes.isEmpty()) {
assert element.isVarArgs() || node.getArguments().size() == element.getParameters().size(); GeneratedExecutableElement element =
new GeneratedExecutableElement(node.getExecutableElement());
element.addParametersPlaceholderFront(parameterTypes);
node.setExecutableElement(element);
assert element.isVarArgs() || node.getArguments().size() == element.getParameters().size();
}
return true; return true;
} }


Expand Down Expand Up @@ -159,17 +158,35 @@ public boolean visit(ThisExpression node) {


@Override @Override
public void endVisit(SuperConstructorInvocation node) { public void endVisit(SuperConstructorInvocation node) {
Expression outerExpression = node.getExpression(); TreeNode typeNode = TreeUtil.getEnclosingType(node);
if (outerExpression == null) { TypeElement superType = ElementUtil.getSuperclass(
return; (TypeElement) TreeUtil.getDeclaredElement(typeNode));
List<Expression> args = node.getArguments().subList(0, 0);
List<TypeMirror> parameterTypes = new ArrayList<>();

// Outer arg.
if (outerResolver.needsOuterParam(superType)) {
Expression outerArg = TreeUtil.remove(node.getExpression());
if (outerArg == null) {
outerArg = Name.newName(fixPath(outerResolver.getPath(typeNode)));
}
args.add(outerArg);
parameterTypes.add(ElementUtil.getDeclaringClass(superType).asType());
}

// Capture args.
for (List<VariableElement> captureArgPath : outerResolver.getCaptureArgPaths(typeNode)) {
args.add(Name.newName(captureArgPath));
parameterTypes.add(captureArgPath.get(captureArgPath.size() - 1).asType());
}

if (!parameterTypes.isEmpty()) {
GeneratedExecutableElement element =
new GeneratedExecutableElement(node.getExecutableElement());
element.addParametersPlaceholderFront(parameterTypes);
node.setExecutableElement(element);
assert element.isVarArgs() || node.getArguments().size() == element.getParameters().size();
} }
node.setExpression(null);
TypeMirror outerExpressionType = outerExpression.getTypeMirror();
GeneratedExecutableElement element =
new GeneratedExecutableElement(node.getExecutableElement());
node.setExecutableElement(element);
node.addArgument(0, outerExpression);
element.addParameterPlaceholderFront(outerExpressionType);
} }


private List<VariableElement> fixPath(List<VariableElement> path) { private List<VariableElement> fixPath(List<VariableElement> path) {
Expand Down
56 changes: 30 additions & 26 deletions translator/src/main/java/com/google/devtools/j2objc/translate/OuterReferenceResolver.java
View file
Expand Up @@ -97,8 +97,9 @@ public boolean needsOuterReference(TypeElement type) {
} }


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


public VariableElement getOuterField(TypeElement type) { public VariableElement getOuterField(TypeElement type) {
Expand All @@ -119,7 +120,6 @@ public List<VariableElement> getPath(TreeNode node) {
} }


public List<List<VariableElement>> getCaptureArgPaths(TreeNode node) { public List<List<VariableElement>> getCaptureArgPaths(TreeNode node) {
assert node instanceof ClassInstanceCreation || node instanceof LambdaExpression;
List<List<VariableElement>> result = captureArgs.get(node.getKey()); List<List<VariableElement>> result = captureArgs.get(node.getKey());
if (result != null) { if (result != null) {
return result; return result;
Expand Down Expand Up @@ -206,6 +206,14 @@ private String getOuterFieldName(TypeElement type) {
return "this$" + suffix; return "this$" + suffix;
} }


private String getCaptureFieldName(VariableElement var, TypeElement type) {
int suffix = 0;
while ((type = ElementUtil.getSuperclass(type)) != null && ElementUtil.isLocal(type)) {
suffix++;
}
return "val" + (suffix > 0 ? suffix : "") + "$" + var.getSimpleName().toString();
}

private VariableElement getOrCreateOuterField(Scope scope) { private VariableElement getOrCreateOuterField(Scope scope) {
// Check that this isn't a lambda, since we'll always capture the field itself // Check that this isn't a lambda, since we'll always capture the field itself
if (scope.initializingContext && scope == peekScope()) { if (scope.initializingContext && scope == peekScope()) {
Expand Down Expand Up @@ -236,7 +244,7 @@ private VariableElement getOrCreateInnerField(VariableElement var, TypeElement d
} }
if (innerField == null) { if (innerField == null) {
innerField = new GeneratedVariableElement( innerField = new GeneratedVariableElement(
"val$" + var.getSimpleName().toString(), var.asType(), ElementKind.FIELD, declaringType) getCaptureFieldName(var, declaringType), var.asType(), ElementKind.FIELD, declaringType)
.addModifiers(Modifier.PRIVATE, Modifier.FINAL) .addModifiers(Modifier.PRIVATE, Modifier.FINAL)
.addAnnotationMirrors(var.getAnnotationMirrors()); .addAnnotationMirrors(var.getAnnotationMirrors());
captures.put(declaringType, new Capture(var, innerField)); captures.put(declaringType, new Capture(var, innerField));
Expand Down Expand Up @@ -321,11 +329,24 @@ private void popType(TreeNode node) {
// type node because there may be implicit super invocations. // type node because there may be implicit super invocations.
private void addSuperOuterPath(TreeNode node, TypeElement type) { private void addSuperOuterPath(TreeNode node, TypeElement type) {
TypeElement superclass = ElementUtil.getSuperclass(type); TypeElement superclass = ElementUtil.getSuperclass(type);
if (superclass != null && ElementUtil.hasOuterContext(superclass)) { if (superclass != null && needsOuterParam(superclass)) {
addPath(node, getOuterPathInherited(ElementUtil.getDeclaringClass(superclass))); addPath(node, getOuterPathInherited(ElementUtil.getDeclaringClass(superclass)));
} }
} }


private void resolveCaptureArgs(TreeNode node, TypeElement type) {
List<Capture> capturesForType = captures.get(type);
List<List<VariableElement>> capturePaths = new ArrayList<>(capturesForType.size());
for (Capture capture : capturesForType) {
List<VariableElement> path = getPathForLocalVar(capture.var);
if (path.isEmpty()) {
path = Collections.singletonList(capture.var);
}
capturePaths.add(path);
}
captureArgs.put(node.getKey(), capturePaths);
}

@Override @Override
public boolean visit(TypeDeclaration node) { public boolean visit(TypeDeclaration node) {
pushType(node.getTypeElement()); pushType(node.getTypeElement());
Expand All @@ -342,6 +363,7 @@ public void endVisit(TypeDeclaration node) {
if (currentScope.constructorCount > currentScope.constructorsNotNeedingSuperOuterScope) { if (currentScope.constructorCount > currentScope.constructorsNotNeedingSuperOuterScope) {
addSuperOuterPath(node, node.getTypeElement()); addSuperOuterPath(node, node.getTypeElement());
} }
resolveCaptureArgs(node, ElementUtil.getSuperclass(node.getTypeElement()));
popType(node); popType(node);
} }


Expand Down Expand Up @@ -400,16 +422,7 @@ public void endVisit(LambdaExpression node) {
if (outerField != null) { if (outerField != null) {
addPath(node, getOuterPathInherited(TypeUtil.asTypeElement(outerField.asType()))); addPath(node, getOuterPathInherited(TypeUtil.asTypeElement(outerField.asType())));
} }
List<Capture> capturesForType = captures.get(typeElement); resolveCaptureArgs(node, typeElement);
List<List<VariableElement>> capturePaths = new ArrayList<>(capturesForType.size());
for (Capture capture : capturesForType) {
List<VariableElement> path = getPathForLocalVar(capture.var);
if (path.isEmpty()) {
path = Collections.singletonList(capture.var);
}
capturePaths.add(path);
}
captureArgs.put(node.getKey(), capturePaths);
} }


@Override @Override
Expand Down Expand Up @@ -486,21 +499,12 @@ public void endVisit(SuperMethodInvocation node) {
@Override @Override
public void endVisit(ClassInstanceCreation node) { public void endVisit(ClassInstanceCreation node) {
TypeElement typeElement = (TypeElement) node.getExecutableElement().getEnclosingElement(); TypeElement typeElement = (TypeElement) node.getExecutableElement().getEnclosingElement();
if (node.getExpression() == null && ElementUtil.hasOuterContext(typeElement)) { if (node.getExpression() == null && needsOuterParam(typeElement)) {
TypeElement enclosingTypeElement = ElementUtil.getDeclaringClass(typeElement); TypeElement enclosingTypeElement = ElementUtil.getDeclaringClass(typeElement);
addPath(node, getOuterPathInherited(enclosingTypeElement)); addPath(node, getOuterPathInherited(enclosingTypeElement));
} }
if (ElementUtil.isLocal(typeElement) && !revisitScope(typeElement)) { if (ElementUtil.isLocal(typeElement) && !revisitScope(typeElement)) {
List<Capture> capturesForType = captures.get(typeElement); resolveCaptureArgs(node, typeElement);
List<List<VariableElement>> capturePaths = new ArrayList<>(capturesForType.size());
for (Capture capture : capturesForType) {
List<VariableElement> path = getPathForLocalVar(capture.var);
if (path.isEmpty()) {
path = Collections.singletonList(capture.var);
}
capturePaths.add(path);
}
captureArgs.put(node.getKey(), capturePaths);
} }
} }


Expand Down
15 changes: 14 additions & 1 deletion translator/src/test/java/com/google/devtools/j2objc/translate/OuterReferenceFixerTest.java
View file
Expand Up @@ -43,6 +43,19 @@ public void testRecursiveConstructionOfLocalClass() throws IOException {
String translation = translateSourceFile( String translation = translateSourceFile(
"public class Test { void test(final Object bar) { " "public class Test { void test(final Object bar) { "
+ "class Foo { void foo() { bar.toString(); new Foo(); } } } }", "Test", "Test.m"); + "class Foo { void foo() { bar.toString(); new Foo(); } } } }", "Test", "Test.m");
assertTranslation(translation, "create_Test_1Foo_initWithTest_withId_(this$0_, val$bar_)"); assertTranslation(translation, "create_Test_1Foo_initWithId_(val$bar_)");
}

public void testLocalClassExtendsLocalClassCapturesVariables() throws IOException {
String translation = translateSourceFile(
"public class Test { void test(final int i, final int j) { "
+ "class A { int sum() { return i + j; } }; class B extends A {} } }", "Test", "Test.m");
// Local class B must also capture the locals and pass them to A's constructor.
assertTranslatedLines(translation,
"void Test_1B_initWithInt_withInt_(Test_1B *self, jint capture$0, jint capture$1) {",
" self->val1$i_ = capture$0;",
" self->val1$j_ = capture$1;",
" Test_1A_initWithInt_withInt_(self, self->val1$i_, self->val1$j_);",
"}");
} }
} }

0 comments on commit 7d35ec5

Please sign in to comment.