Bug 242797 - problem with generic itds
Summary: problem with generic itds
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: DEVELOPMENT   Edit
Hardware: PC Mac OS X - Carbon (unsup.)
: P3 normal (vote)
Target Milestone: 1.6.2   Edit
Assignee: Andrew Clement CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 166514 (view as bug list)
Depends on:
Blocks: 257437
  Show dependency tree
 
Reported: 2008-07-31 18:20 EDT by Andrew Clement CLA
Modified: 2009-01-09 18:53 EST (History)
2 users (show)

See Also:


Attachments
Sources that fail to compile (2.41 KB, application/x-zip-compressed)
2008-08-01 12:55 EDT, Andrew Clement CLA
no flags Details
java files changed to try and improve MethodVerifier (32.92 KB, application/octet-stream)
2008-08-01 16:52 EDT, Andrew Clement CLA
no flags Details
Patch for ajde.jar (13.47 KB, application/octet-stream)
2008-08-12 20:32 EDT, Andrew Clement CLA
no flags Details
replace one file in ajde.jar (13.58 KB, application/octet-stream)
2008-08-15 17:11 EDT, Andrew Clement CLA
no flags Details
Reproduces generic method override bug (4.94 KB, application/zip)
2008-08-19 11:51 EDT, Dave Whittaker CLA
no flags Details
Patch to org.aspectj.weaver plugin (4.08 KB, application/octet-stream)
2008-08-19 14:01 EDT, Andrew Clement CLA
no flags Details
Reproduces generic method override bug, take 2 (6.86 KB, application/zip)
2008-08-19 15:04 EDT, Dave Whittaker CLA
no flags Details
Another org.aspectj.weaver patch (3.46 KB, application/octet-stream)
2008-08-19 16:28 EDT, Andrew Clement CLA
no flags Details
Replicates inconsistent classfile error (6.61 KB, application/zip)
2008-09-11 21:33 EDT, Dave Whittaker CLA
no flags Details
Replicates aspectpath bug (28.54 KB, application/zip)
2008-09-18 14:38 EDT, Dave Whittaker CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Clement CLA 2008-07-31 18:20:17 EDT
Reported on the list by Dave Whittaker:

ENTRY org.eclipse.ajdt.ui 4 0 2008-07-31 14:12:24.251
!MESSAGE Compile error: NullPointerException thrown: null
!STACK 0
java.lang.NullPointerException
        at org.aspectj.ajdt.internal.compiler.lookup.InterTypeMethodBinding.<init>(InterTypeMethodBinding.java:73)
        at org.aspectj.ajdt.internal.compiler.lookup.EclipseTypeMunger.mungeNewMethod(EclipseTypeMunger.java:111)
        at org.aspectj.ajdt.internal.compiler.lookup.EclipseTypeMunger.munge(EclipseTypeMunger.java:100)
        at org.aspectj.ajdt.internal.compiler.lookup.AjLookupEnvironment.weaveInterTypeDeclarations(AjLookupEnvironment.java:647)
        at org.aspectj.ajdt.internal.compiler.lookup.AjLookupEnvironment.weaveInterTypeDeclarations(AjLookupEnvironment.java:519)
        at org.aspectj.ajdt.internal.compiler.lookup.AjLookupEnvironment.doPendingWeaves(AjLookupEnvironment.java:371)
        at org.aspectj.ajdt.internal.compiler.lookup.AjLookupEnvironment.completeTypeBindings(AjLookupEnvironment.java:187)
        at org.aspectj.org.eclipse.jdt.internal.compiler.Compiler.internalBeginToCompile(Compiler.java:605)
        at org.aspectj.org.eclipse.jdt.internal.compiler.Compiler.beginToCompile(Compiler.java:357)
Comment 1 Andrew Clement CLA 2008-08-01 00:10:35 EDT
More info:
Maybe a little more info will help to track this down.  I have the following interfaces:

public interface Finder {
	...
}

public interface PartitionedFinder<P extends Partitioned<?>> extends Finder{
	...
}

public interface LocalizedFinder<L extends Localized> extends Finder{
	...
}

Each one of these interfaces has a corresponding aspect that defines default intertype implementations of it's methods.  Then I have this abstract class:

public abstract class OnetFinder<S, P extends Partitioned<?>, L extends Localized, O extends OnetElement<?>> 
		implements Finder, PartitionedFinder<O>, LocalizedFinder<O> {
	...
}

Up to this point it works great.  No compile errors and I can see from within the cross references view that the intertype methods are identified in the aspects, and all applied to OnetFinder.  Then I try to subclass OnetFinder:

public class CMEFinder extends OnetFinder<CMEStub, PartitionedCME, LocalizedCME, ContentModelElement>{
	...
}

And here is where the problems come in.  I get an error that one of the methods from PartitionedFinder and one of the methods from LocalizedFinder, that are declared as intertype methods, need to be implemented, and all the advice markers and cross references disappear for every class.  The methods that aren't getting inherited properly are:

public <T extends Localized> List<T> bestLanguageMatch(List<T> list, List<String> languageOrder);

And

public <T extends Partitioned<?>> List<T> bestPartitionMatch(List<T> list, List<String> partitionOrder);

The strange thing there is that the cross references view showed both of those methods as being correctly applied to OnetFinder before I subclassed it.  They are also the only generic methods involved, so it seems that has something to do with that.  Hopefully that helps.
Comment 2 Andrew Clement CLA 2008-08-01 00:11:10 EDT
And more:
public aspect PartitionedFinderAspect {
	
	public Class<? extends P> PartitionedFinder<P>.getPartitionedType(){
		return ClassUtils.guessGenericType(getClass());
	}
	
	public List<String> PartitionedFinder<P>.getPartitionOrder(){
		return H2Deployment.instance().getPartitionOrder(getPartitionedType());
	}
	
	public <T extends Partitioned<?>> List<T> PartitionedFinder<P>.bestPartitionMatch(List<T> list, List<String> partitionOrder){
		return new OrderComparator<T, String>(partitionOrder){
			
			@Override
			public String getOrdering(T partitioned){
				return partitioned.getPartitionId();
			}
			
		}.bestMatch(list);
- Hide quoted text -
	}

}
Comment 3 Andrew Clement CLA 2008-08-01 12:55:28 EDT
Created attachment 108960 [details]
Sources that fail to compile

Some exciting generics.  I've created some kind of thing based on the information here.  It does fail but I'm not positive it is the same error that Dave is reporting.  There is an awful lot going on in the program and just in developing it I hit 3 or 4 internal compiler errors (nice) - but it is advanced use of ITDs and generics.

One peculiarity is this construct:

