8033718: Inference ignores capture variable as upper bound

Summary: Split Types.lowerBound into two methods; fix bugs in inference handling of capture variables.
Reviewed-by: vromero

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

@@ -703,10 +703,10 @@ public abstract class Symbol extends AnnoConstruct implements Element {
         }
 
         /**
-         * A total ordering between type symbols that refines the
+         * A partial ordering between type symbols that refines the
          * class inheritance graph.
          *
-         * Typevariables always precede other kinds of symbols.
+         * Type variables always precede other kinds of symbols.
          */
         public final boolean precedes(TypeSymbol that, Types types) {
             if (this == that)

src/share/classes/com/sun/tools/javac/code/Type.java

@@ -1445,12 +1445,19 @@ public abstract class Type extends AnnoConstruct implements TypeMirror {
          * Inference variable bound kinds
          */
         public enum InferenceBound {
-            /** upper bounds */
-            UPPER,
-            /** lower bounds */
-            LOWER,
-            /** equality constraints */
-            EQ;
+            UPPER {
+                public InferenceBound complement() { return LOWER; }
+            },
+             /** lower bounds */
+            LOWER {
+                public InferenceBound complement() { return UPPER; }
+            },
+             /** equality constraints */
+            EQ {
+                public InferenceBound complement() { return EQ; }
+            };
+
+            public abstract InferenceBound complement();
         }
 
         /** inference variable bounds */
@@ -1636,6 +1643,9 @@ public abstract class Type extends AnnoConstruct implements TypeMirror {
                 //only change bounds if request comes from substBounds
                 super.addBound(ib, bound, types, update);
             }
+            else if (bound.hasTag(UNDETVAR) && !((UndetVar) bound).isCaptured()) {
+                ((UndetVar) bound).addBound(ib.complement(), this, types, false);
+            }
         }
 
         @Override

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

@@ -152,31 +152,31 @@ public class Types {
         };
     // </editor-fold>
 
-    // <editor-fold defaultstate="collapsed" desc="lowerBound">
+    // <editor-fold defaultstate="collapsed" desc="wildLowerBound">
     /**
-     * The "lvalue conversion".<br>
-     * The lower bound of most types is the type
-     * itself.  Wildcards, on the other hand have upper
-     * and lower bounds.
-     * @param t a type
-     * @return the lower bound of the given type
+     * Get a wildcard's lower bound, returning non-wildcards unchanged.
+     * @param t a type argument, either a wildcard or a type
      */
-    public Type lowerBound(Type t) {
-        return lowerBound.visit(t);
+    public Type wildLowerBound(Type t) {
+        if (t.hasTag(WILDCARD)) {
+            WildcardType w = (WildcardType) t;
+            return w.isExtendsBound() ? syms.botType : wildLowerBound(w.type);
+        }
+        else return t;
     }
-    // where
-        private final MapVisitor<Void> lowerBound = new MapVisitor<Void>() {
-
-            @Override
-            public Type visitWildcardType(WildcardType t, Void ignored) {
-                return t.isExtendsBound() ? syms.botType : visit(t.type);
-            }
+    // </editor-fold>
 
-            @Override
-            public Type visitCapturedType(CapturedType t, Void ignored) {
-                return visit(t.getLowerBound());
-            }
-        };
+    // <editor-fold defaultstate="collapsed" desc="cvarLowerBound">
+    /**
+     * Get a capture variable's lower bound, returning other types unchanged.
+     * @param t a type
+     */
+    public Type cvarLowerBound(Type t) {
+        if (t.hasTag(TYPEVAR) && ((TypeVar) t).isCaptured()) {
+            return cvarLowerBound(t.getLowerBound());
+        }
+        else return t;
+    }
     // </editor-fold>
 
     // <editor-fold defaultstate="collapsed" desc="isUnbounded">
@@ -828,9 +828,14 @@ public class Types {
             return true;
         }
 
-        Type lower = lowerBound(s);
-        if (s != lower)
-            return isSubtype(capture ? capture(t) : t, lower, false);
+        // Generally, if 's' is a type variable, recur on lower bound; but
+        // for alpha <: CAP, alpha should get upper bound CAP
+        if (!t.hasTag(UNDETVAR)) {
+            // TODO: JDK-8039198, bounds checking sometimes passes in a wildcard as s
+            Type lower = cvarLowerBound(wildLowerBound(s));
+            if (s != lower)
+                return isSubtype(capture ? capture(t) : t, lower, false);
+        }
 
         return isSubtype.visit(capture ? capture(t) : t, s);
     }
@@ -1137,7 +1142,7 @@ public class Types {
                     return visit(s, t);
 
                 if (s.isSuperBound() && !s.isExtendsBound())
-                    return visit(t, upperBound(s)) && visit(t, lowerBound(s));
+                    return visit(t, upperBound(s)) && visit(t, wildLowerBound(s));
 
                 if (t.isCompound() && s.isCompound()) {
                     if (!visit(supertype(t), supertype(s)))
@@ -1292,7 +1297,7 @@ public class Types {
                         break;
                     }
                     case SUPER: {
-                        Type bound = lowerBound(s);
+                        Type bound = wildLowerBound(s);
                         undetvar.addBound(InferenceBound.LOWER, bound, this);
                         break;
                     }
@@ -1385,9 +1390,9 @@ public class Types {
 //                                  t.isSuperBound()
 //                                  || isSubtypeNoCapture(upperBound(s), U(t)));
 //                System.err.format(" %s L(%s) <: L(%s) %s = %s%n",
-//                                  L(t), t, s, lowerBound(s),
+//                                  L(t), t, s, wildLowerBound(s),
 //                                  t.isExtendsBound()
-//                                  || isSubtypeNoCapture(L(t), lowerBound(s)));
+//                                  || isSubtypeNoCapture(L(t), wildLowerBound(s)));
 //                System.err.println();
 //            }
 
@@ -1399,7 +1404,7 @@ public class Types {
 //                    debugContainsType(t, s);
                     return isSameWildcard(t, s)
                         || isCaptureOf(s, t)
-                        || ((t.isExtendsBound() || isSubtypeNoCapture(L(t), lowerBound(s))) &&
+                        || ((t.isExtendsBound() || isSubtypeNoCapture(L(t), wildLowerBound(s))) &&
                             (t.isSuperBound() || isSubtypeNoCapture(upperBound(s), U(t))));
                 }
             }
@@ -1761,7 +1766,7 @@ public class Types {
                     if (s.isExtendsBound())
                         return !isCastableRecursive(t.type, upperBound(s));
                     else if (s.isSuperBound())
-                        return notSoftSubtypeRecursive(lowerBound(s), t.type);
+                        return notSoftSubtypeRecursive(wildLowerBound(s), t.type);
                 } else if (t.isSuperBound()) {
                     if (s.isExtendsBound())
                         return notSoftSubtypeRecursive(t.type, upperBound(s));
@@ -1771,19 +1776,13 @@ public class Types {
         };
     // </editor-fold>
 
-    // <editor-fold defaultstate="collapsed" desc="lowerBoundArgtypes">
-    /**
-     * Returns the lower bounds of the formals of a method.
-     */
-    public List<Type> lowerBoundArgtypes(Type t) {
-        return lowerBounds(t.getParameterTypes());
-    }
-    public List<Type> lowerBounds(List<Type> ts) {
-        return map(ts, lowerBoundMapping);
+    // <editor-fold defaultstate="collapsed" desc="cvarLowerBounds">
+    public List<Type> cvarLowerBounds(List<Type> ts) {
+        return map(ts, cvarLowerBoundMapping);
     }
-    private final Mapping lowerBoundMapping = new Mapping("lowerBound") {
+    private final Mapping cvarLowerBoundMapping = new Mapping("cvarLowerBound") {
             public Type apply(Type t) {
-                return lowerBound(t);
+                return cvarLowerBound(t);
             }
         };
     // </editor-fold>
@@ -2252,7 +2251,8 @@ public class Types {
 
     // <editor-fold defaultstate="collapsed" desc="makeCompoundType">
     /**
-     * Make a compound type from non-empty list of types
+     * Make a compound type from non-empty list of types.  The list should be
+     * ordered according to {@link Symbol#precedes(TypeSymbol,Types)}.
      *
      * @param bounds            the types from which the compound type is formed
      * @param supertype         is objectType if all bounds are interfaces,
@@ -3342,12 +3342,15 @@ public class Types {
      * Insert a type in a closure
      */
     public List<Type> insert(List<Type> cl, Type t) {
-        if (cl.isEmpty() || t.tsym.precedes(cl.head.tsym, this)) {
+        if (cl.isEmpty()) {
             return cl.prepend(t);
-        } else if (cl.head.tsym.precedes(t.tsym, this)) {
-            return insert(cl.tail, t).prepend(cl.head);
-        } else {
+        } else if (t.tsym == cl.head.tsym) {
             return cl;
+        } else if (t.tsym.precedes(cl.head.tsym, this)) {
+            return cl.prepend(t);
+        } else {
+            // t comes after head, or the two are unrelated
+            return insert(cl.tail, t).prepend(cl.head);
         }
     }
 
@@ -3359,12 +3362,15 @@ public class Types {
             return cl2;
         } else if (cl2.isEmpty()) {
             return cl1;
+        } else if (cl1.head.tsym == cl2.head.tsym) {
+            return union(cl1.tail, cl2.tail).prepend(cl1.head);
         } else if (cl1.head.tsym.precedes(cl2.head.tsym, this)) {
             return union(cl1.tail, cl2).prepend(cl1.head);
         } else if (cl2.head.tsym.precedes(cl1.head.tsym, this)) {
             return union(cl1, cl2.tail).prepend(cl2.head);
         } else {
-            return union(cl1.tail, cl2.tail).prepend(cl1.head);
+            // unrelated types
+            return union(cl1.tail, cl2).prepend(cl1.head);
         }
     }
 
@@ -3474,18 +3480,31 @@ public class Types {
     private List<Type> closureMin(List<Type> cl) {
         ListBuffer<Type> classes = new ListBuffer<>();
         ListBuffer<Type> interfaces = new ListBuffer<>();
+        Set<Type> toSkip = new HashSet<>();
         while (!cl.isEmpty()) {
             Type current = cl.head;
-            if (current.isInterface())
-                interfaces.append(current);
-            else
-                classes.append(current);
-            ListBuffer<Type> candidates = new ListBuffer<>();
-            for (Type t : cl.tail) {
-                if (!isSubtypeNoCapture(current, t))
-                    candidates.append(t);
+            boolean keep = !toSkip.contains(current);
+            if (keep && current.hasTag(TYPEVAR)) {
+                // skip lower-bounded variables with a subtype in cl.tail
+                for (Type t : cl.tail) {
+                    if (isSubtypeNoCapture(t, current)) {
+                        keep = false;
+                        break;
+                    }
+                }
             }
-            cl = candidates.toList();
+            if (keep) {
+                if (current.isInterface())
+                    interfaces.append(current);
+                else
+                    classes.append(current);
+                for (Type t : cl.tail) {
+                    // skip supertypes of 'current' in cl.tail
+                    if (isSubtypeNoCapture(current, t))
+                        toSkip.add(t);
+                }
+            }
+            cl = cl.tail;
         }
         return classes.appendList(interfaces).toList();
     }
@@ -3645,7 +3664,19 @@ public class Types {
             return s;
 
         List<Type> closure = union(closure(t), closure(s));
-        List<Type> bounds = closureMin(closure);
+        return glbFlattened(closure, t);
+    }
+    //where
+    /**
+     * Perform glb for a list of non-primitive, non-error, non-compound types;
+     * redundant elements are removed.  Bounds should be ordered according to
+     * {@link Symbol#precedes(TypeSymbol,Types)}.
+     *
+     * @param flatBounds List of type to glb
+     * @param errT Original type to use if the result is an error type
+     */
+    private Type glbFlattened(List<Type> flatBounds, Type errT) {
+        List<Type> bounds = closureMin(flatBounds);
 
         if (bounds.isEmpty()) {             // length == 0
             return syms.objectType;
@@ -3653,11 +3684,21 @@ public class Types {
             return bounds.head;
         } else {                            // length > 1
             int classCount = 0;
-            for (Type bound : bounds)
-                if (!bound.isInterface())
+            List<Type> lowers = List.nil();
+            for (Type bound : bounds) {
+                if (!bound.isInterface()) {
                     classCount++;
-            if (classCount > 1)
-                return createErrorType(t);
+                    Type lower = cvarLowerBound(bound);
+                    if (bound != lower && !lower.hasTag(BOT))
+                        lowers = insert(lowers, lower);
+                }
+            }
+            if (classCount > 1) {
+                if (lowers.isEmpty())
+                    return createErrorType(errT);
+                else
+                    return glbFlattened(union(bounds, lowers), errT);
+            }
         }
         return makeCompoundType(bounds);
     }
@@ -4137,7 +4178,7 @@ public class Types {
             if (source.isExtendsBound())
                 adaptRecursive(upperBound(source), upperBound(target));
             else if (source.isSuperBound())
-                adaptRecursive(lowerBound(source), lowerBound(target));
+                adaptRecursive(wildLowerBound(source), wildLowerBound(target));
             return null;
         }
 
@@ -4149,7 +4190,7 @@ public class Types {
             Type val = mapping.get(source.tsym);
             if (val != null) {
                 if (val.isSuperBound() && target.isSuperBound()) {
-                    val = isSubtype(lowerBound(val), lowerBound(target))
+                    val = isSubtype(wildLowerBound(val), wildLowerBound(target))
                         ? target : val;
                 } else if (val.isExtendsBound() && target.isExtendsBound()) {
                     val = isSubtype(upperBound(val), upperBound(target))
@@ -4263,7 +4304,7 @@ public class Types {
         }
 
         public Type visitType(Type t, Void s) {
-            return high ? upperBound(t) : lowerBound(t);
+            return high ? upperBound(t) : t;
         }
 
         @Override

src/share/classes/com/sun/tools/javac/comp/Check.java

@@ -621,7 +621,7 @@ public class Check {
          } else if (a.isExtendsBound()) {
              return types.isCastable(bound, types.upperBound(a), types.noWarnings);
          } else if (a.isSuperBound()) {
-             return !types.notSoftSubtype(types.lowerBound(a), bound);
+             return !types.notSoftSubtype(types.wildLowerBound(a), bound);
          }
          return true;
      }
@@ -2685,7 +2685,7 @@ public class Check {
         if (types.isSameType(type, syms.stringType)) return;
         if ((type.tsym.flags() & Flags.ENUM) != 0) return;
         if ((type.tsym.flags() & Flags.ANNOTATION) != 0) return;
-        if (types.lowerBound(type).tsym == syms.classType.tsym) return;
+        if (types.cvarLowerBound(type).tsym == syms.classType.tsym) return;
         if (types.isArray(type) && !types.isArray(types.elemtype(type))) {
             validateAnnotationType(pos, types.elemtype(type));
             return;

src/share/classes/com/sun/tools/javac/comp/Infer.java

@@ -1407,7 +1407,7 @@ public class Infer {
             Type solve(UndetVar uv, InferenceContext inferenceContext) {
                 Infer infer = inferenceContext.infer();
                 List<Type> hibounds = filterBounds(uv, inferenceContext);
-                //note: lobounds should have at least one element
+                //note: hibounds should have at least one element
                 Type owntype = hibounds.tail.tail == null  ? hibounds.head : infer.types.glb(hibounds);
                 if (owntype.isPrimitive() || owntype.hasTag(ERROR)) {
                     throw infer.inferenceException

src/share/classes/com/sun/tools/javac/comp/Resolve.java

@@ -1549,7 +1549,7 @@ public class Resolve {
             currentResolutionContext.methodCheck =
                     prevResolutionContext.methodCheck.mostSpecificCheck(actuals, !allowBoxing);
             Type mst = instantiate(env, site, m2, null,
-                    adjustArgs(types.lowerBounds(types.memberType(site, m1).getParameterTypes()), m1, maxLength, useVarargs), null,
+                    adjustArgs(types.cvarLowerBounds(types.memberType(site, m1).getParameterTypes()), m1, maxLength, useVarargs), null,
                     allowBoxing, useVarargs, noteWarner);
             return mst != null &&
                     !noteWarner.hasLint(Lint.LintCategory.UNCHECKED);

test/tools/javac/generics/inference/7086586/T7086586.out

-T7086586.java:14:20: compiler.err.cant.apply.symbol: kindname.method, m, java.util.List<? super T>, java.util.List<compiler.misc.type.captureof: 1, ?>, kindname.class, T7086586, (compiler.misc.infer.no.conforming.assignment.exists: T, (compiler.misc.inconvertible.types: java.util.List<compiler.misc.type.captureof: 1, ?>, java.util.List<? super T>))
-T7086586.java:15:20: compiler.err.cant.apply.symbol: kindname.method, m, java.util.List<? super T>, java.util.List<compiler.misc.type.captureof: 1, ?>, kindname.class, T7086586, (compiler.misc.infer.no.conforming.assignment.exists: T, (compiler.misc.inconvertible.types: java.util.List<compiler.misc.type.captureof: 1, ?>, java.util.List<? super T>))
-T7086586.java:16:23: compiler.err.cant.apply.symbol: kindname.method, m, java.util.List<? super T>, java.util.List<compiler.misc.type.captureof: 1, ?>, kindname.class, T7086586, (compiler.misc.infer.no.conforming.assignment.exists: T, (compiler.misc.inconvertible.types: java.util.List<compiler.misc.type.captureof: 1, ?>, java.util.List<? super T>))
-T7086586.java:17:9: compiler.err.cant.apply.symbol: kindname.method, m, java.util.List<? super T>, java.util.List<compiler.misc.type.captureof: 1, ?>, kindname.class, T7086586, (compiler.misc.infer.no.conforming.assignment.exists: T, (compiler.misc.inconvertible.types: java.util.List<compiler.misc.type.captureof: 1, ?>, java.util.List<? super T>))
+T7086586.java:14:28: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: compiler.misc.type.captureof: 1, ?, java.lang.String)
+T7086586.java:15:28: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: compiler.misc.type.captureof: 1, ?, java.lang.Number)
+T7086586.java:16:31: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: compiler.misc.type.captureof: 1, ?, java.lang.Exception)
+T7086586.java:17:13: compiler.err.cant.resolve.location.args: kindname.method, nonExistentMethod, , , (compiler.misc.location: kindname.interface, java.util.List<compiler.misc.type.captureof: 1, ?>, null)
 4 errors

test/tools/javac/generics/inference/7086586/T7086586b.java

@@ -23,9 +23,10 @@
 
 /*
  * @test
- * @bug 7086586
+ * @bug 7086586 8033718
  *
- * @summary Inference producing null type argument
+ * @summary Inference producing null type argument; inference ignores capture
+ *          variable as upper bound
  */
 import java.util.List;
 
@@ -40,8 +41,8 @@ public class T7086586b {
         assertionCount++;
     }
 
-    <T> void m(List<? super T> dummy) { assertTrue(false); }
-    <T> void m(Object dummy) { assertTrue(true); }
+    <T> void m(List<? super T> dummy) { assertTrue(true); }
+    <T> void m(Object dummy) { assertTrue(false); }
 
     void test(List<?> l) {
         m(l);

test/tools/javac/generics/inference/LowerBoundGLB.java

+/*
+ * Copyright (c) 2014, 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 8033718
+ * @author dlsmith
+ * @summary GLB for two capture variables with lower bounds
+ * @compile LowerBoundGLB.java
+ */
+
+public class LowerBoundGLB {
+
+    interface Box<T> {
+        T get();
+        void set(T arg);
+    }
+
+    <T> T doGLB(Box<? super T> b1, Box<? super T> b2) {
+        return null;
+    }
+
+    void test(Box<? super String> l1, Box<? super CharSequence> l2) {
+        doGLB(l1, l2).substring(3);
+    }
+
+}