Skip to content

Commit

Permalink
Code review driven incremental clean up of sealed types implementation (
Browse files Browse the repository at this point in the history 
#3141)

* Get rid of stale @noreference tags
* Fix javadoc
* Fix indentation problems
* Align method names with JLS terminology
* Reuse standard code patterns
* Prevent further checks on a `iterator.remove()`d element
* Get rid of type annotation handling code where no type annotations are legal
  • Loading branch information
@srikanth-sankaran
srikanth-sankaran authored Oct 22, 2024
1 parent f80c910 commit ea94dfa
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 31 deletions.
2 changes: 1 addition & 1 deletion org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ClassFile.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6043,7 +6043,7 @@ public void initialize(SourceTypeBinding aType, ClassFile parentClassFile, boole
if (aType.isAnonymousType()) {
ReferenceBinding superClass = aType.superclass;
if (superClass == null || !(superClass.isEnum() && superClass.isSealed()))
accessFlags &= ~ClassFileConstants.AccFinal;
accessFlags &= ~ClassFileConstants.AccFinal;
}
int finalAbstract = ClassFileConstants.AccFinal | ClassFileConstants.AccAbstract;
if ((accessFlags & finalAbstract) == finalAbstract) {
Expand Down
23 changes: 12 additions & 11 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 @@ -367,7 +367,7 @@ public boolean visit(TNode node) {
}
if (node.type instanceof ReferenceBinding ref && ref.isSealed()) {
List<ReferenceBinding> allAllowedTypes = ref.getAllEnumerableReferenceTypes();
this.covers &= isExhaustiveWithCaseTypes(allAllowedTypes, availableTypes);
this.covers &= caseElementsCoverSelectorType(allAllowedTypes, availableTypes);
return this.covers;
}
this.covers = false;
Expand Down Expand Up @@ -1456,7 +1456,7 @@ private boolean checkAndFlagDefaultSealed(BlockScope skope, CompilerOptions comp
if (!(ref.isClass() || ref.isInterface() || ref.isTypeVariable() || ref.isIntersectionType()))
return false;

if (!isExhaustiveWithCaseTypes(ref.getAllEnumerableReferenceTypes(), this.caseLabelElementTypes)) {
if (!caseElementsCoverSelectorType(ref.getAllEnumerableReferenceTypes(), this.caseLabelElementTypes)) {
if (this instanceof SwitchExpression) // non-exhaustive switch expressions will be flagged later.
return false;
skope.problemReporter().enhancedSwitchMissingDefaultCase(this.expression);
Expand All @@ -1470,7 +1470,7 @@ private boolean checkAndFlagDefaultRecord(BlockScope skope, CompilerOptions comp
List<ReferenceBinding> allallowedTypes = new ArrayList<>();
allallowedTypes.add(ref);
if (comps == null || comps.length == 0) {
if (!isExhaustiveWithCaseTypes(allallowedTypes, this.caseLabelElementTypes)) {
if (!caseElementsCoverSelectorType(allallowedTypes, this.caseLabelElementTypes)) {
skope.problemReporter().enhancedSwitchMissingDefaultCase(this.expression);
return true;
}
Expand All @@ -1490,7 +1490,7 @@ private boolean checkAndFlagDefaultRecord(BlockScope skope, CompilerOptions comp
this.switchBits |= SwitchStatement.Exhaustive;
return false;
}
private boolean isExhaustiveWithCaseTypes(List<ReferenceBinding> allAllowedTypes, List<TypeBinding> listedTypes) {
private boolean caseElementsCoverSelectorType(List<ReferenceBinding> allAllowedTypes, List<TypeBinding> listedTypes) {
Iterator<ReferenceBinding> iterator = allAllowedTypes.iterator();
while (iterator.hasNext()) {
ReferenceBinding next = iterator.next();
Expand All @@ -1502,18 +1502,19 @@ private boolean isExhaustiveWithCaseTypes(List<ReferenceBinding> allAllowedTypes
iterator.remove();
continue;
}
for (TypeBinding type : listedTypes) {
// permits specifies classes, not parameterizations
if (next.erasure().isCompatibleWith(type.erasure())) {
iterator.remove();
break;
}
}
if (next.isEnum()) {
int constantCount = this.otherConstants == null ? 0 : this.otherConstants.length;
Set<FieldBinding> unenumeratedConstants = unenumeratedConstants(next, constantCount);
if (unenumeratedConstants.size() == 0) {
iterator.remove();
continue;
}
}
for (TypeBinding type : listedTypes) {
// permits specifies classes, not parameterizations
if (next.erasure().isCompatibleWith(type.erasure())) {
iterator.remove();
break;
}
}
}
Expand Down
6 changes: 2 additions & 4 deletions ...t.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/classfmt/ClassFileReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -528,9 +528,7 @@ public ClassFileReader(byte[] classFileBytes, char[] fileName, boolean fullyInit
offset += 2;
this.permittedSubtypesNames = new char[this.permittedSubtypesCount][];
for (int j = 0; j < this.permittedSubtypesCount; j++) {
utf8Offset =
this.constantPoolOffsets[u2At(this.constantPoolOffsets[u2At(offset)] + 1)];
this.permittedSubtypesNames[j] = CharDeduplication.intern(utf8At(utf8Offset + 3, u2At(utf8Offset + 1)));
this.permittedSubtypesNames[j] = CharDeduplication.intern(getConstantClassNameAt(u2At(offset)));
offset += 2;
}
}
Expand Down Expand Up @@ -1113,7 +1111,7 @@ && hasStructuralTypeAnnotationChanges(getTypeAnnotations(), newClassFile.getType
return true;
}

// permitted sub-types
// permitted subtypes
char[][] newPermittedSubtypesNames = newClassFile.getPermittedSubtypesNames();
if (this.permittedSubtypesNames != newPermittedSubtypesNames) {
int newPermittedSubtypesLength = newPermittedSubtypesNames == null ? 0 : newPermittedSubtypesNames.length;
Expand Down
6 changes: 3 additions & 3 deletions ...clipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/env/IBinaryType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,11 @@ public interface IBinaryType extends IGenericType, IBinaryInfo {
char[][] getInterfaceNames();

/**
* Answer the unresolved names of the receiver's permitted sub types
* Answer the unresolved names of the receiver's permitted subtypes in the
* class file format as specified in section 4.2 of the Java 2 VM spec
* or null if the array is empty.
*
* A name is a simple name or a qualified, dot separated name.
* For example, Hashtable or java.util.Hashtable.
* For example, java.lang.String is java/lang/String.
*/
default char[][] getPermittedSubtypesNames() {
return null;
Expand Down
13 changes: 3 additions & 10 deletions ...t.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -567,12 +567,6 @@ void cachePartsFrom(IBinaryType binaryType, boolean needFieldsAndMethods) {
break;
}
}
for (TypeBinding permsub : this.permittedTypes) {
if (permsub.hasNullTypeAnnotations()) {
this.externalAnnotationStatus = ExternalAnnotationStatus.TYPE_IS_ANNOTATED;
break;
}
}
}
}

Expand Down Expand Up @@ -2558,9 +2552,8 @@ public ReferenceBinding[] permittedTypes() {
return this.permittedTypes = this.prototype.permittedTypes();
}
for (int i = this.permittedTypes.length; --i >= 0;)
this.permittedTypes[i] = (ReferenceBinding) resolveType(this.permittedTypes[i], this.environment, false);
this.permittedTypes[i] = (ReferenceBinding) resolveType(this.permittedTypes[i], this.environment, false); // re-resolution seems harmless

// Note: unlike for superinterfaces() hierarchy check not required here since these are subtypes
return this.permittedTypes;
}
@Override
Expand Down Expand Up @@ -2635,13 +2628,13 @@ public String toString() {
if (this.permittedTypes != Binding.NO_PERMITTED_TYPES) {
buffer.append("\n\tpermits : "); //$NON-NLS-1$
for (int i = 0, length = this.permittedTypes.length; i < length; i++) {
if (i > 0)
if (i > 0)
buffer.append(", "); //$NON-NLS-1$
buffer.append((this.permittedTypes[i] != null) ? this.permittedTypes[i].debugName() : "NULL TYPE"); //$NON-NLS-1$
}
}
} else {
buffer.append("NULL PERMITTEDSUBTYPES"); //$NON-NLS-1$
buffer.append("NULL PERMITTED SUBTYPES"); //$NON-NLS-1$
}

if (this.enclosingType != null) {
Expand Down
1 change: 0 additions & 1 deletion org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/BinaryType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,6 @@ public boolean isRecord() throws JavaModelException {
}
/**
* @see IType#isSealed()
* @noreference This method is not intended to be referenced by clients as it is a part of Java preview feature.
*/
@Override
public boolean isSealed() throws JavaModelException {
Expand Down
1 change: 0 additions & 1 deletion org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/SourceType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,6 @@ public boolean isRecord() throws JavaModelException {
}
/**
* @see IType#isSealed()
* @noreference This method is not intended to be referenced by clients as it is a part of Java preview feature.
*/
@Override
public boolean isSealed() throws JavaModelException {
Expand Down

0 comments on commit ea94dfa

Please sign in to comment.