public <T extends Partitioned<?>> List<T> PartitionedFinder<P>.bestPartitionMatch(List<T> list, List<String> partitionOrder){

and the <P> is not required since it is not used elsewhere in the definition.  But if it is removed then a different error happens! 

java.lang.IllegalStateException
at org.aspectj.weaver.TypeVariable.canBeBoundTo(TypeVariable.java:187)
at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:350)
at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:321)
at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:277)
at org.aspectj.weaver.ResolvedType.checkLegalOverride(ResolvedType.java:1653)
at org.aspectj.weaver.Resol ... clipse.core.internal.jobs.Worker.run(Worker.java:55)
Comment 4 Dave Whittaker CLA 2008-08-01 13:12:52 EDT
Removing the ITDs from the places where they are not necessary hasn't changed what I'm seeing in the Eclipse log.  Still seems to be failing with an NPE in the constructor of InterTypeMethodBinding line 73.
Comment 5 Andrew Clement CLA 2008-08-01 13:18:17 EDT
that NPE is about the only failure I didn't see in getting the code I attached into the current state it is in.  Are you able to tinker with what I attached to try and change it so that the NPE happens?  I had to make up some of the definitions for some of the types, and maybe because they are different to what you have, it isn't giving me the NPE.


Comment 6 Andrew Clement CLA 2008-08-01 16:50:57 EDT
I haven't fixed the NPE or the IllegalStateException - but I have progressed on the incorrect message about needing to implement a method.  I've uploaded a new ajde.jar - this sits in the org.aspectj.ajde plugin.  Download this one:

http://www.eclipse.org/downloads/download.php?file=/tools/aspectj/dev/ajde_242797_1.jar

and replace the ajde.jar in your plugins/org.aspectj.ajde_<LATESTDATE> with it.

I will also attach a zip of my changes.  They pass all the tests for AspectJ but I'd like some confirmation that they help before I commit them into HEAD.

The problems are all in the MethodVerifier JDT code because it is completely unaware of ITDs.

---

Also - I saw your message on the mailing list.  If you let me have the ftp details (send them to my gmail ID) then I will grab your code and debug the NPE.
Comment 7 Andrew Clement CLA 2008-08-01 16:51:26 EDT
forgot to say - I only just uploaded that ajde**.jar so it will take a while to replicate around the download servers before it is available for download.
Comment 8 Andrew Clement CLA 2008-08-01 16:52:38 EDT
Created attachment 108984 [details]
java files changed to try and improve MethodVerifier
Comment 9 Andrew Clement CLA 2008-08-02 13:05:46 EDT
extracted the test project.  white space change in LocalizedFinder, then on save the NPE:

java.lang.NullPointerException
	at org.aspectj.ajdt.internal.compiler.lookup.InterTypeMethodBinding.<init>(InterTypeMethodBinding.java:73)
	at org.aspectj.ajdt.internal.compiler.lookup.EclipseTypeMunger.mungeNewMethod(EclipseTypeMunger.java:111)
	at org.aspectj.ajdt.internal.compiler.lookup.EclipseTypeMunger.munge(EclipseTypeMunger.java:100)
	at org.aspectj.ajdt.internal.compiler.lookup.AjLookupEnvironment.weaveInterTypeDeclarations(AjLookupEnvironment.java:647)
	at org.aspectj.ajdt.internal.compiler.lookup.AjLookupEnvironment.weaveInterTypeDeclarations(AjLookupEnvironment.java:519)
	at org.aspectj.ajdt.internal.compiler.lookup.AjLookupEnvironment.doPendingWeaves(AjLookupEnvironment.java:371)
	at org.aspectj.ajdt.internal.compiler.lookup.AjLookupEnvironment.completeTypeBindings(AjLookupEnvironment.java:187)
	at org.aspectj.org.eclipse.jdt.internal.compiler.Compiler.internalBeginToCompile(Compiler.java:605)

so it appears to be an incremental compilation problem

The NPE is because on the secondary compile we no longer have the source form around for the ITD declaration of onlyOne(), we just have the bytecode.  And so in trying to access the type variables on the ITD decl (to see if our use of a type variable is from the generic method decl or from the containing type) we blow up.
Comment 10 Dave Whittaker CLA 2008-08-02 13:36:46 EDT
You've probably already seen this, but when I swap out the ajde.jar files, it no longer tells me the generic methods are not defined, but instead I get new errors:

can't override java.util.List<T> h2.core.finder.LocalizedFinder<O>.bestLanguageMatch(java.util.List<T>, java.util.List<java.lang.String>) with java.util.List<T> h2.core.finder.LocalizedFinder.bestLanguageMatch(java.util.List<T>, java.util.List<java.lang.String>) return types don't match

can't override java.util.List<T> h2.core.finder.PartitionedFinder<O>.bestPartitionMatch(java.util.List<T>, java.util.List<java.lang.String>) with java.util.List<T> h2.core.finder.PartitionedFinder.bestPartitionMatch(java.util.List<T>, java.util.List<java.lang.String>) return types don't match

Inconsistent classfile encountered: The undefined type parameter L is referenced from within LocalizedFinderAspect
Comment 11 Andrew Clement CLA 2008-08-02 13:41:28 EDT
Yep - i've seen those now.

i've addressed the NPE.  Now the problem with the incorrect error messages.  It appears to be due to the loss of the bound information for the ITD declared type variable. 

I moved the code out of AJDT so I can debug it with just the compiler rather than in the IDE.  Now on first compile I get the NPE I saw yesterday:


java.lang.IllegalStateException: Can't answer binding questions prior to resolving
	at org.aspectj.weaver.TypeVariable.canBeBoundTo(TypeVariable.java:188)
	at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:361)
	at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:332)
	at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:288)
	at org.aspectj.weaver.ResolvedType.checkLegalOverride(ResolvedType.java:1653)

fun fun fun :)
Comment 12 Andrew Clement CLA 2008-08-02 14:17:44 EDT
fixed that npe.

now, after serializing and deserializing the bytecode representation of the intertype declaration we are working with a bunch of unresolved types (where we just know the signature) - and then just before we compare the return types in the method ResolvedType.checkLegalOverride() we resolve the types.

Because the return types are resolved standalone, outside of the scope of the method in which they are declared:

signature.getReturnType().resolve(world)

we fail to see 'T' in the return type as a reference to the type variable for the generic method.  If we change it to this:

signature.resolve(world).getReturnType()

then we resolve all components of the signature (return type and params) recognizing that any tvar references are to tvars declared on the method.  This ensures List<T> resolves to reference the correct 'T extends Localized' and then checkLegalOverride() succeeds and the errors are gone.  i'm just checking what impact this has on all the tests.
Comment 13 Andrew Clement CLA 2008-08-02 14:29:35 EDT
ok, the tests are passing (shock horror!).

