Community
Participate
Working Groups
Build Identifier: 20100917-0705 This is a very tricky issue related to choosing the most specific method on method invocation, when generic type arguments are involved. Essentially, when overloading methods like this: public <T> void setValue(Parameter<T> parameter, T value) { } public <T> void setValue(Parameter<T> parameter, Field<T> value) { } And calling those methods like this: Parameter<Object> p = null; Field<Object> f = null; setValue(p, f); We *should* get a compilation error telling us that the method invocation is ambiguous. Read my full report (and the accepted answer) on stackoverflow: http://stackoverflow.com/questions/5361513/reference-is-ambiguous-with-generics Reproducible: Always Steps to Reproduce: 1. Compile this class with Eclipse, works ----------------------------------------- public class Test { public <T> void setValue(Parameter<T> parameter, T value) { System.out.println("Object"); } public <T> void setValue(Parameter<T> parameter, Field<T> value) { System.out.println("Field"); } public static void main(String[] args) { new Test().test(); } private void test() { Parameter<String> p1 = p1(); Field<String> f1 = f1(); setValue(p1, f1); Parameter<Object> p2 = p2(); Field<Object> f2 = f2(); setValue(p2, f2); } private Field<String> f1() { Field<String> f1 = null; return f1; } private Parameter<String> p1() { Parameter<String> p1 = null; return p1; } private Parameter<Object> p2() { Parameter<Object> p2 = null; return p2; } private Field<Object> f2() { Field<Object> f2 = null; return f2; } } interface Field<T> {} interface Parameter <T> {} 2. Compile that class with javac, doesn't work ----------------------------------------------
Created attachment 191573 [details] A test file containing an ambiguous method invocation
Even worse, I can pass null as the second argument of the overloaded setValue() method. I.e: setValue(p1, null); setValue(p2, null); Here, clearly, there should be ambiguity indicated by the compiler
Eclipse behavior hasn't changed here since at least 3.4. I agree the difference in behavior needs to be looked into and understood and fixed if needed. Unfortunately, I don't expect to get to this any time soon. For the time being retargetting this to 3.7.
If we want to address it for 3.7, it has to be done for RC1.
>(In reply to comment #3) > Eclipse behavior hasn't changed here since at least 3.4. > I agree the difference in behavior needs to be looked into > and understood and fixed if needed. Unfortunately, I don't > expect to get to this any time soon. > > For the time being retargetting this to 3.7. Didn't have time to look into this yet as most of the cycles in the last few months went into Java 7 work. Postponed to 3.8
There is a similar issue reported in bug 333011.
>Check the update in JDK 1.6.0_25 which fixes an issue with erasure of generics that may be related http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6302954
The accepted answer at the reference stackoverflow concluded the second method is *not* more specific than the first because Field<V> <: V does not hold, mentioning that "V is not Object in this analysis; the analysis considers all possible values of V." However, in JLS 15.12.2.5 I find the subtype requirement to be Tj <: Sj where Sj is a type *after* type argument inference: let A1 ... Ap be the type arguments inferred (§15.12.2.7) for this invocation under the initial constraints Ti << Ui (1 ≤ i ≤ n), and let Si = Ui[R1=A1,...,Rp=Ap] (1 ≤ i ≤ n). So my first judgement is: the invocation where V = Object must indeed be considered, so JDT would we correct, and javac wrong. From the accepted answer at stackoverflow: > This appears to be a significant bug in Eclipse. The spec quite clearly > indicates that the type variables are not substituted in this step. I don't see this in the spec. By contrast I see let Si = Ui[R1=A1,...,Rp=Ap] (1 ≤ i ≤ n) It seems the JLS itself is inconsistent between the "informal intuition" (speaking of *any invocation*) and the formal details (saying *this invocation*). (In reply to comment #7) > >Check the update in JDK 1.6.0_25 which fixes an issue with erasure of generics that may be related > http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6302954 I don't see a connection as the Sun/Oracle bug talks about finding a substitution for a type parameter, not about overloading. From these initial findings I lean towards closing as INVALID. Any comments?
Thanks for the analysis Stephan.
> let A1 ... Ap be the type arguments inferred (§15.12.2.7) > for this invocation > under the initial constraints Ti << Ui (1 ≤ i ≤ n), Here Ti and Ui are method parameter types, not invocation argument types. The meaning of "for this invocation" is unclear; if we interpret it as that the initial constraints Ei << Ui should also be included, where Ei is the type of invocation argument ei, then we need to solve Ei << Ui Ti << Ui which will usually yield two conflicting results for each type variable Rl. Suppose the inference process favors results from Ei<<Ui, then the mentioning of Ti<<Ui is apparently pointless in the first place. Next, the spec requires For all j from 1 to n, Tj <: Sj. for this example <V> void setValue(Parameter<V> parameter, Field<V> value) <R> void setValue(Parameter<R> parameter, R value) if we infer, from Ei<<Ui, that R=Object, we need to check Parameter<V> <: Parameter<Object> Field<V> <: Object the 1st subtype relation is false, therefore "more specific" is still false.
Zhong Yu, thanks for your comment, this is much more substantial than what was discussed so far. (In reply to comment #10) > > let A1 ... Ap be the type arguments inferred (§15.12.2.7) > > for this invocation > > under the initial constraints Ti << Ui (1 ≤ i ≤ n), > > Here Ti and Ui are method parameter types, not invocation argument types. > > The meaning of "for this invocation" is unclear; ... I'd actually say it a bit stronger: this sentence looks plain inconsistent to me. §15.12.2.7 speaks about "Inferring Type Arguments Based on Actual Arguments " but in 15.12.2.5 that rule seems to be abused to infer type arguments based on another method *declaration* and its formal parameters. If the mentioning of "Ti << Ui" is correct then saying "invocation" is wrong. I admit I believed the "invocation" part more than the "Ti << Ui" part, which may be different from what the JLS authors intended. > if we interpret it as that the > initial constraints Ei << Ui should also be included, where Ei is the type of > invocation argument ei, then we need to solve > > Ei << Ui > Ti << Ui > > which will usually yield two conflicting results for each type variable Rl... I agree, considering both these constraints gets us nowhere. > Next, the spec requires > > For all j from 1 to n, Tj <: Sj. > > for this example > > <V> void setValue(Parameter<V> parameter, Field<V> value) > <R> void setValue(Parameter<R> parameter, R value) > > if we infer, from Ei<<Ui, that R=Object, we need to check > > Parameter<V> <: Parameter<Object> > Field<V> <: Object > > the 1st subtype relation is false, therefore "more specific" is still false. I see, comparing two signatures where one has substituted type parameters while the other has not, will not yield the expected compatibility. Looking at the code the JDT compiler essentially compares these: void setValue(Parameter<Object> parameter, Field<Object> value) void setValue(Parameter<Object> parameter, Object value) i.e., we work on the basis of substitutions for both methods. I've a simple patch under test that changes the JDT compiler to comparing the method declarations disregarding the current invocation. The naive way of fixing this, however, breaks the fixes for bug 216686 and bug 99106 I'm willing to reopen the bug if we agree to realize the semantics based on the constraints "Ti << Ui" while deliberately ignoring the mentioning of an invocation and how 15.12.2.7 is inconsistently used without any actual arguments. However, if we change the compiler ... (a) we need to find how to do this without breaking bug 216686 and bug 99106 (b) we will reject more programs where intuition would see no problem
(In reply to comment #11) > I've a simple patch under test that changes the JDT compiler to comparing the > method declarations disregarding the current invocation. The naive way of > fixing this, however, breaks the fixes for bug 216686 and bug 99106 The change regarding bug 99106 seems acceptable: first we report a name clash, and afterwards it is disputable if those erroneous methods should be considered as overloads, or whether secondary errors should be suppressed.
Created attachment 215541 [details] test & possible fix *Assuming* ... "... we agree to realize the semantics based on the constraints "Ti << Ui" while deliberately ignoring the mentioning of an invocation and how 15.12.2.7 is inconsistently used without any actual arguments" ... then this patch seems to fix the issue. Note, however, that there is a very fine line between fixing this issue and breaking bug 216686. The patch distinguishes both situations be looking for the presence of a wildcard, if one is found the substituted parameters must be used, otherwise go back to the originals. This distinction is more based on experimentation than on findings from the JLS and needs further validation. Unfortunately this part of the implementation is structured differently than the JLS (see comment above Scope#mostSpecificMethodBinding).
Reopening to allow for consideration of comment 10 ff.
Hi Stephan, my understanding is that if wildcards are involved, capture conversion should be applied first to remove the wildcards. 15.12.2.7: (if A<<F) If F has the form G<...> ... then if A has a supertype of the form G<...>... now if A has wildcards, to find super types of A: 4.10.2 : The direct supertypes of the type C<R1,...,Rn> , where at least one of the Ri, 1in, is a wildcard type argument, are the direct supertypes of C<X1,...,Xn>, where C<X1,...,Xn> is the result of applying capture conversion (§5.1.10) to C<R1,...,Rn>. For example ArrayList<? super A> << List<T> apply capture conversion to ArrayList<? super A>, we get ArrayList<x>, where A <: x <: Object it has a super type List<x>, therefore T = x inference succeeds. It is odd if the inference result contains a free/unknown type variable. Usually A in A<<F does not contain wildcards, because the type of a method argument has undergone capture conversion first even before 15.12 starts. Unfortunately I cannot back that up with words in the spec, however I believe it is intended that, the type of any expression must undergo capture conversion before evaluating its enclosing expression. List<?> list = ...; foo(list); // the type of `list` here is List<x> where x<:Object it makes sense because `list` indeed is a List<x> for some concrete but unknown type x (x would be knowable if there's no erasure); narrowing down the type of `list` from List<?> to List<x> increases type information. With the same principle, I think 15.12.2.5 would be better if it says If the second method is generic ... inferred under the initial constraints T'i << Ui, where T'i is the result of applying capture conversion on Ti, ... although in its current form it happens to work too, thank to 4.10.2.
Update: at 1.8 ecj also reports an error: ---------- 1. ERROR in /tmp/Test.java (at line 21) setValue(p2, f2); ^^^^^^^^ The method setValue(Parameter<Object>, Object) is ambiguous for the type Test ---------- This can be taken as a hint that the same was also intended for 1.7-.
If this bug indeed only occurs at 1.7- I'm leaning towards closing WORKSFORME or such. I'm just planning to add some tests before closing.
New Gerrit change created: https://git.eclipse.org/r/125278
Gerrit change https://git.eclipse.org/r/125278 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=355afcea9e91a57a8d79f549075bd38d3046710d
(In reply to Eclipse Genie from comment #19) > Gerrit change https://git.eclipse.org/r/125278 was merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/ > ?id=355afcea9e91a57a8d79f549075bd38d3046710d New test released for 4.9 M2. At 1.8+ the compiler correctly reports ambiguity. Since type inference in 1.7- is a completely different story, I am not planning to 'downport' this behavior, given this bug has been quiet for some years.
Verified with Eclipse 4.9 m2 Build id: I20180731-0800