Community
Participate
Working Groups
Created attachment 179136 [details] testcase I20100915-0100 Sorry to keep opening new bugs in this area, but here is another case where 1.4 project confuses the reconciler when referencing API from a 1.5 project that uses generics. The usecase is rather horrid and it is not a good example for API design. But I was trying to remove all my generic warnings in Equinox and I found some API that returned an array of Dictionary. To get rid of the warning I changed the type to Dictionary<Object, Object>[]. This confuses the reconciler (the editor has an error, but the bytecode runs fine).
Reproduced. I see an error message "Type mismatch: cannot convert from List to List[]" in the reconciler. Compiler works OK. If this problem proves to be as simple as it appears at first glance, I'll deliver a point fix here while retaining bug 324850 for the general case.
Created attachment 179221 [details] Patch under test This patch is deliberately kept simple. In particular I have not folded the multiple checks against the has1_5Compliance field into one block so that the change set is simpler to study and understand.
All tests pass, Satyam please review, TIA. Basically the problem was that when we encountered a type encoded as a string of the form "List<String> []", we cannot upon seeing the '<' in 1.4 compliance mode conclude that we have the full type description. We need to skip the type arguments and look past the '>' too see if there are further tokens there that could modify the type seen thus far.
I don't think using the originalComplianceLevel in TypeConverter is correct. I think the compliance level is modified so that the TypeConverter sees the files according to the project compliance level of the files rather than the requesting project. The testcase attached here works fine using complianceLevel rather than originalComplianceLevel. However, it breaks one test with 323633.
Created attachment 179247 [details] Test Patch This patch is not complete nor tested, but I think the fix should be something like this.
(In reply to comment #4) > However, it breaks one test with 323633. I meant it breaks the test added for 323633. With the patch added with comment 5, this test also passes.
(In reply to comment #4) Sorry, I should have apprised you of some history around this defect. Recently, there has been a spate of defects around mixing of 1.4 and 1.5 projects with generics in its APIs. https://bugs.eclipse.org/bugs/show_bug.cgi?id=305259 (fixed) https://bugs.eclipse.org/bugs/show_bug.cgi?id=323633 (fixed) https://bugs.eclipse.org/bugs/show_bug.cgi?id=325633 (current) https://bugs.eclipse.org/bugs/show_bug.cgi?id=324850 (open) The problems are mostly in the reconciler, but as bug 324850 shows even the compiler is unprepared to properly handle mixing of 1.4 & 1.5 projects. > I don't think using the originalComplianceLevel in TypeConverter is correct. I > think the compliance level is modified so that the TypeConverter sees the files > according to the project compliance level of the files rather than the > requesting project. Yes, but this is a recent change. Prior to the fix for bug 323633 and bug 305259, whether it is the compiler or the reconciler, there was only one compliance level and only one source level: which is that of the "requesting project" as you put it. So the TypeConverter used to operate only under the compliance level of the requesting project and so if that is 1.4, would simply drop type parameters. This results in semi erased forms of signatures leading to various problems in the reconciler. So to fix this, I introduced a "sand box mode", where we temporarily switch the compliance and source setting to that of the project defining the type that is requested. > The testcase attached here works fine using complianceLevel rather than > originalComplianceLevel. However, it breaks one test with 323633. Yes, using the (switched) complianceLevel everywhere introduces its own problems since that results in "too much" generic information being digested, which is why the change was made to use "originalComplianceLevel" i.e, the "original" settings cached from the erstwhile (requesting project's) settings while we are on a rendezvous using the defining project's settings. (In reply to comment #6) > (In reply to comment #4) > > However, it breaks one test with 323633. > I meant it breaks the test added for 323633. With the patch added with comment > 5, this test also passes. So, what you have done here is to provide an alternate fix to the approach of using the original compliance level. While it also works, it leaves the blatant bug in TypeConverter as it is. Having seen a '<' we cannot simply conclude that we have the full (erased) type description. We must walk past the type arguments to see if there are valid tokens past the '>'. Secondly, as the (open) bug 324850 shows, we cannot simply open the flood gates and internalize all generic information as leads to its own set of problems. That further speaks to the incremental approach. That being said, as mentioned in comment# 1, this is something of a point fix and the whole area is likely to change quite a bit to fix bug 324850 Let me know if you have more questions. Thanks!
(In reply to comment #7) Sorry, I didn't realize that you have put the sand box. I thought you had missed it :(. > So, what you have done here is to provide an alternate fix to the > approach of using the original compliance level. While it also works, > it leaves the blatant bug in TypeConverter as it is. Having seen a '<' > we cannot simply conclude that we have the full (erased) type description. > We must walk past the type arguments to see if there are valid tokens > past the '>'. Secondly, as the (open) bug 324850 shows, we cannot simply > open the flood gates and internalize all generic information as leads to > its own set of problems. That further speaks to the incremental approach. > I agree that the TypeConverter should completely walk through the type argument, but the compliance should be the project of the source code rather than the requesting project. In that case, we should not see '<' in 1.4 mode. The 1.4 files could use the erasure types. As this fix is good and bug 324850 will handle it in cleaner way, +1 for the fix.
(In reply to comment #8) > (In reply to comment #7) > I agree that the TypeConverter should completely walk through the type > argument, but the compliance should be the project of the source code rather > than the requesting project. Doing this "opens the flood gates" as I described earlier - it is not clear to me that the issue you found and fixed in comment #5 via the change to org.eclipse.jdt.internal.compiler.ast.ParameterizedSingleTypeReference.internalResolveType(Scope, ReferenceBinding, boolean) (which I had fixed alternatively via the use of originalComplianceLevel) is the only issue raised due to full internalizing of all generics. > As this fix is good and bug 324850 will handle it in cleaner way, +1 for the > fix. Just to be clear, I do intend to use bug 324850 to handle the general case (to the extent I can foresee it) - By this I mean, we will internalize as much generics as necessary and no more - the current fix is a clean fix in and of itself, but is specific to the current problem at hand. I'll release the patch after 3.7 M2a stabilizes -- Thanks!
Released in HEAD for 3.7 M3
Verified for 3.7M3 using build I20101025-0901 I also noticed that there is a syntax error (compilation unit prematurely closed) in the List.java that is used in testGenericAPIUsageFromA14Project5(). It's not affecting the result, though.
