Skip to content

Commit

Permalink
Parse multiple Patterns in a case statement
Browse files Browse the repository at this point in the history
- Multiple patterns are reported as an error unless all variables are
  unnamed
- GuardedPattern now has a list of Patterns as a child,
  deprecate the methods that assume it has one
- Modified tests to adapt to the parser not handling multiple guards in
  one case statement any more
  (this was never allowed, but now the error has changed)
- Modify tests that allow variables after record patterns
  (this feature was removed before record patterns moved out of preview)
- AST & generator changes

Signed-off-by: David Thompson <davthomp@redhat.com>
  • Loading branch information
datho7561 committed Feb 21, 2024
1 parent a9de560 commit 588a3e3
Show file tree
Hide file tree
Showing 60 changed files with 1,652 additions and 924 deletions.
34 changes: 21 additions & 13 deletions org.eclipse.jdt.core.compiler.batch/grammar/java.g
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ Goal ::= '->' SwitchLabelCaseLhs
Goal ::= RestrictedIdentifiersealed Modifiersopt
Goal ::= RestrictedIdentifierpermits PermittedSubclasses
-- jsr 427 --
Goal ::= BeginCaseElement Pattern
Goal ::= CaseLabelPatternElements
Goal ::= RestrictedIdentifierWhen Expression
/:$readableName Goal:/

Expand Down Expand Up @@ -1588,33 +1588,41 @@ SwitchLabelCaseLhs ::= 'case' CaseLabelElements

-- END SwitchExpression (JEP 325) --

CaseLabelElements -> CaseLabelElement
CaseLabelElements ::= CaseLabelElements ',' CaseLabelElement
CaseLabelElements -> CaseLabelConstantElements
CaseLabelElements -> CaseLabelPatternElements

CaseLabelConstantElements ::= CaseLabelConstantElement
CaseLabelConstantElements ::= CaseLabelConstantElements ',' CaseLabelConstantElement
/.$putCase consumeCaseLabelElements(); $break ./
/:$readableName CaseLabelElements:/
/:$readableName CaseLabelConstantElements:/

-- Production name hardcoded in parser. Must be ::= and not -> (need to hook at cCLE)
CaseLabelElement ::= ConstantExpression
CaseLabelConstantElement ::= ConstantExpression
/.$putCase consumeCaseLabelElement(CaseLabelKind.CASE_EXPRESSION); $break ./
/:$readableName CaseLabelElement:/

-- following 'null' in CASE_EXPRESSION - passes through existing grammar
-- CaseLabelElement -> 'null'

CaseLabelElement ::= 'default'
CaseLabelConstantElement ::= 'default'
/.$putCase consumeCaseLabelElement(CaseLabelKind.CASE_DEFAULT); $break ./
/:$readableName CaseLabelElement:/

CaseLabelElement ::= CaseLabelElementPattern
/.$putCase consumeCaseLabelElement(CaseLabelKind.CASE_PATTERN); $break ./
CaseLabelPatternElements ::= CaseLabelPatternElementsInternal
/:$readableName CaseLabelElement:/

CaseLabelElement ::= CaseLabelElementPattern Guard
/.$putCase consumeCaseLabelElement(CaseLabelKind.CASE_PATTERN); $break ./
/:$readableName CaseLabelElement:/
CaseLabelPatternElementsInternal ::= BeginCaseElement CasePatternList
CaseLabelPatternElementsInternal ::= BeginCaseElement CasePatternList Guard

CaseLabelElementPattern -> BeginCaseElement Pattern
/:$readableName CaseLabelElementPattern:/
CasePatternList ::= CasePattern
/:$readableName CasePatternList :/
CasePatternList ::= CasePatternList ',' CasePattern
/.$putCase consumeCaseLabelElements(); $break ./
/:$readableName CasePatternList :/

CasePattern ::= Pattern
/.$putCase consumeCaseLabelElement(CaseLabelKind.CASE_PATTERN); $break ./
/:$readableName CasePattern:/

