Community
Participate
Working Groups
I call ASTParser.createASTs, passing in a compilation unit for a class 'X' containing a nested class 'Y', and asking for the binding to java.lang.Object. Then, within the ASTRequestor.acceptBinding() callback, I use createBindings() to request a binding for the nested type. I get back a null. I'm not sure if this is *supposed* to work, but there's nothing saying it shouldn't (unlike for local elements in acceptAST callbacks, which are documented to not work). Under the covers, the problem seems to be that in BindingKeyResolver.getCompilationUnitDeclaration(), the SourceTypeBinding obtained for the outer type has a null scope. This causes the method to return null, which ultimately causes a failure in accessing a binding to the inner type. When the outer type binding is first created during compilation, its scope is properly set. However, it then gets nulled out in CompilationUnitDeclaration.cleanUp(), called from CompilationUnitResolver *before* requestor.acceptResult() (where my code takes place) is called. This occurs in Eclipse 3.2.2, 3.3.1, and HEAD. Should this work?
Created attachment 84583 [details] test case Test case demonstrating the problem. Note the lines commented with "this will make the test fail" or "this would make the test pass"; these change what binding is being requested.
After scopes got cleaned up, little is expected to work in lazy fashion... need to investigated whether the binding shouldn't have been created earlier on.
At present, the Java 5 version of APT always works this way: it calls createASTs() with the compilation unit being processed but only requests java.lang.Object, and then requests additional bindings as needed by the annotation processor from inside the acceptBinding() callback. See BuildEnv in apt.core for details, and note that AbstractCompilationEnv.getTypeBindingFromKey() overrides BaseProcessorEnv.getTypeBindingFromKey(). I am not totally sure why it was done that way, but a comment in BuildEnv mentions the need to deal with the "batch mode" case where a processor is given all the annotated files at once. If this shouldn't be relied upon, I can imagine two workarounds. The first is to do an additional createASTs() whenever we get a null binding, to see if by asking for a specific binding key we can be successful. Disadvantages are that it is a performance hit, and more importantly, it triggers the "multiverse" problem where bindings from different parse pipelines are not comparable. The second workaround would be to always ask for bindings for all the types we are compiling, in addition to java.lang.Object, in the outermost call to createASTs(). But I don't think this would work, because I think we can't anticipate what classes a processor will request bindings for. We could be processing unit Z when the processor asks for a binding for X$Y.
Maybe something to backport to 3.3.x and 3.2.x if easy.
Created attachment 85022 [details] test case #2 Here is a test case that illustrates the same problem differently. Given two classes X and Z, each with inner classes, I create a parser and ask it to parse both and give me bindings for both. The acceptBinding() callback will thus be called first for X and then for Z. Within the callback I try to create bindings for X, X.Y, Z, and Z.W. This works when accepting the X binding, but fails when accepting the Z binding.
Created attachment 88560 [details] Proposed patch A definitely naive patch which simply moves the compilation units declaration clean-up at the end of the CompilationUnitResolver.resolve(...) method instead of doing it inside the loop. This makes the two tests green but may have an impact on performance as memory will not be released until the end of the method. I know this could not be done for builder or search resolving loop as it may have a large number of compilation units in the given array, but my feeling would be that for DOM ASTParser, it should not be the case... Olivier, Philippe, your review on this proposal is mandatory and some other hints if this solution wouldn't be acceptable, TIA
Unfortunately the main client of this API is refactoring and they rely on us releasing the memory early as a fair amount of compilation units can be analyzed at the same time.
(In reply to comment #7) > Unfortunately the main client of this API is refactoring and they rely on us > releasing the memory early as a fair amount of compilation units can be > analyzed at the same time. > So, perhaps could we do the clean-up at the end only if there's not too much CUs to resolve (in the 2 test cases there are only one and two CUs). The other possibility may be to add a specific option for this behavior on the ASTParser and warn the clients that using it may imply high memory usage...
Created attachment 88801 [details] Better patch Better patch after having discussed with Jerome. This patch simply fixes how the BindingKeyResolver get type bindings for member types...
Jerome, please review, thanks
Patch looks good.
Better patch released for 3.4M5 in HEAD stream.
Walter, could you set this bug as VERIFIED if you agree that it's fixed now? TIA
