Skip to content

Commit 415328c

Browse files
rlublecopybara-github
rluble
authored and
copybara-github
committed
Issue #2580: Fix the translation of instanceof patterns to support the general form.
Introduce a pass to normalize isntanceof patterns out instead of handling special cases in the initial construction of the AST. PiperOrigin-RevId: 822414708
1 parent 883105d commit 415328c

File tree

9 files changed

+314
-132
lines changed

9 files changed

+314
-132
lines changed

‎translator/src/main/java/com/google/devtools/j2objc/ast/CastExpression.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.devtools.j2objc.ast;
1616

17+
import com.google.errorprone.annotations.CanIgnoreReturnValue;
1718
import javax.lang.model.type.TypeMirror;
1819

1920
/**
@@ -54,6 +55,7 @@ public Type getType() {
5455
return type.get();
5556
}
5657

58+
@CanIgnoreReturnValue
5759
public CastExpression setType(Type newType) {
5860
type.set(newType);
5961
return this;
@@ -63,6 +65,7 @@ public Expression getExpression() {
6365
return expression.get();
6466
}
6567

68+
@CanIgnoreReturnValue
6669
public CastExpression setExpression(Expression newExpression) {
6770
expression.set(newExpression);
6871
return this;
@@ -72,8 +75,10 @@ public boolean needsCastChk() {
7275
return needsCastChk;
7376
}
7477

75-
public void setNeedsCastChk(boolean value) {
78+
@CanIgnoreReturnValue
79+
public CastExpression setNeedsCastChk(boolean value) {
7680
needsCastChk = value;
81+
return this;
7782
}
7883

7984
@Override

‎translator/src/main/java/com/google/devtools/j2objc/ast/SimpleName.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package com.google.devtools.j2objc.ast;
1616

1717
import com.google.common.base.Preconditions;
18+
import com.google.errorprone.annotations.CanIgnoreReturnValue;
1819
import javax.lang.model.element.Element;
1920
import javax.lang.model.type.TypeMirror;
2021

@@ -68,6 +69,7 @@ public String getIdentifier() {
6869
return identifier;
6970
}
7071

72+
@CanIgnoreReturnValue
7173
public SimpleName setIdentifier(String identifier) {
7274
this.identifier = identifier;
7375
return this;

‎translator/src/main/java/com/google/devtools/j2objc/javac/TreeConverter.java‎

Lines changed: 4 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,102 +1017,10 @@ private TreeNode convertIdent(IdentifierTree node, TreePath parent) {
10171017

10181018
private TreeNode convertIf(IfTree node, TreePath parent) {
10191019
TreePath path = getTreePath(parent, node);
1020-
Expression condition = convertWithoutParens(node.getCondition(), path);
1021-
Pattern pattern = null;
1022-
InstanceofExpression instanceofExpr = null;
1023-
Expression subExpr = null;
1024-
boolean negate = false;
1025-
if (condition.getKind() == TreeNode.Kind.INSTANCEOF_EXPRESSION) {
1026-
instanceofExpr = (InstanceofExpression) condition;
1027-
pattern = instanceofExpr.getPattern();
1028-
instanceofExpr.setPattern(null);
1029-
} else if (condition.getKind() == TreeNode.Kind.PREFIX_EXPRESSION) {
1030-
PrefixExpression prefixExpr = (PrefixExpression) condition;
1031-
Expression operand = prefixExpr.getOperand();
1032-
if (operand.getKind() == TreeNode.Kind.PARENTHESIZED_EXPRESSION) {
1033-
operand = ((ParenthesizedExpression) operand).getExpression();
1034-
}
1035-
if (operand.getKind() == TreeNode.Kind.INSTANCEOF_EXPRESSION) {
1036-
instanceofExpr = (InstanceofExpression) operand;
1037-
pattern = instanceofExpr.getPattern();
1038-
instanceofExpr.setPattern(null);
1039-
negate = true;
1040-
}
1041-
} else if (condition.getKind() == TreeNode.Kind.INFIX_EXPRESSION) {
1042-
InfixExpression infixExpr = (InfixExpression) condition;
1043-
List<Expression> operands = infixExpr.getOperands();
1044-
for (int i = 0; i < operands.size(); i++) {
1045-
Expression operand = operands.get(i);
1046-
if (operand.getKind() == TreeNode.Kind.INSTANCEOF_EXPRESSION) {
1047-
instanceofExpr = (InstanceofExpression) operand;
1048-
pattern = instanceofExpr.getPattern();
1049-
if (pattern != null) {
1050-
instanceofExpr.setPattern(null);
1051-
condition = instanceofExpr.copy();
1052-
List<Expression> subOperands = new ArrayList<>();
1053-
while (++i < operands.size()) {
1054-
subOperands.add(operands.get(i));
1055-
}
1056-
if (subOperands.size() == 1) {
1057-
subExpr = subOperands.get(0).copy();
1058-
} else if (subOperands.size() > 1) {
1059-
InfixExpression newInfix =
1060-
new InfixExpression(infixExpr.getTypeMirror(), infixExpr.getOperator());
1061-
for (Expression subOperand : subOperands) {
1062-
newInfix.addOperand(subOperand.copy());
1063-
}
1064-
subExpr = newInfix;
1065-
}
1066-
break;
1067-
} else {
1068-
// No pattern, reset instanceofExpr and pattern.
1069-
instanceofExpr = null;
1070-
pattern = null;
1071-
}
1072-
}
1073-
}
1074-
}
1075-
1076-
Statement thenStatement = (Statement) convert(node.getThenStatement(), path);
1077-
if (pattern != null) {
1078-
// Create local variable with pattern variable element.
1079-
VariableElement localVar =
1080-
((Pattern.BindingPattern) pattern).getVariable().getVariableElement();
1081-
CastExpression castExpr =
1082-
new CastExpression(localVar.asType(), instanceofExpr.getLeftOperand().copy());
1083-
castExpr.setNeedsCastChk(false);
1084-
VariableDeclarationStatement localVarDecl =
1085-
new VariableDeclarationStatement(localVar, castExpr);
1086-
Block thenBlock;
1087-
if (thenStatement.getKind() == TreeNode.Kind.BLOCK) {
1088-
thenBlock = (Block) thenStatement;
1089-
} else {
1090-
thenBlock = new Block();
1091-
thenBlock.addStatement(thenStatement);
1092-
}
1093-
if (negate) {
1094-
trailingPatternDeclaration = localVarDecl;
1095-
} else {
1096-
thenBlock.addStatement(0, localVarDecl);
1097-
}
1098-
if (subExpr != null) {
1099-
// Move statements inside of if statement with subExpr condition.
1100-
IfStatement subIf = new IfStatement().setExpression(subExpr);
1101-
Block subIfBlock = new Block();
1102-
subIf.setThenStatement(subIfBlock);
1103-
while (thenBlock.getStatements().size() > 1) {
1104-
subIfBlock.addStatement(thenBlock.getStatements().remove(1));
1105-
}
1106-
thenBlock.addStatement(subIf);
1107-
}
1108-
thenStatement = thenBlock;
1109-
}
1110-
Statement newNode =
1111-
new IfStatement()
1112-
.setExpression(condition)
1113-
.setThenStatement(thenStatement)
1114-
.setElseStatement((Statement) convert(node.getElseStatement(), path));
1115-
return newNode;
1020+
return new IfStatement()
1021+
.setExpression(convertWithoutParens(node.getCondition(), path))
1022+
.setThenStatement((Statement) convert(node.getThenStatement(), path))
1023+
.setElseStatement((Statement) convert(node.getElseStatement(), path));
11161024
}
11171025

11181026
private TreeNode convertInstanceOf(InstanceOfTree node, TreePath parent) {

‎translator/src/main/java/com/google/devtools/j2objc/pipeline/TranslationProcessor.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import com.google.devtools.j2objc.translate.GwtConverter;
4141
import com.google.devtools.j2objc.translate.InitializationNormalizer;
4242
import com.google.devtools.j2objc.translate.InnerClassExtractor;
43+
import com.google.devtools.j2objc.translate.InstanceOfPatternRewriter;
4344
import com.google.devtools.j2objc.translate.JavaCloneWriter;
4445
import com.google.devtools.j2objc.translate.JavaToIOSMethodTranslator;
4546
import com.google.devtools.j2objc.translate.LabelRewriter;
@@ -212,6 +213,9 @@ public static void applyMutations(
212213
new VariableRenamer(unit).run();
213214
ticker.tick("VariableRenamer");
214215

216+
new InstanceOfPatternRewriter(unit).run();
217+
ticker.tick("InstanceOfPatternRewriter");
218+
215219
// Rewrite enhanced for loops into correct C code.
216220
new EnhancedForRewriter(unit).run();
217221
ticker.tick("EnhancedForRewriter");

‎translator/src/main/java/com/google/devtools/j2objc/translate/InstanceOfPatternRewriter.java‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
15+
package com.google.devtools.j2objc.translate;
16+
17+
import com.google.devtools.j2objc.ast.Assignment;
18+
import com.google.devtools.j2objc.ast.Block;
19+
import com.google.devtools.j2objc.ast.CastExpression;
20+
import com.google.devtools.j2objc.ast.CommaExpression;
21+
import com.google.devtools.j2objc.ast.CompilationUnit;
22+
import com.google.devtools.j2objc.ast.ConditionalExpression;
23+
import com.google.devtools.j2objc.ast.InfixExpression;
24+
import com.google.devtools.j2objc.ast.InfixExpression.Operator;
25+
import com.google.devtools.j2objc.ast.InstanceofExpression;
26+
import com.google.devtools.j2objc.ast.NullLiteral;
27+
import com.google.devtools.j2objc.ast.Pattern;
28+
import com.google.devtools.j2objc.ast.SimpleName;
29+
import com.google.devtools.j2objc.ast.UnitTreeVisitor;
30+
import com.google.devtools.j2objc.ast.VariableDeclarationStatement;
31+
import com.google.devtools.j2objc.types.GeneratedVariableElement;
32+
import java.util.ArrayDeque;
33+
import java.util.Deque;
34+
import java.util.HashSet;
35+
import java.util.Set;
36+
import javax.lang.model.element.VariableElement;
37+
38+
/**
39+
* Rewrites instance of patterns.
40+
*
41+
* <p>Rewrites instance of patterns by extracting the variable declaration and rewriting {@code expr
42+
* instanceof Class c} as {@code (X tmp = expr, c = tmp instanceof C ? (c = (C) tmp): null, c !=
43+
* null)}
44+
*
45+
* @author Roberto Lublinerman
46+
*/
47+
public class InstanceOfPatternRewriter extends UnitTreeVisitor {
48+
49+
private Deque<Block> enclosingScopes = new ArrayDeque<>();
50+
private int tempCount;
51+
private int patternCount;
52+
53+
private Set<VariableElement> patternVariables = new HashSet<>();
54+
55+
public InstanceOfPatternRewriter(CompilationUnit unit) {
56+
super(unit);
57+
}
58+
59+
@Override
60+
public boolean visit(Block node) {
61+
enclosingScopes.push(node);
62+
return true;
63+
}
64+
65+
@Override
66+
public void endVisit(Block node) {
67+
enclosingScopes.pop();
68+
}
69+
70+
@Override
71+
public void endVisit(SimpleName node) {
72+
if (patternVariables.contains(node.getElement())) {
73+
// Rename all existing access to the variable to the new name.
74+
node.setIdentifier(nameTable.getVariableShortName((VariableElement) node.getElement()));
75+
}
76+
}
77+
78+
@Override
79+
public void endVisit(InstanceofExpression node) {
80+
if (node.getPattern() == null) {
81+
return;
82+
}
83+
84+
VariableElement patternVariable =
85+
((Pattern.BindingPattern) node.getPattern()).getVariable().getVariableElement();
86+
87+
// TODO(b/454053746): Have a general pass that renames variables to avoid collisions.
88+
// Create a unique name to avoid collisions.
89+
nameTable.setVariableName(
90+
patternVariable,
91+
nameTable.getVariableShortName(patternVariable) + "$pattern$" + patternCount++);
92+
patternVariables.add(patternVariable);
93+
94+
// Generate a temporary variable to preserve evaluation semantics.
95+
VariableElement tempVariable =
96+
GeneratedVariableElement.newLocalVar(
97+
"tmp$" + tempCount++,
98+
node.getLeftOperand().getTypeMirror(),
99+
patternVariable.getEnclosingElement());
100+
101+
enclosingScopes.peek().addStatement(0, new VariableDeclarationStatement(tempVariable, null));
102+
enclosingScopes.peek().addStatement(0, new VariableDeclarationStatement(patternVariable, null));
103+
104+
CommaExpression replacement =
105+
new CommaExpression(
106+
// tmp = expr
107+
new Assignment(new SimpleName(tempVariable), node.getLeftOperand().copy()),
108+
// patternVariable = tmp instanceof T ? (T) tmp : null
109+
new Assignment(
110+
getResolvedSimpleName(patternVariable),
111+
new ConditionalExpression()
112+
.setExpression(
113+
new InstanceofExpression(node)
114+
.setLeftOperand(new SimpleName(tempVariable))
115+
.setPattern(null))
116+
.setThenExpression(
117+
new CastExpression(patternVariable.asType(), new SimpleName(tempVariable))
118+
.setNeedsCastChk(false))
119+
.setElseExpression(new NullLiteral(patternVariable.asType()))
120+
.setTypeMirror(patternVariable.asType())),
121+
// patternVariable != null
122+
new InfixExpression()
123+
.setTypeMirror(typeUtil.getBoolean())
124+
.setOperator(Operator.NOT_EQUALS)
125+
.addOperand(new SimpleName(getResolvedSimpleName(patternVariable)))
126+
.addOperand(new NullLiteral(patternVariable.asType())));
127+
node.replaceWith(replacement);
128+
}
129+
130+
/**
131+
* Return the unique simple name for a pattern variable.
132+
*
133+
* <p>Since pattern variables end up being declared in an enclosing scope, to avoid name clashes a
134+
* unique name is synthesized for them.
135+
*/
136+
private SimpleName getResolvedSimpleName(VariableElement patternVariable) {
137+
return new SimpleName(patternVariable)
138+
.setIdentifier(nameTable.getVariableShortName(patternVariable));
139+
}
140+
}