So I'm making these two jars available that should be applied to org.aspectj.ajde and org.aspectj.weaver respectively, to replace the ajde.jar and aspectjweaver.jar files within those plugins:

http://www.eclipse.org/downloads/download.php?file=/tools/aspectj/dev/ajde_242797_2.jar

http://www.eclipse.org/downloads/download.php?file=/tools/aspectj/dev/aspectjweaver_242797_2.jar

(they will take a while to appear on the download servers)

this includes all the changes so far.  Some pretty radical stuff in there so I hope it works for you.  I've patched my AJDT and it now compiles your test project cleanly.
Comment 14 Dave Whittaker CLA 2008-08-02 15:57:12 EDT
That's great.  I don't think I'll be able to do too much testing until Monday, but I'll bang away with the new jars next week and give you feedback on how they works out.
Comment 15 Dave Whittaker CLA 2008-08-04 16:25:42 EDT
So far things are working out well.  When a class that references CMEFinder triggers an incremental compile, I get this error:

Inconsistent classfile encountered: The undefined type parameter L is referenced from within LocalizedFinderAspect

However, cleaning the project and allowing it to rebuild fixes the problem, which is a far better situation than we had before.  If you want to try to replicate, add this method to h2.onet.cme.CMEFinder:
	
public static CMEFinder instance(){
  return H2Lookup.instance().lookup(CMEFinder.class);
}

Then replace h2.onet.cme.CMEStub with the code below, and force an incremental compile while editing it.

package h2.onet.cme;

import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.Id;
import javax.persistence.Inheritance;
import javax.persistence.InheritanceType;
import javax.persistence.Table;

@Entity
@Table(name = "onet_content_model_element_stub")
@Inheritance(strategy = InheritanceType.JOINED)
public class CMEStub implements ContentModelElement{

	@Id
	@Column(name = "element_id")
	private String elementId;
	
	public boolean equalAcrossPartitions(ContentModelElement cme){
		return elementId.equals(cme.getElementId());
	}
	
	public PartitionedCME getPartitioned(){
		return CMEFinder.instance().getPartitioned(elementId);
	}
	
	public String getDescription() {
	    return getPartitioned().getDescription();
    }

	public String getName() {
	    return getPartitioned().getName();
    }
	
	public String getPartitionId(){
		return getPartitioned().getPartitionId();
	}

	@Override
    public Double getOnetVersion() {
	    return getPartitioned().getOnetVersion();
    }

	@Override
    public String getLanguage() {
	    return getPartitioned().getLanguage();
    }
	
	public String getElementId() {
		return this.elementId;
	}

	public void setElementId(String elementId) {
		this.elementId = elementId;
	}

}
Comment 16 Andrew Clement CLA 2008-08-07 12:59:54 EDT
I've recreated this final problem but it is extremely complicated to address.

The stack is this when the problem is raised:
DefaultProblem.<init>(char[], String, int, String[], int, int, int, int, int) line: 47	
DefaultProblemFactory.createProblem(char[], int, String[], String[], int, int, int, int, int) line: 74	
AjProblemReporter(ProblemHandler).createProblem(char[], int, String[], String[], int, int, int, int, int) line: 69	
AjProblemReporter(ProblemHandler).handle(int, String[], String[], int, int, int, ReferenceContext, CompilationResult) line: 111	
AjProblemReporter.handle(int, String[], String[], int, int, int, ReferenceContext, CompilationResult) line: 318	
AjProblemReporter(ProblemReporter).handle(int, String[], String[], int, int, int) line: 1776	
AjProblemReporter(ProblemReporter).undefinedTypeVariableSignature(char[], ReferenceBinding) line: 6082	
AjLookupEnvironment(LookupEnvironment).getTypeFromTypeSignature(SignatureWrapper, TypeVariableBinding[], ReferenceBinding) line: 1114	
AjLookupEnvironment(LookupEnvironment).getTypeFromVariantTypeSignature(SignatureWrapper, TypeVariableBinding[], ReferenceBinding, ReferenceBinding, int) line: 1173	
AjLookupEnvironment(LookupEnvironment).getTypeArgumentsFromSignature(SignatureWrapper, TypeVariableBinding[], ReferenceBinding, ReferenceBinding) line: 985	
AjLookupEnvironment(LookupEnvironment).getTypeFromTypeSignature(SignatureWrapper, TypeVariableBinding[], ReferenceBinding) line: 1124	
BinaryTypeBinding.createMethod(IBinaryMethod, long) line: 464	
BinaryTypeBinding.createMethods(IBinaryMethod[], long) line: 568	
BinaryTypeBinding.cachePartsFrom(IBinaryType, boolean) line: 327	
AjLookupEnvironment(LookupEnvironment).createBinaryTypeFrom(IBinaryType, PackageBinding, boolean, AccessRestriction) line: 620	
AjLookupEnvironment.createBinaryTypeFrom(IBinaryType, PackageBinding, boolean, AccessRestriction) line: 1095	

and it is due to creating the binary type binding during incremental compilation for the type that contains the generic ITD that targets LocalizedFinder.  The eclipse compiler cannot cope with it naming a type variable that exists in some other type.  The compiler has only been upgraded to cope with this properly when compiling from source - which is why everything is fine for full source compilation.
Comment 17 Andrew Clement CLA 2008-08-07 14:20:56 EDT
all fixes so far now committed to AspectJ.  I'll leave this bug open to cover the incremental compilation problem
Comment 18 Andrew Clement CLA 2008-08-07 14:52:11 EDT
fixes also committed to AJDT1.6 - will be in a dev build shortly.
Comment 19 Dave Whittaker CLA 2008-08-11 12:26:45 EDT
After about 2 weeks of use the fix seams to be pretty effective, I haven't been able to break the compiler with any generic signatures on my ITDs.  As my project is growing, the need to to full rebuilds any time a class with generic ITDs is referenced is getting to be a bit of a pain though.  Is there anything I can do to help with a fix, or any idea on a time frame for that?
Comment 20 Andrew Clement CLA 2008-08-11 18:36:38 EDT
I'll have another look at it, but it is very complex to fix in a proper way as I need to extend the JDT compiler in yet another direction.  I've been trying to think about a workaround for you - by slightly adjusting the declarations - but nothing is quite right.
Comment 21 Andrew Clement CLA 2008-08-12 20:32:54 EDT
Created attachment 109853 [details]
Patch for ajde.jar

This is a patch for a class file in ajde.jar (part of the org.aspectj.ajde plugin).

