Bug 172745 - [jsr269] How do we edit or remove compilation units within Compiler loop?
Summary: [jsr269] How do we edit or remove compilation units within Compiler loop?
Status: VERIFIED WORKSFORME
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.5 M2   Edit
Assignee: Kent Johnson CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-02-02 20:16 EST by Walter Harley CLA
Modified: 2008-09-15 09:55 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Walter Harley CLA 2007-02-02 20:16:06 EST
(per discussion with Olivier)

When annotation processors run in the IDE, they may re-generate a file that was already generated on a previous run, possibly causing it to change.  Also, a change to an annotation could mean that a file that was previously generated is no longer generated on the new run.

In Java 5 annotation processing, we dealt with this outside of the Compiler's compilation loop; see IncrementalImageBuilder.processAnnotationResults().  If new files are added, it triggers an additional build loop in IncrementalImageBuilder.buildAfterBatchBuild.  Deleted files are not handled correctly, though; see Bug 163733.

In Java 6 annotation processing, the processors are called from within the Compiler's own compilation loop, that is, at a lower level than Java 5 processors.  It correctly handles adding new files.  But I am not sure how the loop should handle files that are removed or modified.
Comment 1 Olivier Thomann CLA 2007-02-06 09:59:55 EST
Kent, could you please see if you can provide some insights for this issue?
Thank you.
Comment 2 Kent Johnson CLA 2007-02-06 13:58:48 EST
Can we assume that outside of the IDE, we are always provided with all source files?

If so, then we could start by deleting all generated files.

If not, and we're called with a few source files, then we would need to know who owns each generated file so that the corresponding generated files can be deleted prior to processing the original file (or the processor could do it).

Inside the batch compiler, there is no dependency propagation so how do structural changes get propagated if only a few source files are compiled?
Comment 3 Walter Harley CLA 2007-02-06 14:22:55 EST
(In reply to comment #2)
> If not, and we're called with a few source files, then we would need to know
> who owns each generated file so that the corresponding generated files can be
> deleted prior to processing the original file (or the processor could do it).

The processor manager takes care of figuring out what generated files need to be deleted, and it communicates the list at the end of each round of processing via AbstractAnnotationProcessorManager.getDeletedUnits(), which is visible to the Compiler.

My question amounts to this: within the IDE, assuming that some compilation unit has been generated and compiled (and thus exists in the LookupEnvironment), what do I need to do to make it go away again?  

It seems like within the Compiler.compile() loop, if the annotation processor manager says that files have been deleted, we need to remove them from the typesystem and re-compile.

For comparison: In Java 5 annotation processing, which operates on the Java Model rather than on the compiler internal typesystem, we discard the working copy, physically delete the file, and/or set the compilation unit's contents to be empty, depending on whether we're in build or reconcile.  We then re-start the build loop, via IncrementalImageBuilder.buildAfterBatchBuild().  

Maybe we can treat Java 6 annotation processing exactly the same way, i.e., outside of the Compiler loop?  I'll experiment with that.  Would still like to understand the answer to the question of "how do we remove a type from the typesystem", though.
Comment 4 Kent Johnson CLA 2007-02-06 14:28:11 EST
I'm confused.

So its possible to compile a file, have a processor generate a file & then somehow process the same source file again & delete the generated file?

All in one call to the compiler?
Comment 5 Walter Harley CLA 2007-02-06 15:32:47 EST
Not in one call to the compiler, no.

The scenario is:

1. File A contains annotation.  File B contains (broken) reference to type AGen.
2. Compiling A produces new type AGen.  
3. Second loop of compilation resolves reference in B.
4. User edits A, removing annotation.
5. Compiling A removes AGen.  Reference in B should now show as broken again, i.e., a problem should be created.

So, the removal takes place during a subsequent incremental build or reconcile.
Comment 6 Kent Johnson CLA 2007-03-13 14:27:33 EDT
Walter : with the fix for bug 163733, are you ok with removing generated files using a Participant ?
Comment 7 Walter Harley CLA 2007-03-13 21:12:54 EDT
I think that should work, but I'm not sure.  I haven't had a chance to actually try it yet.  Intuitively it seems there is a possibility that types will be removed too late.  That is, suppose type A generates AGen, and B refers to both A and AGen.  Suppose the user then modifies A so that it no longer generates AGen, and then saves and builds.  A is compiled; AGen is *marked* as being deleted, but is not yet actually deleted; B is compiled, but AGen still exists, so there is no error; only then is AGen actually deleted, leaving things in an inconsistent state.  But as long as AGen's deletion triggers a subsequent recompilation after that, then I think everything is fine.
Comment 8 Kent Johnson CLA 2007-03-14 10:11:13 EDT
With the change to bug 163733, B will be recompiled (assuming it references AGen) AFTER the participant tells us that AGen.java has been deleted & the builder deletes the AGen.class .

A.java likely won't be recompiled since it should not contain a reference to AGen anymore.

Let us know when you've had a chance to see if this case works out.
Comment 9 Kent Johnson CLA 2007-06-08 14:47:12 EDT
Walter, when you get a chance, let us know if there is anything else we need to do for this case.

thx
Comment 10 Walter Harley CLA 2007-06-08 14:51:07 EDT
(In reply to comment #9)
> Walter, when you get a chance, let us know if there is anything else we need to
> do for this case.

This got put on the back burner in the effort to closely match the full javac functionality at the command line.  I'll be back to the IDE side of things as soon as 3.3 ships, and will let you know then.  Thanks!
Comment 11 Kent Johnson CLA 2007-08-09 13:12:40 EDT
Walter - any news on this one?
Comment 12 Walter Harley CLA 2007-08-15 19:32:44 EDT
Sorry for the long delay in responding.  

I have just gotten file deletion working, I believe.  It appears that the current code will work okay, although I haven't yet tested thoroughly.  See bug 188559 for details on the algorithm; it uses the existing AptCompilationParticipant to handle the deletion, so at the least, it should work as well as it ever did.
Comment 13 Jerome Lanneluc CLA 2008-08-22 10:58:04 EDT
Changing resolution to WORKSFORME since to action was taken to fix this bug.
Comment 14 Jerome Lanneluc CLA 2008-09-15 09:55:15 EDT
Verified for 3.5M2