changeset 2797:11743872bfc9

8068517: Compiler may generate wrong InnerClasses attribute for static enum reference Summary: Making sure enum's abstractness is resolved before writing InnerClasses entry about it. Reviewed-by: mcimadamore
author jlahoda
date Fri, 13 Feb 2015 17:18:21 +0100
parents e59ced856c92
children 20bf47dc2a91
files src/share/classes/com/sun/tools/javac/code/Symbol.java src/share/classes/com/sun/tools/javac/code/Types.java src/share/classes/com/sun/tools/javac/comp/Attr.java src/share/classes/com/sun/tools/javac/comp/Check.java src/share/classes/com/sun/tools/javac/jvm/ClassWriter.java test/tools/javac/classfiles/InnerClasses/T8068517.java
diffstat 6 files changed, 202 insertions(+), 62 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/classes/com/sun/tools/javac/code/Symbol.java	Wed Feb 11 12:18:57 2015 -0800
+++ b/src/share/classes/com/sun/tools/javac/code/Symbol.java	Fri Feb 13 17:18:21 2015 +0100
@@ -1153,6 +1153,16 @@
         public <R, P> R accept(Symbol.Visitor<R, P> v, P p) {
             return v.visitClassSymbol(this, p);
         }
+
+        public void markAbstractIfNeeded(Types types) {
+            if (types.enter.getEnv(this) != null &&
+                (flags() & ENUM) != 0 && types.supertype(type).tsym == types.syms.enumSym &&
+                (flags() & (FINAL | ABSTRACT)) == 0) {
+                if (types.firstUnimplementedAbstract(this) != null)
+                    // add the ABSTRACT flag to an enum
+                    flags_field |= ABSTRACT;
+            }
+        }
     }
 
 
--- a/src/share/classes/com/sun/tools/javac/code/Types.java	Wed Feb 11 12:18:57 2015 -0800
+++ b/src/share/classes/com/sun/tools/javac/code/Types.java	Fri Feb 13 17:18:21 2015 +0100
@@ -47,6 +47,7 @@
 import com.sun.tools.javac.util.*;
 import static com.sun.tools.javac.code.BoundKind.*;
 import static com.sun.tools.javac.code.Flags.*;
+import static com.sun.tools.javac.code.Kinds.MTH;
 import static com.sun.tools.javac.code.Scope.*;
 import static com.sun.tools.javac.code.Symbol.*;
 import static com.sun.tools.javac.code.Type.*;
@@ -85,6 +86,7 @@
     final boolean allowBoxing;
     final boolean allowCovariantReturns;
     final boolean allowObjectToPrimitiveCast;
+    final boolean allowDefaultMethods;
     final ClassReader reader;
     final Check chk;
     final Enter enter;
@@ -111,6 +113,7 @@
         allowBoxing = source.allowBoxing();
         allowCovariantReturns = source.allowCovariantReturns();
         allowObjectToPrimitiveCast = source.allowObjectToPrimitiveCast();
+        allowDefaultMethods = source.allowDefaultMethods();
         reader = ClassReader.instance(context);
         chk = Check.instance(context);
         enter = Enter.instance(context);
@@ -2763,6 +2766,59 @@
     // </editor-fold>
 
 
+    /** Return first abstract member of class `sym'.
+     */
+    public MethodSymbol firstUnimplementedAbstract(ClassSymbol sym) {
+        try {
+            return firstUnimplementedAbstractImpl(sym, sym);
+        } catch (CompletionFailure ex) {
+            chk.completionError(enter.getEnv(sym).tree.pos(), ex);
+            return null;
+        }
+    }
+        //where:
+        private MethodSymbol firstUnimplementedAbstractImpl(ClassSymbol impl, ClassSymbol c) {
+            MethodSymbol undef = null;
+            // Do not bother to search in classes that are not abstract,
+            // since they cannot have abstract members.
+            if (c == impl || (c.flags() & (ABSTRACT | INTERFACE)) != 0) {
+                Scope s = c.members();
+                for (Scope.Entry e = s.elems;
+                     undef == null && e != null;
+                     e = e.sibling) {
+                    if (e.sym.kind == MTH &&
+                        (e.sym.flags() & (ABSTRACT|IPROXY|DEFAULT)) == ABSTRACT) {
+                        MethodSymbol absmeth = (MethodSymbol)e.sym;
+                        MethodSymbol implmeth = absmeth.implementation(impl, this, true);
+                        if (implmeth == null || implmeth == absmeth) {
+                            //look for default implementations
+                            if (allowDefaultMethods) {
+                                MethodSymbol prov = interfaceCandidates(impl.type, absmeth).head;
+                                if (prov != null && prov.overrides(absmeth, impl, this, true)) {
+                                    implmeth = prov;
+                                }
+                            }
+                        }
+                        if (implmeth == null || implmeth == absmeth) {
+                            undef = absmeth;
+                        }
+                    }
+                }
+                if (undef == null) {
+                    Type st = supertype(c.type);
+                    if (st.hasTag(CLASS))
+                        undef = firstUnimplementedAbstractImpl(impl, (ClassSymbol)st.tsym);
+                }
+                for (List<Type> l = interfaces(c.type);
+                     undef == null && l.nonEmpty();
+                     l = l.tail) {
+                    undef = firstUnimplementedAbstractImpl(impl, (ClassSymbol)l.head.tsym);
+                }
+            }
+            return undef;
+        }
+
+
     //where
     public List<MethodSymbol> interfaceCandidates(Type site, MethodSymbol ms) {
         Filter<Symbol> filter = new MethodFilter(ms, site);
--- a/src/share/classes/com/sun/tools/javac/comp/Attr.java	Wed Feb 11 12:18:57 2015 -0800
+++ b/src/share/classes/com/sun/tools/javac/comp/Attr.java	Fri Feb 13 17:18:21 2015 +0100
@@ -4269,6 +4269,8 @@
             chk.validate(tree.implementing, env);
         }
 
