ECJ fails: Problem detected during type inference: Unknown error at invocation of getReadOnly(P)

[hohwille]

Problem

I have a case with generics that is not compiled by EJC even though it seems perfectly valid and is accepted by javac as well.
It occurs in my OSS project exactly here:
https://github.com/m-m-m/entity/blob/f64a597454f32c7544826a2c4a171459f1937eb5/bean/src/test/java/io/github/mmm/entity/property/PropertyTest.java#L78

P readOnly = WritableProperty.getReadOnly(property);

The inital generic P here is defined like this:
https://github.com/m-m-m/entity/blob/f64a597454f32c7544826a2c4a171459f1937eb5/bean/src/test/java/io/github/mmm/entity/property/PropertyTest.java#L23

PropertyTest<V, P extends Property<V>>

The signature of the static method is as following:
https://github.com/m-m-m/property/blob/b71d7b3c8a320beaa841e24ee29bd5039b647bda/core/src/main/java/io/github/mmm/property/WritableProperty.java#L51

static <P extends WritableProperty<?>> P getReadOnly(P property) { ... }

And of course Property implements WritableProperty:
https://github.com/m-m-m/property/blob/b71d7b3c8a320beaa841e24ee29bd5039b647bda/core/src/main/java/io/github/mmm/property/Property.java#L24

public abstract class Property<V> extends AbstractWritableObservableValue<V> implements WritableProperty<V>, Cloneable

Expected result

Code compiles in Eclipse without errors.

Actual result

Compiler error:

Problem detected during type inference: Unknown error at invocation of getReadOnly(P)

If you need further details, I should test anything or I should isolate a minimum code-snippet to reproduce the error, do not hesitate to ask.

Side note

I seem to be one of the few Java developers on this planet that uses generics extensively and have also found many bugs in javac in this regard that Oracle seems to ignore. If you want to waste some time feel invited to have a look into my collection of such bugs:
m-m-m/util#166
I never implemented a compiler with type-inference but already implementing Java code to resolve java.lang.reflect.Type with optionally its surrounding Type to Class (or Classes with upper and lower bound) was a nightmare that took me ~50x more time than planned. So you have my deepest respect implementing EJC.

Workaround

Avoid type inference and cast manually:

    P readOnly = (P) WritableProperty.getReadOnly((Property<V>) property);

However, the entire purpose of the static getReadOnly method is to avoid the ugly casts.
Then I can also directly do this:

    P readOnly = (P) property.getReadOnly();

However, good about these kind of type-inference bugs is that there typically is a workaround so you are not blocked.

[hohwille]

For the record, this code compiles without errors

WritableProperty.getReadOnly(property);

When I click [Ctrl][1] and select Extract to local variable then Eclipse IDE produces this code causing the same compiler error again:

P readOnly = WritableProperty.getReadOnly(property);

Really strange...

[iloveeclipse]

Which Eclipse /Javac version was used?
Do you see the error with latest nightly build from https://download.eclipse.org/eclipse/downloads/drops4/I20240504-1800?
Could you isolate this to a small self containing example?

[jukzi]

@hohwille please provide a single file snippet that reproduces the error - without dependencies to anything then jdk.

[hohwille]

Wow, this was more tricky than expected and it seems only to happen in a rather advanced scenario but here is my so far minimal code to reproduce the problem without any imports all in a single java file:

public class EjcBug2413 {

  public interface EventListener<E> { }

  public interface EventSource<E, L extends EventListener<?/* super E */> > { }

  public interface ObservableEvent<V> { }

  public interface ObservableEventListener<V> extends EventListener<ObservableEvent<V>> { }

  public interface WritableObservableValue<V> extends EventSource<ObservableEvent<V>, ObservableEventListener<? super V>> { }

  public interface WritableProperty<V> extends WritableObservableValue<V> {
    static <P extends WritableProperty<?>> P getReadOnly(P property) {
      return null;
    }
  }