Guard ::= RestrictedIdentifierWhen Expression
/.$putCase consumeGuard(); $break ./
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2576,6 +2576,10 @@ public interface IProblem {
*/
int IllegalRecordPattern = TypeRelated + 1941;

/**
* @since 3.37
*/
int CaseLabelWithPatternMustHaveExactlyOnePattern = Internal + 1942;

/**
* @since 3.35
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,4 +156,5 @@ public boolean isUnnamed(BlockScope scope) {
&& scope.compilerOptions().sourceLevel >= ClassFileConstants.JDK21
&& scope.compilerOptions().enablePreviewFeatures;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ public FlowInfo analyseCode(
local.useFlag = LocalVariableBinding.USED; // these are structurally required even if not touched
}
nullPatternCount += e instanceof NullLiteral ? 1 : 0;
if (i > 0 && (e instanceof Pattern)) {
if (i > 0 && (e instanceof Pattern)
&& (currentScope.compilerOptions().sourceLevel < ClassFileConstants.JDK21 || !currentScope.compilerOptions().enablePreviewFeatures)) {
if (!(i == nullPatternCount && e instanceof TypePattern))
currentScope.problemReporter().IllegalFallThroughToPattern(e);
}
Expand Down Expand Up @@ -160,18 +161,24 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream) {
}

private void casePatternExpressionGenerateCode(BlockScope currentScope, CodeStream codeStream) {
if (this.patternIndex != -1) {
Pattern pattern = ((Pattern) this.constantExpressions[this.patternIndex]);
if (containsPatternVariable()) {
this.trueLabel = new BranchLabel(codeStream);
this.falseLabel = new BranchLabel(codeStream);
if (this.patternIndex != -1 && containsPatternVariable()) {
this.trueLabel = new BranchLabel(codeStream);
this.falseLabel = new BranchLabel(codeStream);
for (int i = 0; i < this.constantExpressions.length - 1; i++) {
BranchLabel nextPatternCheck = new BranchLabel(codeStream);
Pattern pattern = ((Pattern)this.constantExpressions[i]);
LocalVariableBinding local = currentScope.findVariable(SwitchStatement.SecretPatternVariableName, null);
codeStream.load(local);
pattern.generateCode(currentScope, codeStream, this.trueLabel, this.falseLabel);
// Srikanth, check this goto.
if (!(pattern instanceof GuardedPattern))
codeStream.goto_(this.trueLabel);
pattern.generateCode(currentScope, codeStream, this.trueLabel, nextPatternCheck);
codeStream.goto_(this.trueLabel);
nextPatternCheck.place();
}
Pattern pattern = ((Pattern)this.constantExpressions[this.constantExpressions.length - 1]);
LocalVariableBinding local = currentScope.findVariable(SwitchStatement.SecretPatternVariableName, null);
codeStream.load(local);
pattern.generateCode(currentScope, codeStream, this.trueLabel, this.falseLabel);
if (!(pattern instanceof GuardedPattern))
codeStream.goto_(this.trueLabel);
}
}

Expand Down Expand Up @@ -249,10 +256,16 @@ private Expression getFirstValidExpression(BlockScope scope, SwitchStatement swi
if (e instanceof Pattern) {
scope.problemReporter().validateJavaFeatureSupport(JavaFeature.PATTERN_MATCHING_IN_SWITCH,
e.sourceStart, e.sourceEnd);
if (this.constantExpressions.length > 1) {
scope.problemReporter().illegalCaseConstantCombination(e);
return e;
if (this.constantExpressions.length > 1 || e instanceof GuardedPattern gp && gp.patterns.length > 1) {
int count = 0;
for (Expression expr : this.constantExpressions) {
count += ((Pattern)expr).countNamedVariables(scope);
}
if (count > 0) {
scope.problemReporter().illegalCaseLabelWithMultiplePatterns(this);
}
}
return e;
} else if (e instanceof NullLiteral) {
scope.problemReporter().validateJavaFeatureSupport(JavaFeature.PATTERN_MATCHING_IN_SWITCH,
e.sourceStart, e.sourceEnd);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,27 +27,40 @@

public class GuardedPattern extends Pattern {

public Pattern primaryPattern;
public Pattern[] patterns;
public Expression condition;
public int restrictedIdentifierStart = -1; // used only for 'when' restricted keyword.

public GuardedPattern(Pattern primaryPattern, Expression conditionalAndExpression) {
this.primaryPattern = primaryPattern;
public GuardedPattern(Pattern patterns[], Expression conditionalAndExpression) {
this.patterns = patterns;
this.condition = conditionalAndExpression;
this.sourceStart = primaryPattern.sourceStart;
if (patterns.length > 0) {
this.sourceStart = this.patterns[0].sourceStart;
} else {
this.sourceStart = conditionalAndExpression.sourceStart;
}
this.sourceEnd = conditionalAndExpression.sourceEnd;
}

@Override
public LocalVariableBinding[] bindingsWhenTrue() {
return LocalVariableBinding.merge(this.primaryPattern.bindingsWhenTrue(),
LocalVariableBinding[] patternBindings = new LocalVariableBinding[0];
for (Pattern pattern : this.patterns) {
patternBindings = LocalVariableBinding.merge(patternBindings, pattern.bindingsWhenTrue());
}
return LocalVariableBinding.merge(patternBindings,
this.condition.bindingsWhenTrue());
}

@Override
public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, FlowInfo flowInfo) {
flowInfo = this.primaryPattern.analyseCode(currentScope, flowContext, flowInfo);
currentScope.methodScope().recordInitializationStates(flowInfo);
if (this.patterns.length >= 1) {
flowInfo = this.patterns[0].analyseCode(currentScope, flowContext, flowInfo);
currentScope.methodScope().recordInitializationStates(flowInfo);
}
for (int i = 1; i < this.patterns.length; i++) {
flowInfo = this.patterns[i].analyseCode(currentScope, flowContext, flowInfo);
}
FlowInfo mergedFlow = this.condition.analyseCode(currentScope, flowContext, flowInfo);
mergedFlow = mergedFlow.safeInitsWhenTrue();
currentScope.methodScope().recordInitializationStates(mergedFlow);
Expand All @@ -56,7 +69,20 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl

@Override
public void generateCode(BlockScope currentScope, CodeStream codeStream, BranchLabel trueLabel, BranchLabel falseLabel) {
this.primaryPattern.generateCode(currentScope, codeStream, trueLabel, falseLabel);
LocalVariableBinding local = currentScope.findVariable(SwitchStatement.SecretPatternVariableName, null);
BranchLabel toGuard = new BranchLabel(codeStream);

for (int i = 0; i < this.patterns.length - 1; i++) {
BranchLabel nextPatternCheck = new BranchLabel(codeStream);
Pattern pattern = this.patterns[i];
pattern.generateCode(currentScope, codeStream, toGuard, nextPatternCheck);
codeStream.goto_(toGuard);
nextPatternCheck.place();
codeStream.load(local);
}
Pattern pattern = this.patterns[this.patterns.length - 1];
pattern.generateCode(currentScope, codeStream, toGuard, falseLabel);
toGuard.place();
this.condition.generateOptimizedBoolean(
currentScope,
codeStream,
Expand All @@ -67,12 +93,12 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream, BranchL

@Override
public boolean matchFailurePossible() {
return !isAlwaysTrue() || this.primaryPattern.matchFailurePossible();
return !isAlwaysTrue() || this.patterns[0].matchFailurePossible();
}

@Override
public boolean isAlwaysTrue() {
if (this.primaryPattern.isAlwaysTrue()) {
if (this.patterns[0].isAlwaysTrue()) {
Constant cst = this.condition.optimizedBooleanConstant();
return cst != Constant.NotAConstant && cst.booleanValue() == true;
}
Expand All @@ -81,23 +107,41 @@ public boolean isAlwaysTrue() {

@Override
public boolean coversType(TypeBinding type) {
return this.primaryPattern.coversType(type) && isAlwaysTrue();
if (!isAlwaysTrue()) {
return false;
}
for (Pattern pattern : this.patterns) {
if (pattern.coversType(type)) {
return true;
}
}
return false;
}

@Override
public boolean dominates(Pattern p) {
if (isAlwaysTrue())
return this.primaryPattern.dominates(p);
if (isAlwaysTrue()) {
for (Pattern pattern : this. patterns) {
if (pattern.dominates(p)) {
return true;
}
}
}
return false;
}

@Override
public TypeBinding resolveType(BlockScope scope) {
if (this.resolvedType != null || this.primaryPattern == null)
if (this.resolvedType != null || this.patterns == null || this.patterns.length == 0)
return this.resolvedType;
this.resolvedType = this.primaryPattern.resolveType(scope);

this.condition.resolveTypeExpectingWithBindings(this.primaryPattern.bindingsWhenTrue(), scope, TypeBinding.BOOLEAN);
this.resolvedType = this.patterns[0].resolveType(scope);
for (int i = 1; i < this.patterns.length; i++) {
this.patterns[i].resolveType(scope);
}
// The following call (as opposed to resolveType() ensures that
// the implicitConversion code is set properly and thus the correct
// unboxing calls are generated.
this.condition.resolveTypeExpectingWithBindings(this.patterns[0].bindingsWhenTrue(), scope, TypeBinding.BOOLEAN);
Constant cst = this.condition.optimizedBooleanConstant();
if (cst.typeID() == TypeIds.T_boolean && cst.booleanValue() == false) {
scope.problemReporter().falseLiteralInGuard(this.condition);
Expand All @@ -123,26 +167,35 @@ public boolean visit(
return false;
}
}, scope);
return this.resolvedType = this.primaryPattern.resolvedType;
return this.resolvedType = this.patterns[0].resolvedType;
}

@Override
public StringBuilder printExpression(int indent, StringBuilder output) {
this.primaryPattern.print(indent, output).append(" when "); //$NON-NLS-1$
for (int i = 0; i < this.patterns.length; i++) {
this.patterns[i].print(indent, output);
if (i < this.patterns.length - 1) {
output.append(", "); //$NON-NLS-1$
}
}
output.append(" when "); //$NON-NLS-1$
return this.condition.print(indent, output);
}

@Override
public void traverse(ASTVisitor visitor, BlockScope scope) {
if (visitor.visit(this, scope)) {
this.primaryPattern.traverse(visitor, scope);
this.condition.traverse(visitor, scope);
for (Pattern pattern : this.patterns) {
pattern.traverse(visitor, scope);
}
if (this.condition != null)
this.condition.traverse(visitor, scope);
}
visitor.endVisit(this, scope);
}

@Override
protected boolean isApplicable(TypeBinding other, BlockScope scope) {
return this.primaryPattern.isApplicable(other, scope);
return this.patterns[0].isApplicable(other, scope);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*******************************************************************************/
package org.eclipse.jdt.internal.compiler.ast;

import org.eclipse.jdt.internal.compiler.ASTVisitor;
import org.eclipse.jdt.internal.compiler.codegen.BranchLabel;
import org.eclipse.jdt.internal.compiler.codegen.CodeStream;
import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
Expand Down Expand Up @@ -105,8 +106,28 @@ protected boolean isApplicable(TypeBinding other, BlockScope scope) {

public abstract boolean dominates(Pattern p);

public int countNamedVariables(BlockScope scope) {
var pvc = new PatternVariableCounter();
this.traverse(pvc, scope);
return pvc.numVars;
}

@Override
public StringBuilder print(int indent, StringBuilder output) {
return this.printExpression(indent, output);
}
}

private class PatternVariableCounter extends ASTVisitor {

public int numVars = 0;

@Override
public boolean visit(TypePattern pattern, BlockScope scope) {
if (!pattern.isUnnamed()) {
this.numVars++;
}
return true;
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,12 @@ public boolean dominates(Pattern p) {
return false;
}
}
} else if (p instanceof GuardedPattern gp) {
for (Pattern gpChild : gp.patterns) {
if (!this.dominates(gpChild)) {
return false;
}
}
}
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ private void generateCodePatternCaseEpilogue(CodeStream codeStream, int caseInde
if (this.switchPatternRestartTarget != null && caseStatement != null
&& caseStatement.patternIndex != -1 // for null
) {
Pattern pattern = (Pattern) caseStatement.constantExpressions[caseStatement.patternIndex];
Pattern pattern = (Pattern) caseStatement.constantExpressions[caseStatement.constantExpressions.length - 1];
if (caseStatement.falseLabel != null)
caseStatement.falseLabel.place();
if (pattern.matchFailurePossible()) {
Expand Down
Loading

0 comments on commit 588a3e3

Please sign in to comment.