+        c.markAbstractIfNeeded(types);
+
         // If this is a non-abstract class, check that it has no abstract
         // methods or unimplemented methods of an implemented interface.
         if ((c.flags() & (ABSTRACT | INTERFACE)) == 0) {
--- a/src/share/classes/com/sun/tools/javac/comp/Check.java	Wed Feb 11 12:18:57 2015 -0800
+++ b/src/share/classes/com/sun/tools/javac/comp/Check.java	Fri Feb 13 17:18:21 2015 +0100
@@ -2049,70 +2049,15 @@
      *  @param c            The class.
      */
     void checkAllDefined(DiagnosticPosition pos, ClassSymbol c) {
-        try {
-            MethodSymbol undef = firstUndef(c, c);
-            if (undef != null) {
-                if ((c.flags() & ENUM) != 0 &&
-                    types.supertype(c.type).tsym == syms.enumSym &&
-                    (c.flags() & FINAL) == 0) {
-                    // add the ABSTRACT flag to an enum
-                    c.flags_field |= ABSTRACT;
-                } else {
-                    MethodSymbol undef1 =
-                        new MethodSymbol(undef.flags(), undef.name,
-                                         types.memberType(c.type, undef), undef.owner);
-                    log.error(pos, "does.not.override.abstract",
-                              c, undef1, undef1.location());
-                }
-            }
-        } catch (CompletionFailure ex) {
-            completionError(pos, ex);
+        MethodSymbol undef = types.firstUnimplementedAbstract(c);
+        if (undef != null) {
+            MethodSymbol undef1 =
+                new MethodSymbol(undef.flags(), undef.name,
+                                 types.memberType(c.type, undef), undef.owner);
+            log.error(pos, "does.not.override.abstract",
+                      c, undef1, undef1.location());
         }
     }
-//where
-        /** Return first abstract member of class `c' that is not defined
-         *  in `impl', null if there is none.
-         */
-        private MethodSymbol firstUndef(ClassSymbol impl, ClassSymbol c) {
-            MethodSymbol undef = null;
-            // Do not bother to search in classes that are not abstract,
-            // since they cannot have abstract members.
-            if (c == impl || (c.flags() & (ABSTRACT | INTERFACE)) != 0) {
-                Scope s = c.members();
-                for (Scope.Entry e = s.elems;
-                     undef == null && e != null;
-                     e = e.sibling) {
-                    if (e.sym.kind == MTH &&
-                        (e.sym.flags() & (ABSTRACT|IPROXY|DEFAULT)) == ABSTRACT) {
-                        MethodSymbol absmeth = (MethodSymbol)e.sym;
-                        MethodSymbol implmeth = absmeth.implementation(impl, types, true);
-                        if (implmeth == null || implmeth == absmeth) {
-                            //look for default implementations
-                            if (allowDefaultMethods) {
-                                MethodSymbol prov = types.interfaceCandidates(impl.type, absmeth).head;
-                                if (prov != null && prov.overrides(absmeth, impl, types, true)) {
-                                    implmeth = prov;
-                                }
-                            }
-                        }
-                        if (implmeth == null || implmeth == absmeth) {
-                            undef = absmeth;
-                        }
-                    }
-                }
-                if (undef == null) {
-                    Type st = types.supertype(c.type);
-                    if (st.hasTag(CLASS))
-                        undef = firstUndef(impl, (ClassSymbol)st.tsym);
-                }
-                for (List<Type> l = types.interfaces(c.type);
-                     undef == null && l.nonEmpty();
-                     l = l.tail) {
-                    undef = firstUndef(impl, (ClassSymbol)l.head.tsym);
-                }
-            }
-            return undef;
-        }
 
     void checkNonCyclicDecl(JCClassDecl tree) {
         CycleChecker cc = new CycleChecker();
--- a/src/share/classes/com/sun/tools/javac/jvm/ClassWriter.java	Wed Feb 11 12:18:57 2015 -0800
+++ b/src/share/classes/com/sun/tools/javac/jvm/ClassWriter.java	Fri Feb 13 17:18:21 2015 +0100
@@ -1027,6 +1027,7 @@
              l.nonEmpty();
              l = l.tail) {
             ClassSymbol inner = l.head;
+            inner.markAbstractIfNeeded(types);
             char flags = (char) adjustFlags(inner.flags_field);
             if ((flags & INTERFACE) != 0) flags |= ABSTRACT; // Interfaces are always ABSTRACT
             if (inner.name.isEmpty()) flags &= ~FINAL; // Anonymous class: unset FINAL flag
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/tools/javac/classfiles/InnerClasses/T8068517.java	Fri Feb 13 17:18:21 2015 +0100
@@ -0,0 +1,126 @@
+/*
+ * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/** @test
+ *  @bug 8068517
+ *  @summary Verify that nested enums have correct abstract flag in the InnerClasses attribute.
+ *  @library /tools/javac/lib
+ *  @build ToolBox T8068517
+ *  @run main T8068517
+ */
+
+import com.sun.tools.javac.util.Assert;
+import java.io.File;
+import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import javax.tools.JavaCompiler;
+import javax.tools.ToolProvider;
+
+public class T8068517 {
+
+    public static void main(String[] args) throws Exception {
+        new T8068517().run();
+    }
+
+    void run() throws Exception {
+        runTest("class A {\n" +
+                "    enum AInner implements Runnable {\n" +
+                "        A {\n" +
+                "            public void run() {}\n" +
+                "        };\n" +
+                "    }\n" +
+                "}\n",
+                "class B {\n" +
+                "    A.AInner a;\n" +
+                "}");
+        runTest("class A {\n" +
+                "    enum AInner implements Runnable {\n" +
+                "        A {\n" +
+                "            public void run() {}\n" +
+                "        };\n" +
+                "    }\n" +
+                "    AInner aInner;\n" +
+                "}\n",
+                "class B {\n" +
+                "    void test(A a) {;\n" +
+                "        switch (a.aInner) {\n" +
+                "            case A: break;\n" +
+                "        }\n" +
+                "    };\n" +
+                "}");
+        runTest("class A {\n" +
+                "    enum AInner implements Runnable {\n" +
+                "        A {\n" +
+                "            public void run() {}\n" +
+                "        };\n" +
+                "    }\n" +
+                "    AInner aInner;\n" +
+                "}\n",
+                "class B {\n" +
+                "    void test(A a) {;\n" +
+                "        System.err.println(a.aInner.toString());\n" +
+                "    };\n" +
+                "}");
+        runTest("class A {\n" +
+                "    enum AInner implements Runnable {\n" +
+                "        A {\n" +
+                "            public void run() {}\n" +
+                "        };\n" +
+                "    }\n" +
+                "    AInner aInner() {\n" +
+                "        return null;\n" +
+                "    }\n" +
+                "}\n",
+                "class B {\n" +
+                "    void test(A a) {;\n" +
+                "        System.err.println(a.aInner().toString());\n" +
+                "    };\n" +
+                "}");
+    }
+
+    JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
+    int testN = 0;
+
+    void runTest(String aJava, String bJava) throws Exception {
+        File testClasses = new File(System.getProperty("test.classes"));
+        File target1 = new File(testClasses, "T8068517s" + testN++);
+        doCompile(target1, aJava, bJava);
+        File target2 = new File(testClasses, "T8068517s" + testN++);
+        doCompile(target2, bJava, aJava);
+
+        Assert.check(Arrays.equals(Files.readAllBytes(new File(target1, "B.class").toPath()),
+                                   Files.readAllBytes(new File(target2, "B.class").toPath())));
+    }
+
+    void doCompile(File target, String... sources) throws Exception {
+        target.mkdirs();
+        List<String> options = Arrays.asList("-d", target.getAbsolutePath());
+        List<ToolBox.JavaSource> files = Stream.of(sources)
+                                               .map(ToolBox.JavaSource::new)
+                                               .collect(Collectors.toList());
+        compiler.getTask(null, null, null, options, null, files).call();
+    }
+}