Bug 158573 - changing value of variable in aspect results in adviceDidNotMatch warning
Summary: changing value of variable in aspect results in adviceDidNotMatch warning
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: unspecified   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 1.5.3   Edit
Assignee: aspectj inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-09-25 11:09 EDT by Helen Beeken CLA
Modified: 2012-04-03 15:50 EDT (History)
0 users

See Also:


Attachments
failing testcase (2.60 KB, patch)
2006-09-25 11:12 EDT, Helen Beeken CLA
aclement: iplog+
Details | Diff
patch containing proposed fix (2.47 KB, patch)
2006-09-25 11:59 EDT, Helen Beeken CLA
aclement: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Helen Beeken CLA 2006-09-25 11:09:03 EDT
Given the following:

public aspect A {
	public static int i = 0;
	before() : execution(* *.*(..)) {}
}

public class C {
	public void m() {}
}

Changing the value of i to be something else and then saving results in an incremental build and an "adviceDidNotMatch" warning. A full build clears the warning.

This is since the JDTLikeHandleProvider (bug 141730) was made the default in AJDT and is down to the call on line 90 of CrosscuttingMembersSet which is the fix for bug 134541). The reason we now hit this is that the JDTLikeHandleProvider doesn't depend on location.
Comment 1 Helen Beeken CLA 2006-09-25 11:12:36 EDT
Created attachment 50831 [details]
failing testcase

Apply this patch to the tests project.
Comment 2 Helen Beeken CLA 2006-09-25 11:19:30 EDT
The problem is that the new shadowMunger has the field "hasMatchedAtLeastOnce" set to false, as it would do since it was created on this incremental compile. The shadowmunger it's replacing has "hasMatchedAtLeastOnce" set to true because on the full compile we went through BcelAdvice.implementOn(). Currently in CrosscuttingMembersSet.replaceWith() we do a blanket replace of the shadowmungers to ensure we've picked up the latest changes. I guess we could be more specific here and ensure that we don't loose this type of information...... 
Comment 3 Andrew Clement CLA 2006-09-25 11:48:49 EDT
it's ugly but we could do that ... i've put in similarly ugly fixes before ;)
Comment 4 Helen Beeken CLA 2006-09-25 11:59:48 EDT
Created attachment 50839 [details]
patch containing proposed fix

Apply this patch to the weaver project.

This patch contains the fix proposed in the above comment.
Comment 5 Andrew Clement CLA 2006-09-29 10:18:11 EDT
test and 'fix' in ;)

iplog
Comment 6 Andrew Clement CLA 2006-10-03 06:54:12 EDT
fix available in aj dev builds.