  public static abstract class AbstractEventSource<E, L extends EventListener<?/* super E */> > implements EventSource<E, L> { }

  public static class Property<V> extends AbstractEventSource<ObservableEvent<V>, ObservableEventListener<? super V>>
 implements WritableProperty<V> { }

  public static abstract class Bug2413<V, P extends Property<V>> {
    public void foo() {
      P property = createProperty();
      P readOnly = WritableProperty.getReadOnly(property);
    }
    protected abstract P createProperty();
  }
}

And with javac it is working:

$ javac EjcBug2413.java 

$ ls
'EjcBug2413$AbstractEventSource.class'  'EjcBug2413$EventSource.class'              'EjcBug2413$Property.class'                  EjcBug2413.class
'EjcBug2413$Bug2413.class'              'EjcBug2413$ObservableEvent.class'          'EjcBug2413$WritableObservableValue.class'   EjcBug2413.java
'EjcBug2413$EventListener.class'        'EjcBug2413$ObservableEventListener.class'  'EjcBug2413$WritableProperty.class'

Java version:

$ java --version
openjdk 21.0.1 2023-10-17 LTS
OpenJDK Runtime Environment Temurin-21.0.1+12 (build 21.0.1+12-LTS)
OpenJDK 64-Bit Server VM Temurin-21.0.1+12 (build 21.0.1+12-LTS, mixed mode, sharing)

Eclipse version:

Version: 2024-03 (4.31.0)
Build id: 20240307-1437

I also setup a different Java version:

$ java --version
openjdk 17.0.11 2024-04-16
OpenJDK Runtime Environment Temurin-17.0.11+9 (build 17.0.11+9)
OpenJDK 64-Bit Server VM Temurin-17.0.11+9 (build 17.0.11+9, mixed mode, sharing)
$ rm *.class
$ javac EjcBug2413.java
$ ls
'EjcBug2413$AbstractEventSource.class'  'EjcBug2413$EventSource.class'              'EjcBug2413$Property.class'                  EjcBug2413.class
'EjcBug2413$Bug2413.class'              'EjcBug2413$ObservableEvent.class'          'EjcBug2413$WritableObservableValue.class'   EjcBug2413.java
'EjcBug2413$EventListener.class'        'EjcBug2413$ObservableEventListener.class'  'EjcBug2413$WritableProperty.class'

[hohwille]

Do you see the error with latest nightly build from https://download.eclipse.org/eclipse/downloads/drops4/I20240504-1800?

The link gives me Not Found (404 page).

[jukzi]

https://download.eclipse.org/eclipse/downloads/index.html
select newest i-build
-> https://download.eclipse.org/eclipse/downloads/drops4/I20240515-1800/

[iloveeclipse]

Do you see the error with latest nightly build from https://download.eclipse.org/eclipse/downloads/drops4/I20240504-1800?

The link gives me Not Found (404 page).

It was a link to a nightly build. Just take latest build you can find at https://download.eclipse.org/eclipse/downloads/index.html

[hohwille]

One more (inner) class eliminated and comments removed:

public class EjcBug2413 {

  public interface EventListener<E> { }

  public interface EventSource<E, L extends EventListener<?> > { }

  public interface ObservableEvent<V> { }

  public interface ObservableEventListener<V> extends EventListener<ObservableEvent<V>> { }

  public interface WritableObservableValue<V> extends EventSource<ObservableEvent<V>, ObservableEventListener<? super V>> { }

  public interface WritableProperty<V> extends WritableObservableValue<V> {
    static <P extends WritableProperty<?>> P getReadOnly(P property) {
      return null;
    }
  }

  public static class Property<V> implements EventSource<ObservableEvent<V>, ObservableEventListener<? super V>>, WritableProperty<V> { }

  public static abstract class Bug2413<V, P extends Property<V>> {
    public void foo(P property) {
      P readOnly = WritableProperty.getReadOnly(property);
    }
  }
}

