8246486: javac doesn't allow a subclass to be declared before a seale…

…d superclass with no permits clause Reviewed-by: mcimadamore
openjdk · Jun 4, 2020 · 0db1be2 · 0db1be2
1 parent b94314a
commit 0db1be2
Show file tree

Hide file tree

Showing 7 changed files with 128 additions and 14 deletions.
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TypeEnter.java
@@ -717,7 +717,6 @@ protected void attribSuperTypes(Env<AttrContext> env, Env<AttrContext> baseEnv)
             ListBuffer<Symbol> permittedSubtypeSymbols = new ListBuffer<>();
             List<JCExpression> permittedTrees = tree.permitting;
             for (JCExpression permitted : permittedTrees) {
-                permitted = clearTypeParams(permitted);
                 Type pt = attr.attribBase(permitted, baseEnv, false, false, false);
                 permittedSubtypeSymbols.append(pt.tsym);
             }
@@ -731,7 +730,13 @@ protected void attribSuperTypes(Env<AttrContext> env, Env<AttrContext> baseEnv)
                         ? ct.interfaces_field : all_interfaces.toList();
             }
 
-            sym.permitted = permittedSubtypeSymbols.toList();
+            /* it could be that there are already some symbols in the permitted list, for the case
+             * where there are subtypes in the same compilation unit but the permits list is empty
+             * so don't overwrite the permitted list if it is not empty
+             */
+            if (!permittedSubtypeSymbols.isEmpty()) {
+                sym.permitted = permittedSubtypeSymbols.toList();
+            }
             sym.isPermittedExplicit = !permittedSubtypeSymbols.isEmpty();
         }
             //where:

diff --git a/...jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java b/...jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java
@@ -1645,6 +1645,8 @@ public void visitClassDef(JCClassDecl node) {
                         List<JCAnnotation> originalAnnos = rc.getOriginalAnnos();
                         originalAnnos.stream().forEach(a -> visitAnnotation(a));
                     }
+                    // we should empty the list of permitted subclasses for next round
+                    node.sym.permitted = List.nil();
                 }
                 node.sym = null;
             }

diff --git a/test/langtools/lib/combo/tools/javac/combo/CompilationTestCase.java b/test/langtools/lib/combo/tools/javac/combo/CompilationTestCase.java
@@ -65,6 +65,10 @@ protected void setCompileOptions(String... options) {
         compileOptions = options.clone();
     }
 
