JDK-6337171 : javac should create bridge methods when type variable bounds restricted
  • Type: Bug
  • Component: tools
  • Sub-Component: javac
  • Affected Version: 5.0,6
  • Priority: P4
  • Status: Open
  • Resolution: Unresolved
  • OS: linux,windows_xp
  • CPU: x86
  • Submitted: 2005-10-14
  • Updated: 2017-11-15
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbd_majorUnresolved
Related Reports
Duplicate :  
Duplicate :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Description
A DESCRIPTION OF THE REQUEST :
When a subtype restricts the bounds of a type variable of its supertype, any inherited methods with a return type of that variable should have a bridge method created. Currently, moving such a method from the derived to base type will cause binary incompatibility.

Care needs to be exercised for protected methods inherited across packages.

JUSTIFICATION :
It should be possible to move such methods to super types, without binary incompatibility.

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
$ /usr/java/jdk1.6.0/bin/javap Derived
Compiled from "Derived.java"
public class Derived extends Base{
    public Derived();
    Derived getThis();
    Base getThis();
    public Derived function();
}
ACTUAL -
$ /usr/java/jdk1.6.0/bin/javap Derived
Compiled from "Derived.java"
public class Derived extends Base{
    public Derived();
    Derived getThis();
    Base getThis();
}

with 1.5.0_04:

C:\>javac Derived.java

C:\>javap Derived
Compiled from "Derived.java"
public class Derived extends Base{
    public Derived();
    Derived getThis();
    Base getThis();
}


---------- BEGIN SOURCE ----------
abstract class Base<THIS extends Base> {
    abstract THIS getThis();
    public THIS function() {
         return getThis();
    }
}
public class Derived extends Base<Derived> {
    Derived getThis() {
        return this;
    }
}
---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
Override all methods with this problem.

Comments
FWIW, this patch does not cause the infinite loop problem anymore on the test case in JDK-6996415. Hard to say what else has changed that makes this problem "go away" This is the test case I used: class Expression {} abstract class ExpVisitor<R,D> { protected R visitExpression (Expression exp, D d) { System.out.println(exp); return null; } } abstract class ExpExpVisitor<D> extends ExpVisitor<Expression,D> { } public class FindTail extends ExpExpVisitor<Expression> { protected Expression visitExpression (Expression exp, Expression returnContinuation) { return super.visitExpression(exp, exp); } public static void main(String [] args) { new FindTail().visitExpression(new Expression(), new Expression()); ExpVisitor<Expression, Expression> e = new FindTail(); e.visitExpression(new Expression(), new Expression()); } } Which compiles and runs fine with a compiler patched with Maurizio's changes brought forward to tip. (I do see the bridges emitted, but no loop)
15-11-2017

Maurizio's patch from above brought forward to tip: diff -r be620a591379 src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransTypes.java --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransTypes.java Mon Oct 30 21:23:10 2017 +0100 +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransTypes.java Wed Nov 15 11:33:36 2017 +0530 @@ -910,7 +910,101 @@ return types.erasure(t); } -/************************************************************************** + + + private boolean areBoundsRestricted(ClassSymbol c) { + List<Type> supertypes = types.interfaces(c.type); + supertypes = supertypes.prepend(types.supertype(c.type)); + for (Type st : supertypes) { + if (st.isParameterized()) { // TODO: inner classes? + 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 void addOverrideBridgesIfNeeded(DiagnosticPosition pos, + ClassSymbol c, + ListBuffer<JCTree> bridges) { + List<Type> closure = types.closure(c.type); + for (Type t : closure) { + if (t.tsym == c) + continue; + if (t.isParameterized()) { // TODO: inner classes? + Scope s = t.tsym.members(); + for (Symbol e : s.getSymbols()) { + if (e.kind != MTH) continue; + MethodSymbol m = (MethodSymbol)e; + MethodSymbol member = (MethodSymbol)m.asMemberOf(c.type, types); + MethodSymbol impl = m.implementation(c, types, true); + impl = impl != null ? impl : m; + //System.err.format(" %s:%n", e.sym.type); + + //visitor.visit(e.sym.type, e.sym); + if (!c.isInterface() && + !m.isConstructor() && + m.isInheritedIn(c, types) && + impl.getEnclosingElement() != c && + (impl.flags() & FINAL) == 0 && + !types.isSameType(member.erasure(types), impl.erasure(types))) + addOverrideBridges(pos, impl != null ? impl : m, member, c, bridges); + } + } + } + } + + private void addOverrideBridges(DiagnosticPosition pos, + MethodSymbol impl, + MethodSymbol member, + ClassSymbol c, + ListBuffer<JCTree> bridges) { + + Type implErasure = impl.erasure(types); + + /*System.err.format(" %s : %s%n => %s : %s%n", + m, t, member, member.type); + + System.err.format(" impl: %s : %s @ %s%n", + impl, null, null);*/ + + long flags = impl.flags() & AccessFlags | 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().getTag() == 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); + //System.err.println(md); + } + + + + + + + /************************************************************************** * main method *************************************************************************/ @@ -966,6 +1060,10 @@ super.visitClassDef(tree); make.at(tree.pos); ListBuffer<JCTree> bridges = new ListBuffer<>(); + if (areBoundsRestricted(tree.sym)) { + //System.err.format("Restricted bounds on %s%n", tree.sym); + addOverrideBridgesIfNeeded(tree.pos(), tree.sym, bridges); + } if (allowInterfaceBridges || (tree.sym.flags() & INTERFACE) == 0) { addBridges(tree.pos(), c, bridges); }
15-11-2017