It is a possible start on a fix for the remaining issue.  It passes my test program but I need to optimize it and test out some scenarios in which it may still not work correctly.  But I thought I'd share it with you since I *think* it will address your situation.

To apply, put this in the org.aspectj.ajde* plugin folder, then

jar -xvf patch.zip
jar -uvf ajde.jar org

and BinaryTypeBinding will be replaced in the jar file.  Restart eclipse.
Comment 22 Andrew Clement CLA 2008-08-14 11:28:46 EDT
Dave - did you get a chance to try this patch yet?  I'm keen to get it into base AspectJ if it helps your situation.
Comment 23 Dave Whittaker CLA 2008-08-14 11:35:18 EDT
I just grabbed it.  I'll get back to you today.
Comment 24 Dave Whittaker CLA 2008-08-14 14:08:17 EDT
Andy,

The good news is that the patch seems to have fixed this problem.  That bad news is an older problem is back.

I no longer get an error when a class with generic ITDs applied to it is referenced, but within the class the generic ITDs are applied to I am once again seeing an error stating that:

public <T extends Localized> List<T> bestLanguageMatch(List<T> list,
List<String>);

And

public <T extends Partitioned<?>> List<T> bestPartitionMatch(List<T> list,
List<String>);

must be implemented, even though they are mixed in from the aspect.  A clean and full compile is not clearing the error this time.

I'm also seeing the stack below in the log, although I'm not entirely sure if it's directly related (it's from 20 minutes ago and doesn't seem to be recurring on subsequent compiles)

!ENTRY org.eclipse.jdt.ui 4 0 2008-08-14 13:39:28.862
!MESSAGE Error in JDT Core during reconcile
!STACK 0
java.lang.ArrayIndexOutOfBoundsException: -1
        at org.aspectj.org.eclipse.jdt.internal.compiler.parser.RecoveryScanner.getNextToken(RecoveryScanner.java:143)
        at org.aspectj.org.eclipse.jdt.internal.compiler.parser.TheOriginalJDTParserClass.parse(TheOriginalJDTParserClass.java:7359)
        at org.aspectj.org.eclipse.jdt.internal.compiler.parser.TheOriginalJDTParserClass.parseStatements(TheOriginalJDTParserClass.java:7897)
        at org.aspectj.org.eclipse.jdt.internal.compiler.parser.TheOriginalJDTParserClass$1$MethodVisitor.endVisitMethod(TheOriginalJDTParserClass.java:81
61)
        at org.aspectj.org.eclipse.jdt.internal.compiler.parser.TheOriginalJDTParserClass$1$MethodVisitor.endVisit(TheOriginalJDTParserClass.java:8147)
        at org.aspectj.org.eclipse.jdt.internal.compiler.ast.MethodDeclaration.traverse(MethodDeclaration.java:247)
        at org.aspectj.org.eclipse.jdt.internal.compiler.parser.TheOriginalJDTParserClass.recoverStatements(TheOriginalJDTParserClass.java:8253)
        at org.aspectj.org.eclipse.jdt.internal.compiler.parser.TheOriginalJDTParserClass.parse(TheOriginalJDTParserClass.java:7461)
        at org.aspectj.org.eclipse.jdt.internal.compiler.parser.TheOriginalJDTParserClass.parse(TheOriginalJDTParserClass.java:7738)
        at org.aspectj.org.eclipse.jdt.internal.compiler.ast.MethodDeclaration.parseStatements(MethodDeclaration.java:124)
        at org.aspectj.org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.parseMethod(TypeDeclaration.java:825)
        at org.aspectj.org.eclipse.jdt.internal.compiler.parser.TheOriginalJDTParserClass.getMethodBodies(TheOriginalJDTParserClass.java:6654)
        at org.eclipse.ajdt.core.parserbridge.AJSourceElementParser.parseCompilationUnit(AJSourceElementParser.java:1643)
        at org.eclipse.ajdt.internal.ui.editor.CompilationUnitAnnotationModelWrapper.beginReporting(CompilationUnitAnnotationModelWrapper.java:216)
        at org.eclipse.jdt.internal.core.ReconcileWorkingCopyOperation.reportProblems(ReconcileWorkingCopyOperation.java:129)
        at org.eclipse.jdt.internal.core.ReconcileWorkingCopyOperation.executeOperation(ReconcileWorkingCopyOperation.java:104)
        at org.eclipse.jdt.internal.core.JavaModelOperation.run(JavaModelOperation.java:709)
        at org.eclipse.jdt.internal.core.JavaModelOperation.runOperation(JavaModelOperation.java:770)
        at org.eclipse.jdt.internal.core.CompilationUnit.reconcile(CompilationUnit.java:1224)
        at org.eclipse.jdt.internal.ui.text.java.JavaReconcilingStrategy.reconcile(JavaReconcilingStrategy.java:124)
        at org.eclipse.jdt.internal.ui.text.java.JavaReconcilingStrategy.access$0(JavaReconcilingStrategy.java:108)
        at org.eclipse.jdt.internal.ui.text.java.JavaReconcilingStrategy$1.run(JavaReconcilingStrategy.java:89)
        at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37)
        at org.eclipse.jdt.internal.ui.text.java.JavaReconcilingStrategy.reconcile(JavaReconcilingStrategy.java:87)
        at org.eclipse.jdt.internal.ui.text.java.JavaReconcilingStrategy.reconcile(JavaReconcilingStrategy.java:149)
        at org.eclipse.jdt.internal.ui.text.CompositeReconcilingStrategy.reconcile(CompositeReconcilingStrategy.java:86)
        at org.eclipse.jdt.internal.ui.text.JavaCompositeReconcilingStrategy.reconcile(JavaCompositeReconcilingStrategy.java:102)
        at org.eclipse.jface.text.reconciler.MonoReconciler.process(MonoReconciler.java:77)
        at org.eclipse.jface.text.reconciler.AbstractReconciler$BackgroundThread.run(AbstractReconciler.java:206)
Comment 25 Dave Whittaker CLA 2008-08-14 14:16:17 EDT
I'm also a little confused here to see that the error only shows up if I force one of the classes that LocalizedFinderAspect and PartitionedFinderAspect are applied to to recompile by changing something and then saving.  If I just clean the project and let the whole thing recompile I don't get any new error markers, put it doesn't clear any of the existing error markers either.  I would have suspected that a problem with incremental compiles like we saw before would be cleared on a rebuild, while a problem with the compiler at large would show up on all affected classes once they were rebuilt......
Comment 26 Dave Whittaker CLA 2008-08-14 14:54:13 EDT
I take that last comment back.  After a restart of Eclipse the error markers are still appearing when an incremental compile occurs, but they are now clearing when I do a clean and full rebuild.
Comment 27 Andrew Clement CLA 2008-08-14 16:53:15 EDT
urgh...  For the latest patch I only developed a small testcase scenario so I could recreate and fix it.  I haven't tried incrementally building the original scenario too - i'll create a more sophisticated incremental testcase now and see what happens.
Comment 28 Andrew Clement CLA 2008-08-15 17:11:26 EDT
Created attachment 110133 [details]
replace one file in ajde.jar

