Bug 547029 - [1.8][inference] JDT compiler fails to infer correctly generic types in Eclipse 2019-03
Summary: [1.8][inference] JDT compiler fails to infer correctly generic types in Eclip...
Status: NEW
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.11   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: JDT-Core-Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-07 01:52 EDT by Lyor Goldstein CLA
Modified: 2022-08-22 15:57 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Lyor Goldstein CLA 2019-05-07 01:52:24 EDT
import java.util.Map;
import java.util.AbstractMap;
import java.util.function.LongUnaryOperator;

public static final List<? extends Map.Entry<Long, LongUnaryOperator>> DEFAULT_REPORT_THRESHOLDS =
    Collections.unmodifiableList(
        Arrays.asList(
            new AbstractMap.SimpleImmutableEntry<>(10L * 1024L * 1024L, sz -> sz / 3L),            // 10MB
            new AbstractMap.SimpleImmutableEntry<>(100L * 1024L * 1024L, sz -> sz / 4L),           // 100MB
            new AbstractMap.SimpleImmutableEntry<>(1024L * 1024L * 1024L, sz -> sz / 5L),          // 1GB
            new AbstractMap.SimpleImmutableEntry<>(10L * 1024L * 1024L * 1024L, sz -> sz / 10L))); // 10GB

==========================================================================

The code is perfectly compliant with 1.8 and compile without a problem, however Eclipse complains about 2 (!) "problems":

* "Type mismatch: cannot convert from List<AbstractMap.SimpleImmutableEntry<Long,Object>> to List<? extends Map.Entry<Long,LongUnaryOperator>>"
* "The target type of this expression must be a functional interface" - once for each line
Comment 1 Lyor Goldstein CLA 2019-05-07 01:54:15 EDT
Seems that using explicit casting (new AbstractMap.SimpleImmutableEntry<Long, LongUnaryOperator>(...)) "fixes" it - but it should not be an issue from the start
Comment 2 Stephan Herrmann CLA 2019-05-07 07:24:34 EDT
I extended the example in two ways:

- make it self-contained to enable investigation

- added some variants for experimentation


//---
package snippet;
import java.util.function.*;
import java.util.*;
import java.util.AbstractMap.SimpleImmutableEntry;

public class Snippet {
	
	public static final List<? extends Map.Entry<Long, LongUnaryOperator>> DEFAULT_REPORT_THRESHOLDS =
		    Collections.unmodifiableList(
		        Arrays.asList(
		            new AbstractMap.SimpleImmutableEntry<>(10L * 1024L * 1024L, sz -> sz / 3L),            // 10MB
		            new AbstractMap.SimpleImmutableEntry<>(10L * 1024L * 1024L * 1024L, sz -> sz / 10L))); // 10GB
	public static final List<? extends Map.Entry<Long, LongUnaryOperator>> DEFAULT_REPORT_THRESHOLDS2 =
		    Collections.unmodifiableList(
		        Arrays.asList(
		            new AbstractMap.SimpleImmutableEntry<Long, LongUnaryOperator>(10L * 1024L * 1024L, sz -> sz / 3L),            // 10MB
		            new AbstractMap.SimpleImmutableEntry<Long, LongUnaryOperator>(10L * 1024L * 1024L * 1024L, sz -> sz / 10L))); // 10GB
	public static final List<Map.Entry<Long, LongUnaryOperator>> DEFAULT_REPORT_THRESHOLDS3 =
		    Collections.unmodifiableList(
		        Arrays.<Map.Entry<Long, LongUnaryOperator>>asList(
		            new AbstractMap.SimpleImmutableEntry<>(10L * 1024L * 1024L, sz -> sz / 3L),            // 10MB
		            new AbstractMap.SimpleImmutableEntry<>(10L * 1024L * 1024L * 1024L, sz -> sz / 10L))); // 10GB
	public static final List<Map.Entry<Long, LongUnaryOperator>> DEFAULT_REPORT_THRESHOLDS4 =
		    myUnmodifiableList(
		        Arrays.asList(
		            new AbstractMap.SimpleImmutableEntry<>(10L * 1024L * 1024L, sz -> sz / 3L),            // 10MB
		            new AbstractMap.SimpleImmutableEntry<>(10L * 1024L * 1024L * 1024L, sz -> sz / 10L))); // 10GB
	void test() {
		SimpleImmutableEntry<Long, LongUnaryOperator> simpleImmutableEntry = new AbstractMap.SimpleImmutableEntry<>(10L * 1024L * 1024L * 1024L, sz -> sz / 10L);
	}
	static <T> List<T> myUnmodifiableList(List<T> t) { return t; }
}
//---

I can observe the following:

DEFAULT_REPORT_THRESHOLDS: the original problem according to comment 0.

