JDK-8219318 (???): inferred type does not conform to upper bound(s) when collecting to a HashMap

[B. Blaser]

Thanks for your feedback, Maurizio.

Using a lacunal inference mechanism during overload resolution would
definitely lead to inconsistencies like this.

Moreover, JLS §15.12.2.2 [1] is about applicability by strict
invocation and our example refers to applicability by loose invocation
(§15.12.2.3 [2]):

"If m is a generic method and the method invocation does not provide
explicit type arguments, then the applicability of the method is
inferred as specified in §18.5.1."

At my mind, the suggested fix conforms to the JLS and brings the
missing orthogonality revealed here.

You're right that 'toMap()' is under-constrained during overload
resolution in both cases but the problem only appears when adding the
target type constraint during the overload inference of 'putAll()' and
_not_ during the assignment inference of 'broker' which is truly
inconsistent.

But if you stay on your position considering this isn't an issue, I'll
close it as "won't fix"?

Cheers,
Bernard

[1] https://docs.oracle.com/javase/specs/jls/se13/html/jls-15.html#jls-15.12.2.2
[2] https://docs.oracle.com/javase/specs/jls/se13/html/jls-15.html#jls-15.12.2.3

On Fri, 13 Dec 2019 at 22:52, Maurizio Cimadamore
<maurizio.cimadamore at oracle.com> wrote:
>
> I'll need to think more about this - but I think the proposed fix
> doesn't seem conform with the JLS. Note that, during overload
> resolution, certain arguments are 'left behind', see JLS section 15.12.2.2:
>
> "An argument expression is considered pertinent to applicability for a
> potentially applicable method m unless it has one of the following forms:
> ...
>
> An inexact method reference expression (§15.13.1).
>
> "
>
> 'stuck' is javac's way to call an argument that is not pertinent to
> applicability. So, to me, the fact that we get no constraints from
> HashMap::new during overload seems normal.
>
> At the same time, the target type (Map<? extends Integer, ? extends
> Integer>) is _not_ considered during overload inference - which means
> that the M type-variable of the Collectors.toMap method is
> underconstrained (and will be inferred as Map<Object, Object>). As I
> said before, I'd need to think more about it (as this is a tricky
> issue), but it is possible that this is not an issue (for what it's
> worth, pasting the code with IntelliJ reveals the same issue).
>
> Maurizio
>
> On 13/12/2019 19:46, B. Blaser wrote:
> > Hi,
> >
> > I looked at the following interesting issue [1]:
> >
> > public static void main(String... args) {
> >      Map<Integer, Integer> destination = null;
> >      Map<Integer, Integer> source = null;
> >
> >      destination.putAll( source.entrySet().stream().collect(
> > Collectors.toMap( e -> 0, e -> 0, ( e1, e2 ) -> 0, HashMap::new ) ) );
> > }
> >
> > Javac fails to infer the above expression during overload resolution
> > of 'putAll()' when checking the return type of the partially inferred
> > method 'collect()' whereas avoiding it succeeds:
> >
> >      Map<? extends Integer, ? extends Integer> broker =
> > source.entrySet().stream().collect( Collectors.toMap( e -> 0, e -> 0,
> > ( e1, e2 ) -> 0, HashMap::new ) );
> >
> > It seems that the inference context is missing constraints from
> > 'HashMap::new' when checking the partially inferred method return type
> > because of [2].
> >
> > Removing the line 'stuck = true' as here under makes the above example
> > succeed but a couple of tests like [3] are then failing as statements
> > like 'g4(MethodReference46::m)' are no more ambiguous which would be,
> > at my mind, a good thing.
> >
> > What do you think, is the current behavior correct or should we fix
> > this issue with something like below?
> >
> > Thanks,
> > Bernard
> >
> > [1] https://bugs.openjdk.java.net/browse/JDK-8219318
> > [2] http://hg.openjdk.java.net/jdk/jdk/file/cec148db7b55/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java#l1280
> > [3] http://hg.openjdk.java.net/jdk/jdk/file/cec148db7b55/test/langtools/tools/javac/lambda/MethodReference46.java#l43
> >
> > diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java
> > b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java
> > --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java
> > +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java
> > @@ -1276,9 +1276,9 @@
> >           @Override
> >           public void visitReference(JCMemberReference tree) {
> >               super.visitReference(tree);
> > -            if (tree.getOverloadKind() !=
> > JCMemberReference.OverloadKind.UNOVERLOADED) {
> > -                stuck = true;
> > -            }
> > +//            if (tree.getOverloadKind() !=
> > JCMemberReference.OverloadKind.UNOVERLOADED) {
> > +//                stuck = true;
> > +//            }
> >           }
> >       }