Bug 343713

Summary: [compiler] bogus line number in constructor of inner class in 1.5 compliance
Product: [Eclipse Project] JDT Reporter: Stephan Herrmann <stephan.herrmann>
Component: CoreAssignee: Stephan Herrmann <stephan.herrmann>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: Olivier_Thomann
Version: 3.7   
Target Milestone: 3.7 M7   
Hardware: Other   
OS: Linux   
Whiteboard:
Attachments:
Description Flags
test & proposed fix
none
Alternative fix
none
previous patch with copyright updated none

Description Stephan Herrmann CLA 2011-04-24 15:41:54 EDT
Compile the following class with -1.5 -g :

public class LineNumberBug {
	class Inner {
		public Inner() {
			System.out.println("Inner()");
		} // line 10
	}
	public static void main(String[] args) {
		new LineNumberBug().new Inner();
	}
}

Inspecting the generated line number table you'll see that the first few
instructions are mapped to line 10, which is at least confusing.
Comment 1 Stephan Herrmann CLA 2011-04-24 15:57:11 EDT
Created attachment 193971 [details]
test & proposed fix

Here's the same test as a regression test case and a proposed simple fix.

The line in question was last modified for bug 15250 to ensure that the
free return maps to the closing brace rather than the ctor header.

For target < 1.4 this seemed to work OK, but since 1.4 a constructor of
an inner class may have instructions before the super-ctor call, which 
assign to the this$0 synthetic field. PC positions for these instructions
(generated from generateSyntheticFieldInitializationsIfNecessary(..)) 
are never recorded, but this line
  codeStream.recordPositionsFrom(0, this.bodyEnd)
will actually be applied to those leading instructions.

At first look it seems safe to just avoid using a startPC of 0, but simply
remembering the position before the free return is generated.

Unfortunately I couldn't find any tests for bug 15250 to see if the change
breaks anything. Also, the "0" argument has been there since day one,
so I can't see why it is there. Any idea?
Comment 2 Stephan Herrmann CLA 2011-04-24 16:07:34 EDT
Created attachment 193972 [details]
Alternative fix

I guess this fix is even better and has minimal impact:

Instead of changing the final recordPositionsFrom(..) I simply added another
call to this method right where it was missing: immediately after
  generateSyntheticFieldInitializationsIfNecessary(..)
Comment 3 Olivier Thomann CLA 2011-04-25 09:36:19 EDT
Looks good. Targeting M7.
Comment 4 Stephan Herrmann CLA 2011-04-25 18:48:35 EDT
Created attachment 194026 [details]
previous patch with copyright updated

The same patch as before (second strategy) with copyright updated.
Comment 5 Stephan Herrmann CLA 2011-04-25 18:52:07 EDT
Released for 3.7M7.
Comment 6 Olivier Thomann CLA 2011-04-26 10:14:30 EDT
Will be verified once a build with v_B51 is available.
Comment 7 Olivier Thomann CLA 2011-04-27 10:11:47 EDT
Verified.