From 69849ad8565217fefa9a8df3783767424e701adf Mon Sep 17 00:00:00 2001 From: Srikanth Sankaran Date: Wed, 23 Oct 2024 07:28:13 +0530 Subject: [PATCH] Code review driven incremental cleanup of sealed types implementation --- .../internal/compiler/ast/CaseStatement.java | 2 + .../compiler/ast/SwitchStatement.java | 61 ++++++------------- .../internal/compiler/impl/JavaFeature.java | 2 +- .../compiler/lookup/ReferenceBinding.java | 7 +-- .../compiler/lookup/TypeConstants.java | 1 + .../jdt/internal/compiler/parser/Parser.java | 1 - .../regression/SwitchPatternTest.java | 1 - 7 files changed, 24 insertions(+), 51 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CaseStatement.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CaseStatement.java index 3de3773e464..49fb1ecfaff 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CaseStatement.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CaseStatement.java @@ -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); diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java index e33f7e345bf..d2288534f70 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java @@ -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; @@ -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, @@ -839,7 +833,6 @@ public void generateCode(BlockScope currentScope, CodeStream codeStream) { sortedIndexes, this.constMapping, caseLabels); - } } else { codeStream.lookupswitch(defaultLabel, this.constants, sortedIndexes, caseLabels); } @@ -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: { @@ -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; @@ -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) @@ -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 )) { @@ -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; diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/JavaFeature.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/JavaFeature.java index 2edf0038c14..09b4cbe1428 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/JavaFeature.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/JavaFeature.java @@ -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), diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java index a0a9cc5e1d1..5b2d5017050 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java @@ -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; @@ -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) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java index f9e2d8a59b6..2a72d85fa51 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeConstants.java @@ -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$ diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Parser.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Parser.java index 6c32f715eac..6b07efd14d8 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Parser.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Parser.java @@ -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 : diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java index 300b50edee8..a802d08ba5e 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java @@ -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";