blimey this bug is incredibly complex... incremental compilation of generic itds sharing type variables with advised types...

I went back to your original large testcase and managed to recreate this latest problem.  It is happening because my original fixes applied to the compiler type SourceTypeBinding - where I made it avoid considering a type as inheriting 'unresolved' methods from an interface if an aspect was around to supply an implementation of that method.  During incremental compilation we are in the BinaryTypeBinding world and I had to duplicate the fix.  This seems to work (amazing!) - certainly I cannot recreate your problem now.  No doubt you'll come up with another scenario though ;)

Here is a new patch for ajde.jar that updates the same class as before - it contains the latest fix and the 'inconsistent class file' fix.
Comment 29 Dave Whittaker CLA 2008-08-18 18:37:38 EDT
I couldn't let you down with finding a way to break this.  I applied the patch and everything worked great for about 6 hours of coding, until I somehow triggered the stack below.  It seems to have had something to do with me overriding generic methods.  I managed to fix it eventually by just cutting out the offending method, saving, and then pasting it right back where it had been.  Any ideas?

!ENTRY org.eclipse.ajdt.ui 4 0 2008-08-18 18:21:17.493
!MESSAGE Compile error: NullPointerException thrown: null
!STACK 0
java.lang.NullPointerException
        at org.aspectj.weaver.TypeVariable.canBeBoundTo(TypeVariable.java:206)
        at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:361)
        at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:332)
        at org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:288)
        at org.aspectj.weaver.ResolvedType.checkLegalOverride(ResolvedType.java:1653)
        at org.aspectj.weaver.ResolvedType.compareToExistingMembers(ResolvedType.java:1559)
        at org.aspectj.weaver.ResolvedType.compareToExistingMembers(ResolvedType.java:1522)
        at org.aspectj.weaver.ResolvedType.addInterTypeMunger(ResolvedType.java:1471)
        at org.aspectj.weaver.bcel.BcelWeaver.weaveNormalTypeMungers(BcelWeaver.java:1611)
        at org.aspectj.weaver.bcel.BcelWeaver.addNormalTypeMungers(BcelWeaver.java:1426)
        at org.aspectj.weaver.bcel.BcelWeaver.weave(BcelWeaver.java:1162)
        at org.aspectj.ajdt.internal.compiler.AjPipeliningCompilerAdapter.weaveQueuedEntries(AjPipeliningCompilerAdapter.java:454)
        at org.aspectj.ajdt.internal.compiler.AjPipeliningCompilerAdapter.queueForWeaving(AjPipeliningCompilerAdapter.java:391)
        at org.aspectj.ajdt.internal.compiler.AjPipeliningCompilerAdapter.afterProcessing(AjPipeliningCompilerAdapter.java:379)
        at org.aspectj.ajdt.internal.compiler.CompilerAdapter.ajc$after$org_aspectj_ajdt_internal_compiler_CompilerAdapter$5$6b855184(CompilerAdapter.aj:98)
        at org.aspectj.org.eclipse.jdt.internal.compiler.Compiler.process(Compiler.java:641)        at org.aspectj.org.eclipse.jdt.internal.compiler.Compiler.compile(Compiler.java:392)        at org.aspectj.ajdt.internal.core.builder.AjBuildManager.performCompilation(AjBuildManager.java:992)
        at org.aspectj.ajdt.internal.core.builder.AjBuildManager.doBuild(AjBuildManager.java:266)        at org.aspectj.ajdt.internal.core.builder.AjBuildManager.batchBuild(AjBuildManager.java:181)        at org.aspectj.ajde.core.internal.AjdeCoreBuildManager.doBuild(AjdeCoreBuildManager.java:95)        at org.aspectj.ajde.core.AjCompiler.build(AjCompiler.java:118)        at org.eclipse.ajdt.core.builder.AJBuilder.build(AJBuilder.java:198)        at org.eclipse.core.internal.events.BuildManager$2.run(BuildManager.java:633)        at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37)        at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:170)        at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:201)
        at org.eclipse.core.internal.events.BuildManager$1.run(BuildManager.java:253)
        at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37)
        at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:256)
        at org.eclipse.core.internal.events.BuildManager.basicBuildLoop(BuildManager.java:309)
        at org.eclipse.core.internal.events.BuildManager.build(BuildManager.java:341)
        at org.eclipse.core.internal.events.AutoBuildJob.doBuild(AutoBuildJob.java:140)
        at org.eclipse.core.internal.events.AutoBuildJob.run(AutoBuildJob.java:238)
        at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55)


Comment 30 Dave Whittaker CLA 2008-08-18 18:46:07 EDT
Delving a little deeper into it, I have an interface Manager with the method

public <T> List<T> getAll()

Then I create a default implementation of that method with an ITD and I apply the interface to class OnetElementManager which has it's own implementation of the method.  This seems like it should be ok to me, but I'm getting an error in the ITD aspect claiming

can't override java.util.List<T> h2.core.manager.Manager.getAll() with java.util.List h2.onet.OnetElementManager.getAll() return types don't match

It seems like ajde isn't seeing that the override is also a generic method?


Comment 31 Andrew Clement CLA 2008-08-18 23:02:59 EDT
thanks for testing these things out for me.  I'll take a look at the next problem soon.

As a bit of a long shot, do you happen to use the same type variable name in the target of the ITD as in the generic method?

If the method is 

<T> List<T> foo()

and the ITD is

<T> List<T> OnetThingy.foo() {}

Does OnetThingy happen to use T? (I know the ITD itself may not be using it, but bare with me ;) )

interface OnetThingY<T> {
...
}

if so - can you try changing the type variables so they don't clash - it is a quirk of the fix I sent you that it will have problems with clashing names.  You might not be hitting this, just thought I'd suggest it as a possible problem.


Comment 32 Dave Whittaker CLA 2008-08-19 11:51:08 EDT
Created attachment 110356 [details]
Reproduces generic method override bug

I'm happy to test the stuff, thanks for offering up the fixes so quickly.  It doesn't appear to be a name clash.  I don't have any generic declarations of T on any of the classes within the hierarchy, but I tried changing names up just in case with no luck.  As I make changes, it seems to swap back and forth between an error marker on the aspect that defines the ITD (ManagerAspect) 