DEFAULT_REPORT_THRESHOLDS2: a recommended workaround (is this what you meant in comment 1? this is *not* casting): if inference doesn't find a solution always prefer providing type arguments over *casting*. The former helps the compiler to complete its type check, while the latter may hide existing type bugs (which will then blow up at runtime).

DEFAULT_REPORT_THRESHOLDS3: removed the useless wildcard in the target type, but still ecj needs help. This time I added the explicit type argument one level up,  thus avoiding the repeated declaration from #2

DEFAULT_REPORT_THRESHOLDS4: here I ensured that no wildcards are needed to reproduce the issue. This is done, because wildcards in type inference are a thorny issue, for which even JLS is reported to be insufficient. Having a repro without wildcards increases the chances of finding the root problem.


Still, on what grounds do you say that the example is "perfectly compliant with 1.8"? Can you demonstrate using JLS how inference should resolve this? Otherwise, let's please not jump to conclusions :)
Comment 3 Lyor Goldstein CLA 2019-05-08 01:13:10 EDT
>> DEFAULT_REPORT_THRESHOLDS2: a recommended workaround (is this what you meant in comment 1? this is *not* casting)

Yes, this is what I meant about the workaround.

>> Still, on what grounds do you say that the example is "perfectly compliant with 1.8"? Can you demonstrate using JLS how inference should resolve this? Otherwise, let's please not jump to conclusions :)

Perhaps I mis-spoke - all I can say is that the OpenJDK 1.8 build 212 (and lower) as well as Oracle JDK have no problem compiling and correctly running the original code, so I assume it has no issue with inferring correctly the types. In my book, this is equivalent to being 1.8 compliant. I think that we can agree that if the compiler finds no fault in the code neither should the IDE, otherwise it may be lead to a trust issue, and if one cannot trust the IDE, then I think it is not healthy....

BTW, this is not the 1st such incident I reported (and not even the 2nd, or 3rd - all verified and unfortunately some never fixed) - where perfectly good code (and I mean code that the compiler does not complain - not even a warning) is being reported as ERROR and Eclipse cannot run it - so how can one debug such code ?

My subjective (!) feeling is that as Java becomes more "complex" (and I have not even begun to use Java 9, 10, 11 and the constructs they introduce) Eclipse seems to lag behind in compliance more and more whereas other IDE(s) manage to keep up and provide a seamless experience. I am a great supporter (and defender) of Eclipse and open source in general, but it feels (again - entirely subjective) that it is slowly becoming un-usable or not trustworthy. In this case, I feel that (very) reluctantly I may have to switch to another IDE (and I am not the only one who feels this way).
Comment 4 Lyor Goldstein CLA 2019-05-08 01:54:16 EDT
Here is something interesting - on a specific host of a team member we ran into an "Error: Unresolved compiler error" when trying to use the original compiled code that I have posted. The fix was to explicitly declare the lambda's type:

public static final List<? extends Map.Entry<Long, LongUnaryOperator>> DEFAULT_REPORT_THRESHOLDS =
    Collections.unmodifiableList(
        Arrays.asList(
            new AbstractMap.SimpleImmutableEntry<>(10L * 1024L * 1024L, (LongUnaryOperator) sz -> sz / 3L),             // 10MB
            new AbstractMap.SimpleImmutableEntry<>(100L * 1024L * 1024L, (LongUnaryOperator) sz -> sz / 4L),            // 100MB
            new AbstractMap.SimpleImmutableEntry<>(1024L * 1024L * 1024L, (LongUnaryOperator) sz -> sz / 5L),           // 1GB
            new AbstractMap.SimpleImmutableEntry<>(10L * 1024L * 1024L * 1024L, (LongUnaryOperator) sz -> sz / 10L)));  // 10GB


