Community
Participate
Working Groups
Created attachment 99986 [details] 2 Projects mentioned in step 1 showing the error. Build ID: (3.4M7) I20080502-0100 Steps To Reproduce: 1. Import the attached projects into a 3.4M3 or later workspace. 2. Watch an error about the method Person#put This error does not happen with 3.3.2, and neither with javac from JDK 1.6.0_03. It gets even weirder: 3. Uncomment the superfluous import of base.Value 4. Watch the error go away. Even more weird: 5. Move the Person class into base. 6. Get the same error as in step 2 7. Add an import for base.Value (!) 8. Watch the error go away again. More information: This is a reduced testcase from a more elaborate framework. I have tried this in a newly unpacked 3.4M7 SDK with a new workspace, to make sure it is not an interaction with the host of plugins I have in my normal installation. I have tried each 3.4 Milestones, M2 works, M3 and later fail. Fully generifying the Model class (using Value<?>) fixes the testcase, but is impossible in the full framework (too many dependent generifications that end in "capture#1 does not match capture#2" errors). Please update the summary if you have better words.
Reduced the testcase to these 3 files: package p; interface Value<B> {} public class BaseValue<B> implements Value<B> {} package p; public class Model { public java.util.Map<String, Value> map = new java.util.LinkedHashMap<String, Value>(); } package p2; public class Person extends p.Model { void test() { this.map.put("name", new p.BaseValue<String>()); } } A full build compiles the case correctly, but an incremental change to p2.Person fails to initialize the field in the BinaryType Model correctly. Instead of reading the type args for the field map as String and Value#RAW, we read them in as String and Unresolved Value, which becomes Value<B>
Created attachment 100553 [details] Possible solution Philippe - please take a look.
The fix is near this area, but I think it would be rather on caller side. I think your change will cause raw conversion in places we do not want it (i.e. too many raw conversions instead of too little as in this bug). Adopting.
Added GenericTypeTest#test1330-1331.
Created attachment 100620 [details] Proposed patch Small patch for 3.4. It is close to Kent's, except it moves the raw conversion on argument resolution, instead of forcing it down in the process (where it could still be required to not do - though from looking at the code, this path would not be used any longer). Post 3.4, we should do some cleanup in this code.
Kent - pls review patch for 3.4RC2 Maxime - pls review patch for 3.4RC2
forgot to cc maxime.
Looking at the code, I cannot see any way parameterizedType could be non-null or rank non-zero. If I'm right and your patch is correct and we can further simplify the code right away with no risk at all (making a try on my side). Moreover, if it holds true, that property would make your patch a bit more intrusive than Kent's. (I may have missed something though. Let's discuss this face-to-face.)
You got the idea for the cleanup. Also the 2 methods for resolving type on BinaryTypeBinding could be merged together, in the same pass. But this is not essential at this point.
(In reply to comment #3) > ... I > think your change will cause raw conversion in places we do not want it (i.e. > too many raw conversions instead of too little as in this bug). As we know that the second fix is more intrusive than the first, do we have any doubt about this left? (I mean, do we believe we may cause too many raw conversions?)
I do not think the 2nd patch is more intrusive than the first one. It guarantees that arguments are being resolved consistently to any other type for which a raw type conversion is to be applied. It does not change the mechanics of the internal resolution to always ignore the parameterizedType... so if in this branch we were to invoke with proper parameterizedType, it wouldn't change the semantics. Today, I do not believe Kent's patch was actually causing trouble, but in theory it could have disrupted other senders. So this is more about aesthetics in the patch.
either patch is fine with me
The second patch is readily isomorphic with code that eliminates the two latest parameters (the only call that does not pass null, 0 is the recursive call in the static method itself). The first might be so, but I've not dug into a proof of this (involves checking whether or not we can ever have parameterizedType not null on BinaryTypeBinding line 132, knowing that we pass this for bindings which kind could be - unless we know otherwise - be GENERIC). In all cases, a patch without the parameters would leave us with simpler code (that may or may not have a different behavior than patch 1).
As far as speeding things up is concerned, I'd consider that we can go with patch 2 and see how it fares, opening a fup bug for simplification that would reference this one. I'll attach a patch 3 that is isomorphic to patch 2 for further reference.
Created attachment 100652 [details] Alternative to patch 2 (isomorphic)
Released for 3.4RC2. (2nd patch which got reviewed)
Created attachment 100659 [details] Patch for suggested cleanup (post 3.4)
Created attachment 100663 [details] Patch for 3.3.x
Released to 3.3.x maintenance stream.
Verified for 3.4RC2 using I20080523-0100
*** Bug 235460 has been marked as a duplicate of this bug. ***