Note:as part of JDK-8005542, test OverrideBridge (which was previously @ignored) has been removed from langtools/test and added as an attachment here.
04-10-2013

This bug was "deferred" in the old manner. As it describes an existing condition in the compiler, it should be targetedd to some future release rather than closed.
04-10-2013

EVALUATION The override bridge technique has proven to be quite problematic; first of all there are some incompatibilities regarding override bridge and raw methods (previously working examples now start to fail with a CCE at runtime). Another, more problematic case is 6996415, in which override bridge interacts badly with overload resolution causing javac to generate code that produces and endless loop if executed. Because of the above reason I'm now marking the fix as failed. The override bridge method generation as been temporarily disabled, awaiting for a more in depth investigation (not sure if we can target 7, as a complete fix might require new classfile flags that are unlikely to be added at this stage). A followup bug will be created to keep track of this issue.
02-11-2010

SUGGESTED FIX A webrev of this fix is available at the following URL: http://hg.openjdk.java.net/jdk7/tl/langtools/rev/7ae4016c5938
07-09-2010

SUGGESTED FIX I was unable to get Peter's fix working with a recent version of the compiler as it was - once I got that fix working the results were not as good as I expected: the fix generated a lot of binary incompatibilities that resulted in over 30 javac regression failures and 9 JCK failures. Here's a refined version of Peter's fix that (hopefully) tackles all the issues; it passes all regression and JCK tests. diff -r 1d1f34b36535 src/share/classes/com/sun/tools/javac/comp/TransTypes.java --- a/src/share/classes/com/sun/tools/javac/comp/TransTypes.java Wed Nov 26 11:07:07 2008 +0000 +++ b/src/share/classes/com/sun/tools/javac/comp/TransTypes.java Fri Nov 28 17:21:54 2008 +0000 @@ -738,6 +738,101 @@ public class TransTypes extends TreeTran return types.erasure(t); } + private boolean areBoundsRestricted(ClassSymbol c) { + List<Type> supertypes = types.interfaces(c.type); + supertypes = supertypes.prepend(types.supertype(c.type)); + for (Type st : supertypes) { + if (st.isParameterized()) { // TODO: inner classes? + 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 void addOverrideBridgesIfNeeded(DiagnosticPosition pos, + ClassSymbol c, + ListBuffer<JCTree> bridges) { + List<Type> closure = types.closure(c.type); + for (Type t : closure) { + if (t.tsym == c) + continue; + if (t.isParameterized()) { // TODO: inner classes? + Scope s = t.tsym.members(); + if (s.elems != null) { + for (Scope.Entry e = s.elems; + e.scope != null; + e = e.next()) { + if (e.sym.kind != MTH) continue; + MethodSymbol m = (MethodSymbol)e.sym; + MethodSymbol member = (MethodSymbol)m.asMemberOf(c.type, types); + MethodSymbol impl = m.implementation(c, types, true); + impl = impl != null ? impl : m; + //System.err.format(" %s:%n", e.sym.type); + + //visitor.visit(e.sym.type, e.sym); + if (!c.isInterface() && + !m.isConstructor() && + m.isInheritedIn(c, types) && + impl.getEnclosingElement() != c && + (impl.flags() & FINAL) == 0 && + !types.isSameType(member.erasure(types), impl.erasure(types))) + addOverrideBridges(pos, impl != null ? impl : m, member, c, bridges); + } + } + } + } + } + + private void addOverrideBridges(DiagnosticPosition pos, + MethodSymbol impl, + MethodSymbol member, + ClassSymbol c, + ListBuffer<JCTree> bridges) { + + Type implErasure = impl.erasure(types); + + /*System.err.format(" %s : %s%n => %s : %s%n", + m, t, member, member.type); + + System.err.format(" impl: %s : %s @ %s%n", + impl, null, null);*/ + + long flags = impl.flags() & AccessFlags | 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); + //System.err.println(md); + } + + + + + /************************************************************************** * main method *************************************************************************/ @@ -770,6 +865,10 @@ public class TransTypes extends TreeTran make.at(tree.pos); if (addBridges) { ListBuffer<JCTree> bridges = new ListBuffer<JCTree>(); + if (areBoundsRestricted(tree.sym)) { + //System.err.format("Restricted bounds on %s%n", tree.sym); + addOverrideBridgesIfNeeded(tree.pos(), tree.sym, bridges); + } if ((tree.sym.flags() & INTERFACE) == 0) addBridges(tree.pos(), tree.sym, bridges); tree.defs = bridges.toList().prependList(tree.defs); diff -r 1d1f34b36535 src/share/classes/com/sun/tools/javac/jvm/ClassWriter.java --- a/src/share/classes/com/sun/tools/javac/jvm/ClassWriter.java Wed Nov 26 11:07:07 2008 +0000 +++ b/src/share/classes/com/sun/tools/javac/jvm/ClassWriter.java Fri Nov 28 17:21:54 2008 +0000 @@ -430,7 +430,7 @@ public class ClassWriter extends ClassFi } else if (t.tag == ARRAY) { return typeSig(types.erasure(t)); } else { - throw new AssertionError("xClassName"); + throw new AssertionError("xClassName: " + t); } }
28-11-2008

