Community
Participate
Working Groups
Created attachment 227415 [details] Project that shows the error Error is: The method addListener(App.Command_1<Object>) in the type App.ObservableEventWithArg.Monitor is not applicable for the arguments (new App.Command_1<String>(){}) I've narrowed down the code to simple repro, see attached project. The source is: import java.util.List; public class App { public interface Command_1<T> { public void execute(T o); } public static class ObservableEventWithArg<T> { public class Monitor { public Object addListener(final Command_1<T> l) { return null; } } } public static class Context<T> { public ObservableEventWithArg<String>.Monitor getSubmissionErrorEventMonitor() { return null; } } public static void main(String[] args) { compileError(new Context<List<String>>()); } private static void compileError(Context context) { context.getSubmissionErrorEventMonitor().addListener(new Command_1<String>() { @Override public void execute(String o) { } }); } }
Thanks for the concise repro! Investigating.
Created attachment 227429 [details] tentative fix
Here's what I see: A PMB for getSubmissionErrorEventMonitor() is created using Context#RAW as the substitution context. Since substitution.isRawSubstitution() answers true, the return type ObservableEventWithArg<java.lang.String>.Monitor will be substituted to become ObservableEventWithArg#RAW.Monitor. Using this as the receiver type for addListener() the method parameter is substituted to Command_1<Object>, which doesn't accept the actual argument of type Command_1<String>. The patch in comment 2 cheats by trying to make the substitution raw only at the inner level of nesting. This breaks a number of tests: - org.eclipse.jdt.core.tests.compiler.regression.GenericTypeTest.test0939() - org.eclipse.jdt.core.tests.compiler.regression.GenericTypeTest.test0956() plus different type printout in - org.eclipse.jdt.core.tests.compiler.regression.GenericsRegressionTest_1_7.test001h_1() I will see if I find a simple and "more correct" patch.
At a closer look I fail to see how the rawness of Context should ever affect the interpretation of ObservableEventWithArg<java.lang.String>.Monitor. Thus the whole concept of Substitution.isRawSubstituation() looks bogus to me. The commit 3af20c8fa9116cf8e52dcf498c1b4eea06f21754 introducing this concept mentions bug 81721 and bug 82590, but I don't see any raw types in those bugs :(
Hm, so, when substituting a type via a raw type, all enclosing types are made raw. I have no clue why that is so. I'm backing out for now, sorry. Ah, here's another find: if I remove this rawification, among the many tests changing their outcome I see org.eclipse.jdt.core.tests.compiler.regression.GenericTypeTest.test1132() which has the following comment: // Type Mismatch error - Since // myThing is unbounded, return // type List<Integer> // incorrectly becomes unbounded Is this saying we want the compiler to produce an incorrect result?
Final note for today: Looking at GenericTypeTest.test1132() I see - javac reports the same error, of which the comment explains that it is wrong - the test was introduced in commit 671203b650cf0c5ccde429cd68a99e555968a741 assumably on behalf of bug 175409 (inferred from looking at the build note changes in that commit), which takes us further away from understanding the situation, because that bug is about a DOM issue, not compilation proper.
(In reply to comment #5) > Hm, so, when substituting a type via a raw type, all enclosing types are > made raw. > I have no clue why that is so. Because class X<T> { class Y<P> {} X<String>.Y y = null; } is a compile error ? The inner type cannot be raw when outer type is parameterized. > > I'm backing out for now, sorry. For that reason, we should double check this plan.
(In reply to comment #7) > Because > > class X<T> { > class Y<P> {} > X<String>.Y y = null; > } > > is a compile error ? The inner type cannot be raw when outer type is > parameterized. Thanks, I didn't know. Alright, JLS7 sect 4.8 has this: "It is a compile-time error to attempt to use a type member of a parameterized type as a raw type. " I must have been sleeping already when I searched last night :) So, on the one hand we have the rule that mixed Parameterized.Raw types are actually illegal, on the other hand type inference makes some types raw on its way, in order to find a match. With a naive fix lots of tests relating to type inference fail. Coming back to the example in this bug: Resolving this expression: context.getSubmissionErrorEventMonitor() we currently make the return type raw, because context has a raw type. This would be correct if the return type would depend on the value of context (i.e., an inner class), but it's not correct for an independent parameterized type. The actual return type ObservableEventWithArg<java.lang.String>.Monitor should be perfectly legal, no? I've experimented with an additional check inside Scope.substitute: - only make types raw when originalType in some way 'dependsOn' the raw type (substitution). Dependency could be: - _ is an inner of _ - _ uses a type variable declared in the original of the raw type _ This fixes the immediate issue but still causes regressions in GenericTypeTests. Srikanth, what priority would you give to this issue? I guess that fixing would involve looking at approx. 30 individual tests, re-check expectations, compare with javac, improve the patch to better distinguish whether or not to rawify. Much seems to depend on this piece of code, but if my conclusions are right we're indeed rejecting legal code. OTOH, an obvious workaround exists: avoid the raw type "Context"!
We can split the issue into two questions: (A) Should the return type of a method, that is accessed via a raw receiver, be converted to raw? (B) Is it legal to assign that value to a non-raw variable? Ad (A): ------- Looking at GenericTypeTest.test0523() we see both compilers agree that the return type is indeed raw: import java.util.*; public class X { public X() { M m = new M(); List<String> ls = m.list(); // rawified even though wasn't using T parameter } Zork z; static class M<T> { List<String> list() { return null; } } } It seems the author of the test was puzzled himself. Frankly, I don't see this motivated by anything in the JLS, but since everybody seems to agree, not much we can/should change here. Ad (B): ------- Here the answer is obviously: yes, assigning raw to parameterized is legal. Looking just at this part of the problem, ecj has a pretty simple bug: While computing the raw type of ObservableEventWithArg<String>.Monitor we construct a bogus type ObservableEventWithArg<Object>.Monitor#RAW. I have a simple patch under test which addresses just this bogus conversion. All GenericTypeTests plus the test for this bug are green.
(In reply to comment #9) > (A) Should the return type of a method, that is accessed via a raw receiver, > be converted to raw? > > [...] > It seems the author of the test was puzzled himself. > Frankly, I don't see this motivated by anything in the JLS, > but since everybody seems to agree, not much we can/should change here. I'm withdrawing my critique, seeing that JLS7 Sect. 4.8 indeed says: "The type of a constructor (§8.8), instance method (§8.4, §9.4), or non-static field (§8.3) M of a raw type C that is not inherited from its superclasses or superinterfaces is the raw type that corresponds to the erasure of its type in the generic declaration corresponding to C." So the answer to (A) is clearly: yes. I will document this insight in the corresponding tests (see also comment 5).
(In reply to comment #9) > (B) Is it legal to assign that value to a non-raw variable? > ------- > Here the answer is obviously: yes, assigning raw to parameterized is legal. > > Looking just at this part of the problem, ecj has a pretty simple bug: > While computing the raw type of ObservableEventWithArg<String>.Monitor > we construct a bogus type ObservableEventWithArg<Object>.Monitor#RAW. > > I have a simple patch under test which addresses just this bogus conversion. > All GenericTypeTests plus the test for this bug are green. After all back-and-forth (sorry about that), I found this simple patch to succeed in fixing the bug without causing regressions elsewhere. Fix is well localized so no particularly high risk involved. Released for 4.3 M6 via commit 8200b6cfd64a575249a0f66c65d0e9c9e4a95b92
Verified for 4.3 M3 using Build id: I20130310-2000
