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
--- a/langtools/src/share/classes/com/sun/tools/javac/code/Flags.java Mon Sep 06 12:55:09 2010 -0700
+++ b/langtools/src/share/classes/com/sun/tools/javac/code/Flags.java Tue Sep 07 17:31:54 2010 +0100
@@ -241,6 +241,12 @@
*/
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
--- a/langtools/src/share/classes/com/sun/tools/javac/code/Scope.java Mon Sep 06 12:55:09 2010 -0700
+++ b/langtools/src/share/classes/com/sun/tools/javac/code/Scope.java Tue Sep 07 17:31:54 2010 +0100
@@ -272,6 +272,12 @@
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 @@
* 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 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 @@
}
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;
}
}
};
--- a/langtools/src/share/classes/com/sun/tools/javac/code/Symbol.java Mon Sep 06 12:55:09 2010 -0700
+++ b/langtools/src/share/classes/com/sun/tools/javac/code/Symbol.java Tue Sep 07 17:31:54 2010 +0100
@@ -1217,7 +1217,18 @@
* 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
--- a/langtools/src/share/classes/com/sun/tools/javac/code/Types.java Mon Sep 06 12:55:09 2010 -0700
+++ b/langtools/src/share/classes/com/sun/tools/javac/code/Types.java Tue Sep 07 17:31:54 2010 +0100
@@ -1974,45 +1974,74 @@
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));
+ // <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 impl = cache.get(origin);
- if (impl == null) {
+
+ 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
--- a/langtools/src/share/classes/com/sun/tools/javac/comp/TransTypes.java Mon Sep 06 12:55:09 2010 -0700
+++ b/langtools/src/share/classes/com/sun/tools/javac/comp/TransTypes.java Tue Sep 07 17:31:54 2010 +0100
@@ -283,12 +283,13 @@
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 @@
// 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 @@
}
}
// 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 @@
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 @@
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);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/src/share/classes/com/sun/tools/javac/util/Filter.java Tue Sep 07 17:31:54 2010 +0100
@@ -0,0 +1,39 @@
+/*
+ * 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);
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/generics/OverrideBridge.java Tue Sep 07 17:31:54 2010 +0100
@@ -0,0 +1,163 @@
+/*
+ * 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");
+ }
+ }
+ }
+ }
+}