EVALUATION I agree this is not an RFE but a bug - however fixing this could affect binary compatibility so extra care is needed. Moreover it seems like other compilers don't get this right (same as javac - no bridge is added if there's no explicit overriding). The anomaly described in this CR is only reproducible when the generated code is accessed through reflection - javac will forbid any attempt to use non-explicitly overriden method in an unsound way. For these reasons I'm going to lower the priority of this bug from 2 to 3. In any case, I'd like to take care of this in time for 7 - the fact that javac generates potentially unsafe code (at least w.r.t. reflection) is something that should be fixed - perhaps by tackling this problem at the JLS level, too.
28-11-2008

EVALUATION The solution is to have the compiler add "implicit override methods": "In the case that a concrete method in a superclass is not overridden explicitly and doing so would require a different descriptor, the compiler must add an implicit override method which forwards to the method in the superclass." Bridge methods and the like are not specified in the JLS but see bug 6337203. Since the compiler will add a bridge method to the implicit override method, adding this also fixes bug 6481176.
19-10-2006

SUGGESTED FIX Index: j2se/src/share/classes/com/sun/tools/javac/code/Scope.java --- /tmp/geta13219 2006-10-17 17:58:12.000000000 -0700 +++ Scope.java 2006-10-13 22:34:54.000000000 -0700 @@ -25,7 +25,7 @@ * deletion without notice.</b> */ @Version("%W% %E%") -public class Scope { +public class Scope implements Iterable<Symbol> { /** The number of scopes that share this scope's hash table. */ @@ -308,6 +308,13 @@ } + public Iterator<Symbol> iterator() { + if (elems == null) + return List.<Symbol>nil().iterator(); + else + return elems.iterator(); + } + public String toString() { StringBuilder result = new StringBuilder(); result.append("Scope["); @@ -324,7 +331,7 @@ /** A class for scope entries. */ - public static class Entry { + public static class Entry implements Iterable<Symbol> { /** The referenced symbol. * sym == null iff this == sentinel @@ -372,6 +379,25 @@ // in many cases. return scope; } + + public Iterator<Symbol> iterator() { + return new Iterator<Symbol>() { + Entry next = Entry.this; + public boolean hasNext() { + return next != null; + } + public Symbol next() { + if (next == null) + throw new java.util.NoSuchElementException(); + Symbol sym = next.sym; + next = next.sibling; + return sym; + } + public void remove() { + throw new UnsupportedOperationException(); + } + }; + } } public static class ImportScope extends Scope { Index: j2se/src/share/classes/com/sun/tools/javac/comp/TransTypes.java --- /tmp/geta13229 2006-10-17 17:58:53.000000000 -0700 +++ TransTypes.java 2006-10-17 17:56:42.000000000 -0700 @@ -717,6 +717,112 @@ return types.erasure(t); } + private boolean areBoundsRestricted(ClassSymbol c) { + List<Type> supertypes = types.interfaces(c.type); + supertypes = supertypes.prepend(types.supertype(c.type)); + for (Type st : supertypes) { + if (st.isParameterized()) { // TODO: inner classes? + List<Type> actuals = st.getTypeArguments(); + List<Type> formals = st.tsym.type.getTypeArguments(); + while (!actuals.isEmpty() && !formals.isEmpty()) { + Type actual = actuals.head; + Type formal = formals.head; + + if (actual.tag != TYPEVAR) + return true; + + if (!types.isSameType(actual.getUpperBound(), formal.getUpperBound())) + return true; + + actuals = actuals.tail; + formals = formals.tail; + } + } + } + return false; + } + + private void addOverrideBridges(final DiagnosticPosition pos, + final ClassSymbol c, + final ListBuffer<JCTree> bridges) { + class Visitor extends Types.DefaultTypeVisitor<Void,Symbol> { + @Override + public Void visitType(Type t, Symbol sym) { + return null; + } + /** + * @param t the type corresponding to sym + * @param sym the symbol of the current member of c + */ + @Override + public Void visitMethodType(Type.MethodType t, Symbol sym) { + MethodSymbol m = (MethodSymbol)sym; + MethodSymbol member = (MethodSymbol)m.asMemberOf(c.type, types); + if (types.isSameType(types.erasure(t), types.erasure(member.type))) + return null; + MethodSymbol impl = m.implementation(c, types, true); + if (impl.getEnclosingElement() == c) + return null; + + System.err.format(" %s : %s%n => %s : %s%n", + m, t, member, member.type); + + System.err.format(" impl: %s : %s @ %s%n", + impl, impl.type, impl.getEnclosingElement()); + + long flags = impl.flags() & AccessFlags | BRIDGE; // | SYNTHETIC ; + member = new MethodSymbol(flags, m.name, member.type, c); + // TODO: what about interface methods? + JCMethodDecl md = make.MethodDef(member, null); + JCExpression receiver = make.Super(impl.owner.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), + member.type.getParameterTypes(), null)) + .setType(calltype); + JCStatement stat = (t.getReturnType().tag == VOID) + ? make.Exec(call) + : make.Return(coerce(call, member.type.getReturnType())); + md.body = make.Block(0, List.of(stat)); + c.members().enter(member); + bridges.append(md); + System.err.println(md); + // overridden.put(member, m); + + return null; + } + /** + * @param t the type corresponding to sym + * @param sym the symbol of the current member of c + */ + @Override + public Void visitForAll(Type.ForAll t, Symbol sym) { + MethodSymbol m = (MethodSymbol)sym; + Symbol member = m.asMemberOf(c.type, types); + if (types.isSameType(types.erasure(t), types.erasure(member.type))) + return null; + System.err.format(" %s : %s%n => %s : %s%n", + m, t, member, member.type); + return null; + } + } + Visitor visitor = new Visitor(); + List<Type> closure = types.closure(c.type); + for (Type t : closure) { + if (t.tsym == c) + continue; + if (t.isParameterized()) { // TODO: inner classes? + System.err.format(" %s:%n", t); + for (Symbol s : t.tsym.members()) { + visitor.visit(s.type, s); + } + } + } + } + + /************************************************************************** * main method *************************************************************************/ @@ -743,6 +849,10 @@ make.at(tree.pos); if (addBridges) { ListBuffer<JCTree> bridges = new ListBuffer<JCTree>(); + if (areBoundsRestricted(tree.sym)) { + System.err.format("Restricted bounds on %s%n", tree.sym); + addOverrideBridges(tree.pos(), tree.sym, bridges); + } if ((tree.sym.flags() & INTERFACE) == 0) addBridges(tree.pos(), tree.sym, bridges); tree.defs = bridges.toList().prependList(tree.defs);
18-10-2006

EVALUATION This is a bug, not a request for enhancement.
14-10-2005