Summary: | generic pertypewithin advice fails to match though pointcut does | ||
---|---|---|---|
Product: | [Tools] AspectJ | Reporter: | Wes Isberg <wes> |
Component: | Compiler | Assignee: | Andrew Clement <aclement> |
Status: | RESOLVED FIXED | QA Contact: | |
Severity: | normal | ||
Priority: | P3 | ||
Version: | unspecified | ||
Target Milestone: | 1.5.0RC1 | ||
Hardware: | PC | ||
OS: | Windows XP | ||
Whiteboard: |
Description
Wes Isberg
2005-12-06 20:52:35 EST
Bah!!!! I've fixed this - and my fix is great. but it breaks an incremental compilation test ... the reason is that the incremental test aspect now matches where it didnt before (it used a generic pertypewithin). It relates to bug 115251 which is where Adrian was investigating changes to abstract aspects during incremental compilation. He put some stuff in, then commented it out - if I uncomment it then my pertypewithin fix works, but other tests fail ... here be dragons.... (and it's 1am, and I need to go to bed...). That abstract aspect fix I tried to put in and then backed out again really does need to happen to be solid in all cases. But as you discovered.... it has knock-on consequences that seemed unpleasant when I pulled on it (I was getting a 1.2.1 compatability test failure amongst other nasties). If you can find a clean way to include the absract aspect fix though, it's the right thing to do... Right. I have fixed this properly. the abstract aspect fix is in. the cflow optimizations are sorted (and more bullet proof than before) and pertypewithin(TypeVariable) works. 1. ReferenceType.getPerClause() needed to parameterize what it was returning when necessary. 2. EclipseSourceType.getPerClause() needed to return something intelligible when it could, rather than defaulting to PerSingleton(). It now looks on the aspect declaration and returns the right thing. 3. CrosscuttingMembersSet.addOrReplaceAspect(). I've uncommented Adrians stuff to handle abstract aspects. The problem is that addOrReplace.. was running twice - once during compile time, once during weave time. The type mungers to add the cflow counters were put into the crosscutting set whilst resolving the pointcuts the first time through addOrReplace. When the second addOrReplace occurs - the type mungers are deleted, and they never get readded because the fields are cached in the CflowPointcut logic (for reuse). The fix is that if we are trashing the type mungers, we need to trash the CflowPointcut cache - causing things to be recreated correctly. (Whether we need to call addOrReplaceAspect twice - compile time and weave time - i didnt investigate). 4. Finally, the logic in cflowpointcut for caching and reusing stacks/counters was a little shaky and I think could break if you used the same pointcut in two different aspects (because it maintained a cache of pointcut>cflowfield). Depending on the order in which ajc saw the aspects, the field might go into one aspect or the other - real problem for separate compilation. The real fix is to include the aspect name in the key for the cache - so we only share stacks/counters if a cflow entry pointcut is reused within the same aspect. i will check this in shortly. should help with incremental bugs relating to abstract aspects! fixes checked in. fix available. fyi, my original code for this bug (and some other code) now produces VerifyError when no-inline is on with the released AspectJ 5. I'm still isolating test cases, but as a workaround users should use the compiler's default inline setting. |