can't override java.util.List<T> h2.core.manager.Manager.getAll() with
java.util.List h2.onet.OnetElementManager.getAll() return types don't match

and the stack trace that I originally sent.  Since the error marker is on the aspect it seems to me like aspectj doesn't see that the overriden method in OnetElementManager is also defined as

public <T> List<T> getAll();

It's like the generic portion of the method signature is lost somewhere in the process.  I'm attaching a small project that you can (hopefully) use to reproduce.
Comment 33 Andrew Clement CLA 2008-08-19 13:55:41 EDT
Ok, thanks for the new test case :)

First problem:

java.lang.NullPointerException
        at org.aspectj.weaver.TypeVariable.canBeBoundTo(TypeVariable.java:206)
        at
org.aspectj.weaver.ReferenceType.isAssignableFrom(ReferenceType.java:361)

is an ordering (pipelining) problem.  I can recreate it building your example code on the command line if I compile the files in a certain order.  Easy to fix (hurrah!)

onto second problem now
Comment 34 Andrew Clement CLA 2008-08-19 14:01:38 EDT
Created attachment 110377 [details]
Patch to org.aspectj.weaver plugin

I'm having a bit of trouble recreating:

can't override java.util.List<T> h2.core.manager.Manager.getAll() with
java.util.List h2.onet.OnetElementManager.getAll() return types don't match

It is vaguely possible I've fixed it by addressing the stack trace but why would I be so lucky...

I've attached a patch for the problem in the stack trace (although I will be committing the fix into HEAD shortly).  This needs applying to the aspectjweaver.jar in the org.aspectj.weaver plugin.  I'd be interested to know if you still get the 'cant override' message with it applied - or if you can tell me what incremental change you make in the same project to trigger that message?

Note to self: With this fix committed the only change not in HEAD will be the one to cope with resolving type variables from an ITD member in a binary type binding.
Comment 35 Dave Whittaker CLA 2008-08-19 15:04:17 EDT
Created attachment 110382 [details]
Reproduces generic method override bug, take 2

The new attachment is broken even with the patch applied.  Strangely, if you change Type1 from an interface to a class, the error goes away.  So how many defects have we addressed with this one bug at this point? :)
Comment 36 Andrew Clement CLA 2008-08-19 16:28:02 EDT
Created attachment 110392 [details]
Another org.aspectj.weaver patch

Getting better - errors from the weaver now rather than the compiler.

Here we are comparing return types for two methods to check compatibility.

The parent is

java.util.List<T> GenericMethodInterface.getStuff()

The child is

java.util.List<T> GenericMethodImpl.getStuff()

In the second case T is T extends D where D is D extends Type1.

The bounds comparison logic in TypeVariable.matchingBounds() is rather naive and hence we fail to consider them compatible.

Here is a comment I put in the code a few months ago next to the call for matchingBounds():

// TODO 2 is this next bit bogus, what is it for?

So I got rid of the call and the method - all current tests pass and the new failure here passes (I don't get the override message).

---
we are certainly stressing out the generics code here.  Certainly the issue of incremental of generic ITDs was something I knew was going to be an issue (we have some open bugs on it) but some of these other things have surprised me.

I've now committed ALL CHANGES so far discussed in this bug into AspectJ HEAD, including the one to fix this.

Because the AJ build will take a while (and the AJDT builds are still a bit shaky), I've attached a new version of the TypeVariable weaver patch that I previously attached earlier today.  This new version passes your new failing testcase - apply it over the top of the old change to aspectjweaver.jar

phew.  next...
Comment 37 Dave Whittaker CLA 2008-08-22 14:05:38 EDT
Got through a whole day with no errors, but now I've got a new one for you.  The error crops up when an interface defines a return type where one of the bounds of it's declaration is also generic. i.e.

public Class<? extends Partitioned<?>> getPartitionedType();

When I try this ITD
	
public Class<? extends Partitioned<?>> PartitionedManager.getPartitionedType(){
  return (Class<? extends Partitioned<?>>) getManagedType();
}

I get the exception below.  Removing the generic declaration from Partitioned clears the error however.  So

public Class<? extends Partitioned> PartitionedManager.getPartitionedType(){
  return (Class<? extends Partitioned<?>>) getManagedType();
}

does work.  And of course, to complicate matters I can't reproduce this outside of my project.  Hopefully the stack will give you some idea of what's going on.

!ENTRY org.eclipse.ajdt.ui 4 0 2008-08-22 13:57:06.233
!MESSAGE Compile error: NullPointerException thrown: null
!STACK 0
java.lang.NullPointerException
        at org.aspectj.org.eclipse.jdt.internal.compiler.lookup.CaptureBinding.initializeBounds(CaptureBinding.java:85)
        at org.aspectj.org.eclipse.jdt.internal.compiler.lookup.ParameterizedTypeBinding.capture(ParameterizedTypeBinding.java:113)
        at org.aspectj.org.eclipse.jdt.internal.compiler.ast.MessageSend.resolveType(MessageSend.java:550)
        at org.aspectj.org.eclipse.jdt.internal.compiler.ast.MessageSend.resolveType(MessageSend.java:422)
        at org.aspectj.org.eclipse.jdt.internal.compiler.ast.BinaryExpression.resolveType(BinaryExpression.java:1789)
        at org.aspectj.org.eclipse.jdt.internal.compiler.ast.LocalDeclaration.resolve(LocalDeclaration.java:196)
        at org.aspectj.org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration.resolveStatements(AbstractMethodDeclaration.java:433)
        at org.aspectj.org.eclipse.jdt.internal.compiler.ast.MethodDeclaration.resolveStatements(MethodDeclaration.java:196)
        at org.aspectj.ajdt.internal.compiler.ast.InterTypeMethodDeclaration.resolveStatements(InterTypeMethodDeclaration.java:161)
        at org.aspectj.org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration.resolve(AbstractMethodDeclaration.java:404)
        at org.aspectj.ajdt.internal.compiler.ast.InterTypeDeclaration.resolve(InterTypeDeclaration.java:155)
        at org.aspectj.ajdt.internal.compiler.ast.InterTypeMethodDeclaration.resolve(InterTypeMethodDeclaration.java:97)
        at org.aspectj.org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.resolve(TypeDeclaration.java:1109)
        at org.aspectj.ajdt.internal.compiler.ast.AspectDeclaration.resolve(AspectDeclaration.java:114)
        at org.aspectj.org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.resolve(TypeDeclaration.java:1188)
        at org.aspectj.org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.resolve(CompilationUnitDeclaration.java:366)
        at org.aspectj.org.eclipse.jdt.internal.compiler.Compiler.process(Compiler.java:625)
        at org.aspectj.org.eclipse.jdt.internal.compiler.Compiler.compile(Compiler.java:392)
        at org.aspectj.ajdt.internal.core.builder.AjBuildManager.performCompilation(AjBuildManager.java:992)
        at org.aspectj.ajdt.internal.core.builder.AjBuildManager.doBuild(AjBuildManager.java:266)
        at org.aspectj.ajdt.internal.core.builder.AjBuildManager.batchBuild(AjBuildManager.java:181)
        at org.aspectj.ajde.core.internal.AjdeCoreBuildManager.doBuild(AjdeCoreBuildManager.java:95)
        at org.aspectj.ajde.core.AjCompiler.buildFresh(AjCompiler.java:127)
        at org.eclipse.ajdt.core.builder.AJBuilder.build(AJBuilder.java:196)
        at org.eclipse.core.internal.events.BuildManager$2.run(BuildManager.java:633)
        at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37)
        at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:170)
        at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:201)
        at org.eclipse.core.internal.events.BuildManager$1.run(BuildManager.java:253)
        at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37)
        at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:256)
        at org.eclipse.core.internal.events.BuildManager.basicBuildLoop(BuildManager.java:309)
        at org.eclipse.core.internal.events.BuildManager.build(BuildManager.java:341)
        at org.eclipse.core.internal.events.AutoBuildJob.doBuild(AutoBuildJob.java:140)
        at org.eclipse.core.internal.events.AutoBuildJob.run(AutoBuildJob.java:238)
        at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55)
