Skip to content

Commit c24673b

Browse files
tomballcopybara-github
authored andcommitted
Issue #2388: Fixed translation of Java 16+ pattern matching in instanceof expressions within if statements.
This change introduces a `needsCastChk` flag to `CastExpression` to prevent redundant runtime cast checks. When translating `if (expr instanceof Type var)`, a local variable `var` is declared and initialized with a cast of `expr`. Since the `instanceof` check already guarantees type safety, the generated cast does not need an additional runtime check. For complex conditions like `if (expr instanceof Type var && otherCondition)`, the translation now creates a nested `if` statement for `otherCondition` inside the block where `var` is in scope. PiperOrigin-RevId: 812930294
1 parent ec24a53 commit c24673b

File tree

4 files changed

+343
-233
lines changed

4 files changed

+343
-233
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,13 @@ public class CastExpression extends Expression {
2323

2424
private ChildLink<Type> type = ChildLink.create(Type.class, this);
2525
private ChildLink<Expression> expression = ChildLink.create(Expression.class, this);
26+
private boolean needsCastChk = true;
2627

2728
public CastExpression(CastExpression other) {
2829
super(other);
2930
type.copyFrom(other.getType());
3031
expression.copyFrom(other.getExpression());
32+
needsCastChk = other.needsCastChk;
3133
}
3234

3335
public CastExpression(TypeMirror typeMirror, Expression expression) {
@@ -66,6 +68,14 @@ public CastExpression setExpression(Expression newExpression) {
6668
return this;
6769
}
6870

71+
public boolean needsCastChk() {
72+
return needsCastChk;
73+
}
74+
75+
public void setNeedsCastChk(boolean value) {
76+
needsCastChk = value;
77+
}
78+
6979
@Override
7080
protected void acceptInner(TreeVisitor visitor) {
7181
if (visitor.visit(this)) {

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

Lines changed: 63 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,36 +1013,83 @@ private TreeNode convertIdent(IdentifierTree node, TreePath parent) {
10131013
private TreeNode convertIf(IfTree node, TreePath parent) {
10141014
TreePath path = getTreePath(parent, node);
10151015
Expression condition = convertWithoutParens(node.getCondition(), path);
1016-
Statement thenStatement = (Statement) convert(node.getThenStatement(), path);
1017-
Statement newNode = new IfStatement()
1018-
.setExpression(condition)
1019-
.setThenStatement(thenStatement)
1020-
.setElseStatement((Statement) convert(node.getElseStatement(), path));
1016+
Pattern pattern = null;
10211017
InstanceofExpression instanceofExpr = null;
1018+
Expression subExpr = null;
10221019
if (condition.getKind() == TreeNode.Kind.INSTANCEOF_EXPRESSION) {
10231020
instanceofExpr = (InstanceofExpression) condition;
1021+
pattern = instanceofExpr.getPattern();
1022+
instanceofExpr.setPattern(null);
10241023
} else if (condition.getKind() == TreeNode.Kind.INFIX_EXPRESSION) {
1025-
for (Expression operand : ((InfixExpression) condition).getOperands()) {
1024+
InfixExpression infixExpr = (InfixExpression) condition;
1025+
List<Expression> operands = infixExpr.getOperands();
1026+
for (int i = 0; i < operands.size(); i++) {
1027+
Expression operand = operands.get(i);
10261028
if (operand.getKind() == TreeNode.Kind.INSTANCEOF_EXPRESSION) {
10271029
instanceofExpr = (InstanceofExpression) operand;
1028-
break;
1030+
pattern = instanceofExpr.getPattern();
1031+
if (pattern != null) {
1032+
instanceofExpr.setPattern(null);
1033+
condition = instanceofExpr.copy();
1034+
List<Expression> subOperands = new ArrayList<>();
1035+
while (++i < operands.size()) {
1036+
subOperands.add(operands.get(i));
1037+
}
1038+
if (subOperands.size() == 1) {
1039+
subExpr = subOperands.get(0).copy();
1040+
} else if (subOperands.size() > 1) {
1041+
InfixExpression newInfix =
1042+
new InfixExpression(infixExpr.getTypeMirror(), infixExpr.getOperator());
1043+
for (Expression subOperand : subOperands) {
1044+
newInfix.addOperand(subOperand.copy());
1045+
}
1046+
subExpr = newInfix;
1047+
}
1048+
break;
1049+
} else {
1050+
// No pattern, reset instanceofExpr and pattern.
1051+
instanceofExpr = null;
1052+
pattern = null;
1053+
}
10291054
}
10301055
}
10311056
}
1032-
Pattern pattern = instanceofExpr != null ? instanceofExpr.getPattern() : null;
1057+
1058+
Statement thenStatement = (Statement) convert(node.getThenStatement(), path);
10331059
if (pattern != null) {
10341060
// Create local variable with pattern variable element.
10351061
VariableElement localVar =
10361062
((Pattern.BindingPattern) pattern).getVariable().getVariableElement();
1037-
CastExpression castExpr = new CastExpression(localVar.asType(),
1038-
instanceofExpr.getLeftOperand().copy());
1063+
CastExpression castExpr =
1064+
new CastExpression(localVar.asType(), instanceofExpr.getLeftOperand().copy());
1065+
castExpr.setNeedsCastChk(false);
10391066
VariableDeclarationStatement localVarDecl =
10401067
new VariableDeclarationStatement(localVar, castExpr);
1041-
Block block = new Block()
1042-
.addStatement(localVarDecl)
1043-
.addStatement(newNode);
1044-
newNode = block;
1068+
Block thenBlock;
1069+
if (thenStatement.getKind() == TreeNode.Kind.BLOCK) {
1070+
thenBlock = (Block) thenStatement;
1071+
} else {
1072+
thenBlock = new Block();
1073+
thenBlock.addStatement(thenStatement);
1074+
}
1075+
thenBlock.addStatement(0, localVarDecl);
1076+
if (subExpr != null) {
1077+
// Move statements inside of if statement with subExpr condition.
1078+
IfStatement subIf = new IfStatement().setExpression(subExpr);
1079+
Block subIfBlock = new Block();
1080+
subIf.setThenStatement(subIfBlock);
1081+
while (thenBlock.getStatements().size() > 1) {
1082+
subIfBlock.addStatement(thenBlock.getStatements().remove(1));
1083+
}
1084+
thenBlock.addStatement(subIf);
1085+
}
1086+
thenStatement = thenBlock;
10451087
}
1088+
Statement newNode =
1089+
new IfStatement()
1090+
.setExpression(condition)
1091+
.setThenStatement(thenStatement)
1092+
.setElseStatement((Statement) convert(node.getElseStatement(), path));
10461093
return newNode;
10471094
}
10481095

@@ -1201,6 +1248,7 @@ private void maybeAddUnreachableDirective(Block body) {
12011248
}
12021249
}
12031250

1251+
@SuppressWarnings("StatementSwitchToExpressionSwitch")
12041252
private static String getMemberName(ExpressionTree node) {
12051253
switch (node.getKind().name()) {
12061254
case "IDENTIFIER":
@@ -1883,6 +1931,7 @@ private TreeNode getAssociatedJavaDoc(Tree node, TreePath path) {
18831931
return comment != null && comment.isDocComment() ? comment : null;
18841932
}
18851933

1934+
@SuppressWarnings("StatementSwitchToExpressionSwitch")
18861935
private Comment convertAssociatedComment(Tree node, TreePath path) {
18871936
boolean docCommentsEnabled = newUnit.getEnv().options().docCommentsEnabled();
18881937
DocCommentTable docComments = ((JCCompilationUnit) unit).docComments;

translator/src/main/java/com/google/devtools/j2objc/translate/CastResolver.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,11 @@ public void endVisit(CastExpression node) {
111111
return;
112112
}
113113

114-
FunctionInvocation castCheck = createCastCheck(type, expr);
115-
if (castCheck != null) {
116-
node.setExpression(castCheck);
114+
if (node.needsCastChk()) {
115+
FunctionInvocation castCheck = createCastCheck(type, expr);
116+
if (castCheck != null) {
117+
node.setExpression(castCheck);
118+
}
117119
}
118120
}
119121
}

0 commit comments

Comments
 (0)