6337171: javac should create bridge methods when type variable bounds restricted

Summary: javac should add synthetic overrides for inherited abstract methods in order to preserve binary compatibility
Reviewed-by: jjg

langtools/src/share/classes/com/sun/tools/javac/code/Flags.java

@@ -241,6 +241,12 @@ public class Flags {
      */
     public static final long POLYMORPHIC_SIGNATURE = 1L<<40;
 
+    /**
+     * Flag that marks a special kind of bridge methods (the ones that
+     * come from restricted supertype bounds)
+     */
+    public static final long OVERRIDE_BRIDGE = 1L<<41;
+
     /** Modifier masks.
      */
     public static final int

langtools/src/share/classes/com/sun/tools/javac/code/Scope.java

@@ -272,6 +272,12 @@ public class Scope {
         return false;
     }
 
+    static final Filter<Symbol> noFilter = new Filter<Symbol>() {
+        public boolean accepts(Symbol s) {
+            return true;
+        }
+    };
+
     /** Return the entry associated with given name, starting in
      *  this scope and proceeding outwards. If no entry was found,
      *  return the sentinel, which is characterized by having a null in
@@ -279,13 +285,20 @@ public class Scope {
      *  for regular entries.
      */
     public Entry lookup(Name name) {
+        return lookup(name, noFilter);
+    }
+    public Entry lookup(Name name, Filter<Symbol> sf) {
         Entry e = table[name.hashCode() & hashMask];
-        while (e.scope != null && e.sym.name != name)
+        while (e.scope != null && (e.sym.name != name || !sf.accepts(e.sym)))
             e = e.shadowed;
         return e;
     }
 
     public Iterable<Symbol> getElements() {
+        return getElements(noFilter);
+    }
+
+    public Iterable<Symbol> getElements(final Filter<Symbol> sf) {
         return new Iterable<Symbol>() {
             public Iterator<Symbol> iterator() {
                 return new Iterator<Symbol>() {
@@ -301,7 +314,9 @@ public class Scope {
 
                     public Symbol next() {
                         Symbol sym = (currEntry == null ? null : currEntry.sym);
-                        currEntry = currEntry.sibling;
+                        if (currEntry != null) {
+                            currEntry = currEntry.sibling;
+                        }
                         update();
                         return sym;
                     }
@@ -311,9 +326,17 @@ public class Scope {
                     }
 
                     private void update() {
+                        skipToNextMatchingEntry();
                         while (currEntry == null && currScope.next != null) {
                             currScope = currScope.next;
                             currEntry = currScope.elems;
+                            skipToNextMatchingEntry();
+                        }
+                    }
+
+                    void skipToNextMatchingEntry() {
+                        while (currEntry != null && !sf.accepts(currEntry.sym)) {
+                            currEntry = currEntry.sibling;
                         }
                     }
                 };

langtools/src/share/classes/com/sun/tools/javac/code/Symbol.java

@@ -1217,7 +1217,18 @@ public abstract class Symbol implements Element {
          *  as possible implementations.
          */
         public MethodSymbol implementation(TypeSymbol origin, Types types, boolean checkResult) {
-            MethodSymbol res = types.implementation(this, origin, types, checkResult);
+            return implementation(origin, types, checkResult, implementation_filter);
+        }
+        // where
+            private static final Filter<Symbol> implementation_filter = new Filter<Symbol>() {
+                public boolean accepts(Symbol s) {
+                    return s.kind == Kinds.MTH &&
+                            (s.flags() & SYNTHETIC) == 0;
+                }
+            };
+
+        public MethodSymbol implementation(TypeSymbol origin, Types types, boolean checkResult, Filter<Symbol> implFilter) {
+            MethodSymbol res = types.implementation(this, origin, types, checkResult, implFilter);
             if (res != null)
                 return res;
             // if origin is derived from a raw type, we might have missed

langtools/src/share/classes/com/sun/tools/javac/code/Types.java

@@ -1974,45 +1974,74 @@ public class Types {
             hasSameArgs(t, erasure(s)) || hasSameArgs(erasure(t), s);
     }
 
-    private WeakHashMap<MethodSymbol, SoftReference<Map<TypeSymbol, MethodSymbol>>> implCache_check =
-            new WeakHashMap<MethodSymbol, SoftReference<Map<TypeSymbol, MethodSymbol>>>();
-
-    private WeakHashMap<MethodSymbol, SoftReference<Map<TypeSymbol, MethodSymbol>>> implCache_nocheck =
-            new WeakHashMap<MethodSymbol, SoftReference<Map<TypeSymbol, MethodSymbol>>>();
-
-    public MethodSymbol implementation(MethodSymbol ms, TypeSymbol origin, Types types, boolean checkResult) {
-        Map<MethodSymbol, SoftReference<Map<TypeSymbol, MethodSymbol>>> implCache = checkResult ?
-            implCache_check : implCache_nocheck;
-        SoftReference<Map<TypeSymbol, MethodSymbol>> ref_cache = implCache.get(ms);
-        Map<TypeSymbol, MethodSymbol> cache = ref_cache != null ? ref_cache.get() : null;
-        if (cache == null) {
-            cache = new HashMap<TypeSymbol, MethodSymbol>();
-            implCache.put(ms, new SoftReference<Map<TypeSymbol, MethodSymbol>>(cache));
-        }
-        MethodSymbol impl = cache.get(origin);
-        if (impl == null) {
+    // <editor-fold defaultstate="collapsed" desc="Determining method implementation in given site">
+    class ImplementationCache {
+
+        private WeakHashMap<MethodSymbol, SoftReference<Map<TypeSymbol, Entry>>> _map =
+                new WeakHashMap<MethodSymbol, SoftReference<Map<TypeSymbol, Entry>>>();
+
+        class Entry {
+            final MethodSymbol cachedImpl;
+            final Filter<Symbol> implFilter;
+            final boolean checkResult;
+
+            public Entry(MethodSymbol cachedImpl,
+                    Filter<Symbol> scopeFilter,
+                    boolean checkResult) {
+                this.cachedImpl = cachedImpl;
+                this.implFilter = scopeFilter;
+                this.checkResult = checkResult;
+            }
+
+            boolean matches(Filter<Symbol> scopeFilter, boolean checkResult) {
+                return this.implFilter == scopeFilter &&
+                        this.checkResult == checkResult;
+            }
+        }
+
+        MethodSymbol get(MethodSymbol ms, TypeSymbol origin, boolean checkResult, Filter<Symbol> implFilter) {
+            SoftReference<Map<TypeSymbol, Entry>> ref_cache = _map.get(ms);
+            Map<TypeSymbol, Entry> cache = ref_cache != null ? ref_cache.get() : null;
+            if (cache == null) {
+                cache = new HashMap<TypeSymbol, Entry>();
+                _map.put(ms, new SoftReference<Map<TypeSymbol, Entry>>(cache));
+            }
+            Entry e = cache.get(origin);
+            if (e == null ||
+                    !e.matches(implFilter, checkResult)) {
+                MethodSymbol impl = implementationInternal(ms, origin, Types.this, checkResult, implFilter);
+                cache.put(origin, new Entry(impl, implFilter, checkResult));
+                return impl;
+            }
+            else {
+                return e.cachedImpl;
+            }
+        }
+
+        private MethodSymbol implementationInternal(MethodSymbol ms, TypeSymbol origin, Types types, boolean checkResult, Filter<Symbol> implFilter) {
             for (Type t = origin.type; t.tag == CLASS || t.tag == TYPEVAR; t = types.supertype(t)) {
                 while (t.tag == TYPEVAR)
                     t = t.getUpperBound();
                 TypeSymbol c = t.tsym;
-                for (Scope.Entry e = c.members().lookup(ms.name);
+                for (Scope.Entry e = c.members().lookup(ms.name, implFilter);
                      e.scope != null;
                      e = e.next()) {
-                    if (e.sym.kind == Kinds.MTH) {
-                        MethodSymbol m = (MethodSymbol) e.sym;
-                        if (m.overrides(ms, origin, types, checkResult) &&
-                            (m.flags() & SYNTHETIC) == 0) {
-                            impl = m;
-                            cache.put(origin, m);
-                            return impl;
-                        }
-                    }
+                    if (e.sym != null &&
+                             e.sym.overrides(ms, origin, types, checkResult))
+                        return (MethodSymbol)e.sym;
                 }
             }
+            return null;
         }
-        return impl;
     }
 
+    private ImplementationCache implCache = new ImplementationCache();
+
+    public MethodSymbol implementation(MethodSymbol ms, TypeSymbol origin, Types types, boolean checkResult, Filter<Symbol> implFilter) {
+        return implCache.get(ms, origin, checkResult, implFilter);
+    }
+    // </editor-fold>
+
     /**
      * Does t have the same arguments as s?  It is assumed that both
      * types are (possibly polymorphic) method types.  Monomorphic

langtools/src/share/classes/com/sun/tools/javac/comp/TransTypes.java

@@ -283,12 +283,13 @@ public class TransTypes extends TreeTranslator {
                            ListBuffer<JCTree> bridges) {
         if (sym.kind == MTH &&
             sym.name != names.init &&
-            (sym.flags() & (PRIVATE | SYNTHETIC | STATIC)) == 0 &&
+            (sym.flags() & (PRIVATE | STATIC)) == 0 &&
+            (sym.flags() & (SYNTHETIC | OVERRIDE_BRIDGE)) != SYNTHETIC &&
             sym.isMemberOf(origin, types))
         {
             MethodSymbol meth = (MethodSymbol)sym;
             MethodSymbol bridge = meth.binaryImplementation(origin, types);
-            MethodSymbol impl = meth.implementation(origin, types, true);
+            MethodSymbol impl = meth.implementation(origin, types, true, overrideBridgeFilter);
             if (bridge == null ||
                 bridge == meth ||
                 (impl != null && !bridge.owner.isSubClass(impl.owner, types))) {
@@ -304,7 +305,7 @@ public class TransTypes extends TreeTranslator {
                     // reflection design error.
                     addBridge(pos, meth, impl, origin, false, bridges);
                 }
-            } else if ((bridge.flags() & SYNTHETIC) != 0) {
+            } else if ((bridge.flags() & (SYNTHETIC | OVERRIDE_BRIDGE)) == SYNTHETIC) {
                 MethodSymbol other = overridden.get(bridge);
                 if (other != null && other != meth) {
                     if (impl == null || !impl.overrides(other, origin, types, true)) {
@@ -327,6 +328,11 @@ public class TransTypes extends TreeTranslator {
         }
     }
     // where
+        Filter<Symbol> overrideBridgeFilter = new Filter<Symbol>() {
+            public boolean accepts(Symbol s) {
+                return (s.flags() & (SYNTHETIC | OVERRIDE_BRIDGE)) != SYNTHETIC;
+            }
+        };
         /**
          * @param method The symbol for which a bridge might have to be added
          * @param impl The implementation of method
@@ -754,6 +760,90 @@ public class TransTypes extends TreeTranslator {
         return types.erasure(t);
     }
 
+    private boolean boundsRestricted(ClassSymbol c) {
+        Type st = types.supertype(c.type);
+        if (st.isParameterized()) {
+            List<Type> actuals = st.allparams();
+            List<Type> formals = st.tsym.type.allparams();
+            while (!actuals.isEmpty() && !formals.isEmpty()) {
+                Type actual = actuals.head;
+                Type formal = formals.head;
+
+                if (!types.isSameType(types.erasure(actual),
+                        types.erasure(formal)))
+                    return true;
+
+                actuals = actuals.tail;
+                formals = formals.tail;
+            }
+        }
+        return false;
+    }
+
+    private List<JCTree> addOverrideBridgesIfNeeded(DiagnosticPosition pos,
+                                    final ClassSymbol c) {
+        ListBuffer<JCTree> buf = ListBuffer.lb();
+        if (c.isInterface() || !boundsRestricted(c))
+            return buf.toList();
+        Type t = types.supertype(c.type);
+            Scope s = t.tsym.members();
+            if (s.elems != null) {
+                for (Symbol sym : s.getElements(new NeedsOverridBridgeFilter(c))) {
+
+                    MethodSymbol m = (MethodSymbol)sym;
+                    MethodSymbol member = (MethodSymbol)m.asMemberOf(c.type, types);
+                    MethodSymbol impl = m.implementation(c, types, false);
+
+                    if ((impl == null || impl.owner != c) &&
+                            !types.isSameType(member.erasure(types), m.erasure(types))) {
+                        addOverrideBridges(pos, m, member, c, buf);
+                    }
+                }
+            }
+        return buf.toList();
+    }
+    // where
+        class NeedsOverridBridgeFilter implements Filter<Symbol> {
+
+            ClassSymbol c;
+
+            NeedsOverridBridgeFilter(ClassSymbol c) {
+                this.c = c;
+            }
+            public boolean accepts(Symbol s) {
+                return s.kind == MTH &&
+                            !s.isConstructor() &&
+                            s.isInheritedIn(c, types) &&
+                            (s.flags() & FINAL) == 0 &&
+                            (s.flags() & (SYNTHETIC | OVERRIDE_BRIDGE)) != SYNTHETIC;
+            }
+        }
+
+    private void addOverrideBridges(DiagnosticPosition pos,
+                                    MethodSymbol impl,
+                                    MethodSymbol member,
+                                    ClassSymbol c,
+                                    ListBuffer<JCTree> bridges) {
+        Type implErasure = impl.erasure(types);
+        long flags = (impl.flags() & AccessFlags) | SYNTHETIC | BRIDGE | OVERRIDE_BRIDGE;
+        member = new MethodSymbol(flags, member.name, member.type, c);
+        JCMethodDecl md = make.MethodDef(member, null);
+        JCExpression receiver = make.Super(types.supertype(c.type).tsym.erasure(types), c);
+        Type calltype = erasure(impl.type.getReturnType());
+        JCExpression call =
+            make.Apply(null,
+                       make.Select(receiver, impl).setType(calltype),
+                       translateArgs(make.Idents(md.params),
+                                     implErasure.getParameterTypes(), null))
+            .setType(calltype);
+        JCStatement stat = (member.getReturnType().tag == VOID)
+            ? make.Exec(call)
+            : make.Return(coerce(call, member.erasure(types).getReturnType()));
+        md.body = make.Block(0, List.of(stat));
+        c.members().enter(member);
+        bridges.append(md);
+    }
+
 /**************************************************************************
  * main method
  *************************************************************************/
@@ -786,6 +876,7 @@ public class TransTypes extends TreeTranslator {
                 make.at(tree.pos);
                 if (addBridges) {
                     ListBuffer<JCTree> bridges = new ListBuffer<JCTree>();
+                    bridges.appendList(addOverrideBridgesIfNeeded(tree, c));
                     if ((tree.sym.flags() & INTERFACE) == 0)
                         addBridges(tree.pos(), tree.sym, bridges);
                     tree.defs = bridges.toList().prependList(tree.defs);

langtools/src/share/classes/com/sun/tools/javac/util/Filter.java

+/*
+ * Copyright (c) 2010, 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+
+package com.sun.tools.javac.util;
+
+/**
+ * Simple filter acting as a boolean predicate. Method accepts return true if
+ * the supplied element matches against the filter.
+ */
+public interface Filter<T> {
+    /**
+     * Does this element match against the filter?
+     * @param t element to be checked
+     * @return true if the element satisfy constraints imposed by filter
+     */
+    boolean accepts(T t);
+}

langtools/test/tools/javac/generics/OverrideBridge.java

+/*
+ * Copyright (c) 2010, 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 6337171
+ * @summary  javac should create bridge methods when type variable bounds restricted
+ * @run main OverrideBridge
+ */
+
+import java.io.*;
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.HashMap;
+import javax.tools.JavaCompiler;
+import javax.tools.JavaFileObject;
+import javax.tools.SimpleJavaFileObject;
+import javax.tools.ToolProvider;
+
+import com.sun.source.util.JavacTask;
+import com.sun.tools.classfile.ClassFile;
+import com.sun.tools.classfile.ConstantPoolException;
+import com.sun.tools.classfile.Descriptor.InvalidDescriptor;
+import com.sun.tools.classfile.Method;
+
+public class OverrideBridge {
+
+    enum Implementation {
+        IMPLICIT(""),
+        EXPLICIT("@Override public abstract X m(X x);");
+
+        String impl;
+
+        Implementation(String impl) {
+            this.impl = impl;
+        }
+    }
+
+    static class JavaSource extends SimpleJavaFileObject {
+
+        final static String sourceStub =
+                        "abstract class A<X> {\n" +
+                        "   public abstract X m(X x);\n" +
+                        "}\n" +
+                        "interface I<X> {\n" +
+                        "X m(X x);\n" +
+                        "}\n" +
+                        "abstract class B<X extends B<X>> extends A<X> implements I<X> { #B }\n" +
+                        "abstract class C<X extends C<X>> extends B<X>  { #C }\n" +
+                        "abstract class D<X extends D<X>> extends C<X>  { #D }\n";
+
+        String source;
+
+        public JavaSource(Implementation implB, Implementation implC, Implementation implD) {
+            super(URI.create("myfo:/Test.java"), JavaFileObject.Kind.SOURCE);
+            source = sourceStub.replace("#B", implB.impl).replace("#C", implC.impl).replace("#D", implD.impl);
+        }
+
+        @Override
+        public CharSequence getCharContent(boolean ignoreEncodingErrors) {
+            return source;
+        }
+    }
+
+    public static void main(String... args) throws Exception {
+        Map<ClassFile, List<Method>> refMembers =
+                compile(Implementation.EXPLICIT, Implementation.EXPLICIT, Implementation.EXPLICIT, "ref");
+        int i = 0;
+        for (Implementation implB : Implementation.values()) {
+            for (Implementation implC : Implementation.values()) {
+                for (Implementation implD : Implementation.values()) {
+                    Map<ClassFile, List<Method>> membersToCheck = compile(implB, implC, implD, "out_" + i++);
+                    check(refMembers, membersToCheck);
+                }
+            }
+        }
+    }
+
+    static String workDir = System.getProperty("user.dir");
+
+    static Map<ClassFile, List<Method>> compile(Implementation implB, Implementation implC, Implementation implD, String destPath) throws Exception {
+        File destDir = new File(workDir, destPath); destDir.mkdir();
+        final JavaCompiler tool = ToolProvider.getSystemJavaCompiler();
+        JavaSource source = new JavaSource(implB, implC, implD);
+        JavacTask ct = (JavacTask)tool.getTask(null, null, null,
+                Arrays.asList("-d", destPath), null, Arrays.asList(source));
+        ct.generate();
+        Map<ClassFile, List<Method>> members = new HashMap<>();
+        addMembers(destDir, members);
+        return members;
+    }
+
+    static void addMembers(File destDir, Map<ClassFile, List<Method>> members) {
+        String[] names = { "B.class", "C.class", "D.class" };
+        try {
+            for (String name : names) {
+                File f = new File(destDir, name);
+                ClassFile cf = ClassFile.read(f);
+                members.put(cf, readMethod(cf, "m"));
+            }
+        } catch (Exception e) {
+            e.printStackTrace();
+            throw new Error("error reading classes");
+        }
+    }
+
+    static List<Method> readMethod(ClassFile cf, String name) throws ConstantPoolException {
+        List<Method> buf = new ArrayList<>();
+        for (Method m : cf.methods) {
+            if (m.getName(cf.constant_pool).equals(name)) {
+                buf.add(m);
+            }
+        }
+        return buf;
+    }
+
+    static void check(Map<ClassFile, List<Method>> refMembers, Map<ClassFile, List<Method>> membersToCheck) throws ConstantPoolException, InvalidDescriptor {
+        for (Map.Entry<ClassFile, List<Method>> ref : refMembers.entrySet()) {
+            ClassFile cRef = ref.getKey();
+            for (Method mRef : ref.getValue()) {
+                boolean ok = false;
+                for (Map.Entry<ClassFile, List<Method>> toCheck : membersToCheck.entrySet()) {
+                    ClassFile cToCheck = toCheck.getKey();
+                    for (Method mToCheck : toCheck.getValue()) {
+                        if (cRef.getName().equals(cToCheck.getName()) &&
+                                mRef.descriptor.getReturnType(cRef.constant_pool).equals(
+                                mToCheck.descriptor.getReturnType(cToCheck.constant_pool)) &&
+                                mRef.descriptor.getParameterTypes(cRef.constant_pool).equals(
+                                mToCheck.descriptor.getParameterTypes(cToCheck.constant_pool))) {
+                            ok = true;
+                        }
+                    }
+                }
+                if (!ok) {
+                    throw new AssertionError("Matching method descriptor for " + mRef.descriptor.getParameterTypes(cRef.constant_pool) + "not found");
+                }
+            }
+        }
+    }
+}