Community
Participate
Working Groups
Found in 3.3 M20070913-1500; have not yet determined whether it is present in 3.3 or HEAD. The following code, when processed with the GenClass6 processor in pluggable.tests.annotations, generates the class and compiles correctly in a clean build. But then, changing the annotation to name="Bar" and saving causes the NPE below. At that point, a clean build does NOT cause another NPE; but changing the name back to Foo and rebuilding does. So this is something to do with generated file deletion and incremental build. package p; import org.eclipse.jdt.apt.pluggable.tests.annotations.GenClass6; @GenClass6(pkg="g", name="Foo") public class Quux { g.Foo _foo; } java.lang.NullPointerException at org.eclipse.jdt.internal.compiler.Compiler.processAnnotations(Compiler.java:656) at org.eclipse.jdt.internal.compiler.Compiler.compile(Compiler.java:374) at org.eclipse.jdt.internal.core.builder.AbstractImageBuilder.compile(AbstractImageBuilder.java:362) at org.eclipse.jdt.internal.core.builder.IncrementalImageBuilder.compile(IncrementalImageBuilder.java:302) at org.eclipse.jdt.internal.core.builder.AbstractImageBuilder.compile(AbstractImageBuilder.java:299) at org.eclipse.jdt.internal.core.builder.IncrementalImageBuilder.build(IncrementalImageBuilder.java:130) at org.eclipse.jdt.internal.core.builder.JavaBuilder.buildDeltas(JavaBuilder.java:280) at org.eclipse.jdt.internal.core.builder.JavaBuilder.build(JavaBuilder.java:192) at org.eclipse.core.internal.events.BuildManager$2.run(BuildManager.java:624) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37) at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:166) at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:197) at org.eclipse.core.internal.events.BuildManager$1.run(BuildManager.java:246) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37) at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:249) at org.eclipse.core.internal.events.BuildManager.basicBuildLoop(BuildManager.java:302) at org.eclipse.core.internal.events.BuildManager.build(BuildManager.java:334) at org.eclipse.core.internal.events.AutoBuildJob.doBuild(AutoBuildJob.java:137) at org.eclipse.core.internal.events.AutoBuildJob.run(AutoBuildJob.java:235) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55)
The failure is also present in HEAD, and in M20070905. The test case doesn't work at all in 3.3.0; file generation isn't occurring at all there, probably because the GenClass6 processor is making unsupported API calls, though I haven't verified that).
Problem only occurs when the generated type is referenced in the parent file; that's why it's passing the automated tests, which do not (yet) cover that scenario.
Created attachment 78452 [details] Test case Attached patch to apt.pluggable.tests demonstrates the bug. In the Compiler, currentUnit is null, because this.unitsToProcess[] contains a null entry where one might expect to see the parent file's compilation unit.
Olivier, Jess and I think this is ugly but not worth delaying 3.3.1 over, because it only appears to affect parent files that refer to the generated types and there is a workaround of doing a clean build. We this this should be fixed in 3.3.2. Do you agree?
> Olivier, Jess and I think this is ugly but not worth delaying 3.3.1 over, > because it only appears to affect parent files that refer to the generated > types and there is a workaround of doing a clean build. We this this should be > fixed in 3.3.2. Do you agree? In order to fix it for 3.3.1, we need to determine first if this is a regression compare to 3.3.0. If not, then it might wait for 3.3.2 unless the user would get it too many times in "normal" use cases.
This being said, we could add a temporary null checks till we know exactly why null units are there. I would favor this solution only if there is no better way to fix it.
If not a regression, I would consider this for 3.3.2.
This is not a regression: JSR269 file generation was not fully implemented in IDE mode in that case, which prevented the IDE from getting this far.
(In reply to comment #8) > This is not a regression: JSR269 file generation was not fully implemented in > IDE mode in that case, which prevented the IDE from getting this far. Technically I think you could argue it either way. In 3.3.0, if you generated files, they would never be deleted until a clean, but there wouldn't be any errors in the log. In 3.3.1, they get deleted, but (in the particular case where parent files reference generated files and thus get recompiled) there are errors in the log. My feeling is that JSR269 adoption is still very minimal, this is not the only problem that exists, and touching the code that handles the compile loop would be risky without a lot of benefit. People who are adopting JSR269 are probably cutting-edge enough that they can take an early 3.3.2 build or even apply a patch, if they really need the fix. The main risk is if commercial products are being built off 3.3.1, that will not be revised until perhaps a year from now, by which time JSR269 will maybe be more commonplace. But you could go crazy thinking about situations like that.
Created attachment 78460 [details] Proposed fix This patch passes the attached test case, but if I comment out the last line of the new test, I get: Failure while running test org.eclipse.jdt.apt.pluggable.tests.FilerTests#testCreateSourceFileWithGenReference()!!! Actual output is: "Problem : gen6.Generated02 cannot be resolved to a type [ resource : </testproj0002/src/targets/filer/Parent02.java> range : <842,858> category : <40> severity : <2>]\n" Maybe this is expected. Walter, could you please comment on that? This code would pass: final String[] expectedClasses2 = {"gen6.Generated02", "gen6.XxxGenerated02", "targets.filer.Parent02"}; expectingUniqueCompiledClasses(expectedClasses2); expectingNoFile(proj, ".apt_generated/gen6/Generated02.java"); Maybe the test is simply not up-to-date because of the NPE occuring before the commented out code.
(In reply to comment #10) > This code would pass: > final String[] expectedClasses2 = {"gen6.Generated02", "gen6.XxxGenerated02", > "targets.filer.Parent02"}; > expectingUniqueCompiledClasses(expectedClasses2); > expectingNoFile(proj, ".apt_generated/gen6/Generated02.java"); Hmm, if .apt_generated/gen6/Generated02.java does not exist, then I am not sure why the class gen6.Generated02 would show up in the CompiledClasses. The class should no longer be getting generated or compiled, I think, after the incremental build.
The "Generated02 cannot be resolved to a type" problem is expected, since the parent now refers to a non-generated file. Also, the fact that the file is deleted is good. So from an external perspective this patch is working correctly. However, the fact that the type is still compiled seems curious. Olivier, is it possible that it gets compiled initially (ie before it has been deleted) and then subsequently removed (when Parent02 gets compiled and we discover that the generated file is no longer generated)? That seems like it would be okay.
The reason that Generated02 is getting compiled is that it has not yet been deleted at that time. The interaction between Java 6 and Java 5 processing is basically: 1. Compile loop: all Java 6 processing, including file generation 2. CompilationParticipants: all Java 5 processing 3. At very end of CompilationParticipants, delete obsolete files 4. Repeat if necessary In step 2, ASTs are created in order to run Java 5 processors. The parent file contains a reference to Generated02, and Generated02 has not yet been deleted because step 3 has not executed yet, so it is added to the unitsToProcess[]. The Java 5 processors are present in the test case because apt.tests is loaded in the runtime environment. If the apt.tests plugin is removed from the launch configuration, then Generated02 is not recompiled. This is as expected. So, it appears to me that Olivier's patch is doing the right thing. All APT tests pass with this patch. The manual operation that provoked this discovery also now passes. Given that Olivier is comfortable that this will not cause a regression for normal compilation, and if we have time, I think we should try to promote this to 3.3.1.
Created attachment 78487 [details] Improved test case Improved test case is more precise about what compilation problem it's looking for, and works regardless of whether apt.tests is in the launch profile. This test case passes with Olivier's patch, and fails without it.
The fix looks good. +1 for 3.3.1
Released for 3.3.1. Regression tests added in org.eclipse.jdt.apt.pluggable.tests.FilerTests#testCreateSourceFileWithGenReference
Released for 3.4M2. Regression tests added in org.eclipse.jdt.apt.pluggable.tests.FilerTests#testCreateSourceFileWithGenReference
(In reply to comment #14) > Created an attachment (id=78487) [details] > Improved test case > > Improved test case is more precise about what compilation problem it's looking > for, and works regardless of whether apt.tests is in the launch profile. This > test case passes with Olivier's patch, and fails without it. > Verified for 3.3.1 using build M20070917-0900. Verified for 3.4M2 using build I20070917-0010.
*** Bug 207944 has been marked as a duplicate of this bug. ***
