Community
Participate
Working Groups
Build 20020214 I was stepping through the following code in org.eclipse.ui.views.tasklist.TaskListContentProvider: int[] getMarkerCounts(List markers) { int numTasks = 0; int numErrors = 0, numWarnings = 0, numInfos = 0; for (Iterator i = markers.iterator(); i.hasNext();) { IMarker marker = (IMarker) i.next(); if (MarkerUtil.isMarkerType(marker, IMarker.PROBLEM)) { int sev = MarkerUtil.getSeverity(marker); switch (sev) { case IMarker.SEVERITY_ERROR: ++numErrors; break; case IMarker.SEVERITY_WARNING: ++numWarnings; break; case IMarker.SEVERITY_INFO: ++numInfos; break; } } else if (MarkerUtil.isMarkerType(marker, IMarker.TASK)) { ++numTasks; } } return new int[] { numTasks, numErrors, numWarnings, numInfos }; } All the markers have severity IMarker.SEVERITY_WARNING in this case. When I stepped from this case's break statement, the debugger ended up highlighting the case for IMarker.SEVERITY_INFO. Although ++numInfos was highlighed, stepping again had no effect (correct). Suspect problems with the debug attributes. I've attached the current sources and the corresponding .class files for the whole task list, and a screen shot showing the bogus state. In the screen shot, notice that sev=1 (SEVERITY_WARNING, not SEVERITY_INFO).
Created attachment 483 [details] Snap.gif
Created attachment 484 [details] export.jar containing .java and .class files for the tasklist view
It seems that it is missing a line number entry in the debug attribute. For the switch in the getMarkerCounts method the bytecodes are: 0xAA /*53 : tableswitch*/ 0x00 /*padding with 0-3 0x00*/ 0x00 0x0000002A /* default goto 95*/ 0x00000000 /* from 0 to 2 */ 0x00000002 0x00000027 /* for 0 goto 92 */ 0x00000021 /* for 1 goto 86 */ 0x0000001B /* for 2 goto 80 */ 0x84 0x03 0x01 /*80 : iinc local variable 3 with const 1 */ 0xA7 0x00 0x0C /*83 : goto pc 95 : offset 12 */ 0x84 0x04 0x01 /*86 : iinc local variable 4 with const 1 */ 0xA7 0x00 0x06 /*89 : goto pc 95 : offset 6 */ 0x84 0x05 0x01 /*92 : iinc local variable 5 with const 1 */ 0xA7 0x00 0x11 /*95 : goto pc 112 : offset 17 */ And the line number attributes are: 0x0050 /*pc 80 <--> 229 line number*/ 0x00E5 0x0053 /*pc 83 <--> 230 line number*/ 0x00E6 0x0056 /*pc 86 <--> 232 line number*/ 0x00E8 0x0059 /*pc 89 <--> 233 line number*/ 0x00E9 0x005C /*pc 92 <--> 235 line number*/ 0x00EB 0x0062 /*pc 98 <--> 239 line number*/ 0x00EF So the pc 89 is mapped to the line 233 (break;). This is fine. But the pc 95 has not entry in the line number attribute table. It should be mapped to line 223 (the for loop). I need to investigate why we missed this entry. Right now pc 95 is mapped at the same location than pc 92 which is line 235 (++numInfos; of the next case).
Without the "else if", everything is fine.
I checked what javac provides in this case. Javac had an extra line number attribute which points to the last break statement of the switch statement. This is a little bit confusing as well. The ideal solution would be to point to the line of the increment of the for statement.
The test case I am using is: import java.util.ArrayList; import java.util.Iterator; import java.util.List; public class Test { static int[] getMarkerCounts(List markers) { int numTasks = 0; int numErrors = 0, numWarnings = 0, numInfos = 0; for (Iterator i = markers.iterator(); i.hasNext();) { Marker marker = (Marker) i.next(); if (marker.getType() == Marker.PROBLEM) { int sev = marker.getSeverity(); switch (sev) { case Marker.SEVERITY_ERROR : ++numErrors; break; case Marker.SEVERITY_WARNING : ++numWarnings; break; case Marker.SEVERITY_INFO : ++numInfos; break; } } else if (marker.getType() == Marker.TASK) { ++numTasks; } } return new int[] { numTasks, numErrors, numWarnings, numInfos }; } public static void main(String[] args) { ArrayList l = new ArrayList(); l.add(new Marker(Marker.SEVERITY_ERROR)); l.add(new Marker(Marker.SEVERITY_WARNING)); l.add(new Marker(Marker.SEVERITY_WARNING)); l.add(new Marker(Marker.SEVERITY_INFO)); getMarkerCounts(l); } } class Marker { public static final int PROBLEM = 0; public static final int TASK = 1; public static final int SEVERITY_ERROR = 1; public static final int SEVERITY_WARNING = 2; public static final int SEVERITY_INFO = 3; private int sev; Marker(int k) { this.sev = k; } public int getSeverity() { return this.sev; } public int getType() { return PROBLEM; } }
This is a side-effect of removing a goto to the next bytecode in the CodeStream. We might reconsider this optimization with latest VMs. It might not be relevant anymore. If this optimization is removed, the compiler ends up doing exactly what javac does.
If the last "break" is removed, javac has exactly the same bug. So I don't know if it worth fixing it. We might simply comment this in the release notes.
Inlining the switch gotos would solve this problem
We could try several optimizations. 1) Optimizing gotos to a goto. This is quite risky now. 2) When we place the falseLabel of a if statement, we check if the thenStatement is a switch statement or if it is a block which has a switch statement for the last statement. If this is the case, we can copy all the forward references from the switch's break label to the false label. Then when the false label is placed, it will override the gotos from the switch statements. Need to be investigate and intensively tested.
In fact it is the endif label and not the falseLabel that needs to get the forward references of the switch statement break label.
If it's that complicated, I don't feel this is a critical bug for 2.0. Feel free to lower priority or defer.
We have a fix for this problem. I will release it soon even if I need to see if there are other patterns that could lead to the same stepping problem. With the fix the stepping looks natural again.
Fixed and released in HEAD. We might want to use PR 3426 to report further problems in debugger highlights.