[hohwille]

Happy now to further investigate?

[iloveeclipse]

Happy now to further investigate?

Wait for @stephan-herrmann analysis :)

[hohwille]

super wildcard also not needed:

public class EjcBug2413 {

  public interface EventListener<E> { }

  public interface EventSource<E, L extends EventListener<?> > { }

  public interface ObservableEvent<V> { }

  public interface ObservableEventListener<V> extends EventListener<ObservableEvent<V>> { }

  public interface WritableObservableValue<V> extends EventSource<ObservableEvent<V>, ObservableEventListener<V>> { }

  public interface WritableProperty<V> extends WritableObservableValue<V> {
    static <P extends WritableProperty<?>> P getReadOnly(P property) {
      return null;
    }
  }
 
  public static class Property<V> implements EventSource<ObservableEvent<V>, ObservableEventListener<V>>, WritableProperty<V> { }

  public static abstract class Bug2413<V, P extends Property<V>> {
    public void foo(P property) {
      P readOnly = WritableProperty.getReadOnly(property); 
    }
  }
}

[hohwille]

One more interface removed:

public class EjcBug2413 {

  public interface EventListener<E> { }

  public interface EventSource<E, L extends EventListener<?> > { }

  public interface ObservableEvent<V> { }

  public interface ObservableEventListener<V> extends EventListener<ObservableEvent<V>> { }

  public interface WritableProperty<V> extends EventSource<ObservableEvent<V>, ObservableEventListener<V>> {
    static <P extends WritableProperty<?>> P getReadOnly(P property) {
      return null;
    }
  }
 
  public static class Property<V> implements EventSource<ObservableEvent<V>, ObservableEventListener<V>>, WritableProperty<V> { }

  public static abstract class Bug2413<V, P extends Property<V>> {
    public void foo(P property) {
      P readOnly = WritableProperty.getReadOnly(property); 
    }
  } 
}

[hohwille]

Further simplification:

public class EjcBug2413 {

  public interface EventSource<E, L> { }

  public interface ObservableEvent<V> { }

  public interface ObservableEventListener<V> { }

  public interface WritableProperty<V> extends EventSource<ObservableEvent<V>, ObservableEventListener<V>> {
    static <P extends WritableProperty<?>> P getReadOnly(P property) {
      return null;
    }
  }
 
  public static class Property<V> implements EventSource<ObservableEvent<V>, ObservableEventListener<V>>, WritableProperty<V> { }

  public static abstract class Bug2413<V, P extends Property<V>> {
    public void foo(P property) {
      P readOnly = WritableProperty.getReadOnly(property); 
    }
  } 
}

[hohwille]

Getting close to minimum (sorry for the spam, just trying to make analysis simpler for you):

public class EjcBug2413 {

  public interface EventSource<L> { }

  public interface ObservableEventListener<V> { }

  public interface WritableProperty<V> extends EventSource<ObservableEventListener<V>> {
    static <P extends WritableProperty<?>> P getReadOnly(P property) {
      return null;
    }
  }
 
  public static class Property<V> implements EventSource<ObservableEventListener<V>>, WritableProperty<V> { }

  public static abstract class Bug2413<V, P extends Property<V>> {
    public void foo(P property) {
      P readOnly = WritableProperty.getReadOnly(property); 
    }
  } 
}

[iloveeclipse]

sorry for the spam, just trying to make analysis simpler for you

No problem. Smaller is always better.

[stephan-herrmann]

Thanks, @hohwille for a tricky test case!

The compiler actually "wants" to report: Type mismatch: cannot convert from P to P, but then it detects that types are actually identical, so it tries to come up with a better way to report the problem, but eventually runs into the code block which I commented like this:

  // FIXME(stephan): turn into an exception once we are sure about this

At least we had the safety net to report something specific, rather than just exploding.

