Skip to content

Commit

Permalink
Isolate sealing of hierarchy into a separate phase to prevent diagnosing
Browse files Browse the repository at this point in the history
type hierarchy cycles where none exist

* Fixes eclipse-jdt#2093
  • Loading branch information
srikanth-sankaran committed Jun 13, 2024
1 parent b34e3b1 commit a365a11
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1248,7 +1248,6 @@ private boolean connectEnumSuperclass() {
}
return !foundCycle;
}
// Call only when we know there's no explicit permits clause and this is a sealed type
private void connectImplicitPermittedTypes(SourceTypeBinding sourceType) {
List<SourceTypeBinding> types = new ArrayList<>();
for (TypeDeclaration typeDecl : this.referenceCompilationUnit().types) {
Expand All @@ -1270,9 +1269,6 @@ private void connectImplicitPermittedTypes(SourceTypeBinding sourceType) {
}
sourceType.setPermittedTypes(permSubTypes.toArray(new ReferenceBinding[0]));
}
/**
* @see #connectPermittedTypes()
*/
void connectImplicitPermittedTypes() {
TypeDeclaration typeDecl = this.referenceContext;
SourceTypeBinding sourceType = typeDecl.binding;
Expand All @@ -1288,25 +1284,6 @@ void connectImplicitPermittedTypes() {
((SourceTypeBinding) memberType).scope.connectImplicitPermittedTypes();
}
}
/**
* This method only deals with the permitted types that are explicitly declared
* in a type's permits clause. The implicitly permitted types are all filled in
* in {@link #connectImplicitPermittedTypes()}. The reason being, the implicitly
* permitted types require the complete type hierarchy to be ready. Therefore, this
* method is called inside {@link #connectTypeHierarchy()} and connectImplicitPermittedTypes()
* is called after the connectTypeHierarchy(). Why can't we do both after connectTypeHierarchy()?
* That is because, in a very specific case of one of an explicitly permitted type also being
* a member type and is referenced in the permits clause without type qualifier, we would allow
* the following incorrect code:
* <pre>
* public sealed class X permits Y {
* final class Y extends X {}
* }
* </pre>
* If we were to resolve <code>Y</code> in <code>permits Y</code> after resolving
* the hierarchy, Y is resolved in current scope. However, Y should only be
* allowed with the qualifier, in this case, X.Y.
*/
void connectPermittedTypes() {
SourceTypeBinding sourceType = this.referenceContext.binding;
sourceType.setPermittedTypes(Binding.NO_PERMITTEDTYPES);
Expand Down Expand Up @@ -1478,7 +1455,6 @@ void connectTypeHierarchy() {
}
environment().typesBeingConnected.remove(sourceType);
sourceType.tagBits |= TagBits.EndHierarchyCheck;
// connectPermittedTypes();
noProblems &= connectTypeVariables(this.referenceContext.typeParameters, false);
sourceType.tagBits |= TagBits.TypeVariablesAreConnected;
if (noProblems && sourceType.isHierarchyInconsistent())
Expand Down Expand Up @@ -1518,7 +1494,6 @@ private void connectTypeHierarchyWithoutMembers() {
noProblems &= connectSuperInterfaces();
environment().typesBeingConnected.remove(sourceType);
sourceType.tagBits |= TagBits.EndHierarchyCheck;
connectPermittedTypes();
noProblems &= connectTypeVariables(this.referenceContext.typeParameters, false);
sourceType.tagBits |= TagBits.TypeVariablesAreConnected;
if (noProblems && sourceType.isHierarchyInconsistent())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,13 @@ void connectTypeHierarchy() {
for (SourceTypeBinding topLevelType : this.topLevelTypes)
topLevelType.scope.connectTypeHierarchy();
}
void sealTypeHierarchy() {
for (SourceTypeBinding sourceType : this.topLevelTypes) {
sourceType.scope.connectPermittedTypes();
}
for (SourceTypeBinding topLevelType : this.topLevelTypes)
topLevelType.scope.connectImplicitPermittedTypes();
}
void integrateAnnotationsInHierarchy() {
// Only now that all hierarchy information is built we're ready for ...
// ... integrating annotations
Expand All @@ -418,15 +425,6 @@ void integrateAnnotationsInHierarchy() {
} finally {
this.environment.suppressImportErrors = false;
}
// ... checking on permitted types
connectPermittedTypes();
for (SourceTypeBinding topLevelType : this.topLevelTypes)
topLevelType.scope.connectImplicitPermittedTypes();
}
private void connectPermittedTypes() {
for (SourceTypeBinding sourceType : this.topLevelTypes) {
sourceType.scope.connectPermittedTypes();
}
}
void faultInImports() {
if (this.tempImports != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ static enum CompleteTypeBindingsSteps {
NONE,
CHECK_AND_SET_IMPORTS,
CONNECT_TYPE_HIERARCHY,
SEAL_TYPE_HIERARCHY,
BUILD_FIELDS_AND_METHODS,
INTEGRATE_ANNOTATIONS_IN_HIERARCHY,
CHECK_PARAMETERIZED_TYPES;
Expand Down Expand Up @@ -182,6 +183,7 @@ void perform(CompilationUnitScope scope) {
switch (this) {
case CHECK_AND_SET_IMPORTS -> scope.checkAndSetImports();
case CONNECT_TYPE_HIERARCHY -> scope.connectTypeHierarchy();
case SEAL_TYPE_HIERARCHY -> scope.sealTypeHierarchy();
case BUILD_FIELDS_AND_METHODS -> scope.buildFieldsAndMethods();
case INTEGRATE_ANNOTATIONS_IN_HIERARCHY -> scope.integrateAnnotationsInHierarchy();
case CHECK_PARAMETERIZED_TYPES -> scope.checkParameterizedTypes();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5845,4 +5845,36 @@ public void testBug576378_4() {
+ "Type arguments are not allowed here\n"
+ "----------\n");
}
// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2093
// [sealed types] ECJ complains of cycles in hierarchy where none exists
public void testIssue2093() {
runConformTest(
new String[] {
"X.java",
"""
record Bar() implements TheA.FooOrBar {
}
record Foo() implements TheA.FooOrBar {
}
sealed interface Base permits TheA, TheB {
}
record TheA() implements Base {
public sealed interface FooOrBar permits Foo, Bar {
}
}
record TheB<T extends TheA.FooOrBar>() implements Base {
}
public class X {
public static void main(String [] args) {
System.out.println("Compiled and ran fine!");
}
}
"""
},
"Compiled and ran fine!");
}
}

0 comments on commit a365a11

Please sign in to comment.