Skip to content

Commit

Permalink
Code review and clean up of sealed classes implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
srikanth-sankaran committed Oct 16, 2024
1 parent 1eb248b commit 7ae7d60
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,17 @@ void cachePartsFrom(IBinaryType binaryType, boolean needFieldsAndMethods) {
this.tagBits |= TagBits.HasUnresolvedSuperinterfaces;
}
}
char[][] permittedSubtypeNames = binaryType.getPermittedSubtypeNames();
if (permittedSubtypeNames != null) {
this.modifiers |= ExtraCompilerModifiers.AccSealed;
int size = permittedSubtypeNames.length;
if (size > 0) {
this.permittedTypes = new ReferenceBinding[size];
for (short i = 0; i < size; i++)
// attempt to find each permitted type if it exists in the cache (otherwise - resolve it when requested)
this.permittedTypes[i] = this.environment.getTypeFromConstantPoolName(permittedSubtypeNames[i], 0, -1, false, missingTypeNames);
}
}
} else {
// attempt to find the superclass if it exists in the cache (otherwise - resolve it when requested)
this.superclass = (ReferenceBinding) this.environment.getTypeFromTypeSignature(wrapper, typeVars, this, missingTypeNames,
Expand All @@ -544,32 +555,6 @@ void cachePartsFrom(IBinaryType binaryType, boolean needFieldsAndMethods) {
types.toArray(this.superInterfaces);
this.tagBits |= TagBits.HasUnresolvedSuperinterfaces;
}

this.permittedTypes = Binding.NO_PERMITTEDTYPES;
if (!wrapper.atEnd()) {
// attempt to find each permitted type if it exists in the cache (otherwise - resolve it when requested)
java.util.ArrayList types = new java.util.ArrayList(2);
short rank = 0;
do {
types.add(this.environment.getTypeFromTypeSignature(wrapper, typeVars, this, missingTypeNames, toplevelWalker.toSupertype(rank++, wrapper.peekFullType())));
} while (!wrapper.atEnd());
this.permittedTypes = new ReferenceBinding[types.size()];
types.toArray(this.permittedTypes);
this.extendedTagBits |= ExtendedTagBits.HasUnresolvedPermittedSubtypes;
}

}
// fall back, in case we haven't got them from signature
char[][] permittedSubtypeNames = binaryType.getPermittedSubtypeNames();
if (this.permittedTypes == Binding.NO_PERMITTEDTYPES && permittedSubtypeNames != null) {
this.modifiers |= ExtraCompilerModifiers.AccSealed;
int size = permittedSubtypeNames.length;
if (size > 0) {
this.permittedTypes = new ReferenceBinding[size];
for (short i = 0; i < size; i++)
// attempt to find each superinterface if it exists in the cache (otherwise - resolve it when requested)
this.permittedTypes[i] = this.environment.getTypeFromConstantPoolName(permittedSubtypeNames[i], 0, -1, false, missingTypeNames, toplevelWalker.toSupertype(i, null));
}
}
boolean canUseNullTypeAnnotations = this.environment.globalOptions.isAnnotationBasedNullAnalysisEnabled && this.environment.globalOptions.sourceLevel >= ClassFileConstants.JDK1_8;
if (canUseNullTypeAnnotations && this.externalAnnotationStatus.isPotentiallyUnannotatedLib()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -599,8 +599,7 @@ private void checkAndSetModifiers() {
CompilerOptions options = compilerOptions();
boolean is16Plus = compilerOptions().sourceLevel >= ClassFileConstants.JDK16;
boolean isSealedSupported = JavaFeature.SEALED_CLASSES.isSupported(options);
boolean flagSealedNonModifiers = isSealedSupported &&
(modifiers & (ExtraCompilerModifiers.AccSealed | ExtraCompilerModifiers.AccNonSealed)) != 0;
boolean hierarchySealed = (modifiers & (ExtraCompilerModifiers.AccSealed | ExtraCompilerModifiers.AccNonSealed)) != 0;

switch (modifiers & (ExtraCompilerModifiers.AccSealed | ExtraCompilerModifiers.AccNonSealed | ClassFileConstants.AccFinal)) {
case ExtraCompilerModifiers.AccSealed, ExtraCompilerModifiers.AccNonSealed, ClassFileConstants.AccFinal, ClassFileConstants.AccDefault : break;
Expand Down Expand Up @@ -643,7 +642,7 @@ private void checkAndSetModifiers() {
}
final int UNEXPECTED_MODIFIERS =~(ClassFileConstants.AccEnum | ClassFileConstants.AccStrictfp);
if ((modifiers & ExtraCompilerModifiers.AccJustFlag & UNEXPECTED_MODIFIERS) != 0
|| flagSealedNonModifiers) {
|| hierarchySealed) {
problemReporter().illegalModifierForLocalEnumDeclaration(sourceType);
return;
}
Expand Down Expand Up @@ -754,7 +753,7 @@ private void checkAndSetModifiers() {
| ClassFileConstants.AccStrictfp | ClassFileConstants.AccAnnotation
| ((is16Plus && this.parent instanceof ClassScope) ? ClassFileConstants.AccStatic : 0));
if ((realModifiers & UNEXPECTED_MODIFIERS) != 0
|| flagSealedNonModifiers)
|| hierarchySealed)
problemReporter().localStaticsIllegalVisibilityModifierForInterfaceLocalType(sourceType);
// if ((modifiers & ClassFileConstants.AccStatic) != 0) {
// problemReporter().recordIllegalStaticModifierForLocalClassOrInterface(sourceType);
Expand All @@ -776,24 +775,23 @@ private void checkAndSetModifiers() {
modifiers |= ClassFileConstants.AccSynthetic;
}
modifiers |= ClassFileConstants.AccAbstract;
} else if ((realModifiers & ClassFileConstants.AccEnum) != 0) {
} else if ((realModifiers & ClassFileConstants.AccEnum) != 0) {
// detect abnormal cases for enums
if (isMemberType) { // includes member types defined inside local types
final int UNEXPECTED_MODIFIERS = ~(ClassFileConstants.AccPublic | ClassFileConstants.AccPrivate | ClassFileConstants.AccProtected | ClassFileConstants.AccStatic | ClassFileConstants.AccStrictfp | ClassFileConstants.AccEnum);
if ((realModifiers & UNEXPECTED_MODIFIERS) != 0 || flagSealedNonModifiers) {
final int UNEXPECTED_MODIFIERS = ~(ClassFileConstants.AccPublic | ClassFileConstants.AccPrivate
| ClassFileConstants.AccProtected | ClassFileConstants.AccStatic
| ClassFileConstants.AccStrictfp | ClassFileConstants.AccEnum);
if ((realModifiers & UNEXPECTED_MODIFIERS) != 0 || hierarchySealed) {
problemReporter().illegalModifierForMemberEnum(sourceType);
modifiers &= ~ClassFileConstants.AccAbstract; // avoid leaking abstract modifier
realModifiers &= ~ClassFileConstants.AccAbstract;
// modifiers &= ~(realModifiers & UNEXPECTED_MODIFIERS);
// realModifiers = modifiers & ExtraCompilerModifiers.AccJustFlag;
}
} else if (sourceType.isLocalType()) {
// if (flagSealedNonModifiers)
// problemReporter().illegalModifierForLocalEnum(sourceType);
// each enum constant is an anonymous local type and its modifiers were already checked as an enum constant field
} else {
final int UNEXPECTED_MODIFIERS = ~(ClassFileConstants.AccPublic | ClassFileConstants.AccStrictfp | ClassFileConstants.AccEnum);
if ((realModifiers & UNEXPECTED_MODIFIERS) != 0 || flagSealedNonModifiers)
} else if (!sourceType.isLocalType()) { // local types already handled earlier.
final int UNEXPECTED_MODIFIERS = ~(ClassFileConstants.AccPublic | ClassFileConstants.AccStrictfp
| ClassFileConstants.AccEnum);
if ((realModifiers & UNEXPECTED_MODIFIERS) != 0 || hierarchySealed)
problemReporter().illegalModifierForEnum(sourceType);
}
if (!sourceType.isAnonymousType()) {
Expand All @@ -808,14 +806,16 @@ private void checkAndSetModifiers() {
TypeDeclaration typeDeclaration = this.referenceContext;
FieldDeclaration[] fields = typeDeclaration.fields;
int fieldsLength = fields == null ? 0 : fields.length;
if (fieldsLength == 0) break checkAbstractEnum; // has no constants so must implement the method itself
if (fieldsLength == 0)
break checkAbstractEnum; // has no constants so must implement the method itself
AbstractMethodDeclaration[] methods = typeDeclaration.methods;
int methodsLength = methods == null ? 0 : methods.length;
// TODO (kent) cannot tell that the superinterfaces are empty or that their methods are implemented
boolean definesAbstractMethod = typeDeclaration.superInterfaces != null;
for (int i = 0; i < methodsLength && !definesAbstractMethod; i++)
definesAbstractMethod = methods[i].isAbstract();
if (!definesAbstractMethod) break checkAbstractEnum; // all methods have bodies
if (!definesAbstractMethod)
break checkAbstractEnum; // all methods have bodies
boolean needAbstractBit = false;
for (int i = 0; i < fieldsLength; i++) {
FieldDeclaration fieldDecl = fields[i];
Expand All @@ -827,8 +827,10 @@ private void checkAndSetModifiers() {
}
}
}
// tag this enum as abstract since an abstract method must be implemented AND all enum constants define an anonymous body
// as a result, each of its anonymous constants will see it as abstract and must implement each inherited abstract method
// tag this enum as abstract since an abstract method must be implemented AND all enum constants
// define an anonymous body
// as a result, each of its anonymous constants will see it as abstract and must implement each
// inherited abstract method
if (needAbstractBit) {
modifiers |= ClassFileConstants.AccAbstract;
}
Expand Down Expand Up @@ -893,7 +895,7 @@ private void checkAndSetModifiers() {
} else if (sourceType.isLocalType()) {
final int UNEXPECTED_MODIFIERS = ~(ClassFileConstants.AccAbstract | ClassFileConstants.AccFinal | ClassFileConstants.AccStrictfp
| ((is16Plus && this.parent instanceof ClassScope) ? ClassFileConstants.AccStatic : 0));
if ((realModifiers & UNEXPECTED_MODIFIERS) != 0 || flagSealedNonModifiers)
if ((realModifiers & UNEXPECTED_MODIFIERS) != 0 || hierarchySealed)
problemReporter().illegalModifierForLocalClass(sourceType);
} else {
final int UNEXPECTED_MODIFIERS = ~(ClassFileConstants.AccPublic | ClassFileConstants.AccAbstract | ClassFileConstants.AccFinal | ClassFileConstants.AccStrictfp);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,5 @@ public interface ExtraCompilerModifiers { // modifier constant
final int AccImplementing = ASTNode.Bit30; // record fact a method implements another one (it is concrete and overrides an abstract one)
final int AccImplicitlyDeclared = ASTNode.Bit30; // used for implicitly declared classes
final int AccGenericSignature = ASTNode.Bit31; // record fact a type/method/field involves generics in its signature (and need special signature attr)
final int AccOutOfFlowScope = ASTNode.Bit29;
final int AccOutOfFlowScope = ASTNode.Bit29; // used to signal pattern binding variables not being live.
}
Original file line number Diff line number Diff line change
Expand Up @@ -1166,7 +1166,7 @@ protected void checkAndSetModifiers(int flag){
problemReporter().StrictfpNotRequired(this.scanner.startPosition, this.scanner.currentPosition - 1);
}

if ((this.modifiers & flag) != 0){ // duplicate modifier
if ((this.modifiers & flag) != 0) { // duplicate modifier
this.modifiers |= ExtraCompilerModifiers.AccAlternateModifierProblem;
}
this.modifiers |= flag;
Expand All @@ -1176,10 +1176,6 @@ protected void checkAndSetModifiers(int flag){
if (this.currentElement != null) {
this.currentElement.addModifier(flag, this.modifiersSourceStart);
}
if (flag == ExtraCompilerModifiers.AccSealed || flag == ExtraCompilerModifiers.AccNonSealed) {
problemReporter().validateJavaFeatureSupport(JavaFeature.SEALED_CLASSES, this.scanner.startPosition,
this.scanner.currentPosition - 1);
}
}
public void checkComment() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9796,7 +9796,7 @@ public boolean validateRestrictedKeywords(char[] name, ASTNode node) {
close();
}
}
//Returns true if the problem is handled and reported (only errors considered and not warnings)
// Returns true if the problem is handled and reported (only errors considered and not warnings)
public boolean validateJavaFeatureSupport(JavaFeature feature, int sourceStart, int sourceEnd) {
boolean versionInRange = feature.getCompliance() <= this.options.sourceLevel;
String version = CompilerOptions.versionFromJdkLevel(feature.getCompliance());
Expand Down

0 comments on commit 7ae7d60

Please sign in to comment.