The root cause is buried in type inference, of course: applicability inference yields a useful result, but then invocation type inference fails! For robustness we continue with the previous result but create a ProblemMethodBinding which later triggers the problem report, but that ProblemMethodBinding is a contradiction in itself:

• it should signal that the return type could not be inferred to match the expected type (aka target type)
• but in fact the inferred return type is exactly the target type.

From here I'll have to dive into invocation type inference to see why that fails, despite having a good solution already in hands.

Wish me luck for a speedy fix to be smuggled into RC1 😄

[stephan-herrmann]

Interesting history of ecj versions:

• 3.10.0 (v20140604-1726, first version supporting Java 8) actually reports
  Type mismatch: cannot convert from P to P.
• 3.11.0 accepts
• 3.13.0 and newer return the "Unknown error"

[stephan-herrmann]

Things go wrong when we incorporate the following type bounds:

	TypeBound  P#0 :> P
	TypeBound  P#0 <: EjcBug2413.WritableProperty<?>
	TypeBound  P#0 <: P

With type bounds 2 and 3 we apply the following from 18.3.1:

When a bound set contains a pair of bounds α <: S and α <: T, and there exists a supertype of S of the form G<S1, ..., Sn> and a supertype of T of the form G<T1, ..., Tn> (for some generic class or interface, G), then for all i (1 ≤ i ≤ n), if Si and Ti are types (not wildcards), the constraint formula ‹Si = Ti› is implied.

• α is P#0
• S is P
• T is WritableProperty<?>
• matching super type of S is EventSource<ObservableEventListener<V>>
• matching super type of T is EventSource<ObservableEventListener<?>>
• S1 = ObservableEventListener<V> which is not a wildcard
• T1 = ObservableEventListener<?> which is not a wildcard
• hence we generate ‹ObservableEventListener<V> = ObservableEventListener<?>›, which reduces to false

I haven't yet found anything suspicious in our implementation, but I have the feeling that JLS 18.3.1 perhaps wants to say "if neither Si nor Ti is a wildcard nor contains a wildcard as a type argument ..." rather than only requesting that only Si and Ti themselves should not be wildcards.

[stephan-herrmann]

... I have the feeling that JLS 18.3.1 perhaps wants to say "if neither Si nor Ti is a wildcard nor contains a wildcard as a type argument ..." rather than only requesting that only Si and Ti themselves should not be wildcards.

That interpretation would let us accept the test program without causing any regressions in our test suite.

[stephan-herrmann]

Here's a variant that compiles fine:

public class EjcBug2413 {

  public interface EventSource<L> { }

  public interface ObservableEventListener<V> { }

  public interface WritableProperty<V> extends EventSource<ObservableEventListener<V>> {
    static <X,P extends WritableProperty<X>> P getReadOnly(P property) {
      return null;
    }
  }
 
  public static class Property<V> implements EventSource<ObservableEventListener<V>>, WritableProperty<V> { }

  public static abstract class Bug2413<V, P extends Property<V>> {
    public void foo(P property) {
      P readOnly = WritableProperty.getReadOnly(property); 
    }
  } 
}

I only changed this:

- <P extends WritableProperty<?>>
+ <X,P extends WritableProperty<X>>

So rather then saying that WritableProperty has an unknown (but fixed) type ?, let inference compute this type argument, too (via <X>).

For a moment I was wondering if the spec intention is indeed to only accept the variant with <X>, not the one with <?>, but then I found also this variant working, which has the wildcard still in place:

public class EjcBug2413 {

  public interface EventSource<L> { }

  public interface ObservableEventListener<V> { }

  public interface WritableProperty<V> extends EventSource<ObservableEventListener<V>> {
    static <P extends WritableProperty<?>> P getReadOnly(P property) {
      return null;
    }
  }
 
  public static class Property<V> implements EventSource<ObservableEventListener<V>>, WritableProperty<V> { }

  public static abstract class Bug2413<V, P extends Property<V>> {
    public void foo(Property<V> property) {
      Property<V> readOnly = WritableProperty.getReadOnly(property); 
    }
  } 
}

