Bugzilla – Full Text Bug Listing
| Summary: | adviceDidNotMatch's line number doesn't keep up with line number of advice | ||
|---|---|---|---|
| Product: | [Tools] AspectJ | Reporter: | Helen Beeken <hlhawkins> |
| Component: | Compiler | Assignee: | aspectj inbox <aspectj-inbox> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | ||
| Version: | DEVELOPMENT | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | |||
| Attachments: | |||
|
Description
Helen Beeken
Created attachment 37544 [details]
failing testcase
Apply this patch to the tests project.
I think part of the fix for bug 129163 caused this bug. Within CrosscuttingMembers.replaceWith(..) we only calculate the changes to the shadowmungers and typemungers if we careAboutShadowMungers. If we change to always calculate the changes to the shadowmungers and typemungers then this bug no longer occurs. The problem is the equals method in Advice. At the moment its only using kind, pointcut and signature to see if two pieces of advice are the same. In the case of this bug they are the same, however, they have different sourceLocations. Because we're thinking they're equal we're not updating the list of shadowMungers and so the original one is used in the calculation of the xlint warning (which gives the original line number). The reason part of the fix for bug 129163 causes this problem is that if we always check shadowmunger equality the first time we compare we compare something with nothing which is a change and the second time we check nothing against something. This forces the update. This is a regression because for 1.5.0 we always thought there was a change so we always updated (due to various implementations of equals). The fix is to add implementation of the equals and hashcode methods within Advice and EclipseSourceLocation. Within Advice I've added a check for the equality of the SourceLocation to the equals method and in EclipseSourceLocation I've said that two are equal if they have the same start position, same end position and if their sourceFiles are equal. Created attachment 37720 [details]
patch implementing equals and hashcode in EclipseSourceLocation
Apply this patch to the org.aspectj.ajdt.core project.
Created attachment 37721 [details]
patch improving equals and hashcode methods in Advice
Apply this patch to the weaver project.
Created attachment 37722 [details]
testcase patch synchronized with the latest version in HEAD
Apply this patch to the tests project.
Created attachment 37727 [details]
testcase patch which tests that adding whitespace doesn't force full build
Apply this patch to the tests project.
Created attachment 37728 [details]
patch which doesn't force full build if there's a whitespace change
Apply this patch to the weaver project.
An explanation for the previous two patches (comment #8 and comment #9)... By changing the equals methods as described in comment #4 this does have the knock on effect of thinking there's been a change if the sourcelocation changes. Which means that if there's a whitespace change then we force a full build...not really something we want to be doing......An alternative solution is to go ahead and work out whether there's been a change as we have been doing previously, however, to copy over the new list of shadowmungers regardless of whether we think there's been a change (this is all in CrosscuttingMembers.replaceWith(..)). That way we pick up any changes in sourcelocation without forcing a full build. This does require us to reset the list of shadowmungers in CrosscuttingMembersSet if we care about the shadowmungers otherwise we wont pick up the new ones. The fix for this has been checked in and is available in both the latest AJ and AJDT dev builds. Therefore, closing this as fixed. |