Comment 38 Andrew Clement CLA 2008-08-22 15:24:09 EDT
hmmm, tricky without a failing testcase.  I've tried recreating myself and I can't.

A little unpleasant that it is deep in the JDT compiler after starting out in the Intertypemethoddeclaration resolution code.  Hopefully it is just some small piece of state that needs setting on the ITD scope in order for capturing of the bound to work correctly.

but glad to hear the other fixes are holding together!
Comment 39 Andrew Clement CLA 2008-08-26 23:04:18 EDT
*** Bug 166514 has been marked as a duplicate of this bug. ***
Comment 40 Dave Whittaker CLA 2008-09-11 21:17:38 EDT
I think I've got a new one for you.  I have a simple interface for object selection SelectAction<T>, and then I have an ITD SelectActionAspect.  Everything compiles fine, and within that project I can create a class that implements SelectAction and see that it receives the methods I've defined.  The problem is that when I package SelectAction into a jar and reference the jar as part of the aspect path in another project, then create a class that inherits from the interface SelectAction I get this:

Inconsistent classfile encountered: The undefined type parameter T is referenced from within SelectActionAspect.

I am, however, passing an argument to use as "T" in the class declaration.

I am also getting a stack in the eclipse log, but I can't tell if it's directly related, although it does reference AJ code.  All the relevant info follows:

public interface SelectAction<T>{

	public void select(T object);

	public T getSelected();

	public void setSelected(T object);

	public void deselect();

}

public aspect SelectActionAspect {
	
	private T SelectAction<T>.selected;
	
	public void SelectAction<T>.select(T object){
		this.selected = object;
	}

	public T SelectAction<T>.getSelected(){
		return selected;
	}

	public void SelectAction<T>.setSelected(T object){
		this.selected = object;
	}

	public void SelectAction<T>.deselect(){
		this.selected = null;
	}

}

** Here's where I see the error marker on the first line of the file***

public class ProfileAction implements SelectAction<OnetOccupationStub> {

}


And here's the stack:

!ENTRY org.eclipse.core.resources 4 2 2008-09-11 20:58:09.403
!MESSAGE Problems occurred when invoking code from plug-in: "org.eclipse.core.resources".
!STACK 0
java.lang.NullPointerException
        at org.eclipse.ajdt.internal.ui.markers.MarkerUpdating.addNewMarkers(MarkerUpdating.java:171)
        at org.eclipse.ajdt.internal.builder.UIBuildListener.postAJBuild(UIBuildListener.java:144)
        at org.eclipse.ajdt.core.builder.AJBuilder.postCallListeners(AJBuilder.java:833)
        at org.eclipse.ajdt.core.builder.AJBuilder.build(AJBuilder.java:172)
        at org.eclipse.core.internal.events.BuildManager$2.run(BuildManager.java:633)
        at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37)
        at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:170)
        at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:201)
        at org.eclipse.core.internal.events.BuildManager$1.run(BuildManager.java:253)
        at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37)
        at org.eclipse.core.internal.events.BuildManager.basicBuild(BuildManager.java:256)
        at org.eclipse.core.internal.events.BuildManager.basicBuildLoop(BuildManager.java:309)
        at org.eclipse.core.internal.events.BuildManager.build(BuildManager.java:341)
        at org.eclipse.core.internal.events.AutoBuildJob.doBuild(AutoBuildJob.java:140)
        at org.eclipse.core.internal.events.AutoBuildJob.run(AutoBuildJob.java:238)
        at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55)

Any ideas?
Comment 41 Dave Whittaker CLA 2008-09-11 21:33:26 EDT
Created attachment 112373 [details]
Replicates inconsistent classfile error

Thankfully this one wasn't very difficult to replicate.  Just import the two projects into your workspace.  Test2 uses test1 on it's aspect path and fails to compile.
Comment 42 Dave Whittaker CLA 2008-09-18 12:36:03 EDT
Just wondering if these patches had made there way into the 1.6.2 build?  I used the udpate site today to test it out and got a build error, albeit a different one than what I've seen in the past.  Should it have worked?
Comment 43 Andrew Clement CLA 2008-09-18 13:01:36 EDT
All my changes up to comment 36 *should* be in the latest dev builds of AspectJ and AJDT.  I've not yet fixed anything raised in comment 37 or after that.

When you say you used an update site, I presume that was to get the latest AJDT 1.6 builds for eclipse 3.4?  I currently see 1.6.1.200809121132 on the AJDT dev update site, and they should be in there.

These fixes are not in AJDT 1.5 yet as there has not been a new AspectJ drop released in that stream since AJDT 1.5.3 was released.
Comment 44 Dave Whittaker CLA 2008-09-18 13:54:37 EDT
I was kind of afraid of that.  My project, which was compiling fine with the patched plugin, is failing with version 1.6.1 from the dev site.  I get this error:

The class file Manager contains a signature '()Ljava/util/List<?>;' ill-formed at position 18