(replace P in foo() with Property<V>).

(The program declares two type variables <P> and several <V>. I might have to assign distinct names, to avoid mistakes in our reasoning ...)

[hohwille]

Wow, I did not actually expect fast progress and more assumed to get feedback like "strange edge-case, simply workaround with cast" or so or maybe bug confirmation but no reaction for years (what usually happens in Oracle bug tracker with my javac & co. bugs).
Thank you for your impressive dedication on this bug. Even though it is not yet fixed I have the impression that analysis is quite complete and the problem is well understood what is often a very important step to get something fixed. I do not know the details of type inference (e.g. S, T, Si, Ti) but with my 25 years experience in IT I can quickly see that real experts are active here (rather than some 1st or 2nd level support asking me to restart eclipse and try again LOL). If a new release of Eclipse with a fix of this would be available some when in 2024 this would already be more than what I initially dreamed of. Thanks for making Eclipse great!

[stephan-herrmann]

Thanks, @hohwille for your kind words 😄

Seems like I feel closely connected to this implementation of type inference after all these years. So not paying attention to an accurate bug report like this wouldn't make any sense, would it? :)

[srikanth-sankaran]

Thanks, @hohwille for your kind words 😄

Seems like I feel closely connected to this implementation of type inference after all these years. So not paying attention to an accurate bug report like this wouldn't make any sense, would it? :)

Thanks, @hohwille for your kind words 😄

When the famous English mathematician G.H. Hardy was asked what his greatest contribution to mathematics was, Hardy unhesitatingly replied that it was the discovery of Ramanujan! (https://en.m.wikipedia.org/wiki/G._H._Hardy)

I unhesitatingly take (grab!) some credit for your praise being the one who recruited and tasked @stephan-herrmann with the entire of the new type inference project in ECJ 😉

Not just type inference, we only half jokingly say if the defect report has something in between angle brackets, anything at all, assign it to Stephan! So he has been the defacto maintainer for much of generics in ECJ.

(Now if only javac doesn't implement a bunch of extra constitutional behavior and both the reference compiler and JLS are perfectly aligned, it would make his and ECJ's job so much easier but that is a perfect world)

Thank you @stephan-herrmann.

[stephan-herrmann]

Another fun fact: changing the order of super interfaces of Property<V> flips accept / reject with current implementation of ecj. The reason lies in this spec sentence:

When a bound set contains a pair of bounds α <: S and α <: T, and there exists a supertype of S of the form G<S1, ..., Sn> and a supertype of T of the form G<T1, ..., Tn> (for some generic class or interface, G), then ...

Look at "for some generic class or interface, G"! Types P and WritableProperty<?> have two candidates for G: WritableProperty and EventSource. The order of super interfaces determines which of these G is found. And the spec does not require to add constraints for all such G, using just one such witness seems sufficient.

Interestingly, the concept of such a common supertype is quite common in JLS. So, are all applications of the concept underspecified?

In our case the selection makes a difference, because the transition from WritableProperty to EventSource pushes the type argument into one more level of type nesting, where it escapes the current test for wildcard.

Looking at the example one could guess that the implementation should pick the "nearest" supertype, but what is the nearest supertype of A and B in situations like the following?

class A implements I1, I2 ...
class B implements I2, I1 ...

[stephan-herrmann]

Another fun fact: changing the order of super interfaces of Property flips accept / reject with current implementation of ecj.

#2459 demonstrates how we can use the space left open by under-specification can be used to go from reject to accept: we simply prefer a more specific G over a less specific one. This alone doesn't make the selection deterministic in the general case, but it helps in this particular case IFF accept is the desired outcome.

The same PR contains another example that essentially demonstrates that a method parameter with a type of the shape One<Inner<?>> can be satisfied by no other value than null. So what happens if a value "has" both kinds of types: directly typed as Two<?> with supertype Super<Inner<?>>? If a value cannot satisfy type Super<Inner<?>> how can it satisfy its subtype Two<?>?

Ergo: it is clear that the current situation is inconsistent, but I can't read the mind of the specification to decide which direction is intended. I'll seek clarification.

[hohwille]

#2459 demonstrates how we can use the space left open by under-specification can be used to go from reject to accept: we simply prefer a more specific G over a less specific one. This alone doesn't make the selection deterministic in the general case, but it helps in this particular case IFF accept is the desired outcome.

Indeed that sounds absolutely logical (cannot think of a case where this would lead to a wrong/worse result right now, but surely generic type system is complex enough that just with some brain processing you can ensure and think through everything).
Do you have the chance to discuss such thoughts with the makers of JLS regarding generics and type-inference?
If yes, awesome.
In no, I have contacts to Wolfgang Wiedgand from Oracle and he is in contact with Brian Goetz. So let me know, if I should arrange some pointing out of connections :)

