Skip to content

Commit

Permalink
[Switch Expressions] Use control flow analysis rather than structural…
Browse files Browse the repository at this point in the history
… analysis of syntax tree, to flag switch rule blocks that complete normally. (#3456)

* Fixes #3455
  • Loading branch information
srikanth-sankaran authored Dec 14, 2024
1 parent b90780a commit 2c2aefe
Show file tree
Hide file tree
Showing 23 changed files with 44 additions and 269 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ public abstract class ASTNode implements TypeConstants, TypeIds {
public static final int IsUsefulEmptyStatement = Bit1;

// for block and method declaration
public static final int SwitchRuleBlock = Bit1;
public static final int BlockShouldEndDead = Bit1;
public static final int UndocumentedEmptyBlock = Bit4;
public static final int OverridingMethodWithSupercall = Bit5;
public static final int CanBeStatic = Bit9; // used to flag a method that can be declared static
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,16 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
}
}
}
if ((this.bits & ASTNode.SwitchRuleBlock) != 0) { // switch rule blocks don't fall through
if ((this.bits & ASTNode.BlockShouldEndDead) != 0) { // switch rule blocks don't fall through
if (flowInfo != FlowInfo.DEAD_END) {
flowContext.recordBreakFrom(flowInfo);
return FlowInfo.DEAD_END;
if (flowContext.associatedNode instanceof SwitchExpression) { // ... demand that for expression switch ...
currentScope.problemReporter().switchExpressionBlockCompletesNormally(this);
} else { // ... enforce that for statement switch, by having the code generator inject an automagic break ...
flowContext.recordBreakFrom(flowInfo);
return FlowInfo.DEAD_END;
}
} else {
this.bits &= ~ASTNode.BlockShouldEndDead; // Already dead-ends; nothing special needed from code generator
}
}
return flowInfo;
Expand Down Expand Up @@ -182,17 +188,4 @@ public boolean completesByContinue() {
int length = this.statements == null ? 0 : this.statements.length;
return length > 0 && this.statements[length - 1].completesByContinue();
}

@Override
public boolean canCompleteNormally() {
int length = this.statements == null ? 0 : this.statements.length;
return length == 0 || this.statements[length - 1].canCompleteNormally();
}

@Override
public boolean continueCompletes() {
int length = this.statements == null ? 0 : this.statements.length;
return length > 0 && this.statements[length - 1].continueCompletes();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,4 @@ public void traverse(ASTVisitor visitor, BlockScope blockscope) {
public boolean doesNotCompleteNormally() {
return true;
}

@Override
public boolean canCompleteNormally() {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,4 @@ public boolean doesNotCompleteNormally() {
public boolean completesByContinue() {
return true;
}

@Override
public boolean canCompleteNormally() {
return false;
}

@Override
public boolean continueCompletes() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -273,28 +273,4 @@ public boolean doesNotCompleteNormally() {
public boolean completesByContinue() {
return this.action.continuesAtOuterLabel();
}

@Override
public boolean canCompleteNormally() {
Constant cst = this.condition.constant;
boolean isConditionTrue = cst == null || cst != Constant.NotAConstant && cst.booleanValue() == true;
cst = this.condition.optimizedBooleanConstant();
boolean isConditionOptimizedTrue = cst == null ? true : cst != Constant.NotAConstant && cst.booleanValue() == true;

if (!(isConditionTrue || isConditionOptimizedTrue)) {
if (this.action == null || this.action.canCompleteNormally())
return true;
if (this.action != null && this.action.continueCompletes())
return true;
}
if (this.action != null && this.action.breaksOut(null))
return true;

return false;
}
@Override
public boolean continueCompletes() {
return this.action.continuesAtOuterLabel();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -489,23 +489,4 @@ public boolean doesNotCompleteNormally() {
public boolean completesByContinue() {
return this.action.continuesAtOuterLabel();
}
@Override
public boolean canCompleteNormally() {
Constant cst = this.condition == null ? null : this.condition.constant;
boolean isConditionTrue = cst == null || cst != Constant.NotAConstant && cst.booleanValue() == true;
cst = this.condition == null ? null : this.condition.optimizedBooleanConstant();
boolean isConditionOptimizedTrue = cst == null ? true : cst != Constant.NotAConstant && cst.booleanValue() == true;

if (!(isConditionTrue || isConditionOptimizedTrue))
return true;
if (this.action != null && this.action.breaksOut(null))
return true;
return false;
}

@Override
public boolean continueCompletes() {
return this.action.continuesAtOuterLabel();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -688,10 +688,4 @@ public void traverse(
public boolean doesNotCompleteNormally() {
return false; // may not be entered at all.
}

@Override
public boolean canCompleteNormally() {
return true;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -340,14 +340,4 @@ public boolean doesNotCompleteNormally() {
public boolean completesByContinue() {
return this.thenStatement != null && this.thenStatement.completesByContinue() || this.elseStatement != null && this.elseStatement.completesByContinue();
}
@Override
public boolean canCompleteNormally() {
return ((this.thenStatement == null || this.thenStatement.canCompleteNormally()) ||
(this.elseStatement == null || this.elseStatement.canCompleteNormally()));
}
@Override
public boolean continueCompletes() {
return this.thenStatement != null && this.thenStatement.continueCompletes() ||
this.elseStatement != null && this.elseStatement.continueCompletes();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -165,17 +165,4 @@ public boolean doesNotCompleteNormally() {
public boolean completesByContinue() {
return this.statement instanceof ContinueStatement; // NOT this.statement.continuesAtOuterLabel
}

@Override
public boolean canCompleteNormally() {
if (this.statement.canCompleteNormally())
return true;
return this.statement.breaksOut(this.label);
}

@Override
public boolean continueCompletes() {
return this.statement instanceof ContinueStatement; // NOT this.statement.continuesAtOuterLabel
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,8 @@ public boolean doesNotCompleteNormally() {
return true;
}

@Override
public boolean canCompleteNormally() {
return false;
}

/**
* Retrun statement code generation
* Return statement code generation
*
* generate the finallyInvocationSequence.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,23 +104,6 @@ public boolean completesByContinue() {
return false;
}

/**
* Switch Expression analysis: *Assuming* this is reachable, analyze if this completes normally
* i.e control flow can reach the textually next statement, as per JLS 14 Sec 14.22
* For blocks, we don't perform intra-reachability analysis.
* Note: delinking this from a similar (opposite) {@link #doesNotCompleteNormally()} since that was
* coded for a specific purpose of Lambda Shape Analysis.
*/
public boolean canCompleteNormally() {
return true;
}
/**
* The equivalent function of completesByContinue - implements both the rules concerning continue with
* and without a label.
*/
public boolean continueCompletes() {
return false;
}
public static final int NOT_COMPLAINED = 0;
public static final int COMPLAINED_FAKE_REACHABLE = 1;
public static final int COMPLAINED_UNREACHABLE = 2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -376,16 +376,6 @@ private TypeBinding resolveAsType(TypeBinding switchType) {
@Override
public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, FlowInfo flowInfo) {
flowInfo = super.analyseCode(currentScope, flowContext, flowInfo);
if ((this.switchBits & LabeledRules) != 0) { // 15.28.1
for (Statement stmt : this.statements) {
if (stmt instanceof Block && stmt.canCompleteNormally())
currentScope.problemReporter().switchExpressionBlockCompletesNormally(stmt);
}
} else {
Statement ultimateStmt = this.statements[this.statements.length - 1]; // length guaranteed > 0
if (ultimateStmt.canCompleteNormally())
currentScope.problemReporter().switchExpressionBlockCompletesNormally(ultimateStmt);
}

if (currentScope.compilerOptions().enableSyntacticNullAnalysisForFields)
flowContext.expireNullCheckedFieldInfo(); // wipe information that was meant only for this result expression:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ public static record SingletonBootstrap(String id, char[] selector, char[] signa
public final static int Exhaustive = ASTNode.Bit3;
public final static int QualifiedEnum = ASTNode.Bit4;
public final static int LabeledBlockStatementGroup = ASTNode.Bit5;
public final static int BarricadeInjectedDefault = ASTNode.Bit6;

// for switch on strings
private static final char[] SecretSelectorVariableName = " selector".toCharArray(); //$NON-NLS-1$
Expand Down Expand Up @@ -438,6 +439,12 @@ else if ((statement.bits & ASTNode.DocumentedFallthrough) == 0) // the case is n
}
}
}
if (caseInits != FlowInfo.DEAD_END) {
if (isTrulyExpression())
currentScope.problemReporter().switchExpressionBlockCompletesNormally(this.statements[this.statements.length - 1]);
if (this.defaultCase == null)
this.switchBits |= BarricadeInjectedDefault;
}

final TypeBinding resolvedTypeBinding = this.expression.resolvedType;
if (resolvedTypeBinding.isEnum() && !needPatternDispatchCopy()) {
Expand Down Expand Up @@ -607,7 +614,7 @@ public String toString() {
defaultCaseLabel.place(); // branch label gets placed by generateCode below.
}
statement.generateCode(this.scope, codeStream);
if ((this.switchBits & LabeledRules) != 0 && statement instanceof Block && statement.canCompleteNormally())
if (statement instanceof Block block && (block.bits & BlockShouldEndDead) != 0)
codeStream.goto_(this.breakLabel);
}
}
Expand Down Expand Up @@ -759,7 +766,7 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream) {
codeStream.removeNotDefinitelyAssignedVariables(currentScope, this.preSwitchInitStateIndex);
}
statement.generateCode(this.scope, codeStream);
if ((this.switchBits & LabeledRules) != 0 && statement instanceof Block && statement.canCompleteNormally())
if (statement instanceof Block block && (block.bits & BlockShouldEndDead) != 0)
codeStream.goto_(this.breakLabel);
}
}
Expand All @@ -782,7 +789,7 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream) {
*/
if (this.scope.compilerOptions().complianceLevel >= ClassFileConstants.JDK19) {
// since 19 we have MatchException for this
if (this.statements.length > 0 && this.statements[this.statements.length - 1].canCompleteNormally())
if ((this.switchBits & BarricadeInjectedDefault) != 0)
codeStream.goto_(this.breakLabel); // hop, skip and jump over match exception throw.
defaultLabel.place();
codeStream.newJavaLangMatchException();
Expand Down Expand Up @@ -1355,55 +1362,6 @@ public boolean completesByContinue() {
return false;
}

@Override
public boolean canCompleteNormally() {
if (this.statements == null || this.statements.length == 0)
return true;
if ((this.switchBits & LabeledRules) == 0) { // switch labeled statement group
if (this.statements[this.statements.length - 1].canCompleteNormally())
return true; // last statement as well as last switch label after blocks if exists.
if (this.totalPattern == null && this.defaultCase == null)
return true;
for (Statement statement : this.statements) {
if (statement.breaksOut(null))
return true;
}
} else {
// switch block consists of switch rules
for (Statement stmt : this.statements) {
if (stmt instanceof CaseStatement)
continue; // skip case
if (this.totalPattern == null && this.defaultCase == null)
return true;
if (stmt instanceof Expression)
return true;
if (stmt.canCompleteNormally())
return true;
if (stmt instanceof YieldStatement && ((YieldStatement) stmt).isImplicit) // note: artificially introduced
return true;
if (stmt instanceof Block) {
Block block = (Block) stmt;
if (block.canCompleteNormally())
return true;
if (block.breaksOut(null))
return true;
}
}
}
return false;
}

@Override
public boolean continueCompletes() {
if (this.statements == null || this.statements.length == 0)
return false;
for (Statement statement : this.statements) {
if (statement.continueCompletes())
return true;
}
return false;
}

@Override
public StringBuilder printExpression(int indent, StringBuilder output) {
return printStatement(indent, output);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,14 +248,4 @@ public boolean doesNotCompleteNormally() {
public boolean completesByContinue() {
return this.block.completesByContinue();
}

@Override
public boolean canCompleteNormally() {
return this.block.canCompleteNormally();
}

@Override
public boolean continueCompletes() {
return this.block.continueCompletes();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,4 @@ public void traverse(ASTVisitor visitor, BlockScope blockScope) {
public boolean doesNotCompleteNormally() {
return true;
}

@Override
public boolean canCompleteNormally() {
return false;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -1270,34 +1270,4 @@ public boolean completesByContinue() {
}
return this.finallyBlock != null && this.finallyBlock.completesByContinue();
}
@Override
public boolean canCompleteNormally() {
if (this.tryBlock.canCompleteNormally()) {
return (this.finallyBlock != null) ? this.finallyBlock.canCompleteNormally() : true;
}
if (this.catchBlocks != null) {
for (Block catchBlock : this.catchBlocks) {
if (catchBlock.canCompleteNormally()) {
return (this.finallyBlock != null) ? this.finallyBlock.canCompleteNormally() : true;
}
}
}
return false;
}
@Override
public boolean continueCompletes() {
if (this.tryBlock.continueCompletes()) {
return (this.finallyBlock == null) ? true :
this.finallyBlock.canCompleteNormally() || this.finallyBlock.continueCompletes();
}
if (this.catchBlocks != null) {
for (Block catchBlock : this.catchBlocks) {
if (catchBlock.continueCompletes()) {
return (this.finallyBlock == null) ? true :
this.finallyBlock.canCompleteNormally() || this.finallyBlock.continueCompletes();
}
}
}
return this.finallyBlock != null && this.finallyBlock.continueCompletes();
}
}
Loading

0 comments on commit 2c2aefe

Please sign in to comment.