Skip to content

Commit

Permalink
Adds a nonnull attribute to GeneratedVariableElement so that generate…
Browse files Browse the repository at this point in the history 
…d variables can avoid being nil_chk'ed.

Converts NilCheckResolver to use javac elements.

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

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=134668640
  • Loading branch information
@kstanger @tomball
kstanger authored and tomball committed Sep 29, 2016
1 parent b4bf716 commit 5bc691f
Show file tree
Hide file tree
Showing 10 changed files with 137 additions and 71 deletions.
11 changes: 8 additions & 3 deletions translator/src/main/java/com/google/devtools/j2objc/ast/TreeUtil.java
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import javax.lang.model.element.Element; import javax.lang.model.element.Element;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.TypeElement; import javax.lang.model.element.TypeElement;
import javax.lang.model.element.VariableElement; import javax.lang.model.element.VariableElement;
import javax.lang.model.type.TypeMirror; import javax.lang.model.type.TypeMirror;
Expand Down Expand Up @@ -292,13 +293,17 @@ public static VariableElement getVariableElement(Name node) {
} }


public static IMethodBinding getMethodBinding(Expression node) { public static IMethodBinding getMethodBinding(Expression node) {
return BindingConverter.unwrapExecutableElement(getExecutableElement(node));
}

public static ExecutableElement getExecutableElement(Expression node) {
switch (node.getKind()) { switch (node.getKind()) {
case CLASS_INSTANCE_CREATION: case CLASS_INSTANCE_CREATION:
return ((ClassInstanceCreation) node).getMethodBinding(); return ((ClassInstanceCreation) node).getExecutableElement();
case METHOD_INVOCATION: case METHOD_INVOCATION:
return ((MethodInvocation) node).getMethodBinding(); return ((MethodInvocation) node).getExecutableElement();
case SUPER_METHOD_INVOCATION: case SUPER_METHOD_INVOCATION:
return ((SuperMethodInvocation) node).getMethodBinding(); return ((SuperMethodInvocation) node).getExecutableElement();
default: default:
return null; return null;
} }
Expand Down
1 change: 1 addition & 0 deletions translator/src/main/java/com/google/devtools/j2objc/jdt/BindingConverter.java
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ public static IBinding unwrapElement(Element element) {
element.getKind() == ElementKind.PARAMETER, element.getKind() == ElementKind.PARAMETER,
possibleEnclosing != null ? possibleEnclosing.asType() : null, null); possibleEnclosing != null ? possibleEnclosing.asType() : null, null);
newBinding.addAnnotations(element.getAnnotationMirrors()); newBinding.addAnnotations(element.getAnnotationMirrors());
newBinding.setNonnull(((GeneratedVariableElement) element).isNonnull());
return newBinding; return newBinding;
} }
if (element instanceof GeneratedExecutableElement) { if (element instanceof GeneratedExecutableElement) {
Expand Down
121 changes: 59 additions & 62 deletions translator/src/main/java/com/google/devtools/j2objc/translate/NilCheckResolver.java
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -57,18 +57,19 @@
import com.google.devtools.j2objc.ast.VariableDeclarationFragment; import com.google.devtools.j2objc.ast.VariableDeclarationFragment;
import com.google.devtools.j2objc.ast.WhileStatement; import com.google.devtools.j2objc.ast.WhileStatement;
import com.google.devtools.j2objc.types.FunctionBinding; import com.google.devtools.j2objc.types.FunctionBinding;
import com.google.devtools.j2objc.util.BindingUtil;
import com.google.devtools.j2objc.util.ElementUtil; import com.google.devtools.j2objc.util.ElementUtil;
import com.google.devtools.j2objc.util.TypeUtil;
import java.util.Collections; import java.util.Collections;
import java.util.HashMap; import java.util.HashMap;
import java.util.HashSet; import java.util.HashSet;
import java.util.Iterator; import java.util.Iterator;
import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.TypeElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.TypeMirror; import javax.lang.model.type.TypeMirror;
import org.eclipse.jdt.core.dom.IMethodBinding;
import org.eclipse.jdt.core.dom.ITypeBinding;
import org.eclipse.jdt.core.dom.IVariableBinding;


/** /**
* Adds nil_chk calls where required to maintain compatibility Java's * Adds nil_chk calls where required to maintain compatibility Java's
Expand All @@ -85,13 +86,13 @@ public class NilCheckResolver extends TreeVisitor {


// These sets are used to pass down to parent nodes the set of variables that // These sets are used to pass down to parent nodes the set of variables that
// are safe given that the expression is true or false. // are safe given that the expression is true or false.
private Set<IVariableBinding> safeVarsTrue = null; private Set<VariableElement> safeVarsTrue = null;
private Set<IVariableBinding> safeVarsFalse = null; private Set<VariableElement> safeVarsFalse = null;
// Identifies the node from which safeVarsTrue and safeVarsFalse have been // Identifies the node from which safeVarsTrue and safeVarsFalse have been
// assigned. // assigned.
private Expression conditionalSafeVarsNode = null; private Expression conditionalSafeVarsNode = null;


private static final Set<IVariableBinding> EMPTY_VARS = Collections.emptySet(); private static final Set<VariableElement> EMPTY_VARS = Collections.emptySet();


/** /**
* A stack element that tracks which variables are safe and don't need a * A stack element that tracks which variables are safe and don't need a
Expand All @@ -109,10 +110,10 @@ private enum Kind {
// Indicates that control flow does not continue through the end of this // Indicates that control flow does not continue through the end of this
// scope because of a return, throw, break or continue. // scope because of a return, throw, break or continue.
private boolean terminates = false; private boolean terminates = false;
private final Map<IVariableBinding, Boolean> vars = new HashMap<>(); private final Map<VariableElement, Boolean> vars = new HashMap<>();
// Saves unsafe vars to be applied the next time this scope becomes the top // Saves unsafe vars to be applied the next time this scope becomes the top
// of the stack. // of the stack.
private Map<IVariableBinding, Boolean> mergedVars = null; private Map<VariableElement, Boolean> mergedVars = null;


private Scope(Scope next, Kind kind, String label) { private Scope(Scope next, Kind kind, String label) {
this.next = next; this.next = next;
Expand All @@ -125,17 +126,17 @@ private Scope(Scope next, Kind kind, String label) {
} }
} }


private void mergeVars(Map<IVariableBinding, Boolean> varsToMerge) { private void mergeVars(Map<VariableElement, Boolean> varsToMerge) {
if (mergedVars == null) { if (mergedVars == null) {
mergedVars = new HashMap<>(); mergedVars = new HashMap<>();
mergedVars.putAll(varsToMerge); mergedVars.putAll(varsToMerge);
terminates = false; terminates = false;
return; return;
} }
// Remove any safe variables that aren't in both maps. // Remove any safe variables that aren't in both maps.
Iterator<Map.Entry<IVariableBinding, Boolean>> iter = mergedVars.entrySet().iterator(); Iterator<Map.Entry<VariableElement, Boolean>> iter = mergedVars.entrySet().iterator();
while (iter.hasNext()) { while (iter.hasNext()) {
Map.Entry<IVariableBinding, Boolean> entry = iter.next(); Map.Entry<VariableElement, Boolean> entry = iter.next();
if (entry.getValue()) { if (entry.getValue()) {
Boolean mergedValue = varsToMerge.get(entry.getKey()); Boolean mergedValue = varsToMerge.get(entry.getKey());
if (mergedValue == null || !mergedValue) { if (mergedValue == null || !mergedValue) {
Expand All @@ -144,25 +145,25 @@ private void mergeVars(Map<IVariableBinding, Boolean> varsToMerge) {
} }
} }
// Add any unsafe variable from the merging map. // Add any unsafe variable from the merging map.
for (Map.Entry<IVariableBinding, Boolean> entry : varsToMerge.entrySet()) { for (Map.Entry<VariableElement, Boolean> entry : varsToMerge.entrySet()) {
if (!entry.getValue()) { if (!entry.getValue()) {
mergedVars.put(entry.getKey(), false); mergedVars.put(entry.getKey(), false);
} }
} }
} }


private void mergeVars(Set<IVariableBinding> varsToMerge) { private void mergeVars(Set<VariableElement> varsToMerge) {
mergeVars(Maps.asMap(varsToMerge, Functions.constant(true))); mergeVars(Maps.asMap(varsToMerge, Functions.constant(true)));
} }


private void mergeInto(Scope targetScope, Set<IVariableBinding> extraVars) { private void mergeInto(Scope targetScope, Set<VariableElement> extraVars) {
Map<IVariableBinding, Boolean> vars = new HashMap<>(); Map<VariableElement, Boolean> vars = new HashMap<>();
for (IVariableBinding var : extraVars) { for (VariableElement var : extraVars) {
vars.put(var, true); vars.put(var, true);
} }
Scope curScope = this; Scope curScope = this;
while (curScope != targetScope) { while (curScope != targetScope) {
for (Map.Entry<IVariableBinding, Boolean> entry : curScope.vars.entrySet()) { for (Map.Entry<VariableElement, Boolean> entry : curScope.vars.entrySet()) {
if (!vars.containsKey(entry.getKey())) { if (!vars.containsKey(entry.getKey())) {
vars.put(entry.getKey(), entry.getValue()); vars.put(entry.getKey(), entry.getValue());
} }
Expand Down Expand Up @@ -190,7 +191,7 @@ private void backwardMerge() {
vars.clear(); vars.clear();
terminates = false; terminates = false;
} else { } else {
Iterator<Map.Entry<IVariableBinding, Boolean>> iter = vars.entrySet().iterator(); Iterator<Map.Entry<VariableElement, Boolean>> iter = vars.entrySet().iterator();
while (iter.hasNext()) { while (iter.hasNext()) {
if (iter.next().getValue()) { if (iter.next().getValue()) {
iter.remove(); iter.remove();
Expand Down Expand Up @@ -254,42 +255,42 @@ private void popAndMerge() {
} }


private void setConditionalSafeVars( private void setConditionalSafeVars(
Expression node, Set<IVariableBinding> newSafeVarsTrue, Expression node, Set<VariableElement> newSafeVarsTrue,
Set<IVariableBinding> newSafeVarsFalse) { Set<VariableElement> newSafeVarsFalse) {
conditionalSafeVarsNode = node; conditionalSafeVarsNode = node;
safeVarsTrue = newSafeVarsTrue; safeVarsTrue = newSafeVarsTrue;
safeVarsFalse = newSafeVarsFalse; safeVarsFalse = newSafeVarsFalse;
} }


private Set<IVariableBinding> getSafeVarsTrue(Expression expr) { private Set<VariableElement> getSafeVarsTrue(Expression expr) {
if (expr == conditionalSafeVarsNode) { if (expr == conditionalSafeVarsNode) {
return safeVarsTrue; return safeVarsTrue;
} }
return EMPTY_VARS; return EMPTY_VARS;
} }


private Set<IVariableBinding> getSafeVarsFalse(Expression expr) { private Set<VariableElement> getSafeVarsFalse(Expression expr) {
if (expr == conditionalSafeVarsNode) { if (expr == conditionalSafeVarsNode) {
return safeVarsFalse; return safeVarsFalse;
} }
return EMPTY_VARS; return EMPTY_VARS;
} }


private void addSafeVar(IVariableBinding var) { private void addSafeVar(VariableElement var) {
if (scope != null) { if (scope != null) {
scope.vars.put(var, true); scope.vars.put(var, true);
} }
} }


private void addSafeVars(Set<IVariableBinding> vars) { private void addSafeVars(Set<VariableElement> vars) {
if (scope != null && vars != null) { if (scope != null && vars != null) {
for (IVariableBinding var : vars) { for (VariableElement var : vars) {
scope.vars.put(var, true); scope.vars.put(var, true);
} }
} }
} }


private void removeSafeVar(IVariableBinding var) { private void removeSafeVar(VariableElement var) {
if (scope != null) { if (scope != null) {
scope.vars.put(var, false); scope.vars.put(var, false);
} }
Expand All @@ -301,8 +302,8 @@ private void removeNonFinalFields() {
} }
Scope curScope = scope; Scope curScope = scope;
while (curScope != null) { while (curScope != null) {
for (IVariableBinding var : curScope.vars.keySet()) { for (VariableElement var : curScope.vars.keySet()) {
if (var.isField() && !BindingUtil.isFinal(var)) { if (var.getKind().isField() && !ElementUtil.isFinal(var)) {
scope.vars.put(var, false); scope.vars.put(var, false);
} }
} }
Expand All @@ -322,7 +323,7 @@ private void handleThrows() {
} }
} }


private boolean isSafeVar(IVariableBinding var) { private boolean isSafeVar(VariableElement var) {
Scope curScope = scope; Scope curScope = scope;
while (curScope != null) { while (curScope != null) {
Boolean result = curScope.vars.get(var); Boolean result = curScope.vars.get(var);
Expand Down Expand Up @@ -354,37 +355,34 @@ private Scope findScope(Scope.Kind kind, String label) {
} }


// Checks if the given method is a primitive boxing or unboxing method. // Checks if the given method is a primitive boxing or unboxing method.
private boolean isBoxingMethod(IMethodBinding method) { private boolean isBoxingMethod(ExecutableElement method) {
ITypeBinding declaringClass = method.getDeclaringClass(); TypeElement declaringClass = ElementUtil.getDeclaringClass(method);
// Autoboxing methods. // Autoboxing methods.
if (typeEnv.isBoxedPrimitive(declaringClass)) { if (typeEnv.isBoxedPrimitive(declaringClass)) {
String name = method.getName(); String name = ElementUtil.getName(method);
ITypeBinding returnType = method.getReturnType(); TypeMirror returnType = method.getReturnType();
ITypeBinding[] paramTypes = method.getParameterTypes(); List<? extends VariableElement> params = method.getParameters();
if (name.equals("valueOf") && paramTypes.length == 1 && paramTypes[0].isPrimitive()) { if (name.equals("valueOf") && params.size() == 1
&& params.get(0).asType().getKind().isPrimitive()) {
return true; return true;
} }
if (returnType.isPrimitive() && name.equals(returnType.getName() + "Value") if (params.isEmpty() && returnType.getKind().isPrimitive()
&& paramTypes.length == 0) { && name.equals(TypeUtil.getName(returnType) + "Value")) {
return true; return true;
} }
} }
return false; return false;
} }


private boolean needsNilCheck(Expression e) { private boolean needsNilCheck(Expression e) {
IVariableBinding sym = TreeUtil.getVariableBinding(e); VariableElement sym = TreeUtil.getVariableElement(e);
if (sym != null) { if (sym != null) {
// The target in a method reference is checked upon creation. return !ElementUtil.isNonnull(sym) && (ElementUtil.isVolatile(sym) || !isSafeVar(sym));
if (sym.getName().equals("target$")) {
return false;
}
return BindingUtil.isVolatile(sym) || !isSafeVar(sym);
} }
IMethodBinding method = TreeUtil.getMethodBinding(e); ExecutableElement method = TreeUtil.getExecutableElement(e);
if (method != null) { if (method != null) {
// Check for some common cases where the result is known not to be null. // Check for some common cases where the result is known not to be null.
return !method.isConstructor() && !method.getName().equals("getClass") return !ElementUtil.isConstructor(method) && !ElementUtil.getName(method).equals("getClass")
&& !isBoxingMethod(method); && !isBoxingMethod(method);
} }
switch (e.getKind()) { switch (e.getKind()) {
Expand All @@ -405,7 +403,7 @@ private void addNilCheck(Expression node) {
if (!needsNilCheck(node)) { if (!needsNilCheck(node)) {
return; return;
} }
IVariableBinding var = TreeUtil.getVariableBinding(node); VariableElement var = TreeUtil.getVariableElement(node);
if (var != null) { if (var != null) {
addSafeVar(var); addSafeVar(var);
} }
Expand All @@ -432,18 +430,17 @@ public void endVisit(FieldAccess node) {


@Override @Override
public boolean visit(MethodInvocation node) { public boolean visit(MethodInvocation node) {
IMethodBinding binding = node.getMethodBinding();
Expression receiver = node.getExpression(); Expression receiver = node.getExpression();
if (receiver != null) { if (receiver != null) {
receiver.accept(this); receiver.accept(this);
if (!BindingUtil.isStatic(binding)) { if (!ElementUtil.isStatic(node.getExecutableElement())) {
addNilCheck(receiver); addNilCheck(receiver);
} }
} }
for (Expression arg : node.getArguments()) { for (Expression arg : node.getArguments()) {
arg.accept(this); arg.accept(this);
} }
if (!isBoxingMethod(node.getMethodBinding())) { if (!isBoxingMethod(node.getExecutableElement())) {
removeNonFinalFields(); removeNonFinalFields();
handleThrows(); handleThrows();
} }
Expand Down Expand Up @@ -525,8 +522,8 @@ public boolean visit(ConditionalExpression node) {


private boolean handleConditional(Expression expr, TreeNode thenNode, TreeNode elseNode) { private boolean handleConditional(Expression expr, TreeNode thenNode, TreeNode elseNode) {
expr.accept(this); expr.accept(this);
Set<IVariableBinding> safeVarsThen = getSafeVarsTrue(expr); Set<VariableElement> safeVarsThen = getSafeVarsTrue(expr);
Set<IVariableBinding> safeVarsElse = getSafeVarsFalse(expr); Set<VariableElement> safeVarsElse = getSafeVarsFalse(expr);
Scope originalScope = scope; Scope originalScope = scope;
pushScope(); pushScope();
addSafeVars(safeVarsThen); addSafeVars(safeVarsThen);
Expand Down Expand Up @@ -555,11 +552,11 @@ public boolean visit(InfixExpression node) {
if (equals || notEquals) { if (equals || notEquals) {
Expression lhs = node.getOperand(0); Expression lhs = node.getOperand(0);
Expression rhs = node.getOperand(1); Expression rhs = node.getOperand(1);
IVariableBinding maybeNullVar = null; VariableElement maybeNullVar = null;
if (lhs instanceof NullLiteral) { if (lhs instanceof NullLiteral) {
maybeNullVar = TreeUtil.getVariableBinding(rhs); maybeNullVar = TreeUtil.getVariableElement(rhs);
} else if (rhs instanceof NullLiteral) { } else if (rhs instanceof NullLiteral) {
maybeNullVar = TreeUtil.getVariableBinding(lhs); maybeNullVar = TreeUtil.getVariableElement(lhs);
} }
if (maybeNullVar != null) { if (maybeNullVar != null) {
if (equals) { if (equals) {
Expand All @@ -573,14 +570,14 @@ public boolean visit(InfixExpression node) {
} }


private boolean handleConditionalOperator(InfixExpression node, boolean logicalAnd) { private boolean handleConditionalOperator(InfixExpression node, boolean logicalAnd) {
Set<IVariableBinding> newSafeVars = new HashSet<>(); Set<VariableElement> newSafeVars = new HashSet<>();
int pushCount = 0; int pushCount = 0;
for (Iterator<Expression> it = node.getOperands().iterator(); it.hasNext(); ) { for (Iterator<Expression> it = node.getOperands().iterator(); it.hasNext(); ) {
Expression operand = it.next(); Expression operand = it.next();
operand.accept(this); operand.accept(this);
Set<IVariableBinding> safeVarsForBranch = Set<VariableElement> safeVarsForBranch =
logicalAnd ? getSafeVarsTrue(operand) : getSafeVarsFalse(operand); logicalAnd ? getSafeVarsTrue(operand) : getSafeVarsFalse(operand);
Set<IVariableBinding> safeVarsForMerge = Set<VariableElement> safeVarsForMerge =
logicalAnd ? getSafeVarsFalse(operand) : getSafeVarsTrue(operand); logicalAnd ? getSafeVarsFalse(operand) : getSafeVarsTrue(operand);
newSafeVars.addAll(safeVarsForBranch); newSafeVars.addAll(safeVarsForBranch);
if (it.hasNext()) { if (it.hasNext()) {
Expand All @@ -598,7 +595,7 @@ private boolean handleConditionalOperator(InfixExpression node, boolean logicalA
return false; return false;
} }


private void handleAssignment(IVariableBinding var, Expression value) { private void handleAssignment(VariableElement var, Expression value) {
if (needsNilCheck(value)) { if (needsNilCheck(value)) {
removeSafeVar(var); removeSafeVar(var);
} else { } else {
Expand All @@ -609,7 +606,7 @@ private void handleAssignment(IVariableBinding var, Expression value) {
@Override @Override
public void endVisit(Assignment node) { public void endVisit(Assignment node) {
if (node.getOperator() == Assignment.Operator.ASSIGN) { if (node.getOperator() == Assignment.Operator.ASSIGN) {
IVariableBinding var = TreeUtil.getVariableBinding(node.getLeftHandSide()); VariableElement var = TreeUtil.getVariableElement(node.getLeftHandSide());
if (var != null) { if (var != null) {
handleAssignment(var, node.getRightHandSide()); handleAssignment(var, node.getRightHandSide());
} }
Expand All @@ -620,7 +617,7 @@ public void endVisit(Assignment node) {
public void endVisit(VariableDeclarationFragment node) { public void endVisit(VariableDeclarationFragment node) {
Expression initializer = node.getInitializer(); Expression initializer = node.getInitializer();
if (initializer != null) { if (initializer != null) {
handleAssignment(node.getVariableBinding(), initializer); handleAssignment(node.getVariableElement(), initializer);
} }
} }


Expand Down Expand Up @@ -783,7 +780,7 @@ public void endVisit(ReturnStatement node) {
@Override @Override
public void endVisit(FunctionInvocation node) { public void endVisit(FunctionInvocation node) {
if (node.getName().equals("nil_chk")) { if (node.getName().equals("nil_chk")) {
IVariableBinding var = TreeUtil.getVariableBinding(node.getArgument(0)); VariableElement var = TreeUtil.getVariableElement(node.getArgument(0));
if (var != null) { if (var != null) {
addSafeVar(var); addSafeVar(var);
} }
Expand Down
Loading

0 comments on commit 5bc691f

Please sign in to comment.