[ 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)