Skip to content

Commit

Permalink
Last round of polish for Patterns 2.0!
Browse files Browse the repository at this point in the history 
* Use JavaFeature rather than ad-hoc checks for feature availability
* Fix compiler crash when translating non-looping while loops
* Remove superfluous calls to recordInitializationStates and
safeInitsWhenTrue
* Align with JLS on terminology of unguarded patterns
  • Loading branch information
@srikanth-sankaran
srikanth-sankaran committed Mar 5, 2024
1 parent fd09117 commit 4e46b78
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 46 deletions.
6 changes: 2 additions & 4 deletions ...compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/AbstractVariableDeclaration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
*******************************************************************************/
package org.eclipse.jdt.internal.compiler.ast;

import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
import org.eclipse.jdt.internal.compiler.flow.FlowContext;
import org.eclipse.jdt.internal.compiler.flow.FlowInfo;
import org.eclipse.jdt.internal.compiler.impl.JavaFeature;
import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
import org.eclipse.jdt.internal.compiler.lookup.InferenceContext18;
import org.eclipse.jdt.internal.compiler.lookup.InvocationSite;
Expand Down Expand Up @@ -152,8 +152,6 @@ public void setFieldIndex(int depth) {
* @param scope used to determine source level
*/
public boolean isUnnamed(BlockScope scope) {
return this.name.length == 1 && this.name[0] == '_'
&& scope.compilerOptions().sourceLevel >= ClassFileConstants.JDK21
&& scope.compilerOptions().enablePreviewFeatures;
return this.name.length == 1 && this.name[0] == '_' && JavaFeature.UNNAMMED_PATTERNS_AND_VARS.isSupported(scope.compilerOptions().sourceLevel, scope.compilerOptions().enablePreviewFeatures);
}
}
10 changes: 5 additions & 5 deletions ...t.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/EitherOrMultiPattern.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ public void setIsEitherOrPattern() {
}

