From pfreeman@ccs.neu.edu Sat Nov 9 08:47:52 2002 Hi Pengcheng - That is really good to know that about ConstructorExcution. Thanks for pointing that out. If you have a chance, I would like to hear any feedback you have regarding the latest version of the object form checker I just sent out. Paul Pengcheng Wu wrote: >Hi Paul, > >I agree with your statement too, except for one part. :-) > >I had also realized pertarget(Any.Set(..)) was not good, but for >a different reason, i.e., if an object doesn't have any instance variable, >an exception would be thrown when doing checking since there is no >Perthis aspect associated with that object. Although I could walk around >this problem by doing some checking first, I still decided to declare >Perthis aspect to be perthis(Any.ConstructorExecution()) and in the before >advice on Any.Set(..) to collect the immdediate relationships, I used: > > before(Object targetObject): Any.Set(targetObject) { > Perthis.aspectOf(targetObject).addAll(..); > } >which comes back to the solution I suggested in my first mail in this >thread. > >One thing I don't agree with you is that even you have no explicit >constructor declared for a class, the perthis(ConstructorExecution()) >still can associate an aspect instance with a newly constructed object, >which explains why the above code could work. The reason is >that AspectJ will create a default constructor for that class. > >It is a very good comment. Thank you! > >--Pengcheng > >On Fri, 8 Nov 2002, Paul Freeman wrote: > >>Hi Pengcheng - >> >>I agree completely with your statement, except for one part. I would >>argue that the pertarget(Any.Set()) pointcut should actually be >>pertarget(Any.ConstructorCall()) for the following reasons. >>- ConstructorCall picks out only the join points that should be >>captured, no more. Any.Set() could pick out multiple Set join points >>for one object. While AspectJ allows for this by ensuring there is only >>one instance of an Aspect associated with the target object of the >>pertarget statement, it is unnecessary. >>- ConstructorCall clearly defines what we are trying to do, namely >>associate one Aspect instance with one newly created Object instance. >> >>Of course it could also be argued that some Objects in the program may >>not contain instance variables, and therefore would never need the >>associated aspect. So perhaps Any.Set() is more appropriate. I suppose >>it depends on the amount of overhead involved with either statement when >>the chekcer is used with a large application. >> >>Note: in my original solution I used the pointcut ConstructorExecution >>and not ConstructorCall, however this would exclude classes without an >>explicitly defined constructor, i.e. those that rely on the default >>constructor from the class Object. In regard to overhead here, there is >>also the question of whether or not aspects will be associated to >>targets of constructor calls outside the scope of the source code, new >>java.io.PrintWriter(..) for example. If this is the case, then there >>would probably be a large number of useless aspect instances taking up >>memory. Of course this side-effect could not be avoided by using the >>Any.Set pointcut either. >> >>I would be interested in your response to the points I raised. I think >>it depends on how AspectJ handles some of the overhead issues I raised, >>which I am sure you know much more about than I do. >> >>Paul >> >> >>Pengcheng Wu wrote: >> >>>Hi Paul, >>> >>>Sorry for late response. I was busy with grading work in the last two >>>days. >>> >>>I think I agree with what you said in this message. But one thing made >>>me a little confused (which made me not really understand the objective >>>of your statement) is that the reason I used pertarget(Any.Set(Object)) >>>in >>>the object form checker was that it is the way to maintain the correct >>>immediate part relationship between objects(otherwise, if you use >>>perthis(Any.Set(Object)), then it may maintain a wrong relationship if >>>an object's field is set in a context where 'this' is not that object), >>>instead of that the advice don't know on what aspect instance it should >>>be executed. I hope this explanation will make things clear. >>> >>>--Pengcheng >>> >>>On Wed, 6 Nov 2002, Paul Freeman wrote: >>> >>>>Hi Pengcheng - >>>> >>>>>However, the behavior (and perhaps the statements above) seem >>>>>to indicate that the target of the perthis pointcut must also be the >>>>>target found in all advice in the aspect. >>>>> >>>>>I don't really understand this statement. Please can you clarify it? >>>>> >>>>actually, I just wasn't thinking clearly here, ignore this. I was in the early stages of trying to formulate what you did below. >>>> >>>> >>>>>if you declare >>>>>an aspect to be perthis, then for each advice defined in that aspect, it >>>>>will try to find the aspect instance associated with the 'this' object >>>>>on that pointcut and run the advice on that instance; >>>>> >>>>this is the conclusion I was trying to formulate. This seems like very strange behavior to me. >>>> >>>>>if you declare >>>>>it as pertarget, it will do the similiar thing too. Because the pointcut >>>>>you used to declare the aspect could have nothing to do with the >>>>>pointcut >>>>>on which you define an advice in that aspect, but any pointcut does have >>>>>'this' and 'target' object, this semantics helps find out on which >>>>>aspect >>>>>instance you would run the advice. It explains why you get to that situation. >>>>> >>>>Here is where I am confused. My understanding is this: >>>>If aspect A is declared with a perthis(execution(* new(..)) pointcut and we have a program that contains an class B and a class Main. >>>>when when an object b of type B is created, it is the 'this' object at the join point defined by the pointcut in perthis(..), then an aspect ab of type A is also created and directly related to the object B. Lets assume Main only contains the main method. Then it should never have an aspect of type A associated with it as it is not instantiated, i.e. it is never the 'this' object at the join point declared in perthis(..). >>>>Now the advice in the aspect ab will only capture join points in object b and no join points in the Main class will be captured. >>>> >>>>Extending this logic to the pertarget(execution(* new(..)) pointcut, all conclusions should be identitical due to the fact that the target at the point of execution of a constructor equal to the this at the point of execution of a constructor. However this must not be the case as is demonstrated by the fact that when imlimented, join points in Main can be captured using pertarget when they cannot be captured using perthis. >>>> >>>>You must be correct though that the instance of the aspect that is associated with the target of the advice in an Aspect declared with pertarget designation, executes for all such join points in the program execution where the target has an aspect instance. (i think i said that right) >>>> >>>> >>>>>I really don't understand why. It seems to me that if there is an >>>>>aspect associated with an object, that advice should only execute for >>>>>that object, >>>>> >>>>>I don't understand this statement either. Advice are executed on aspect >>>>> >>>>>instances ... >>>>> >>>>I was trying to describe what I just did above. >>>> >>>>Paul >>>> >>>> >>>> >>>> >>>>Pengcheng Wu wrote: >>>> >>>>>Hi Paul, >>>>> >>>>>I know what you mean. I can not understand aspect instantiation >>>>> >>>>>from time to time either. I will try to explain some of your concerns in >>>> >>>>>the below. >>>>> >>>>>On Tue, 5 Nov 2002, Paul Freeman wrote: >>>>> >>>>>>Hi Pengcheng - >>>>>> >>>>>>This still doesn't make sense to me. I tried your solution and verified >>>>>>that it works. Why it works doesn't make sense to me. Here is the >>>>>>excerpt from the programming guide: >>>>>>******* >>>>>> >>>>>>If an aspect A is defined perthis(Pointcut ), then one object of type A >>>>>>is created for every object that is the executing object (i.e., "this") >>>>>>at any of the join points picked out by Pointcut. The advice defined in >>>>>>A may then run at any join point where the currently executing object >>>>>>has been associated with an instance of A. >>>>>> >>>>>>Similarly, if an aspect A is defined pertarget(Pointcut ), then one >>>>>>object of type A is created for every object that is the target object >>>>>>of the join points picked out by Pointcut. The advice defined in A may >>>>>>then run at any join point where the target object has been associated >>>>>>with an instance of A. >>>>>> >>>>>>******* >>>>>> >>>>>>It would seem to me that the intention of these aspect instantiation >>>>>>pointcut designators would be only to associate an object with an aspect >>>>>>instance. >>>>>> >>>>>Yes, it sounds right to me too. But my understanding says it has more as >>>>>I will explain later in the below. >>>>> >>>>>>However, the behavior (and perhaps the statements above) seem >>>>>>to indicate that the target of the perthis pointcut must also be the >>>>>>target found in all advice in the aspect. >>>>>> >>>>>I don't really understand this statement. Please can you clarify it? >>>>> >>>>>>For example, my modification >>>>>>to the Foo test file included a set that occurred in the static main >>>>>>method. Prior to the Set join point, a ConstructorExecution join point >>>>>>is reached: >>>>>> >>>>>>public static void main(String[] args){ >>>>>> Foo.bar(); >>>>>> Foo aFoo = new Foo(new Foo(new Foo())); >>>>>> aFoo.f = new Foo(new Foo()); // *** new line *** >>>>>> >>>>>> aFoo.run(new Foo(), new Bar()); >>>>>> aFoo.toString(); >>>>>>} >>>>>> >>>>>> >>>>>>In the original implimentation, I assumed the before():Any.Set() advice >>>>>>was not executing because there was no ImmediatePartBin aspect >>>>>>associated with the currently executing object for the static method >>>>>>main, i.e. null. Since no Aspect was associated, the set join point was >>>>>>not being affected by any advice. >>>>>> >>>>>I think it is right and we all agree on that. >>>>> >>>>>>However, if you change, as you >>>>>>suggest, the perthis statement to be a pertarget statement, the set join >>>>>>point in the static method main is examined, regardless of whether >>>>>>perthis is Any.Set() or Any.ConstructorExecution(). >>>>>> >>>>>^^^^^^^^ >>>>>I guess it is a typo, you really mean 'pertarget', right? My understanding >>>>>is, there are two meanings when you declare an aspect to be perthis or >>>>>pertarget. First, it is about how this aspect should be instantiated and >>>>>with what object the instance of this aspect is associated. I think we >>>>>both know that. The second is(I just figured it out recently by chance >>>>>and have never seen it in any document; so maybe it is just an >>>>>implementation issue and not the designed semantics), if you declare >>>>>an aspect to be perthis, then for each advice defined in that aspect, it >>>>>will try to find the aspect instance associated with the 'this' object >>>>>on that pointcut and run the advice on that instance; if you declare >>>>>it as pertarget, it will do the similiar thing too. Because the pointcut >>>>>you used to declare the aspect could have nothing to do with the >>>>>pointcut >>>>>on which you define an advice in that aspect, but any pointcut does have >>>>>'this' and 'target' object, this semantics helps find out on which >>>>>aspect >>>>>instance you would run the advice. It explains why you get to that situation. >>>>> >>>>>>I really don't understand why. It seems to me that if there is an >>>>>>aspect associated with an object, that advice should only execute for >>>>>>that object, >>>>>> >>>>>I don't understand this statement either. Advice are executed on aspect >>>>>instances ... >>>>> >>>>>>and perthis/pertarget should only associate an aspect with >>>>>>an object based on this/target for the pointcut described. There should >>>>>>never be an aspect associated with the currently executing or target >>>>>>object during the execution of a static method. >>>>>> >>>>>But there is a 'target' object defined on the set join point, so the >>>>>advice can be executed anyway as I described earlier. >>>>> >>>>>I guess Karl and David would also be interested in this issue. So I >>>>>cc them as well. >>>>> >>>>>--Pengcheng >>>>> >>>>>>I guess I am wrong somewhere though... could you please explain? >>>>>> >>>>>>Paul >>>>>> >>>>>> >>>>>>Pengcheng Wu wrote: >>>>>> >>>>>>>Hi Paul, >>>>>>> >>>>>>>The context in which I mentioned my solution was to fix the bug in the >>>>>>>framework of our solution in the paper, not yours. Sorry for not making >>>>>>>it clear. However, I also believe my solution can help fix the bug in >>>>>>>your framework too. You are right in that if there is no aspect associated >>>>>>>with an executing object,e.g.,a static method, both our frameworks failed >>>>>>>to capture the direct part relationships between the 'right' objects. But >>>>>>>I don't want the Perthis (in our framework) aspect or >>>>>>>ImmediatePartFieldAccesses aspect(in your framework) to be singleton >>>>>>>aspect either, since I then have to manually maintain the relationships >>>>>>>with a HashMap/Hashtable object instead of just a HashSet object. That >>>>>>>is why I suggested to declare Perthis(also ImmediatePartFieldAccesses) >>>>>>>aspect to be >>>>>>> >>>>>>> pertarget(Any.Set(Object)) >>>>>>> >>>>>>>which gurrantees that there is always a aspect instance associated with >>>>>>>a target object of a set join point no matter where the set join point >>>>>>>is captured (even in a static method). And I can easily maintain the >>>>>>>direct part relationship with just a HashSet object. To make this solution >>>>>>>work, I have to make the checking logic in a separate aspect, which will >>>>>>>be a singleton aspect, while in the current version of our solution, it >>>>>>>is in the aspect of Perthis. >>>>>>> >>>>>>>--Pengcheng >>>>>>> >>>>>>>On Tue, 5 Nov 2002, Paul Freeman wrote: >>>>>>> >>>>>>>>Hi Pengcheng - >>>>>>>> >>>>>>>>I don't think I quite understand your solution. You mention putting the >>>>>>>>checking logic into a singleton aspect. The problem I was trying to fix >>>>>>>>with the new singleton aspect ImmediatePartFieldAccesses was one in >>>>>>>>which the gathering advice doesn't have a an aspect associated with the >>>>>>>>currently executing object, for example when you are setting a field >>>>>>>>access from within a static method, there is no gathering advice that >>>>>>>>will execute as there is no gathering aspect associated with a null >>>>>>>>object. I didn't modify the checking portion of the solution at all. >>>>>>>>Is there a problem with the checking part that I missed? >>>>>>>> >>>>>>>>Paul >>>>>>>> >>>>>>>> >>>>>>>>Pengcheng Wu wrote: >>>>>>>> >>>>>>>>>Another solution to this bug, which I tried successfully and can >>>>>>>>>completely >>>>>>>>>fix the bug, is to declare the Perthis aspect to be >>>>>>>>> >>>>>>>>>pertarget(Any.Set(Object)) >>>>>>>>> >>>>>>>>>and the before advice on the set join point will be the same as before, >>>>>>>>>i.e., no Perthis.aspectOf(..) is needed. Meanwhile, you have to put the >>>>>>>>>before advice on Any.MethodCall(..), which basically does the check logic, >>>>>>>>>in another aspect, say, Check aspect, which is a sigleton aspect. And I >>>>>>>>>think put the checking logic in a separate aspect is more natural anyway, >>>>>>>>>since it has nothing to do with the concern of collecting positive >>>>>>>>>objects. >>>>>>>>> >>>>>>>>>--Pengcheng >>>>>>>>> >>>>>>>>>On Tue, 5 Nov 2002, Paul Freeman wrote: >>>>>>>>> >>>>>>>>>>Hi Karl - >>>>>>>>>> >>>>>>>>>>Yes, it is. Although I think I may have come up with a solution that is >>>>>>>>>>slightly different than Pengchengs. I realized there might also be a >>>>>>>>>>case where you wish to catch a "set" join point when there is no aspect >>>>>>>>>>associated with the executing object, for example, I made the following >>>>>>>>>>change to the Foo test class from my HW4 Part 3 solution: >>>>>>>>>>public static void main(String[] args){ >>>>>>>>>> Foo.bar(); >>>>>>>>>> Foo aFoo = new Foo(new Foo(new Foo())); >>>>>>>>>> aFoo.f = new Foo(new Foo()); // *** new >>>>>>>>>>line *** >>>>>>>>>> >>>>>>>>>> aFoo.run(new Foo(), new Bar()); >>>>>>>>>> aFoo.toString(); >>>>>>>>>>} >>>>>>>>>> >>>>>>>>>>In the above case, there is no ImmediatePartBin associated with the >>>>>>>>>>currently executing object since the "this" object in a static method >>>>>>>>>>execution is null. So I realized that the advice that captures the Set >>>>>>>>>>join points must be in a singleton aspect. My solution divides the >>>>>>>>>>ImmediatePartBin aspect into two aspects, ImmediatePartBin and >>>>>>>>>>ImmediatePartFieldAccessMonitor: >>>>>>>>>> >>>>>>>>>>*****************************8 >>>>>>>>>>package lawOfDemeter; >>>>>>>>>>import java.util.*; >>>>>>>>>> >>>>>>>>>>/** >>>>>>>>>>* The ImmediatePartBin aspect is part of a Law of Demeter Checker - a >>>>>>>>>>set of >>>>>>>>>>* classes and aspects that verify a given program does not violate >>>>>>>>>>* the Law of Demeter. This aspect stores the immediate parts, or >>>>>>>>>>* instance variables, of objects as they are set for later >>>>>>>>>>* examination by the Checker aspect. >>>>>>>>>>* >>>>>>>>>>* @author David H. Lorenz >>>>>>>>>>* @author Modifications made by Paul Freeman >>>>>>>>>>*/ >>>>>>>>>>public aspect ImmediatePartBin extends Supplier >>>>>>>>>>perthis(Any.ConstructorExecution()){ >>>>>>>>>> >>>>>>>>>>/** >>>>>>>>>>* Prints the objects currently in the bin. >>>>>>>>>>* Used for debugging. >>>>>>>>>>*/ >>>>>>>>>>public void printBin(){ >>>>>>>>>>System.out.println("-- immediate part objects --"); >>>>>>>>>>printHashMap(targets); >>>>>>>>>>System.out.println("----------------------------"); >>>>>>>>>>} >>>>>>>>>> >>>>>>>>>>public void addField(org.aspectj.lang.JoinPoint thisJoinPoint){ >>>>>>>>>> // There is only one argument to a set join point. >>>>>>>>>> // Becomes a preferred supplier and is therefore added to targets. >>>>>>>>>> addAll(thisJoinPoint.getArgs(), direct+at(thisJoinPoint)); >>>>>>>>>>// System.out.println("Adding--" + thisJoinPoint + >>>>>>>>>>at(thisJoinPoint) + " in " + thisJoinPoint.getTarget()); >>>>>>>>>>} >>>>>>>>>> >>>>>>>>>>} >>>>>>>>>> >>>>>>>>>>public aspect ImmediatePartFieldAccessMonitor { >>>>>>>>>> >>>>>>>>>>/** >>>>>>>>>>* Advice that stores the immediate part objects, or instance >>>>>>>>>>variables, of >>>>>>>>>>* the object currently executing. >>>>>>>>>>*/ >>>>>>>>>>before(): // direct parts >>>>>>>>>>Any.Set() { >>>>>>>>>> try{ >>>>>>>>>> >>>>>>>>>>ImmediatePartBin.aspectOf(thisJoinPoint.getTarget()).addField(thisJoinPoint); >>>>>>>>>> }catch(org.aspectj.lang.NoAspectBoundException e){ >>>>>>>>>> // required since aspect won't be bound to static, i.e. null, >>>>>>>>>>targets. >>>>>>>>>> } >>>>>>>>>>} >>>>>>>>>>} >>>>>>>>>>********************** >>>>>>>>>> >>>>>>>>>>This seems to completely fix the bug Pengcheng found. >>>>>>>>>> >>>>>>>>>>Paul >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>Karl Lieberherr wrote: >>>>>>>>>> >>>>>>>>>>>Does this bug also apply to your solution? >>>>>>>>>>> >>>>>>>>>>>Please don't tell the class yet. >>>>>>>>>>> >>>>>>>>>>>-- Karl >>>>>>>>>>> >>>>>>>>>>>>From wupc@ccs.neu.edu Mon Nov 4 09:58:33 2002 >>>>>>>>>>> >>>>>>>>>>>>From: Pengcheng Wu >>>>>>>>>>>>To: "David H. Lorenz" , >>>>>>>>>>>> Karl Lieberherr >>>>>>>>>>>>Subject: Object form checker >>>>>>>>>>>> >>>>>>>>>>>>Hi David, >>>>>>>>>>>> >>>>>>>>>>>>I found a severe bug in your object form LoD checker, that is, if >>>>>>>>>>>>you declare the Perthis aspect to be perthis(Any.Execution) then >>>>>>>>>>>>the before advice in Perthis aspect on the Any.Set(Object) pointcut >>>>>>>>>>>>should be: >>>>>>>>>>>> >>>>>>>>>>>>Perthis.aspectOf(targetObject).addAll(..) >>>>>>>>>>>> >>>>>>>>>>>>not just, >>>>>>>>>>>> >>>>>>>>>>>>addAll(..) >>>>>>>>>>>> >>>>>>>>>>>>I have updated it. >>>>>>>>>>>> >>>>>>>>>>>>--Pengcheng >>>>>>>>>>>> >>>>>>>>>>>> >>>>> >>> >>> >> > > > To: freeberm@earthlink.net, lieber@ccs.neu.edu Subject: Re: Fixed bug in my LoDChecker... Cc: lorenz@ccs.neu.edu, wupc@ccs.neu.edu Hi Paul: thank you. That is the explanation I was looking for. -- Karl >From pfreeman@ccs.neu.edu Tue Nov 19 10:18:53 2002 > >Hi Karl - > >> Paul, do you have an example where removing old values from the >> ImediatePartBin's target hashmap is still needed? >> oldB.bar(); // now a violation >> becomes >> oldB.bar(); // not a violation >> because oldB contains an object returned by a local method. > >I think it is necessary to remove the old values from the immediate part >bin hashmap for two reasons. >One, when trying to debug the problem that Pengcheng had noticed, I used >some of the printBin functions to examine what was occurring durring >execution. The immediate part bin quickly became extreemly large since >any time an immediate part is set to a new value, a new object was added >to the bin. >The other reason I think it is necessary to remove objects from the >immediate part bin, is it is possible that an old object, one that was >an immediat part but has since been removed, could be returned by a >non-local method, and could be used in a way that violates the LoD. If >it still existed in the immediate part bin, it would not be flagged as a >violation. > >For example: >class LinkedList{ > > ListNode head = null; > > public LinkedList(){ > // build a linked list of 10 node elements > for(int i = 0; i < 10; i++){ > ListNode nextNode = new ListNode( i ); > nextNode.setNext( head ); > head = nextNode; > } > } > > public void printNodes(){ > ListNode thisNode = head; > while(thisNode != null){ > System.out.print(thisNode.getValue() + " "); > thisNode = thisNode.getNext(); > } > } > > public static void main(String[] args){ > LinkedList newList = new LinkedList(); > newList.printNodes(); > } >} >class ListNode{ > private int val; > private ListNode next; > > public ListNode(int newVal){ > val = newVal; > } > > public ListNode getNext(){ > return next; > } > > public void setNext(ListNode newNext){ > next = newNext; > } > > public int getValue(){ > return val; > } >} > >Here is the output running the above with and without the removal of old >immediate parts from the bin: > >================================ >without removal: > >*** running OldImmediatePartTest *** > >Statistics: >Type Count %Total %Valid >Total Checks 62 >Violations 0 0.0 >Valid Checks 62 100.0 >As 1 Supplier 62 100.0 100.0 > >=============================== >with removal: > >*** running OldImmediatePartTest *** >!! LoD violation: call(int testcase.ListNode.getValue()) at >LinkedList.java:19:30 this testcase.LinkedList@29428e target >testcase.ListNode@1434234 >!! LoD violation: call(ListNode testcase.ListNode.getNext()) at >LinkedList.java:20:24 this testcase.LinkedList@29428e target >testcase.ListNode@1434234 >!! LoD violation: call(int testcase.ListNode.getValue()) at >LinkedList.java:19:30 this testcase.LinkedList@29428e target >testcase.ListNode@129f3b5 >!! LoD violation: call(ListNode testcase.ListNode.getNext()) at >LinkedList.java:20:24 this testcase.LinkedList@29428e target >testcase.ListNode@129f3b5 >!! LoD violation: call(int testcase.ListNode.getValue()) at >LinkedList.java:19:30 this testcase.LinkedList@29428e target >testcase.ListNode@148aa23 >!! LoD violation: call(ListNode testcase.ListNode.getNext()) at >LinkedList.java:20:24 this testcase.LinkedList@29428e target >testcase.ListNode@148aa23 >!! LoD violation: call(int testcase.ListNode.getValue()) at >LinkedList.java:19:30 this testcase.LinkedList@29428e target >testcase.ListNode@77158a >!! LoD violation: call(ListNode testcase.ListNode.getNext()) at >LinkedList.java:20:24 this testcase.LinkedList@29428e target >testcase.ListNode@77158a >!! LoD violation: call(int testcase.ListNode.getValue()) at >LinkedList.java:19:30 this testcase.LinkedList@29428e target >testcase.ListNode@1193779 >!! LoD violation: call(ListNode testcase.ListNode.getNext()) at >LinkedList.java:20:24 this testcase.LinkedList@29428e target >testcase.ListNode@1193779 >!! LoD violation: call(int testcase.ListNode.getValue()) at >LinkedList.java:19:30 this testcase.LinkedList@29428e target >testcase.ListNode@1efb836 >!! LoD violation: call(ListNode testcase.ListNode.getNext()) at >LinkedList.java:20:24 this testcase.LinkedList@29428e target >testcase.ListNode@1efb836 >!! LoD violation: call(int testcase.ListNode.getValue()) at >LinkedList.java:19:30 this testcase.LinkedList@29428e target >testcase.ListNode@72ffb >!! LoD violation: call(ListNode testcase.ListNode.getNext()) at >LinkedList.java:20:24 this testcase.LinkedList@29428e target >testcase.ListNode@72ffb >!! LoD violation: call(int testcase.ListNode.getValue()) at >LinkedList.java:19:30 this testcase.LinkedList@29428e target >testcase.ListNode@126804e >!! LoD violation: call(ListNode testcase.ListNode.getNext()) at >LinkedList.java:20:24 this testcase.LinkedList@29428e target >testcase.ListNode@126804e >!! LoD violation: call(int testcase.ListNode.getValue()) at >LinkedList.java:19:30 this testcase.LinkedList@29428e target >testcase.ListNode@388993 >!! LoD violation: call(ListNode testcase.ListNode.getNext()) at >LinkedList.java:20:24 this testcase.LinkedList@29428e target >testcase.ListNode@388993 > >Statistics: >Type Count %Total %Valid >Total Checks 62 >Violations 18 29.03 >Valid Checks 44 70.96 >As 1 Supplier 44 70.96 100.0 > > >I'll make the change to the ReturnValueBin so that it collects local >data accesses as well and update the code in the solution. > >I hope this was what you were looking for. Let me know if I >misunderstood your question. > >Paul > > >Karl Lieberherr wrote: > >>Hi Paul: >> >>thank you for your updates to the checker. >> >>Pengcheng: can you clarify why Paul's transformation described >>in the first paragraph was needed? Have you added a test case to the >>test suite that exposes the need for the transformation? >> >>>But the situation above seems >>>to beg the question, should b be treated as a local method is treated, >>>i.e. is b in the situation above equivalent to an object returned by a >>>local method? It seems to me that it should be treated this way, and if >>>so it would be easy to modify the ReturnValueBin in such a way as to >>>store the object represented by oldB with the proper scope. >>> >> >>Yes, an access to a data member is like a local method call, I view it as >>a call to a get method. >> >>Paul, do you have an example where removing old values from the >>ImediatePartBin's target hashmap is still needed? >>oldB.bar(); // now a violation >> becomes >>oldB.bar(); // not a violation >> because oldB contains an object returned by a local method. >> >>-- Karl >> >>>-----Original Message----- >>>From: Paul Freeman [mailto:pfreeman@ccs.neu.edu] >>>Sent: Monday, November 18, 2002 9:19 AM >>>To: Pengcheng Wu; Dr. Karl J. Lieberherr >>>Cc: David H. Lorenz >>>Subject: Fixed bug in my LoDChecker... >>> >>> >>>Hi Pengcheng, Karl - >>> >>>I fixed the issue you noticed with my version of the LoDChecker. It >>>turns out that the problem was in TargetBinStack involving the use of >>>around advice with EventToStack and EventToGather. Since the >>>ArgumentBin used the same join points with both peices of advice, it >>>seems that the part of the EventToStack advice after the proceed() >>>statement did not always execute, i.e. it appeared that HashMaps were >>>not being consistently popped from the HashMap stack. So I fixed it by >>>moving the EventToStack around advice into seperate pieces of before and >>>after advice. >>> >>>I also changed all of the HashMap instances to instances of the new >>>IdentityHashMap so that there would no longer be an issue when comparing >>>objects with the same value. >>> >>>And finally, I modified the ImediatePartBin so that it would remove old >>>identity values from the targets hashmap prior to setting new values in >>>the identity hashmap. I did this by creating a second hashmap that >>>crossreferenced objects with the name of the variable that references >>>the object. So now a situation like the following will cause a >>>voilation to be thrown: >>>class A{ >>> B b; >>> void foo(){ >>> B oldB = b; >>> b = new B(); >>> >>> oldB.bar(); // now a violation >>> } >>>} >>>class B{ >>> void bar(); >>>} >>> >>>There are obviously other instances where the old object could be >>>referenced outside a method where it is replaced by a new object, so I >>>think removing objects from the ImmediatePartObject targets >>>IdentityHashMap is the correct behavior. But the situation above seems >>>to beg the question, should b be treated as a local method is treated, >>>i.e. is b in the situation above equivalent to an object returned by a >>>local method? It seems to me that it should be treated this way, and if >>>so it would be easy to modify the ReturnValueBin in such a way as to >>>store the object represented by oldB with the proper scope. >>> >>>All of the corrected code has been placed in: >>>/proj/tools/solutions/com3205/hw04/part3_code_solution.zip >>> >>>Let me know what you think about the above statement and Pencheng, let >>>me know if my solution produces the same violations that your >>>solution does. >>> >>>Paul >>> >>> >> > >