+    protected String[] getCompileOptions() {
+        return compileOptions.clone();
+    }
+
     protected void appendCompileOptions(String... additionalOptions) {
         String[] moreOptions = additionalOptions.clone();
         String[] newCompileOptions = Arrays.copyOf(compileOptions, compileOptions.length + additionalOptions.length);

diff --git a/test/langtools/tools/javac/enum/FauxEnum3.java b/test/langtools/tools/javac/enum/FauxEnum3.java
@@ -5,11 +5,10 @@
  * @author Joseph D. Darcy
  *
  * @compile/fail/ref=FauxEnum3.out -XDrawDiagnostics  FauxEnum3.java
- * @compile/fail/ref=FauxEnum3.preview.out -XDrawDiagnostics --enable-preview -source ${jdk.version} FauxEnum3.java
+ * @compile/fail/ref=FauxEnum3.out -XDrawDiagnostics --enable-preview -source ${jdk.version} FauxEnum3.java
  */
 
-public class FauxEnum3 extends SpecializedEnum {
-}
+public final class FauxEnum3 extends SpecializedEnum {}
 
 enum SpecializedEnum {
     RED {

diff --git a/test/langtools/tools/javac/enum/FauxEnum3.out b/test/langtools/tools/javac/enum/FauxEnum3.out
@@ -1,2 +1,2 @@
-FauxEnum3.java:11:8: compiler.err.enum.types.not.extensible
+FauxEnum3.java:11:14: compiler.err.enum.types.not.extensible
 1 error
diff --git a/test/langtools/tools/javac/enum/FauxEnum3.preview.out b/test/langtools/tools/javac/enum/FauxEnum3.preview.out
diff --git a/test/langtools/tools/javac/sealed/SealedCompilationTests.java b/test/langtools/tools/javac/sealed/SealedCompilationTests.java
@@ -35,7 +35,8 @@
  *     jdk.compiler/com.sun.tools.javac.main
  * @build toolbox.ToolBox toolbox.JavacTask
  * @compile --enable-preview -source ${jdk.version} SealedCompilationTests.java
- * @run testng/othervm --enable-preview SealedCompilationTests
+ * @run testng/othervm -DuseAP=false --enable-preview SealedCompilationTests
+ * @run testng/othervm -DuseAP=true --enable-preview SealedCompilationTests
  */
 
 import java.lang.constant.ClassDesc;
@@ -52,6 +53,12 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import javax.annotation.processing.AbstractProcessor;
+import javax.annotation.processing.RoundEnvironment;
+import javax.annotation.processing.SupportedAnnotationTypes;
+
+import javax.lang.model.element.TypeElement;
+
 import com.sun.tools.javac.util.Assert;
 
 import static org.testng.Assert.assertEquals;
@@ -70,12 +77,31 @@ public class SealedCompilationTests extends CompilationTestCase {
     ToolBox tb = new ToolBox();
 
     // When sealed classes become a permanent feature, we don't need these any more
-    private static String[] PREVIEW_OPTIONS = {"--enable-preview", "-source",
-                                               Integer.toString(Runtime.version().feature())};
+    private static String[] PREVIEW_OPTIONS = {
+            "--enable-preview",
+            "-source", Integer.toString(Runtime.version().feature())
+    };
+
+    private static String[] PREVIEW_OPTIONS_WITH_AP = {
+            "--enable-preview",
+            "-source", Integer.toString(Runtime.version().feature()),
+            "-processor", SimplestAP.class.getName()
+    };
+
+    /* simplest annotation processor just to force a round of annotation processing for all tests
+     */
+    public static class SimplestAP extends AbstractProcessor {
+        @Override
+        public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
+            return true;
+        }
+    }
 
-    {
+    public SealedCompilationTests() {
+        boolean useAP = System.getProperty("useAP") == null ? false : System.getProperty("useAP").equals("true");
         setDefaultFilename("SealedTest.java");
-        setCompileOptions(PREVIEW_OPTIONS);
+        setCompileOptions(useAP ? PREVIEW_OPTIONS_WITH_AP : PREVIEW_OPTIONS);
+        System.out.println(useAP ? "running all tests using an annotation processor" : "running all tests without annotation processor");
     }
 
     private static final String NO_SHELL = """
@@ -246,6 +272,7 @@ public void testRestrictedKeyword() {
         }
 
         String[] testOptions = {/* no options */};
+        String[] previousCompOptions = getCompileOptions();
         setCompileOptions(testOptions);
         // now testing with preview disabled
         for (String s : List.of(
@@ -258,7 +285,7 @@ public void testRestrictedKeyword() {
         )) {
             assertFail("compiler.err.preview.feature.disabled.plural", s);
         }
-        setCompileOptions(PREVIEW_OPTIONS);
+        setCompileOptions(previousCompOptions);
     }
 
     public void testRejectPermitsInNonSealedClass() {
@@ -750,4 +777,83 @@ final class Sub2 implements Sub {}
             }
         }
     }
+
+    public void testSubClassBeforeSealedClassInSameCU() {
+        for (String s : List.of(
+            """
+            final class Sub extends Sealed {}
+
+            sealed class Sealed {}
+            """,
+            """
+            final class Sub extends Sealed {}
+
+            sealed class Sealed permits Sub {}
+            """,
+            """
+            final class Sub extends Outer.Super {}
+
+            class Outer {
+                sealed static class Super {}
+            }
+            """,
+            """
+            final class Sub extends Outer.Super {}
+
+            class Outer {
+                sealed static class Super permits Sub {}
+            }
+            """,
+            """
+            class Outer {
+                final class Sub extends Super {}
+            }
+
+            sealed class Super {}
+            """,
+            """
+            class Outer {
+                final class Sub extends Super {}
+            }
+
+            sealed class Super permits Outer.Sub{}
+            """,
+            """
+            class Outer1 {
+                final class Sub extends Outer2.Super {}
+            }
+
+            class Outer2 {
+                sealed static class Super {}
+            }
+            """,
+            """
+            class Outer1 {
+                final class Sub extends Outer2.Super {}
+            }
+
+            class Outer2 {
+                sealed static class Super permits Outer1.Sub {}
+            }
+            """,
+            """
+            class Outer {
+                final class Sub extends Outer.Inner.Super {}
+                static class Inner {
+                    sealed static class Super {}
+                }
+            }
+            """,
+            """
+            class Outer {
+                final class Sub extends Outer.Inner.Super {}
+                static class Inner {
+                    sealed static class Super permits Outer.Sub {}
+                }
+            }
+            """
+        )) {
+            assertOK(s);
+        }
+    }
 }