SfxItemSet provides a API that is inviting errors. SfxPoolItems store a "Which-Id" which can be thought of as a primitive kind of type information. Items are stored in a SfxItemSet, which keeps one or zero items of a specific "Which-Id". However, it also provides a method: virtual const SfxPoolItem* Put( const SfxPoolItem&, USHORT nWhich ); which allows putting a SfxPoolItem at a different Which-Id as the one it was created with. This contradicts developer expectations and also blocks further basic refactoring of the SfxItemSets, which have a huge part in the load/save performance and thus are a valuable target. Here is an random example of a client that abuses the SfxItemSet::Put() method: it gets the item from one Which-Id and puts it at explicitly at multiple locations in another (causing the Which-Id of the Item and the Which-Id where it is recorded in the set to differ). Instead it should create copies of the item with the new Which-Id and call const SfxPoolItem* Put( const SfxPoolItem& rItem ). Once all the call to virtual const SfxPoolItem* Put( const SfxPoolItem&, USHORT nWhich ); are gone, it should be immediately removed. Then SfxItemSet can be refactored allowing better performance and debugging and less developer confusion. I tried to do this in cws new_itemsets on OOo, but the cws is outdated and still incomplete. I started with the reimplementation of the itemsets, which was the wrong point to start: Instead one should: 1) the removal of the bad calls which put items at WhichIds were they do not belong 2) the removal of all other call to virtual const SfxPoolItem* Put( const SfxPoolItem&, USHORT nWhich), by replacing them with call with calls to const SfxPoolItem* Put(const SfxPoolItem& rItem) CAUTION: This cannot be done before step one is completed, as Items are often pulled out of one set and put into another at the same WhichId as it was in the original set. If the WhichId on the item and were it was in the set differ, this is bound to break. 3) Remove virtual const SfxPoolItem* Put( const SfxPoolItem&, USHORT nWhich) 4) Replace SfxItemSet implementation with a faster one.
Comments from the EasyHacks wiki: SfxItemSet provides a API that is inviting errors. SfxPoolItems store a "Which-Id" which can be thought of as a primitive kind of type information. Items are stored in a SfxItemSet, which keeps one or zero items of a specific "Which-Id". However, it also provides a method: [http://opengrok.libreoffice.org/xref/libs-gui/svl/inc/svl/itemset.hxx#146 virtual const SfxPoolItem* Put( const SfxPoolItem&, USHORT nWhich );] which allows putting a SfxPoolItem at a ''different'' Which-Id as the one it was created with. This contradicts developer expectations and also blocks further basic refactoring of the SfxItemSets, which have a huge part in the load/save performance and thus are a valuable target. Here is an random [http://opengrok.libreoffice.org/xref/calc/sc/source/ui/app/inputwin.cxx#974 example of a client] that abuses the SfxItemSet::Put() method: it gets the item from one Which-Id and puts it at explicitly at multiple locations in another (causing the Which-Id of the Item and the Which-Id where it is recorded in the set to differ). Instead it should create copies of the item with the new Which-Id and call [http://opengrok.libreoffice.org/xref/libs-gui/svl/inc/svl/itemset.hxx#147 const SfxPoolItem* Put( const SfxPoolItem& rItem )]. Once all the call to [http://opengrok.libreoffice.org/xref/libs-gui/svl/inc/svl/itemset.hxx#146 virtual const SfxPoolItem* Put( const SfxPoolItem&, USHORT nWhich );] are gone, it should be immediately removed. Then SfxItemSet can be refactored allowing better performance and debugging and less developer confusion.
[This is an automated message.] This bug was filed before the changes to Bugzilla on 2011-10-16. Thus it started right out as NEW without ever being explicitly confirmed. The bug is changed to state NEEDINFO for this reason. To move this bug from NEEDINFO back to NEW please check if the bug still persists with the 3.5.0 beta1 or beta2 prereleases. Details on how to test the 3.5.0 beta1 can be found at: http://wiki.documentfoundation.org/QA/BugHunting_Session_3.5.0.-1 more detail on this bulk operation: http://nabble.documentfoundation.org/RFC-Operation-Spamzilla-tp3607474p3607474.html
An EasyHack should have been checked by developers and thus is confirmed regardless of age. Moving back to NEW from NEEDINFO again. Sorry for the hassle.
CC:ing the author of this and myself
Deleted "Easyhack" from summary.
this one blocks bug 39849 if i read things correctly
I am not working on this. Some info can be found on mailing list. I have some patches but they are not finished.
Maciej: I read about http://nabble.documentfoundation.org/Bug-34465-get-rid-of-all-calls-to-virtual-const-SfxPoolItem-Put-const-SfxPoolItem-amp-USHORT-nWhich-td4017320.html. Perhaps you could attach your current patches so someone may keep on this work? (I'm not speaking about me since I'm not smart enough to do it :) )
Created attachment 82856 [details] Patches Attaching my patches which are a work in progress and should not be committed to master. I am not sure about last two patches, they break things. Here is the list of patches: 0001-Add-CloneAtWhich.patch 0002-changes-in-svx.patch 0003-sc-Replace-two-arg-Put-method-with-one-arg-and-clone.patch 0004-changes-in-svl.patch 0005-sw-Replace-two-arg-Put-method-with-one-arg-and-clone.patch 0006-cui-Replace-two-arg-Put-method-with-one-arg-and-clon.patch 0007-sd-Replace-two-arg-Put-method-with-one-arg-and-clone.patch 0008-editeng-Replace-two-arg-Put-method-with-one-arg-and-.patch 0009-Use-one-arg-Put-method-instead.patch 0010-svl-Replace-two-arg-Put-method-with-one-arg-and-clon.patch 0011-remove-Put-const-SfxPoolItem-sal_uInt16-nWhich.patch 0012-correct-handling-of-disabled-items.patch Master was at eee2fe2e7efe1476d363bfb36b09d7e0d4042438 back then Hope that helps Someone.
adding LibreOffice developer list as CC to unresolved EasyHacks for better visibility. see e.g. http://nabble.documentfoundation.org/minutes-of-ESC-call-td4076214.html for details
SfxItemSet is rather nontrivial ... => UnEasyHackify
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.