Comment 1 Olivier Thomann 2011-07-14 13:44:11 EDT

I'll take care of it.

Comment 2 Olivier Thomann 2011-07-14 14:39:29 EDT

The problem comes from the conditional expression:

lf.add(returnSrc ? f : right);

If you replace it with:

            if (returnSrc) {
                lf.add(f);
            } else {
                lf.add(right);
            }

it works.
Poor workaround, but a workaround. Working on a real fix!

Comment 3 Olivier Thomann 2011-07-14 16:45:05 EDT

I found the problem. I cannot believe such a bug never surfaced before. You actually needed to get the local in position 0 to contain a different type (here a subtype: ArrayList) of the actual local type used with a conditional expression.
We ended up with ArrayList being on the stack item instead of List.

Patch to follow.

Comment 4 Olivier Thomann 2011-07-14 16:45:44 EDT

Created attachment 199707 [details]
Proposed fix + regression tests

Comment 5 Olivier Thomann 2011-07-14 16:52:39 EDT

Created attachment 199708 [details]
Proposed fix + regression tests

The check only needs to be done for constructors (only place with the uninitialized this object). Non constructor methods never need to check for uninitialized this.

Comment 6 Olivier Thomann 2011-07-14 16:56:48 EDT

I'll run all tests before I release that code.
Srikanth, please verify.

Comment 7 Srikanth Sankaran 2011-07-18 05:05:06 EDT

Created attachment 199815 [details]
Slightly simpler equivalent

Is this new patch functionally the same ? Original patch looks good BTW.

org.eclipse.jdt.core.tests.compiler.regression.StackMapAttributeTest.test051() &
org.eclipse.jdt.core.tests.compiler.regression.StackMapAttributeTest.test052()

pass even without the patch. I take it this is intentional.

Comment 8 Srikanth Sankaran 2011-07-18 05:06:47 EDT

Patch from comment#7 also passes all JDT/Core tests.
Patch from comment#5 looks good too.

Comment 9 Olivier Thomann 2011-07-18 11:01:03 EDT

(In reply to comment #7)
> Is this new patch functionally the same ? Original patch looks good BTW.
> org.eclipse.jdt.core.tests.compiler.regression.StackMapAttributeTest.test051()
> org.eclipse.jdt.core.tests.compiler.regression.StackMapAttributeTest.test052()
> pass even without the patch. I take it this is intentional.
Yes, there are simple checks to show that if you add an extra local (lf is no longer the locals at the position 0), then the existing code works.

I'll take your patch.

Comment 10 Olivier Thomann 2011-07-18 11:24:21 EDT

Released in BETA_JAVA7 branch only

Comment 11 Deepak Azad 2011-07-19 08:50:25 EDT

Verified in BETA_JAVA7 branch (as this is not yet in a build).

Comment 12 kurt 2011-07-28 13:46:29 EDT

Will there be an update (via Help/Check for Updates) ?
Last one was abt. 2 weeks ago.
Thanks
Kurt

Comment 13 Olivier Thomann 2011-07-28 14:10:04 EDT

(In reply to comment #12)
> Will there be an update (via Help/Check for Updates) ?
> Last one was abt. 2 weeks ago.
> Thanks
> Kurt
We are merging BETA_JAVA7 contents with HEAD and r3_7_maintenance today. So after that, all M-build (=> 3.7.1) and I-builds (including Milestones => Juno) will contain all Java 7 support and fixes including this fix.