@Override
public void setIsEffectivelyGuarded() {
public void setIsGuarded() {
for (int i = 0; i < this.patternsCount; i++)
this.patterns[i].setIsEffectivelyGuarded();
this.patterns[i].setIsGuarded();
}

@Override
Expand All @@ -66,7 +66,7 @@ public TypeBinding resolveType(BlockScope scope) {

@Override
public boolean matchFailurePossible() {
if (!isEffectivelyUnguarded())
if (!isUnguarded())
return true;
for (Pattern p : this.patterns) {
if (p.matchFailurePossible())
Expand Down Expand Up @@ -99,7 +99,7 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream, BranchL

@Override
public boolean dominates(Pattern p) {
if (!isEffectivelyUnguarded())
if (!isUnguarded())
return false;
for (Pattern thiz : this.patterns) {
if (thiz.dominates(p))
Expand All @@ -110,7 +110,7 @@ public boolean dominates(Pattern p) {

@Override
public boolean coversType(TypeBinding type) {
if (!isEffectivelyUnguarded())
if (!isUnguarded())
return false;
for (Pattern p : this.patterns) {
if (p.coversType(type))
Expand Down
19 changes: 8 additions & 11 deletions ...pse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/GuardedPattern.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,8 @@ public LocalVariableBinding[] bindingsWhenTrue() {
@Override
public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, FlowInfo flowInfo) {
flowInfo = this.primaryPattern.analyseCode(currentScope, flowContext, flowInfo);
currentScope.methodScope().recordInitializationStates(flowInfo);
FlowInfo mergedFlow = this.condition.analyseCode(currentScope, flowContext, flowInfo);
mergedFlow = mergedFlow.safeInitsWhenTrue();
currentScope.methodScope().recordInitializationStates(mergedFlow);
return mergedFlow;
return mergedFlow.safeInitsWhenTrue();
}

@Override
Expand All @@ -63,16 +60,16 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream, BranchL

@Override
public boolean matchFailurePossible() {
return !isEffectivelyUnguarded() || this.primaryPattern.matchFailurePossible();
return !isUnguarded() || this.primaryPattern.matchFailurePossible();
}

@Override
public boolean isUnconditional(TypeBinding t) {
return isEffectivelyUnguarded() && this.primaryPattern.isUnconditional(t);
return isUnguarded() && this.primaryPattern.isUnconditional(t);
}

@Override
public boolean isEffectivelyUnguarded() {
public boolean isUnguarded() {
Constant cst = this.condition.optimizedBooleanConstant();
return cst != null && cst != Constant.NotAConstant && cst.booleanValue() == true;
}
Expand All @@ -84,12 +81,12 @@ public void setIsEitherOrPattern() {

@Override
public boolean coversType(TypeBinding type) {
return isEffectivelyUnguarded() && this.primaryPattern.coversType(type);
return isUnguarded() && this.primaryPattern.coversType(type);
}

@Override
public boolean dominates(Pattern p) {
return isEffectivelyUnguarded() && this.primaryPattern.dominates(p);
return isUnguarded() && this.primaryPattern.dominates(p);
}

@Override
Expand All @@ -113,8 +110,8 @@ public TypeBinding resolveType(BlockScope scope) {
scope.problemReporter().falseLiteralInGuard(this.condition);
}

if (!isEffectivelyUnguarded())
this.primaryPattern.setIsEffectivelyGuarded();
if (!isUnguarded())
this.primaryPattern.setIsGuarded();

return this.resolvedType = this.primaryPattern.resolvedType;
}
Expand Down
25 changes: 10 additions & 15 deletions org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Pattern.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,20 @@

public abstract class Pattern extends Expression {

/* package */ boolean isTotalTypeNode = false;
boolean isTotalTypeNode = false;

private Pattern enclosingPattern;

protected MethodBinding accessorMethod;

public int index = -1; // index of this in enclosing record pattern, or -1 for top level patterns

public boolean isEffectivelyUnguarded = true; // no guard or guard is compile time constant true.
public boolean isUnguarded = true; // no guard or guard is compile time constant true.

/**
* @return the enclosingPattern
*/
public Pattern getEnclosingPattern() {
return this.enclosingPattern;
}

/**
* @param enclosingPattern the enclosingPattern to set
*/
public void setEnclosingPattern(RecordPattern enclosingPattern) {
this.enclosingPattern = enclosingPattern;
}
Expand All @@ -55,20 +50,20 @@ public boolean isUnnamed() {
* @return whether pattern covers the given type or not
*/
public boolean coversType(TypeBinding type) {
if (!isEffectivelyUnguarded())
if (!isUnguarded())
return false;
if (type == null || this.resolvedType == null)
return false;
return (type.isSubtypeOf(this.resolvedType, false));
}

// Given a non-null instance of appropriate type, would the pattern always match ?
// Given a non-null instance of same type, would the pattern always match ?
public boolean matchFailurePossible() {
return false;
}

public boolean isUnconditional(TypeBinding t) {
return isEffectivelyUnguarded() && coversType(t);
return isUnguarded() && coversType(t);
}

public abstract void generateCode(BlockScope currentScope, CodeStream codeStream, BranchLabel patternMatchLabel, BranchLabel matchFailLabel);
Expand Down Expand Up @@ -120,11 +115,11 @@ public Pattern[] getAlternatives() {

public abstract void setIsEitherOrPattern(); // if set, is one of multiple (case label) patterns and so pattern variables can't be named.

public boolean isEffectivelyUnguarded() {
return this.isEffectivelyUnguarded;
public boolean isUnguarded() {
return this.isUnguarded;
}

public void setIsEffectivelyGuarded() {
this.isEffectivelyUnguarded = false;
public void setIsGuarded() {
this.isUnguarded = false;
}
}
6 changes: 2 additions & 4 deletions ...ipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/RecordPattern.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,16 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
for (Pattern p : this.patterns) {
flowInfo = p.analyseCode(currentScope, flowContext, flowInfo);
}
flowInfo = flowInfo.safeInitsWhenTrue(); // TODO: is this really needed?
return flowInfo;
}

@Override
public boolean coversType(TypeBinding t) {

if (!isEffectivelyUnguarded())
if (!isUnguarded())
return false;

if (TypeBinding.equalsEquals(t, this.resolvedType)) {
// return the already computed value
return this.isTotalTypeNode;
}
if (!t.isRecord())
Expand Down Expand Up @@ -199,7 +197,7 @@ with type S and pattern list M if (i) R and S name the same record class, and (i
every component pattern, if any, in L dominates the corresponding component
pattern in M.
*/
if (!isEffectivelyUnguarded())
if (!isUnguarded())
return false;

if (this.resolvedType == null || !this.resolvedType.isValidBinding() || p.resolvedType == null || !p.resolvedType.isValidBinding())
Expand Down
11 changes: 5 additions & 6 deletions ...clipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypePattern.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,11 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream, BranchL

@Override
public boolean dominates(Pattern p) {
if (isEffectivelyUnguarded()) {
if (p.resolvedType == null || this.resolvedType == null)
return false;
return p.resolvedType.erasure().isSubtypeOf(this.resolvedType.erasure(), false);
}
return false;
if (!isUnguarded())
return false;
if (p.resolvedType == null || this.resolvedType == null)
return false;
return p.resolvedType.erasure().isSubtypeOf(this.resolvedType.erasure(), false);
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion ...pse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/WhileStatement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,8 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream) {
// output condition and branch back to the beginning of the repeated action
if (this.continueLabel != null || conditionInjectsBindings) {
if (conditionInjectsBindings) {
codeStream.goto_(this.continueLabel);
if (this.continueLabel != null)
codeStream.goto_(this.continueLabel);
} else {
this.continueLabel.place();
this.condition.generateOptimizedBoolean(
Expand Down
46 changes: 46 additions & 0 deletions ...ts.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/PatternMatching16Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4766,4 +4766,50 @@ public static void foo(Object o) {
"true\n"
+ "true");
}
public void testWhileLoop() {
runConformTest(
new String[] {
"X.java",
"""
public class X {
void foo(Object o) {
while (o instanceof String s) {
System.out.println("while");
return;
}
System.out.println("!while");
}
public static void main(String [] args) {
new X().foo("");
new X().foo(null);
}
}
""",
},
"while\n"
+ "!while");
}
public void testForLoop() {
runConformTest(
new String[] {
"X.java",
"""
public class X {
void foo(Object o) {
for(; (o instanceof String s);) {
System.out.println("for");
return;
}
System.out.println("!for");
}
public static void main(String [] args) {
new X().foo("");
new X().foo(null);
}
}
""",
},
"for\n"
+ "!for");
}
}

0 comments on commit 4e46b78

Please sign in to comment.