[stephan-herrmann]

a method parameter with a type of the shape One<Inner<?>> can be satisfied by no other value than null.

wrong, a value of type One<Inner<?>> can be assigned. This is probably due to the fact, that the inner type argument ? survives even capture of the whole type.

[stephan-herrmann]

Do you have the chance to discuss such thoughts with the makers of JLS regarding generics and type-inference?

For a start see https://bugs.openjdk.org/browse/JDK-8319461?focusedId=14674674&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14674674 😄

More to come on other channels.

[stephan-herrmann]

Let's see if we get an answer to https://mail.openjdk.org/pipermail/compiler-dev/2024-May/026577.html

[stephan-herrmann]

Let's see if we get an answer to https://mail.openjdk.org/pipermail/compiler-dev/2024-May/026577.html

Two answers indeed, showing that none of my hypotheses was right :)

• its wrong to pick only one pair of super types with common generic type, all such pairs must be considered
• the supertype of One<?> is not Super<Inner<?>> but Super<? extends Inner<?>>!

The latter applies two rules we didn't yet consider:

• supertype computation involves capture as per §4.10.2
• supertype computation should involve upwards projection (not yet in JLS, some more details still need to be sorted out)

New fix pushed to #2488

[stephan-herrmann]

New fix pushed to #2488

This resolved it.

[hohwille]

@stephan-herrmann thanks for fixing the bug. 👍

p.s.: Seems like in eclipse you are not using milestone feature of github.
In my projects I use it to transparently associate the release version that first included the fix.
At least I just installed the latest Eclipse Java edition 2024-06 that does seem to include the fix.
Still patient and confident that the fix will arrive in the next release.

[stephan-herrmann]

p.s.: Seems like in eclipse you are not using milestone feature of github.

Some do. Back in the days of bugzilla I would never close a bug without setting the milestone, but somehow this never became a habit in github and so far nobody complained.

I guess my original confusion is this: should the milestone be marked at the issue or at the PR or both?

[hohwille]

I guess my original confusion is this: should the milestone be marked at the issue or at the PR or both?

Yep, github could better treat them more as a single unit but it advanced scenarios there can be multiple PRs for the same issue and vice-versa. So for the answer is both. I have the rule to always assign the milestone to issue + PR before doing the merge.
BTW: just chatting about personal best-practices. Feel free to do whatever you think is best for your project.
If you want to have a look for inspirations:
https://github.com/devonfw/ide/blob/master/CHANGELOG.asciidoc

[srikanth-sankaran]

Lately I have been updating the milestone at both places.

The dichotomy between PR and Issue also very often ends up bifurcation the discussions which is unfortunate.

[stephan-herrmann]

Lots of food for thought.

• milestones => Use of milestones #2627
• dichotomy => Discussions in issue vs PR #2630

[hohwille]

Sorry for delay but just wanted to finally confirm that in Eclipse 2024-09 the bug is fixed.
Thank you very much.