Skip to content

Commit b38df97

Browse files
zuevmaximzuevmaxim
committed
Fix line filter for java8 try-with-resources
1 parent 77b1057 commit b38df97

File tree

3 files changed

+83
-57
lines changed

3 files changed

+83
-57
lines changed

‎instrumentation/src/com/intellij/rt/coverage/instrumentation/filters/lines/BaseLineFilter.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ public abstract class BaseLineFilter extends CoverageFilter {
3838
private boolean myHasInstructions;
3939
private int myCurrentLine = -1;
4040

41+
// Fields used only for debug purposes.
42+
// It is convenient to debug by adding conditional breakpoints to `tryRemoveLine` and `setHasInstructions`.
43+
private static final int DEBUG_LINE = -1;
44+
private static final Class<?> DEBUG_FILTER_CLASS = BaseLineFilter.class;
45+
4146
private void tryRemoveLine() {
4247
if (myCurrentLine != -1 && !myHasInstructions && shouldRemoveLine()) {
4348
myContext.removeLine(myCurrentLine);
@@ -63,12 +68,16 @@ protected int getCurrentLine() {
6368
return myCurrentLine;
6469
}
6570

71+
protected boolean wasLineSeenBefore() {
72+
// do not remove lines that are previously used
73+
return myContext.getLineData(myCurrentLine) != null;
74+
}
75+
6676
@Override
6777
public void visitLineNumber(int line, Label start) {
6878
tryRemoveLine();
69-
// do not remove lines that are previously used
70-
myHasInstructions = myContext.getLineData(line) != null;
7179
myCurrentLine = line;
80+
myHasInstructions = wasLineSeenBefore();
7281
super.visitLineNumber(line, start);
7382
}
7483

‎instrumentation/src/com/intellij/rt/coverage/instrumentation/filters/lines/TryWithResourcesJava8LineFilter.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ protected boolean shouldRemoveLine() {
4444
return myState == 9;
4545
}
4646

47+
@Override
48+
protected boolean wasLineSeenBefore() {
49+
int previousState = myCandidates.get(getCurrentLine());
50+
return previousState == -1;
51+
}
52+
4753
@Override
4854
public void visitLineNumber(int line, Label start) {
4955
myCandidates.put(getCurrentLine(), hasInstructions() ? -1 : myState);

‎instrumentation/src/com/intellij/rt/coverage/instrumentation/filters/lines/TryWithResourcesLineFilter.java

Lines changed: 66 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -28,31 +28,31 @@
2828
public class TryWithResourcesLineFilter extends BaseLineFilter {
2929
private State myState = State.INITIAL;
3030
private int myJumpsToRemove = 0;
31+
private int myExceptionVarIndex = -1;
3132

32-
// INITIAL → STORE_INITIAL_EXCEPTION <--------|
33-
// ↘ ↓ |
34-
// LOAD_RESOURCE ↔ CHECK_RESOURCE_NULL |
35-
// ↕ |
36-
// CALL_CLOSE |
37-
// ↓ |
38-
// GOTO |
39-
// ↓ |
40-
// STORE_ADDITIONAL_EXCEPTION |
41-
// ↓ |
42-
// LOAD_INITIAL_EXCEPTION |
43-
// ↓ |
44-
// LOAD_ADDITIONAL_EXCEPTION |
45-
// ↓ |
46-
// CALL_ADD_SUPPRESSED ↘ |
47-
// ↓ GOTO_2 |
48-
// LOAD_INITIAL_EXCEPTION_2 ↙ |
49-
// ↕ ↘ |
50-
// CALL_CLOSE_2 THROW ------------------|
51-
// ↓
52-
// GOTO_3
33+
// INITIAL → STORE_INITIAL_EXCEPTION/2 <-----|
34+
// ↘ ↓ |
35+
// ---->LOAD_RESOURCE ↔ CHECK_RESOURCE_NULL |
36+
// | ↕ |
37+
// | CALL_CLOSE---------------| |
38+
// | ↓ | |
39+
// | GOTO | |
40+
// | ↓ | |
41+
// | STORE_ADDITIONAL_EXCEPTION | |
42+
// | ↓ | |
43+
// | LOAD_INITIAL_EXCEPTION | |
44+
// | ↓ | |
45+
// | LOAD_ADDITIONAL_EXCEPTION | |
46+
// | ↓ | |
47+
// | CALL_ADD_SUPPRESSED | |
48+
// | ↙ ↓ ↙ |
49+
// GOTO_2 LOAD_INITIAL_EXCEPTION_2 |
50+
// ↓ |
51+
// THROW --------------------|
5352
private enum State {
5453
INITIAL,
5554
STORE_INITIAL_EXCEPTION,
55+
STORE_INITIAL_EXCEPTION_2, // final
5656
LOAD_RESOURCE,
5757
CHECK_RESOURCE_NULL,
5858
CALL_CLOSE, // final
@@ -64,8 +64,6 @@ private enum State {
6464
GOTO_2, // java 8
6565
LOAD_INITIAL_EXCEPTION_2,
6666
THROW, // final
67-
CALL_CLOSE_2, // java 8
68-
GOTO_3, // java 8
6967
}
7068

7169
@Override
@@ -75,39 +73,57 @@ public boolean isApplicable(InstrumentationData context) {
7573

7674
@Override
7775
protected boolean shouldRemoveLine() {
78-
return myState == State.GOTO || myState == State.THROW || myState == State.CALL_CLOSE || myState == State.GOTO_3;
76+
return myState == State.GOTO || myState == State.THROW || myState == State.CALL_CLOSE
77+
|| myState == State.STORE_INITIAL_EXCEPTION_2;
7978
}
8079

8180
@Override
8281
public void visitLineNumber(int line, Label start) {
8382
super.visitLineNumber(line, start);
8483
myState = State.INITIAL;
8584
myJumpsToRemove = 0;
85+
myExceptionVarIndex = -1;
8686
}
8787

8888
@Override
8989
public void visitVarInsn(int opcode, int var) {
9090
mv.visitVarInsn(opcode, var);
91-
if ((myState == State.INITIAL || myState == State.THROW) && opcode == Opcodes.ASTORE) {
92-
myState = State.STORE_INITIAL_EXCEPTION;
93-
} else if (opcode == Opcodes.ALOAD
94-
&& (myState == State.STORE_INITIAL_EXCEPTION || myState == State.CHECK_RESOURCE_NULL
95-
|| myState == State.INITIAL || myState == State.CALL_CLOSE)) {
96-
myState = State.LOAD_RESOURCE;
97-
} else if (myState == State.GOTO && opcode == Opcodes.ASTORE) {
98-
myState = State.STORE_ADDITIONAL_EXCEPTION;
99-
} else if (myState == State.STORE_ADDITIONAL_EXCEPTION && opcode == Opcodes.ALOAD) {
100-
myState = State.LOAD_INITIAL_EXCEPTION;
101-
} else if (myState == State.LOAD_INITIAL_EXCEPTION && opcode == Opcodes.ALOAD) {
102-
myState = State.LOAD_ADDITIONAL_EXCEPTION;
103-
} else if ((myState == State.CALL_ADD_SUPPRESSED || myState == State.GOTO_2 || myState == State.CALL_CLOSE_2)
104-
&& opcode == Opcodes.ALOAD) {
105-
myState = State.LOAD_INITIAL_EXCEPTION_2;
106-
} else if (myState == State.CALL_CLOSE_2 && opcode == Opcodes.GOTO) {
107-
myState = State.GOTO_3;
91+
if (opcode == Opcodes.ASTORE) {
92+
if (myState == State.INITIAL) {
93+
myState = State.STORE_INITIAL_EXCEPTION;
94+
myExceptionVarIndex = var;
95+
} else if (myState == State.THROW) {
96+
myState = State.STORE_INITIAL_EXCEPTION_2;
97+
myExceptionVarIndex = var;
98+
} else if (myState == State.GOTO) {
99+
myState = State.STORE_ADDITIONAL_EXCEPTION;
100+
} else {
101+
setHasInstructions();
102+
myState = State.INITIAL;
103+
}
104+
} else if (opcode == Opcodes.ALOAD) {
105+
if (myExceptionVarIndex == var && myState == State.CALL_CLOSE) {
106+
myState = State.LOAD_INITIAL_EXCEPTION_2;
107+
} else if (myState == State.INITIAL
108+
|| myState == State.STORE_INITIAL_EXCEPTION
109+
|| myState == State.STORE_INITIAL_EXCEPTION_2
110+
|| myState == State.CHECK_RESOURCE_NULL
111+
|| myState == State.GOTO_2
112+
|| myState == State.CALL_CLOSE) {
113+
myState = State.LOAD_RESOURCE;
114+
} else if (myState == State.STORE_ADDITIONAL_EXCEPTION) {
115+
myState = State.LOAD_INITIAL_EXCEPTION;
116+
} else if (myState == State.LOAD_INITIAL_EXCEPTION) {
117+
myState = State.LOAD_ADDITIONAL_EXCEPTION;
118+
} else if (myState == State.CALL_ADD_SUPPRESSED) {
119+
myState = State.LOAD_INITIAL_EXCEPTION_2;
120+
} else {
121+
setHasInstructions();
122+
myState = State.INITIAL;
123+
}
108124
} else {
109-
myState = State.INITIAL;
110125
setHasInstructions();
126+
myState = State.INITIAL;
111127
}
112128
}
113129

@@ -125,8 +141,8 @@ public void visitJumpInsn(int opcode, Label label) {
125141
} else if (myState == State.CALL_ADD_SUPPRESSED && opcode == Opcodes.GOTO) {
126142
myState = State.GOTO_2;
127143
} else {
128-
myState = State.INITIAL;
129144
setHasInstructions();
145+
myState = State.INITIAL;
130146
}
131147
}
132148

@@ -135,24 +151,19 @@ public void visitMethodInsn(int opcode, String owner, String name, String descri
135151
mv.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
136152
if ((opcode == Opcodes.INVOKEINTERFACE || opcode == Opcodes.INVOKEVIRTUAL)
137153
&& "close".equals(name)
138-
&& "()V".equals(descriptor)) {
139-
if (myState == State.LOAD_RESOURCE) {
140-
myState = State.CALL_CLOSE;
141-
return;
142-
} else if (myState == State.LOAD_INITIAL_EXCEPTION_2) {
143-
myState = State.CALL_CLOSE_2;
144-
return;
145-
}
154+
&& "()V".equals(descriptor)
155+
&& myState == State.LOAD_RESOURCE) {
156+
myState = State.CALL_CLOSE;
146157
} else if (myState == State.LOAD_ADDITIONAL_EXCEPTION
147158
&& opcode == Opcodes.INVOKEVIRTUAL
148159
&& "java/lang/Throwable".equals(owner)
149160
&& "addSuppressed".equals(name)
150161
&& "(Ljava/lang/Throwable;)V".equals(descriptor)) {
151162
myState = State.CALL_ADD_SUPPRESSED;
152-
return;
163+
} else {
164+
setHasInstructions();
165+
myState = State.INITIAL;
153166
}
154-
myState = State.INITIAL;
155-
setHasInstructions();
156167
}
157168

158169
@Override
@@ -161,8 +172,8 @@ public void visitInsn(int opcode) {
161172
if (myState == State.LOAD_INITIAL_EXCEPTION_2 && opcode == Opcodes.ATHROW) {
162173
myState = State.THROW;
163174
} else {
164-
myState = State.INITIAL;
165175
setHasInstructions();
176+
myState = State.INITIAL;
166177
}
167178
}
168179
}

0 commit comments

Comments
 (0)