Make synthetic classes used in a try with resources statement implement AutoCloseable #337
Conversation
… try-with-resources
… try-with-resources
There was a problem hiding this comment.
Besides one comment regarding the test case's behavior, the code changes look good to me. By the way, before I realized you were working on this issue, I was looking into this issue and created a minimal reproducible test case for the IllegalStateException, if it helps:
package test;
public class Foo {
// target method
public void foo() {
Baz.test();
}
public void bar() {
Baz<String> baz;
}
}|
|
||
| class Simple { | ||
|
|
||
| private ThirdResource r; |
There was a problem hiding this comment.
For this test case, we might want to change this to private final ThirdResource r = new ThirdResource() to prevent the compilation error we see in the CI. Not sure how Specimin will handle this (if it will remove the initializer or not) but we could force Specimin to preserve all used fields if they are used with try-with-resources.
There was a problem hiding this comment.
I changed it to just initialize it to null, which is what Specimin would do anyway. The actual value of the resource doesn't matter here, since we're not going to run the code - javac just checks that it's final.
Definitely! I actually haven't investigated that part of the crash yet at all - I looked at the code that was causing the problem and immediately noticed the try-with-resources and realized that Specimin didn't handle that case at all, so I just started working on fixing that. I'll look into this problem next, starting with your test case. |
Based on a problem I noticed while investigating why Specimin wasn't handling na-176 properly. There are probably other issues with that bug, but this one seemed like a good place to start.