‎translator/src/test/java/com/google/devtools/j2objc/SmallTests.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import com.google.devtools.j2objc.translate.GwtConverterTest;
6060
import com.google.devtools.j2objc.translate.InitializationNormalizerTest;
6161
import com.google.devtools.j2objc.translate.InnerClassExtractorTest;
62+
import com.google.devtools.j2objc.translate.InstanceOfPatternRewriterTest;
6263
import com.google.devtools.j2objc.translate.JavaCloneWriterTest;
6364
import com.google.devtools.j2objc.translate.JavaToIOSMethodTranslatorTest;
6465
import com.google.devtools.j2objc.translate.LambdaTypeElementAdderTest;
@@ -142,6 +143,7 @@ public class SmallTests {
142143
InfixExpressionTest.class,
143144
InitializationNormalizerTest.class,
144145
InnerClassExtractorTest.class,
146+
InstanceOfPatternRewriterTest.class,
145147
J2ObjCIncompatibleStripperTest.class,
146148
J2ObjCTest.class,
147149
JavaCloneWriterTest.class,

‎translator/src/test/java/com/google/devtools/j2objc/gen/StatementGeneratorTest.java‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1996,9 +1996,11 @@ public void testInstanceOfPatternVariableTranslation() throws IOException {
19961996
"Test.m");
19971997
assertTranslatedLines(
19981998
translation,
1999-
"if ([o isKindOfClass:[NSString class]]) {",
2000-
"NSString *s = (NSString *) o;",
2001-
"return [s java_length];",
1999+
"NSString *s$pattern$0;",
2000+
"id tmp$0;",
2001+
"if ((tmp$0 = o, s$pattern$0 = [tmp$0 isKindOfClass:[NSString class]] ? (NSString *) tmp$0"
2002+
+ " : nil, !JreStringEqualsEquals(s$pattern$0, nil))) {",
2003+
"return [s$pattern$0 java_length];",
20022004
"}",
20032005
"return 0;");
20042006
}
@@ -2026,11 +2028,12 @@ public void testInstanceOfPatternVariableTranslationWithGuards() throws IOExcept
20262028
"Test.m");
20272029
assertTranslatedLines(
20282030
translation,
2029-
"if ([o isKindOfClass:[Point class]]) {",
2030-
"Point *p = (Point *) o;",
2031-
"if (x_ == p->x_ && y_ == p->y_) {",
2032-
"return true;",
2033-
"}",
2031+
"Point *p$pattern$0;",
2032+
"id tmp$0;",
2033+
"if ((tmp$0 = o, p$pattern$0 = [tmp$0 isKindOfClass:[Point class]] ? (Point *) tmp$0 : nil,"
2034+
+ " !JreObjectEqualsEquals(p$pattern$0, nil)) && x_ == p$pattern$0->x_ && y_ =="
2035+
+ " p$pattern$0->y_) {",
2036+
" return true;",
20342037
"}",
20352038
"else {",
20362039
"return false;",

0 commit comments

Comments
 (0)