Manager is an interface which corresponds to an ITD aspect.  I'll see if I can get a testcase together to replicate.
Comment 45 Andrew Clement CLA 2008-09-18 14:16:24 EDT
that is a shame :( Must be a side effect of one of the other numerous generics 'fixes' that have gone in to 1.6.2 (Bugzilla query to see them all: http://tinyurl.com/5x7j99 ).

On a positive note, the only internal compiler changes were for your bugs and so this is likely to be something much more straight forward.  If you can email me the broken .class file and the declaration that leads to the problem, that might be enough.
Comment 46 Dave Whittaker CLA 2008-09-18 14:38:20 EDT
Created attachment 112937 [details]
Replicates aspectpath bug

Good news and bad news.  The bad news... It might be a little more complicated, it only happens if the aspect is in a different project and included on the aspect path.  The good... I can replicate it and I'm going to include two projects that you should be able to import into your workspace and see what's happening.  This also seems like it may very well be related to the other bug I had noticed as that also occurred when the aspect was contained in another project.
Comment 47 Dave Whittaker CLA 2008-09-18 16:03:28 EDT
Andy,

I'm sure you're busy, but do you have any idea when you might have a chance to take a look at this?  I hate to be a pain, but eclipse wouldn't let me revert to the previous version and this is a real headache right now.  If it's going to be a while that's fine, I'd just like to plan out what I'll need to do to work around it in the meantime.

Thanks.
Comment 48 Andrew Clement CLA 2008-09-18 16:09:17 EDT
as you have gone to the trouble of attaching a testcase (which i always appreciate) I will take a look either this afternoon or tomorrow morning.  the other thing I was working on has just gotten too complicated anyway ;) - i could use a break.

Comment 49 Dave Whittaker CLA 2008-09-18 16:16:31 EDT
Thanks, I really appreciate it.
Comment 50 Andrew Clement CLA 2008-09-18 20:16:09 EDT
As suspected, a side effect of another fix, this one I think:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=211146

Thanks for the great testcase, integrated it, recreated it and just committed the fix.  It was due to incorrect use of a ? when we should have used *.

I'll put the next working AspectJ build into AJDT.

Just to remind (myself) - I have not looked at the problems described in the bug from comment 37 to 41, but I have committed the test code for the code in comment 41 (called MultiProjectIncrementalTests.testAspectPath_pr242797_c41())
Comment 51 Dave Whittaker CLA 2008-09-18 20:56:52 EDT
Thanks again Andy that's a huge help.  I've checked out the modules referenced in http://www.eclipse.org/ajdt/developers.php from CVS HEAD to build the plugin and I'm seeing an error:

The type CoreOutputLocationManager must implement the inherited abstract method IOutputLocationManager.getSourceFolderForFile(File)	CoreOutputLocationManager.java	org.eclipse.ajdt.core/src/org/eclipse/ajdt/internal/core/ajde	line 45	Java Problem

Am I missing something?
 
Comment 52 Andrew Clement CLA 2008-09-18 21:03:16 EDT
HEAD currently has an issue (the one you are seeing) for AJDT - we are in the midst of some big changes to improve incremental compilation.  I will fix that problem and commit the new AspectJ shortly, I haven't done it yet.  Are you sure you want to extract and build AJDT yourself?  After a build the update site should get refreshed with the change in just a few hours.
Comment 53 Dave Whittaker CLA 2008-09-18 21:07:29 EDT
Oh, I hadn't realized it would get pushed up so quickly.  In that case I'll
probably wait for the update site, but hopefully now that I've got the correct
modules checked out I can at least be a bit more helpful with debugging future
issues.
Comment 54 Andrew Clement CLA 2008-09-18 22:06:38 EDT
New AspectJ committed into AJDT containing this fix, next dev build should include it.  Also fixed HEAD compilation error.  Hopefully no regressions in AJDT, but we are refactoring the model support in AJDT to better support incremental compilation.
Comment 55 Dave Whittaker CLA 2008-09-19 10:17:26 EDT
Picked up the fix this morning and it works great.

I just wanted to let you know, when you get a chance to look at the other issue
it has a similar test case (Comment #41) which consists of 2 projects that
should just need to be imported into your workspace.  No rush on that though,
whenever you get to it.

Thanks again.
Comment 56 Andrew Clement CLA 2008-09-26 17:16:07 EDT
Trying to sort out what is and is not dealt with in this bug.

Appears to be work to do on:

Comment 37:
java.lang.NullPointerException
org.aspectj.org.eclipse.jdt.internal.compiler.lookup.CaptureBinding.initializeBounds(CaptureBinding.java:85)
org.aspectj.org.eclipse.jdt.internal.compiler.lookup.ParameterizedTypeBinding.capture(ParameterizedTypeBinding.java:113)
org.aspectj.org.eclipse.jdt.internal.compiler.ast.MessageSend.resolveType(MessageSend.java:550)

Comment 40:
java.lang.NullPointerException
org.eclipse.ajdt.internal.ui.markers.MarkerUpdating.addNewMarkers(MarkerUpdating.java:171)
org.eclipse.ajdt.internal.builder.UIBuildListener.postAJBuild(UIBuildListener.java:144)

Comment 41:
inconsistent class file - probably what I've described in bug 244619
Comment 57 Andrew Clement CLA 2008-09-26 17:18:09 EDT
I see the attachment in comment 41 is for the inconsistent class file problem - do you still see the AJDT npe mentioned in comment 40:

java.lang.NullPointerException
org.eclipse.ajdt.internal.ui.markers.MarkerUpdating.addNewMarkers(MarkerUpdating.java:171)

?
Comment 58 Dave Whittaker CLA 2008-09-26 17:29:51 EDT
I haven't seen either of comment 37 or comment 40 since they happened.  Comment 40 however seems to be somehow related to bug 244619.  It's something I've seen in the past where an aspect that causes an inconsistent class file error has it's marker show up in the wrong place.  I'm not sure what causes it, but the inconsistent class file itself seems to be the real issue.
Comment 59 Andrew Clement CLA 2008-09-26 18:55:56 EDT
Just fixed the problem with comment 41 and inconsistent class files.  Same
problem as before only I fixed it just for generic methods before and this time
it was a generic field problem.  The classfile is still inconsistent but the
compiler doesn't mind ;) Proper fix would be under 244619.
Comment 60 Andrew Clement CLA 2008-09-26 18:57:15 EDT
when the latest inconsistent class file fix is in AJDT I will close this bug to mark all the work done in 1.6.2.  We'll open a new bug if the capture problem (c37) reappears.
Comment 61 Andrew Clement CLA 2008-09-26 20:06:21 EDT
Just committed a new AspectJ into AJDT 1.6 so the inconsistent issue should be fixed in the next dev build of AJDT 1.6, phew.  Thanks for all your help with test programs getting to the bottom of these issues.