From a365a11ad34156f63c15283b4bd82937514b8315 Mon Sep 17 00:00:00 2001 From: Srikanth Sankaran Date: Thu, 13 Jun 2024 12:40:36 +0530 Subject: [PATCH] Isolate sealing of hierarchy into a separate phase to prevent diagnosing type hierarchy cycles where none exist * Fixes https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2093 --- .../internal/compiler/lookup/ClassScope.java | 25 --------------- .../compiler/lookup/CompilationUnitScope.java | 16 ++++------ .../compiler/lookup/LookupEnvironment.java | 2 ++ .../compiler/regression/SealedTypesTests.java | 32 +++++++++++++++++++ 4 files changed, 41 insertions(+), 34 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java index 4c770913cf1..dbb1497a595 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java @@ -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 types = new ArrayList<>(); for (TypeDeclaration typeDecl : this.referenceCompilationUnit().types) { @@ -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; @@ -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: - * 
-	 * 	public sealed class X permits Y {
-	 *		final class Y extends X {}
-	 *	}
-	 *
- * If we were to resolve Y in permits Y 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); @@ -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()) @@ -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()) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/CompilationUnitScope.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/CompilationUnitScope.java index b38e2e5a536..3ed1c71e60a 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/CompilationUnitScope.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/CompilationUnitScope.java @@ -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 @@ -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) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/LookupEnvironment.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/LookupEnvironment.java index e796b8430b8..9791c4756d8 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/LookupEnvironment.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/LookupEnvironment.java @@ -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; @@ -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(); diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SealedTypesTests.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SealedTypesTests.java index 86253672e33..f61fceeb2ee 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SealedTypesTests.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SealedTypesTests.java @@ -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() implements Base { + } + public class X { + public static void main(String [] args) { + System.out.println("Compiled and ran fine!"); + } + } + """ + }, + "Compiled and ran fine!"); + } }