Skip to content

Commit

Permalink
Functionizes super invocations where possible. This helps to avoid in…
Browse files Browse the repository at this point in the history 
…stanceMethodForSelector calls in some scenarios.

Remove the Objective-C wrapper method for private instance methods when reflection is stripped.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=160951605
  • Loading branch information
@kstanger @tomball
kstanger authored and tomball committed Jul 14, 2017
1 parent ff14cbc commit 03b33a0
Show file tree
Hide file tree
Showing 8 changed files with 154 additions and 125 deletions.
215 changes: 131 additions & 84 deletions translator/src/main/java/com/google/devtools/j2objc/translate/Functionizer.java
View file
Expand Up @@ -14,7 +14,6 @@

package com.google.devtools.j2objc.translate;

import com.google.common.collect.Sets;
import com.google.devtools.j2objc.ast.AbstractTypeDeclaration;
import com.google.devtools.j2objc.ast.AnnotationTypeDeclaration;
import com.google.devtools.j2objc.ast.Block;
Expand Down Expand Up @@ -57,6 +56,7 @@
import java.lang.reflect.Modifier;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -84,86 +84,116 @@ public Functionizer(CompilationUnit unit) {

@Override
public boolean visit(CompilationUnit node) {
functionizableMethods = determineFunctionizableMethods(node);
FunctionizableFinder finder = new FunctionizableFinder();
node.accept(finder);
functionizableMethods = finder.getFunctionizableMethods();
return true;
}

/**
* Determines the set of methods to functionize. In addition to a method being
* final we must also find an invocation for that method. Static methods, though,
* are always functionized since there are no dynamic dispatch issues.
*/
private Set<ExecutableElement> determineFunctionizableMethods(final CompilationUnit unit) {
final Set<ExecutableElement> functionizableDeclarations = Sets.newHashSet();
final Set<ExecutableElement> invocations = Sets.newHashSet();
unit.accept(new TreeVisitor() {
@Override
public void endVisit(MethodDeclaration node) {
if (canFunctionize(node)) {
functionizableDeclarations.add(node.getExecutableElement());
private static boolean isNonVirtual(ExecutableElement method) {
return ElementUtil.isPrivate(method) || ElementUtil.isFinal(method);
}

static class MethodInfo {

private Boolean functionizable = null;
private Set<ExecutableElement> superCalls = new HashSet<>();

private boolean resolveFunctionizable(Map<ExecutableElement, MethodInfo> infoMap) {
for (ExecutableElement superCall : superCalls) {
MethodInfo superCallInfo = infoMap.get(superCall);
if (superCallInfo == null || !superCallInfo.isFunctionizable(infoMap)) {
return false;
}
}
return true;
}

@Override
public void endVisit(MethodInvocation node) {
invocations.add(node.getExecutableElement());
private boolean isFunctionizable(Map<ExecutableElement, MethodInfo> infoMap) {
if (functionizable == null) {
functionizable = resolveFunctionizable(infoMap);
}
});
return Sets.intersection(functionizableDeclarations, invocations);
}
return functionizable;
}

@Override
public boolean visit(AnnotationTypeDeclaration node) {
return false;
private void addSuperCall(ExecutableElement method) {
superCalls.add(method);
}
}

@Override
public boolean visit(NormalAnnotation node) {
return false;
}
private static class FunctionizableFinder extends TreeVisitor {

@Override
public boolean visit(SingleMemberAnnotation node) {
return false;
// Don't need a stack here because local types have already been extracted.
private MethodInfo currentMethod = null;
private Map<ExecutableElement, MethodInfo> infoMap = new HashMap<>();
private Set<ExecutableElement> invocations = new HashSet<>();

@Override
public boolean visit(MethodDeclaration node) {
if (isFunctionizingCandidate(node)) {
infoMap.put(node.getExecutableElement(), currentMethod = new MethodInfo());
}
return true;
}

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

@Override
public void endVisit(MethodInvocation node) {
ExecutableElement method = node.getExecutableElement();
// Regular invocations can only be functionized if the target is private or final, otherwise
// the target method might be overridden by a subclass.
if (isNonVirtual(method)) {
invocations.add(method);
}
}

@Override
public void endVisit(SuperMethodInvocation node) {
ExecutableElement method = node.getExecutableElement();
// Super invocations can always be functionized because it is not a "virtual" invocation.
invocations.add(method);

if (currentMethod != null) {
currentMethod.addSuperCall(method);
}
}

private Set<ExecutableElement> getFunctionizableMethods() {
Iterator<ExecutableElement> iter = invocations.iterator();
while (iter.hasNext()) {
ExecutableElement method = iter.next();
MethodInfo info = infoMap.get(method);
if (info == null || !info.isFunctionizable(infoMap)) {
iter.remove();
}
}
return invocations;
}
}

/**
* Determines whether an instance method can be functionized.
*/
private boolean canFunctionize(MethodDeclaration node) {
ExecutableElement m = node.getExecutableElement();
private static boolean isFunctionizingCandidate(MethodDeclaration node) {
ExecutableElement method = node.getExecutableElement();
int modifiers = node.getModifiers();

// Never functionize these types of methods.
if (Modifier.isStatic(modifiers) || Modifier.isAbstract(modifiers) || !node.hasDeclaration()
|| ElementUtil.isAnnotationMember(m)) {
// Default methods, static methods and constructors are always functionized. We only care about
// regular instance methods.
if (!ElementUtil.isInstanceMethod(method) || ElementUtil.isDefault(method)
|| Modifier.isAbstract(modifiers) || !node.hasDeclaration()) {
return false;
}

// Don't functionize equals/hash, since they are often called by collections.
String name = ElementUtil.getName(m);
if ((name.equals("hashCode") && m.getParameters().isEmpty())
|| (name.equals("equals") && m.getParameters().size() == 1)) {
String name = ElementUtil.getName(method);
if ((name.equals("hashCode") && method.getParameters().isEmpty())
|| (name.equals("equals") && method.getParameters().size() == 1)) {
return false;
}

if (!ElementUtil.isPrivate(m) && !ElementUtil.isFinal(m)) {
return false;
}

return !hasSuperMethodInvocation(node);
}

private static boolean hasSuperMethodInvocation(MethodDeclaration node) {
final boolean[] result = new boolean[1];
result[0] = false;
node.accept(new TreeVisitor() {
@Override
public void endVisit(SuperMethodInvocation node) {
result[0] = true;
}
});
return result[0];
return true;
}

private FunctionElement newFunctionElement(ExecutableElement method) {
Expand Down Expand Up @@ -199,41 +229,40 @@ private void transferParams(
}
}

@Override
public void endVisit(MethodInvocation node) {
ExecutableElement element = node.getExecutableElement();
if (!ElementUtil.isStatic(element) && !functionizableMethods.contains(element)) {
return;
}

private void functionizeInvocation(
Expression node, ExecutableElement element, Expression receiver,
List<Expression> methodArgs) {
FunctionInvocation functionInvocation =
new FunctionInvocation(newFunctionElement(element), node.getTypeMirror());
List<Expression> args = functionInvocation.getArguments();
TreeUtil.moveList(node.getArguments(), args);
List<Expression> funcArgs = functionInvocation.getArguments();
TreeUtil.moveList(methodArgs, funcArgs);

if (!ElementUtil.isStatic(element)) {
Expression expr = node.getExpression();
if (expr == null) {
expr = new ThisExpression(TreeUtil.getEnclosingTypeElement(node).asType());
if (receiver == null) {
receiver = new ThisExpression(TreeUtil.getEnclosingTypeElement(node).asType());
}
args.add(0, TreeUtil.remove(expr));
funcArgs.add(0, TreeUtil.remove(receiver));
}

node.replaceWith(functionInvocation);
}

@Override
public void endVisit(SuperMethodInvocation node) {
ExecutableElement element = node.getExecutableElement();
// Yes, super method invocations can be static.
if (!ElementUtil.isStatic(element)) {
return;
public void endVisit(MethodInvocation node) {
ExecutableElement method = node.getExecutableElement();
if (ElementUtil.isStatic(method)
|| (functionizableMethods.contains(method) && isNonVirtual(method))) {
functionizeInvocation(node, method, node.getExpression(), node.getArguments());
}
}

FunctionInvocation functionInvocation =
new FunctionInvocation(newFunctionElement(element), node.getTypeMirror());
TreeUtil.moveList(node.getArguments(), functionInvocation.getArguments());
node.replaceWith(functionInvocation);
@Override
public void endVisit(SuperMethodInvocation node) {
ExecutableElement method = node.getExecutableElement();
if (ElementUtil.isStatic(method) || functionizableMethods.contains(method)
|| ElementUtil.isDefault(method)) {
functionizeInvocation(node, method, node.getReceiver(), node.getArguments());
}
}

@Override
Expand Down Expand Up @@ -339,9 +368,12 @@ public void endVisit(MethodDeclaration node) {
node.setBody(null);
node.addModifiers(Modifier.ABSTRACT);
} else if (isInstanceMethod) {
// Instance methods must be kept in case they are invoked using "super".
// TODO(kstanger): Is this true for final methods?
setFunctionCaller(node, element);
// We can remove private instance methods if reflection is stripped.
if (translationUtil.needsReflection(declaringClass) || !ElementUtil.isPrivate(element)) {
setFunctionCaller(node, element);
} else {
node.remove();
}
} else if (isEnumConstructor) {
// Enum constructors are never needed.
node.remove();
Expand Down Expand Up @@ -598,4 +630,19 @@ public void endVisit(SuperMethodInvocation node) {
}
}
}

@Override
public boolean visit(AnnotationTypeDeclaration node) {
return false;
}

@Override
public boolean visit(NormalAnnotation node) {
return false;
}

@Override
public boolean visit(SingleMemberAnnotation node) {
return false;
}
}
20 changes: 1 addition & 19 deletions ...tor/src/main/java/com/google/devtools/j2objc/translate/SuperMethodInvocationRewriter.java
View file
Expand Up @@ -24,7 +24,6 @@
import com.google.devtools.j2objc.ast.NativeExpression;
import com.google.devtools.j2objc.ast.NativeStatement;
import com.google.devtools.j2objc.ast.SuperMethodInvocation;
import com.google.devtools.j2objc.ast.ThisExpression;
import com.google.devtools.j2objc.ast.TreeUtil;
import com.google.devtools.j2objc.ast.TypeDeclaration;
import com.google.devtools.j2objc.ast.UnitTreeVisitor;
Expand Down Expand Up @@ -105,24 +104,7 @@ public void endVisit(SuperMethodInvocation node) {
ExecutableElement method = node.getExecutableElement();
TypeMirror exprType = node.getTypeMirror();

// Handle default method invocation: SomeInterface.super.method(...)
if (ElementUtil.isDefault(method)) {
FunctionElement element = new FunctionElement(
nameTable.getFullFunctionName(method), exprType, ElementUtil.getDeclaringClass(method))
.addParameters(TypeUtil.ID_TYPE)
.addParameters(ElementUtil.asTypes(method.getParameters()));
FunctionInvocation invocation = new FunctionInvocation(element, exprType);
List<Expression> args = invocation.getArguments();
if (receiver == null) {
args.add(new ThisExpression(TreeUtil.getEnclosingTypeElement(node).asType()));
} else {
// OuterReferenceResolver has provided an outer path.
args.add(TreeUtil.remove(receiver));
}
TreeUtil.copyList(node.getArguments(), args);
node.replaceWith(invocation);
return;
}
assert !ElementUtil.isDefault(method) : "Default methods are handled in Functionizer.java";

if (receiver == null) {
return;
Expand Down
5 changes: 2 additions & 3 deletions translator/src/test/java/com/google/devtools/j2objc/ast/MethodReferenceTest.java
View file
Expand Up @@ -283,10 +283,9 @@ public void testQualifiedSuperMethodReference() throws IOException {
+ "class Test { void foo() {} static class TestSub extends Test { void foo() {}"
+ "class Inner { I test() { return TestSub.super::foo; } } } }",
"Test", "Test.m");
assertTranslatedSegments(translation,
"static void (*Test_TestSub_super$_foo)(id, SEL);",
assertTranslatedLines(translation,
"- (void)bar {",
" Test_TestSub_super$_foo(this$0_->this$0_, @selector(foo));",
" Test_foo(this$0_->this$0_);",
"}");
}

Expand Down
4 changes: 2 additions & 2 deletions ...r/src/test/java/com/google/devtools/j2objc/gen/ObjectiveCImplementationGeneratorTest.java
View file
Expand Up @@ -181,9 +181,9 @@ public void testNSObjectMessageQualifiedNameRename() throws IOException {
}

public void testNSObjectMessageSuperRename() throws IOException {
addSourceFile("public class SuperClass { int load() { return 1; }}", "SuperClass.java");
String translation = translateSourceFile(
"public class Example { int load() { return 1; }} "
+ "class SubClass extends Example { int load() { return super.load(); }}",
"class Example extends SuperClass { int load() { return super.load(); }}",
"Example", "Example.m");
assertTranslation(translation, "return [super load__];");
}
Expand Down
2 changes: 1 addition & 1 deletion translator/src/test/java/com/google/devtools/j2objc/translate/AutoboxerTest.java
View file
Expand Up @@ -391,7 +391,7 @@ public void testInvokeSuperMethodAutoboxing() throws IOException {
+ "public class Test extends Base {"
+ "@Override public void print(Object o) { super.print(123.456f); }}", "Test", "Test.m");
assertTranslation(translation,
"[super printWithId:JavaLangFloat_valueOfWithFloat_(123.456f)];");
"Base_printWithId_(self, JavaLangFloat_valueOfWithFloat_(123.456f));");
}

public void testAssignIntLiteralToNonIntBoxedType() throws Exception {
Expand Down
11 changes: 6 additions & 5 deletions translator/src/test/java/com/google/devtools/j2objc/translate/FunctionizerTest.java
View file
Expand Up @@ -107,7 +107,8 @@ public void testSuperFieldAccessInFunction() throws IOException {
"return self->hello_;");
}

// Verify there isn't any super method invocations in functions.
// Verify a method with super invocations can be funcitonized if those super invocations can also
// be functionized.
public void testSuperMethodInvocationInFunction() throws IOException {
String translation = translateSourceFile(
"class A { "
Expand All @@ -120,11 +121,11 @@ public void testSuperMethodInvocationInFunction() throws IOException {
+ " void use() { test1(); test2(); }}}",
"A", "A.m");
assertTranslatedLines(translation,
"- (NSString *)test1 {",
"return [super hello];");
"NSString *A_B_test1(A_B *self) {",
"return A_hello(self);");
assertTranslatedLines(translation,
"- (NSString *)test2 {",
"return [super shout];");
"NSString *A_B_test2(A_B *self) {",
"return A_shout(self);");
}

// Verify functions can call other functions, correctly passing the instance variable.
Expand Down

0 comments on commit 03b33a0

Please sign in to comment.