Summary: | [1.4/1.5] Unexpected ambiguous error for 1.4 project | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Thomas Watson <tjwatson> | ||||||||||
Component: | Core | Assignee: | Srikanth Sankaran <srikanth_sankaran> | ||||||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||||||
Severity: | major | ||||||||||||
Priority: | P3 | CC: | jarthana, Olivier_Thomann | ||||||||||
Version: | 3.7 | ||||||||||||
Target Milestone: | 3.7 M4 | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | Mac OS X - Carbon (unsup.) | ||||||||||||
Whiteboard: | |||||||||||||
Attachments: |
|
Srikanth, please investigate. FYI, Srikanth and Olivier, this error can also be observed from the latest OGGi git repo. I released the disabled regression test for this issue: org.eclipse.jdt.core.tests.compiler.regression.MethodVerifyTest#_test331446 Since this is causing grief in the OSGi workspace, setting the severity to major. (In reply to comment #2) > FYI, Srikanth and Olivier, this error can also be observed from the latest OGGi > git repo. Jay, How come we didn't see this in our set up ? Is it because, we have a snap shot and haven't synchronized with the top of trunk ? In any case we should upgrade to the latest bits and test for the upcoming milestone build input. We should also test PDE sources + test projects and JDT/UI sources + test projects. Testing early would help us to react better. Thanks for your help with this. (In reply to comment #5) > Jay, How come we didn't see this in our set up ? Is it because, we have > a snap shot and haven't synchronized with the top of trunk ? In any case > we should upgrade to the latest bits and test for the upcoming milestone > build input. We should also test PDE sources + test projects and JDT/UI > sources + test projects. Testing early would help us to react better. > Thanks for your help with this. The OSGi sources we have was last updated two weeks ago. I will update the clone and see if the errors show up in our set-up. Reduced test case: //------------- 1.5 project ----------------- class List<T> {} public class P15 { static <T> void foo(List<T> expected) {} public static <T> void foo(T expected) {} } //------------- 1.4 project ----------------- public class Client { Client(List l) { P15.foo(l); } } If both projects are 1.5+ or if both are 1.4- (suitable erasure applied), all is well. Released several more tests: Some codify the current & right behavior, some codify the expected, but not current behavior, some codify current behavior that needs to be double checked. org.eclipse.jdt.core.tests.compiler.regression.MethodVerifyTest.test1415Mix() org.eclipse.jdt.core.tests.compiler.regression.MethodVerifyTest.test1415Mix2() show some interesting behavior, in that the behavior is different from what is exhibited by javac. I think eclipse behavior is the correct one, Olivier could you double check this ? (We complain about missing foo(String) method while javac complains about missing foo(Object). Created attachment 184250 [details] Plausible patch - Under test (In reply to comment #8) > org.eclipse.jdt.core.tests.compiler.regression.MethodVerifyTest.test1415Mix() > org.eclipse.jdt.core.tests.compiler.regression.MethodVerifyTest.test1415Mix2() > > show some interesting behavior, in that the behavior is different from > what is exhibited by javac. I think eclipse behavior is the correct one, > Olivier could you double check this ? (We complain about missing foo(String) > method while javac complains about missing foo(Object). Thinking more about this and testing some more, I am actually inclined to think that javac is the right one here. A 1.4 type should look for contract fulfillment only in terms of erased types and should not consider type parameters, substitutions and such. Created attachment 184261 [details]
Proposed fix + regression tests
Same patch with reorganized tests.
(In reply to comment #10) > Created an attachment (id=184261) [details] > Proposed fix + regression tests > > Same patch with reorganized tests. Passes all JDT/Core tests. Will test some more before releasing. I could successfully compile the OSGi workspace. I also passed the AutomatedSuite of JDT/UI tests and I am running the refactoring tests. All refactoring tests passed. In the original example from comment# 0 as well as in the reduced example from comment# 7, I could verify that the problem is gone in the compiler. Reconciler still shows the issue as equivalent fix has to go in the SourceTypeConvertor. I'll look into it - Not sure yet if we should spawn a new bug for this and address it post M4. It is relatively less severe. The latest patch we have fails, on the workaround resorted to by bug 324850. Unfortunately the stripped down example projects from bug 324850 comment 2 lose some nuance that is germane here. I have added two regressions tests and released them: org.eclipse.jdt.core.tests.compiler.regression.MethodVerifyTest.test213b() and org.eclipse.jdt.core.tests.compiler.regression.MethodVerifyTest.test213c() Both pass on HEAD. The former fails with the patch we have while compiling alright with javac. Investigating further... (In reply to comment #14) > Reconciler still shows the > issue as equivalent fix has to go in the SourceTypeConvertor. Released disabled test org.eclipse.jdt.core.tests.model.ReconcilerTests._testGenericAPIUsageFromA14Project9() to test for this. Created attachment 184322 [details]
Proposed patch + tests - Take 2
This patch uses the same approach, but limits the application
of erasure to the operation in hand which is to determine
the most specific method.
This addresses the reconciler test issue reported earlier also.
Under test.
(In reply to comment #9) > Created an attachment (id=184250) [details] > Plausible patch - Under test > > (In reply to comment #8) > > > org.eclipse.jdt.core.tests.compiler.regression.MethodVerifyTest.test1415Mix() > > org.eclipse.jdt.core.tests.compiler.regression.MethodVerifyTest.test1415Mix2() > > > > show some interesting behavior, in that the behavior is different from > > what is exhibited by javac. I think eclipse behavior is the correct one, > > Olivier could you double check this ? (We complain about missing foo(String) > > method while javac complains about missing foo(Object). > > Thinking more about this and testing some more, I am actually inclined > to think that javac is the right one here. Thinking even more about it and testing even more, I don't know what to think :) Olivier, please take a look and let me have your opinion. In essence, we complain about missing foo(String) while javac complains about missing foo(String). The fact of the matter is foo(String) *is* missing. (In reply to comment #17) > Created an attachment (id=184322) [details] > Proposed patch + tests - Take 2 > Under test. RunAllJDTCoreTests is green. (In reply to comment #17) > Created an attachment (id=184322) [details] > Proposed patch + tests - Take 2 I am still having some difficulties in building the OSGi workspace consistently. But I managed to get it build couple of times properly and verified that this patch fixes the 'ambiguous' errors (There are as many as 84 errors without the patch) The OSGi workspace compiles fine with this patch. (In reply to comment #21) > The OSGi workspace compiles fine with this patch. I also tried out the patch and confirmed it works for my OSGi workspace. Passes all JDT/UI tests too. Released in HEAD for 3.7 M4 Verified for 3.7M4 using build I20101206-1800 |
Created attachment 184140 [details] ambiguity test projects I20101123-0800 Not reproducible in 3.6. The attached projects reproduce the ambiguous error for a 1.4 project which references some API from a 1.5 project.