Spruced up @Cleanup's position settings and also forced initialization, because the error appears in a screwed up place if you don't, and we can't seem to move it.
Tried tactics, and why they won't work:
- Replace every 'return', 'break', and 'continue' statement (for the latter 2, only if they break/continue out of the try block) with a block that first sets a uniquely named flag before doing the operation.
Then, check that flag in the finally block to see if the cleanup call should be guarded by a try/catchThrowable. This doesn't work, because its not possible to instrument the 4th way out of a try block without throwing an exception: Just letting it run its course. Tossing a "#flag = true;" at the end may cause a compile time error if the code is not reachable, but figuring that out requires resolution and quite a bit of analysis.
- Put catch blocks in for all relevant exceptions (RuntimeException, Error, all exceptions declared as thrown by the method, and all types of exceptions of the catch blocks of encapsulating try blocks.
This doesn't work, partly because it'll fail for sneakily thrown exceptions, but mostly because you can't just catch an exception listed in the 'throws' clause of the method body; catching an exception
that no statement in the try block can throw is a compile time error, but it is perfectly allright to declare these as 'thrown'.
- Put in a blanket catch Throwable to set the flag. Two problems with this:
First, sneaky throw can't be done. Thread.stop invokes a security manager and triggers a warning, Calling a sneakyThrow method creates a runtime dependency on lombok, constructing a sneakyThrow in-class creates visible methods or at least visible class files, and creating a new class via Class.loadClass would be very slow without caching - which gets you the same issues.
Secondly, this would mean that any statements in the try body that throw an exception aren't flagged to the user as needing to be handled.
The Cleanup annotation now also calls the cleanup method for you, and will call it at the END of the current scope. The following plans have been tried and abandoned:
- Cleanup right after the final mention. This doesn't work, because the final mention may not be the final use-place. Example:
@Cleanup InputStream in = new FileInputStream(someFile);
InputStreamReader reader = new InputStreamReader(in);
reader.read(); //oops - in is already closed by now.
- Require an explicit var.cleanup() call and consider that the cue to close off the try block.
This doesn't work either, because now variables set in between the @Cleanup declaration and the var.cleanup() call become invisible to following statements. Example:
@Cleanup InputStream in = new FileInputStream(someFile);
int i = in.read();
in.close();
System.out.println(i); //fail - i is inside the generated try block but this isn't, so 'i' is not visible from here.
By running to the end of visible scope, all these problems are avoided. This does remove the flexibility of declaring where you want a close call to be executed, but there are two mitigating factors available:
1) Create an explicit scope block. You can just stick { code } in any place where you can legally write a statement, in java. This is relatively unknown, so I expect most users will go for:
2) Just call close explicitly. I've yet to see a cleanup method which isn't idempotent anyway (calling it multiple times is no different than calling it once).
During the course of investigating these options, the AST code has been extended to support live replacement of any child node, including updating the actual underlying system AST as well as our own.
Unfortunately, this code has NOT been tested. It was rather a lot of work so I'm leaving it in, and at least for eclipse it even seemed to work.
Eclipse will now also hold off on running @PrintAST handlers until the very end. Simple generators such as @Getter didn't need this, because PrintAST's handler will hold off until eclipse does a full parse,
but when changing the innards of methods, you would likely not see what you did. Fixed that.
Also, PrintAST has an option to, instead of diving into the ASTNodes of bodies (methods, initializers, etc), to just render the java code, to see if the AST creation/rewriting you've been doing looks like the java code you intended.
There's one serious problem though: The cleanup routine modifies the eclipse internal AST, but doesn't update our bi-directional AST. Thus, or example, having a @Cleanup annotation inside the scope
of another @Cleanup fails, because the application of the second one climbs up to the wrong block level (the original block level instead of newly built try block).
the TypeReference object represents a generic type (like 'T') - each instance of a generic type has a different resolution, but 1 TypeReference object can only hold 1 resolution.
Thus, a copyType() method has been written, and the Handle* classes have been updated to use it.
Also, generateEquals() is half-finished in HandleData.
** DO NOT REUSE TYPEREFERENCE OBJECTS **
because that makes the binding process go pearshaped - after hte first run, that TypeReference object's binding parameter is set, and as its set, the resolver won't bother re-resolving it.
However, each parse run starts with new scope objects, and any 2 bindings created by different scopes aren't equal to each other. urrrrrrgh!
Fortunately, a lot of code that 'fixed' methods by adding bindings and scope have all been removed, as the parser patch point is well before these bindings are created. Thus:
** NEVER CREATE YOUR OWN BINDINGS AND SCOPE OBJECTS **
because if it comes down to that, you're doing it entirely wrong. That's eclipse's job. We're patching where we are so you don't have to do this.
Reinier Zwitserloot