Skip to content

Commit

Permalink
Code review driven incremental cleanup of sealed types implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
@srikanth-sankaran
srikanth-sankaran committed Oct 23, 2024
1 parent 01f090e commit 69849ad
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 51 deletions.
2 changes: 2 additions & 0 deletions ...ipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CaseStatement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,8 @@ private Constant resolveCasePattern(BlockScope scope, TypeBinding caseType, Type
if (type != null) {
constant = IntConstant.fromValue(switchStatement.constantIndex);
switchStatement.caseLabelElements.add(e);
if (e instanceof RecordPattern)
switchStatement.containsRecordPatterns = true;

if (isUnguarded)
switchStatement.caseLabelElementTypes.add(type);
Expand Down
61 changes: 18 additions & 43 deletions ...se.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public static record SingletonBootstrap(String id, char[] selector, char[] signa
public int switchBits;

public boolean containsPatterns;
public boolean containsRecordPatterns;
public boolean containsNull;
boolean nullProcessed = false;
BranchLabel switchPatternRestartTarget;
Expand Down Expand Up @@ -824,13 +825,6 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream) {
int max = localKeysCopy[constantCount - 1];
int min = localKeysCopy[0];
if ((long) (constantCount * 2.5) > ((long) max - (long) min)) {

// work-around 1.3 VM bug, if max>0x7FFF0000, must use lookup bytecode
// see http://dev.eclipse.org/bugs/show_bug.cgi?id=21557
if (max > 0x7FFF0000 && currentScope.compilerOptions().complianceLevel < ClassFileConstants.JDK1_4) {
codeStream.lookupswitch(defaultLabel, this.constants, sortedIndexes, caseLabels);

} else {
codeStream.tableswitch(
defaultLabel,
min,
Expand All @@ -839,7 +833,6 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream) {
sortedIndexes,
this.constMapping,
caseLabels);
}
} else {
codeStream.lookupswitch(defaultLabel, this.constants, sortedIndexes, caseLabels);
}
Expand Down Expand Up @@ -1114,7 +1107,7 @@ public void resolve(BlockScope upperScope) {
boolean isStringSwitch = false;
TypeBinding expressionType = this.expression.resolveType(upperScope);
CompilerOptions compilerOptions = upperScope.compilerOptions();
boolean isEnhanced = checkAndSetEnhanced(upperScope, expressionType);
boolean isEnhanced = isEnhancedSwitch(upperScope, expressionType);
if (expressionType != null) {
this.expression.computeConversion(upperScope, expressionType, expressionType);
checkType: {
Expand All @@ -1131,14 +1124,11 @@ public void resolve(BlockScope upperScope) {
break checkType;
} else if (expressionType.isEnum()) {
isEnumSwitch = true;
if (compilerOptions.complianceLevel < ClassFileConstants.JDK1_5) {
upperScope.problemReporter().incorrectSwitchType(this.expression, expressionType); // https://bugs.eclipse.org/bugs/show_bug.cgi?id=360317
}
break checkType;
} else if (!this.containsPatterns && !this.containsNull && upperScope.isBoxingCompatibleWith(expressionType, TypeBinding.INT)) {
this.expression.computeConversion(upperScope, TypeBinding.INT, expressionType);
break checkType;
} else if (compilerOptions.complianceLevel >= ClassFileConstants.JDK1_7 && expressionType.id == TypeIds.T_JavaLangString) {
} else if (expressionType.id == TypeIds.T_JavaLangString) {
if (this.containsPatterns || this.containsNull) {
isStringSwitch = !JavaFeature.PATTERN_MATCHING_IN_SWITCH.isSupported(compilerOptions);
this.isNonTraditional = true;
Expand Down Expand Up @@ -1310,7 +1300,7 @@ && defaultFound && isExhaustive()) {
}
}
// Exhaustiveness check for enum switch
if (isEnumSwitch && compilerOptions.complianceLevel >= ClassFileConstants.JDK1_5) {
if (isEnumSwitch) {
if (this.defaultCase == null || compilerOptions.reportMissingEnumCaseDespiteDefault) {
int constantCount = this.otherConstants == null ? 0 : this.otherConstants.length;
if (isEnhanced)
Expand Down Expand Up @@ -1389,7 +1379,7 @@ private boolean isExhaustive() {
return (this.switchBits & SwitchStatement.Exhaustive) != 0;
}

private boolean checkAndSetEnhanced(BlockScope upperScope, TypeBinding expressionType) {
private boolean isEnhancedSwitch(BlockScope upperScope, TypeBinding expressionType) {
if (JavaFeature.PATTERN_MATCHING_IN_SWITCH.isSupported(upperScope.compilerOptions())
&& expressionType != null && !(this instanceof SwitchExpression )) {

Expand Down Expand Up @@ -1430,37 +1420,22 @@ private boolean checkAndSetEnhanced(BlockScope upperScope, TypeBinding expressio
}
return false;
}

private boolean checkAndFlagDefaultSealed(BlockScope skope, CompilerOptions compilerOptions) {
if (this.defaultCase != null) {
this.switchBits |= SwitchStatement.Exhaustive;
return false;
}
if (this.expression.resolvedType instanceof ReferenceBinding ref && ref.isRecord()) {
boolean isRecordPattern = false;
for (Pattern pattern : this.caseLabelElements) {
if (pattern instanceof RecordPattern) {
isRecordPattern = true;
break;
}
final TypeBinding selectorType = this.expression.resolvedType;
if (this.defaultCase == null && selectorType != null) {
if (selectorType.isRecord() && this.containsRecordPatterns) {
return checkAndFlagDefaultRecord(skope, compilerOptions, (ReferenceBinding) selectorType);
}
if (isRecordPattern)
return checkAndFlagDefaultRecord(skope, compilerOptions, ref);
}
boolean checkSealed = JavaFeature.SEALED_CLASSES.isSupported(compilerOptions)
&& JavaFeature.PATTERN_MATCHING_IN_SWITCH.isSupported(compilerOptions)
&& this.expression.resolvedType instanceof ReferenceBinding
&& this.expression.resolvedType.isSealed();

if (!checkSealed) return false;
ReferenceBinding ref = (ReferenceBinding) this.expression.resolvedType;
if (!(ref.isClass() || ref.isInterface() || ref.isTypeVariable() || ref.isIntersectionType()))
return false;

if (!caseElementsCoverSelectorType(ref.getAllEnumerableReferenceTypes(), this.caseLabelElementTypes)) {
if (this instanceof SwitchExpression) // non-exhaustive switch expressions will be flagged later.
if (!(JavaFeature.PATTERN_MATCHING_IN_SWITCH.isSupported(compilerOptions) && selectorType.isSealed()))
return false;
skope.problemReporter().enhancedSwitchMissingDefaultCase(this.expression);
return true;

if (!caseElementsCoverSelectorType(selectorType.getAllEnumerableReferenceTypes(), this.caseLabelElementTypes)) {
if (this instanceof SwitchExpression) // non-exhaustive switch expressions will be flagged later.
return false;
skope.problemReporter().enhancedSwitchMissingDefaultCase(this.expression);
return true;
}
}
this.switchBits |= SwitchStatement.Exhaustive;
return false;
Expand Down
2 changes: 1 addition & 1 deletion ...lipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/JavaFeature.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public enum JavaFeature {

SEALED_CLASSES(ClassFileConstants.JDK17,
Messages.bind(Messages.sealed_types),
new char[][] {TypeConstants.SEALED, TypeConstants.PERMITS},
new char[][] {TypeConstants.SEALED, TypeConstants.NON_SEALED, TypeConstants.PERMITS},
false),
PATTERN_MATCHING_IN_SWITCH(ClassFileConstants.JDK21,
Messages.bind(Messages.pattern_matching_switch),
Expand Down
7 changes: 2 additions & 5 deletions ...dt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
import org.eclipse.jdt.internal.compiler.ast.NullAnnotationMatching;
import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
import org.eclipse.jdt.internal.compiler.impl.JavaFeature;
import org.eclipse.jdt.internal.compiler.impl.ReferenceContext;
import org.eclipse.jdt.internal.compiler.util.SimpleLookupTable;

Expand Down Expand Up @@ -2368,10 +2367,8 @@ public MethodBinding getSingleAbstractMethod(Scope scope, boolean replaceWildcar
return this.singleAbstractMethod[index];
} else {
this.singleAbstractMethod = new MethodBinding[2];
// Sec 9.8 of sealed preview - A functional interface is an interface that is not declared sealed...
if (JavaFeature.SEALED_CLASSES.isSupported(scope.compilerOptions())
&& this.isSealed())
return this.singleAbstractMethod[index] = samProblemBinding;
if (this.isSealed())
return this.singleAbstractMethod[index] = samProblemBinding; // JLS 9.8
}

if (this.compoundName != null)
Expand Down
1 change: 1 addition & 0 deletions ...e.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ public interface TypeConstants {
// JEP 360 Sealed
char[] PERMITS = "permits".toCharArray(); //$NON-NLS-1$
char[] SEALED = "sealed".toCharArray(); //$NON-NLS-1$
char[] NON_SEALED = "non-sealed".toCharArray(); //$NON-NLS-1$
String KEYWORD_EXTENDS = "extends"; //$NON-NLS-1$
String IMPLEMENTS = "implements"; //$NON-NLS-1$

Expand Down
1 change: 0 additions & 1 deletion org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Parser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9936,7 +9936,6 @@ protected void consumeToken(int type) {
pushOnIntStack(this.scanner.startPosition);
break;
case TokenNameRestrictedIdentifierpermits:
problemReporter().validateJavaFeatureSupport(JavaFeature.SEALED_CLASSES, this.scanner.startPosition,this.scanner.currentPosition - 1);
pushOnIntStack(this.scanner.startPosition);
break;
case TokenNamecase :
Expand Down
1 change: 0 additions & 1 deletion ....tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ public class SwitchPatternTest extends AbstractRegressionTest9 {
// TESTS_NUMBERS = new int [] { 40 };
// TESTS_RANGE = new int[] { 1, -1 };
// TESTS_NAMES = new String[] { "testBug575053_002"};
// TESTS_NAMES = new String[] { "testBug575571_1"};
}

private static String previewLevel = "21";
Expand Down

0 comments on commit 69849ad

Please sign in to comment.