Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Switch Expressions] Use control flow analysis rather than structural analysis of syntax tree, to flag switch rule blocks that complete normally. #3456

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading