[ 
https://issues.apache.org/jira/browse/BCEL-127?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Sebb resolved BCEL-127.
-----------------------
       Resolution: Fixed
    Fix Version/s: 6.0

URL: http://svn.apache.org/r1696766
Log:
BCEL-127 Document that Instruction Factory returns singleton instances

Modified:
    commons/proper/bcel/trunk/src/changes/changes.xml



> Document that Instruction Factory returns singleton instances
> -------------------------------------------------------------
>
>                 Key: BCEL-127
>                 URL: https://issues.apache.org/jira/browse/BCEL-127
>             Project: Commons BCEL
>          Issue Type: Bug
>          Components: Main
>    Affects Versions: 5.2
>         Environment: Operating System: Linux
> Platform: PC
>            Reporter: zagi
>            Assignee: Apache Commons Developers
>             Fix For: 6.0
>
>
> The implementation of the creation of some instructions e.g. DUPs / NOPs  is 
> really misleading. If you create an invokeinstruction or an constant like ldc 
> a new instruction object is created. Pretty straight forward! But if you 
> create a DUP instruction you always will get the same object back. This is of 
> course is great for saving memory, but in the following example it leads to 
> lots of problems.
> Imagine you instrument a code sequence and add your own instructions one 
> after the other. You add invokestatic instructionsbut also the dups. At the 
> end you iterate again over the whole list to add some missing instructions ( 
> doing it in the first iteration is not possible or too much work, due to the 
> semantics of the algorithm). If you add new instructions again it will not 
> work properly if your insertion point is a DUP! The new instruction will be 
> inserted at the first match of the list.
> e.g.
> i1,... are instructions other than dup,
> dup_ are DUP instructions created with InstructionFactory.createDUP();
> you iterate from i1 to i6;
> init:       i1, i2, i3, dup_1, i4, i5, dup_2, i6
> 1.)  insert(i3, new_i)
> result:  i1, i2, new_i, i3, dup_1, i4, i5, dup_2, i6
> 2.) insert(dup_1, new_i2)
> result:  i1, i2, new_i, i3, new_i2,  dup_1, i4, i5, dup_2, i6
> 3.) insert(dup_2, new_i3)
> result:  i1, i2, new_i, i3, new_i2,  new_i3, dup_1, i4, i5, dup_2, i6
> The reason for this is that dup_1 dup_2 point to the same static DUP instance 
> (there is always one). Your call to insert()
> calls a method findInstruction() and this method only compares references 
> (==) and returns the first match.
> Even if you use copy() you get the same reference! To omit this problem you 
> have to create an instance without the factory by using a constructor.
> My point is, you can implement it this way BUT you have to mention this in 
> the API doc that this createDUP method does not behave like a createInvoke 
> method and always returns the same reference. I know usually the code is the 
> documentation but I think a lot of pain could be spared with a tiny little 
> sentence in the doc.
> My suggestions for the doc of those methods:
> "Returns always a static reference. Note: may cause issues when used as 
> insertion points"
> or something like that



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to