it seems to be some kind of difference in the build number of the JDK being used or perhaps the specific O/S (mine is Fedora 29, his is Fedora 27). In any case, the code works just fine with some JDK's and not so fine with others, so perhaps it is a compiler error where it should have complained about inference and perhaps not - unfortunately I cannot invest time in researching this further.
Comment 5 Stephan Herrmann CLA 2019-05-08 16:43:04 EDT
(In reply to Lyor Goldstein from comment #3)
> My subjective (!) feeling is that as Java becomes more "complex" (and I have
> not even begun to use Java 9, 10, 11 and the constructs they introduce)
> Eclipse seems to lag behind in compliance more and more whereas other IDE(s)
> manage to keep up and provide a seamless experience. I am a great supporter
> (and defender) of Eclipse and open source in general, but it feels (again -
> entirely subjective) that it is slowly becoming un-usable or not
> trustworthy. In this case, I feel that (very) reluctantly I may have to
> switch to another IDE (and I am not the only one who feels this way).

Thanks for sharing your - carefully worded - views.

In one aspect I wholeheartedly agree: the growing complexity of Java is a real issue that more and more affects all Java developers. 

It would also be silly to deny that Eclipse has hard times catching up with this development, where users inevitably hit on bugs.

Recently Brian Goetz acknowledged that "implementability" of new features is an important factor, because putting too much pressure on implementors will in the end hit all developers.

I don't, however, completely agree that Eclipse badly lags behind any other entity in this game. As a compiler engineer I don't see Eclipse vs. Java, but I see three participants in this game: JLS and its two implementations javac & ecj.

We as a community are only done, when all three participants define the same language, which simply isn't true if you look closely. Instead of spamming this bug with long winded explanations, let me point you to my personal post mortem of Java 8 (written 2 years ago): https://objectteams.wordpress.com/2017/04/02/several-languages-java-8/

My main point is: ecj has bugs, so has javac, and even JLS is not what it is intended to be. In some points, ecj is blamed for behavior that is closer to JLS than javac.

I know that most users see javac as defining the language, but this is not correct.

Regarding other IDEs, well those simply rely on javac. This implies: users of other IDEs will simply see a different set of bugs: those of javac - which by many developers will just not be perceived as bugs.


Positively speaking, the Eclipse community is contributing an invaluable service to Java: we have received a great number of excellently written bug reports, _some_ of which led to revealing bugs in javac and/or JLS. Like for every "second implementation", we (you and me) are performing the optimal quality assurance for the language one could possibly dream of: just check if two implementations to the same spec exhibit the same behavior :)

Let's just go into this game with an open mind, thanks.
Comment 6 Mat Gessel CLA 2019-11-02 19:43:33 EDT
So, it seems that reproduction requires 3 elements:

1) an inner *constructor* supplying an argument to an outer method/constructor
2) a lambda of unspecified class as an argument to the inner constructor
3) inference of the class of the lambda from outside of the outer method/constructor (e.g. assignment, outer method arg)

For #1, when I use a factory method instead of the constructor, inference works fine.

JDT 3.18.100.v20190916-1045 ; tried ecj 1.8 & 12, javac 1.8 & 12


Workaround addition to the previous example
----------------
    // use a factory method instead of SimpleImmutableEntry()
    public static final List<Map.Entry<Long, LongUnaryOperator>> DEFAULT_REPORT_THRESHOLDS5 =
        unmodifiableList2(
            Arrays.asList(
                newMapEntry(1L, sz -> sz)
            )
        );
    static <K,V> AbstractMap.SimpleImmutableEntry<K,V> newMapEntry(K k, V v)
    {
        return new AbstractMap.SimpleImmutableEntry<>(k, v);
    }



Full test case: 
-----------------
void simplified()
{
    Inner<LongUnaryOperator> inner1 = new Inner<>(l -> l + 1);
    Outer<Inner<LongUnaryOperator>> outer1 = new Outer<>(inner1);
    
    // type inference for arg to Inner<>() constructor
    // works in javac 8 & 12
    // ERROR: Cannot infer type arguments for Outer<>
    Outer<Inner<LongUnaryOperator>> outer2 = new Outer<>(new Inner<>(l -> l + 1));
    // ERROR: Type mismatch: cannot convert from Filename.Outer<_547029.Inner<Object>> 
    //   to Filename.Outer<_547029.Inner<LongUnaryOperator>>
    Outer<Inner<LongUnaryOperator>> outer3 = createOuter(new Inner<>(l -> l + 1));
    
    // ERROR: same errors happen when inferring lambda type from method arguments
    Consumer<Outer<Inner<LongUnaryOperator>>> f = (Outer<Inner<LongUnaryOperator>> t) -> {};
    f.accept(new Outer<>(new Inner<>(l -> l + 1)));
    f.accept(createOuter(new Inner<>(l -> l + 1)));
    
    // workaround 1: explicitly type lambda arg
    Outer<Inner<LongUnaryOperator>> outer4 = new Outer<>(new Inner<>((LongUnaryOperator) l -> l + 1));
    Outer<Inner<LongUnaryOperator>> outer5 = createOuter(new Inner<>((LongUnaryOperator) l -> l + 1));
    f.accept(new Outer<>(new Inner<>((LongUnaryOperator) l -> l + 1)));
    
    // workaround 2: type inference works for Inner factory method
    Outer<Inner<LongUnaryOperator>> outer6 = new Outer<>(createInner(l -> l + 1));
    Outer<Inner<LongUnaryOperator>> outer7 = createOuter(createInner(l -> l + 1));
    f.accept(new Outer<>(createInner((LongUnaryOperator) l -> l + 1)));
}

static class Outer<T> { public Outer(T t) {} }

static class Inner<T> { Inner(T t) {} }

static <T> Outer<T> createOuter(T t)
{
    return new Outer<T>(t);
}

static <T> Inner<T> createInner(T t)
{
    return new Inner<T>(t);
}
Comment 7 Eclipse Genie CLA 2022-08-22 15:57:10 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.