eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0
139 stars 107 forks source link

ECJ fails to compile method invocation while javac successfully compiles #1591

Closed coehlrich closed 1 month ago

coehlrich commented 6 months ago
    public void test() {
        Supplier<? extends List<? extends Serializable>> supplier = () -> null;
        error(supplier.get());
    }

    public <T, V extends Serializable> void error(List<V> v2) {}

ECJ fails to compile with the message The method error(List<V>) in the type Outer is not applicable for the arguments (capture#1-of ? extends List<? extends Serializable>) whereas javac successfully compiles the method.

jukzi commented 6 months ago

probably related to https://github.com/eclipse-jdt/eclipse.jdt.core/issues/1332

stephan-herrmann commented 1 month ago

probably related to #1332

I don't think so. Cast compatibility and type inference use different algorithms.

But I can report that the problem vanishes when you remove the unused type parameter <T>

OTOH, the problem is not cause by the fact that <T> is unused, as this variant demonstrates:

import java.io.Serializable;
import java.util.List;
import java.util.function.Supplier;

public class Outer {
    public void test() {
        Supplier<? extends List<? extends Serializable>> supplier = () -> null;
        error(supplier.get(), "");
    }

    public <T, V extends Serializable> void error(List<V> v2, T t) {}

}

Now <T> should have a suitable solution (String), but still we report

The method error(List, T) in the type Outer is not applicable for the arguments (capture#1-of ? extends List<? extends Serializable>, String)

stephan-herrmann commented 1 month ago

Inference fails when it encounters this constraint: ⟨V#1 = ? extends java.io.Serializable⟩

Legally, JLS 18.2.4 has no provision to accept this constraint (because for accepting, both sides must be of the same kind: both wildcards or none a wildcard).

So one of the following must be true:

stephan-herrmann commented 1 month ago
  • somewhere implicitly the spec author assumed that an equality constraint involving an inference variable doesn't require reduction / can directly be translated into an equality type bound, or can it be dropped?

Interestingly, dropping the constraint or converting it into a type bound both produce the same effect vis-a-vis GenericsRegressionTest_1_8:

stephan-herrmann commented 1 month ago
  • our inference shouldn't produce that constraint in the first place

The constraint is produced from incorporating these type bounds

What? Here incorporation re-produces a constraint that expresses the same thing as one of the type bounds being incorporated. The intention of incorporation is to combine information from two type bounds to produce a new constraint, but in this case nothing is new.

Here's the rule from 18.3.1:

α = U and S = T imply ‹S[α:=U] = T[α:=U]›

Since in our case the substitution [α:=U] doesn't change anything, we transform the type bound S = T into the type constraint ‹S = T› d'uh.

And again, an opportunistic fix causes regression in the same GenericsRegressionTest_1_8.testBug472851() -- perhaps that test should find a different path for rejecting the program?

stephan-herrmann commented 1 month ago

Next step deeper in the rabbit hole, the type bound V#1 = ? extends Serializable is created when reducing the type argument containment constraint ⟨? extends java.io.Serializable <= V#1⟩, but look, normally we should answer FALSE already here:

    // TODO: speculative addition:
    if (this.right instanceof InferenceVariable)
        return new TypeBound((InferenceVariable) this.right, this.left, SAME, this.isSoft);
    return FALSE;

This block is a greeting from https://github.com/eclipse-jdt/eclipse.jdt.core/commit/84c10c2837d11d68071e121c7bfdefafa4489d33 which apparently was very successful, see the tail in https://bugs.eclipse.org/bugs/show_bug.cgi?id=111208 (after re-opening and reverting that old bogus fix).

To clarify:

After reverting the bogus change in capturing, we can fix the issue (in 1.8) by reducing ? extends A <= E#0 to a type bound E#0 = ? extends A

To me this looks like a fair extension of JLS 18.2.3, I've asked Dan Smith for comments.

Yes, I asked, but no, I can't find a reply to this question in my mailbox.

stephan-herrmann commented 1 month ago

To manage expectations: from initial analysis I don't see how the program could be accepted when following the rules of JLS. Given the long tail of bugs against JLS and javac in this area, some of which might explain this difference between compilers, I'll stop working on this for now.

jukzi commented 1 month ago

Anyway thanks for the analysis

Jörg Kubitz

Am 09.04.2024 um 18:50 schrieb Stephan Herrmann @.***>:

 To manage expectations: from initial analysis I don't see how the program could be accepted when following the rules of JLS. Given the long tail of bugs against JLS and javac in this area, some of which might explain this difference between compilers, I'll stop working on this for now.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.

Chicken-Bones commented 1 month ago

@stephan-herrmann you are correct that the JLS has no provision for accepting an equality constraint where one side is a type, and the other is a wildcard.

However, in Javac, both sides are types. The upper bound of capture#1 of ? extends List<? extends Serializable> is a type, not a wildcard, specifically capture#2 of ? extends Serializable.

I find the JLS quite unclear (perhaps even underspecified) on when capture conversion is performed in the course of operations such as GLB or assignability, inference or overload resolution, but nevertheless this seems to make sense.

You can see the capture conversion at play in the following:

public class Regression {

    public void test(Supplier<? extends List<? extends Serializable>> supplier, List<String> lString) {
        error(supplier.get(), lString);
    }

    public <T, V extends Serializable> void error(List<V> v2, List<V> v3) { }
java: method error in class Regression cannot be applied to given types;
  required: java.util.List<V>,java.util.List<V>
  found:    capture#1 of ? extends java.util.List<? extends java.io.Serializable>,java.util.List<java.lang.String>
  reason: inference variable V has incompatible equality constraints java.lang.String,capture#2 of ? extends java.io.Serializable

However, as a counter-case. Javac does not apply capture conversion to the upper bounds of type parameters. This appears to be a deficiency of the same form.

    public <A extends List<? extends Serializable>> void test(A a) {
        error(a);
    }

    public <T, V extends Serializable> void error(List<V> v2) { }
java: method error in class Regression cannot be applied to given types;
  required: java.util.List<V>
  found:    A
  reason: cannot infer type-variable(s) T,V
    (argument mismatch; A cannot be converted to java.util.List<V>)
stephan-herrmann commented 1 month ago

Thanks, @Chicken-Bones for your in-depth comments.

@stephan-herrmann you are correct that the JLS has no provision for accepting an equality constraint where one side is a type, and the other is a wildcard.

However, in Javac, both sides are types. The upper bound of capture#1 of ? extends List<? extends Serializable> is a type, not a wildcard, specifically capture#2 of ? extends Serializable.

Outch, that's a thorny area. Please see that JLS is specific in that capture conversion is not recursive, spelled out in 5.1.10:

So I don't see any provision in JLS that would capture ? extends Serializable in that position.

I find the JLS quite unclear (perhaps even underspecified) on when capture conversion is performed in the course of operations such as GLB or assignability, inference or overload resolution, but nevertheless this seems to make sense.

In fact I have more doubts regarding javac than regarding JLS in these regards, see e.g., JDK-8016207, which starts:

javac's treatment of capture is often inconsistent with the specification. Sometimes capture occurs in places other than where it is specified. Sometimes a capture variable is "widened" to its upper bound.

Bug filed 2013, not much activity other than linking various related bugs ...

stephan-herrmann commented 1 month ago

Please see that JLS is specific in that capture conversion is not recursive

In fact we are going in circles:

I don't see anybody being on firm grounds in this area.

Chicken-Bones commented 1 month ago

Indeed, applying capture conversion recursively will reject valid code, like in the regression case

Collection<? super Collection<? super Number>> coll_lower_coll_lower_number = null;
List<java.lang.Number> n = null;
coll_lower_coll_lower_number.add(n);

It's not that capture conversion is performed recursively on the type, it's just performed when evaluating the upper bound of the capture type itself. Consider again:

    public void test(Supplier<? extends List<? extends Serializable>> supplier) {
        supplier.get(); // capture#1 of ? extends List<? extends Serializable>
        supplier.get().get(0); // capture#2 of ? extends Serializable
    }

Where is capture#2 created here? At some point you have a type List<capture#2 of ? extends Serializable> on which you bind the .get(0) method call.

The implementation detail is in whether

  1. the upper bound of capture#1 is List<? extends Serializable> and a second capture conversion happens when trying to bind the .get(0) call or
  2. the upper bound of capture#1 is List<capture#2 of ? extends Serializable>

I'm struggling to come up with any examples which would shed light on how lower bounds behave, because it's generally pretty hard to 'unwrap' them

stephan-herrmann commented 1 month ago

There are only few steps leading to the point where type inference relates an inference variable to a wildcard:

@Chicken-Bones tell me, if you see any step were JLS would allow us to capture the "inner" wildcard. As a general rule, wildcards are captured where an expression is evaluated. but the only expression in all this is supplier.get(), and its captured type is capture#1-of ? extends List<? extends Serializable>

stephan-herrmann commented 1 month ago

The implementation detail is in whether

  • the upper bound of capture#1 is List<? extends Serializable> and a second capture conversion happens when trying to bind the .get(0) call or
  • the upper bound of capture#1 is List<capture#2 of ? extends Serializable>

It's the former, evaluating get(), and get(0) each perform one level of capturing.

stephan-herrmann commented 1 month ago

But I can report that the problem vanishes when you remove the unused type parameter <T>

That was a butterfly effect: a type bound for T#0 plus a type bound for V#0 where incorporated to create the bogus constraint ⟨V#1 = ? extends java.io.Serializable⟩. Even though no information from the first bound was used, its mere presence is required for that step of incorporation.

stephan-herrmann commented 1 month ago

For the records, here's the full transcript of our type inference (produced with setting these to true: InferenceContext18.DEBUG and InferenceContext18.DEBUG_FINE):

Infer applicability for error(supplier.get(), ""):
Inference Context (initial) (strict)
Inference Variables:
    T#0 :   NOT INSTANTIATED
    V#1 :   NOT INSTANTIATED
Initial Constraints:
    ⟨supplier.get() → java.util.List<V#1>⟩
    ⟨"" → T#0⟩
Type Bounds:
    TypeBound  V#1 <: java.io.Serializable
Capture Bounds: <empty>

Reduced ⟨supplier.get() → java.util.List<V#1>⟩
  to    ⟨capture#1-of ? extends java.util.List<? extends java.io.Serializable> → java.util.List<V#1>⟩
Reduced ⟨capture#1-of ? extends java.util.List<? extends java.io.Serializable> → java.util.List<V#1>⟩
  to    ⟨capture#1-of ? extends java.util.List<? extends java.io.Serializable> <: java.util.List<V#1>⟩
Reduced ⟨capture#1-of ? extends java.util.List<? extends java.io.Serializable> <: java.util.List<V#1>⟩
  to    ⟨? extends java.io.Serializable <= V#1⟩
Reduced ⟨? extends java.io.Serializable <= V#1⟩
  to    TypeBound  V#1 = ? extends java.io.Serializable
Reduced ⟨"" → T#0⟩
  to    ⟨java.lang.String → T#0⟩
Reduced ⟨java.lang.String → T#0⟩
  to    ⟨java.lang.String <: T#0⟩
Reduced ⟨java.lang.String <: T#0⟩
  to    TypeBound  T#0 :> java.lang.String
Reduced all to:
Inference Context (initial) (strict)
Inference Variables:
    T#0 :   NOT INSTANTIATED
    V#1 :   ? extends java.io.Serializable
Type Bounds:
    TypeBound  V#1 = ? extends java.io.Serializable
    TypeBound  V#1 <: java.io.Serializable
    TypeBound  T#0 :> java.lang.String
Capture Bounds: <empty>

Reduced ⟨? extends java.io.Serializable <: java.io.Serializable⟩
  to    TRUE
Incorporated:
Type Bounds:
    TypeBound  V#1 = ? extends java.io.Serializable
    TypeBound  V#1 <: java.io.Serializable
    TypeBound  T#0 :> java.lang.String
Capture Bounds: <empty>

Reduced ⟨T#0 = java.lang.String⟩
  to    TypeBound  T#0 = java.lang.String
Reduced ⟨V#1 = ? extends java.io.Serializable⟩
  to    FALSE
Incorporated:
Type Bounds:
    TypeBound  V#1 = ? extends java.io.Serializable
    TypeBound  V#1 <: java.io.Serializable
    TypeBound  T#0 :> java.lang.String
    TypeBound  T#0 = java.lang.String
Capture Bounds: <empty>

Reduced ⟨T#0 = <Z#0-T#0>⟩
  to    TypeBound  T#0 = Z#0-T#0
Reduced ⟨V#1 = ? extends java.io.Serializable⟩
  to    FALSE
Incorporated:
Type Bounds:
    TypeBound  V#1 = ? extends java.io.Serializable
    TypeBound  V#1 <: java.io.Serializable
    TypeBound  T#0 :> java.lang.String
    TypeBound  T#0 = Z#0-T#0
Capture Bounds: <empty>

Result=
null
stephan-herrmann commented 1 month ago

In 1.8 javac supported a hidden switch -XDverboseResolution=all which in the current example produces:

Outer.java:5: Note: resolving method <init> in type Object to candidate 0
                                public class Outer {
                                       ^
  phase: BASIC
  with actuals: no arguments
  with type-args: no arguments
  candidates:
      #0 applicable method found: Object()
Outer.java:8: Note: resolving method get in type Supplier to candidate 0
                                                error(supplier.get(), "");
                                                              ^
  phase: BASIC
  with actuals: no arguments
  with type-args: no arguments
  candidates:
      #0 applicable method found: get()
  where T is a type-variable:
    T extends Object declared in interface Supplier
Outer.java:8: Note: resolving method error in type Outer to candidate 0
                                                error(supplier.get(), "");
                                                ^
  phase: BASIC
  with actuals: CAP#1,String
  with type-args: no arguments
  candidates:
      #0 applicable method found: <T,V>error(List<V>,T)
        (partially instantiated to: (List<CAP#2>,String)void)
  where T,V are type-variables:
    T extends Object declared in method <T,V>error(List<V>,T)
    V extends Serializable declared in method <T,V>error(List<V>,T)
  where CAP#1,CAP#2 are fresh type-variables:
    CAP#1 extends List<? extends Serializable> from capture of ? extends List<? extends Serializable>
    CAP#2 extends Serializable from capture of ? extends Serializable
Outer.java:8: Note: Deferred instantiation of method <T,V>error(List<V>,T)
                                                error(supplier.get(), "");
                                                     ^
  instantiated signature: (List<CAP#1>,String)void
  target-type: <none>
  where T,V are type-variables:
    T extends Object declared in method <T,V>error(List<V>,T)
    V extends Serializable declared in method <T,V>error(List<V>,T)
  where CAP#1 is a fresh type-variable:
    CAP#1 extends Serializable from capture of ? extends Serializable
Outer.java:7: Note: resolving method metafactory in type LambdaMetafactory to candidate 0
                                                Supplier<? extends List<? extends Serializable>> supplier = () -> null;
                                                                                                            ^
  phase: BASIC
  with actuals: Lookup,String,MethodType,MethodType,MethodHandle,MethodType
  with type-args: no arguments
  candidates:
      #0 applicable method found: metafactory(Lookup,String,MethodType,MethodType,MethodHandle,MethodType)

Not really enough information to see how inference came to its conclusion, we only see that javac

@srikanth-sankaran do you happen to know if recent versions of javac have a similar option to -XDverboseResolution=all? I don't see it taking any effect since 9.

stephan-herrmann commented 1 month ago

We're done here, see https://github.com/eclipse-jdt/eclipse.jdt.core/pull/2305#issuecomment-2053730390