ITEM: Diverse further changes/cleanups/preparation

Added a counter for SfxItemSet usages, similar to the
already added one for SfxPoolItems to allow quick info
about evtl. problems/drawbacks of changes.

Replaced enum SfxItemKind in favour of settable boolean flags
directly at SfxPoolItem. These are organized as bitfield, do
not need more space as the enum and allow to be set separately
and multiple ones at the same time.

Flags for PoolDefault/StaticDefault/DeleteOnIdle use this now
and are quickly accessible booleans. It is not a problem that
theoretically the flags for PoolDefault/StaticDefault could now
both be set - this is internal to SfxItem stuff and not accessible
from normal code, so can be managed.

Added for debug build a bitfield boolean m_bDeleted
that will be set in the SfxPoolItem destructor. Of course
it's usability will depend on the freed space not yet being
re-used, but will hopefully help in the debugger to detect
reasons for failure (would have helped at least me before).

Added for replacement of virtual method IsVoidItem() another
bitfield bool m_bIsVoidItem that is set in the constructors
of SfxVoidItem. Also had to add some constructors to do that
which were defaulted before. This is mainly because the base
class SfxPoolItem does *not* have a copy-constructor that
copies the members (flags/RefCnt/WhichID) and we should keep
that 'indirect reset' when Cloning. isVoidItem() is now a simple
boolean member access - the bitfield does the needed masking.
This spares one entry in the virtual function table of
SfxPoolItem which is derived more than 500 times.

Used the results of the experiment at
https://gerrit.libreoffice.org/c/core/+/156774 to change
some accesses to IsVoidItem() to use SfxItemState instead.
This is for preparation of splitting up the two usages of
SfxVoidItems, see commit text in the experiment.
If this shows problems in the future those six places documented
there may have to be changed back to use the new isVoidItem(),
but should also check the ptr this time to be non-zero.

Removed SFX_ITEMS_SPECIAL that is no more used anywhere.

Change-Id: Ib687ca2362d72a4651c75aee0c67029088f68947
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/156805
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>

3 files changed, 52 insertions, 55 deletions

diff --git a/include/svl/itemset.hxx b/include/svl/itemset.hxx
index ba14c21b8e6c..6d95ec505c0f 100644
--- a/include/svl/itemset.hxx
+++ b/include/svl/itemset.hxx
@@ -33,6 +33,11 @@
 
 class SfxItemPool;
 
+#ifdef DBG_UTIL
+SVL_DLLPUBLIC size_t getAllocatedSfxItemSetCount();
+SVL_DLLPUBLIC size_t getUsedSfxItemSetCount();
+#endif
+
 class SAL_WARN_UNUSED SVL_DLLPUBLIC SfxItemSet
 {
     friend class SfxItemIter;
diff --git a/include/svl/poolitem.hxx b/include/svl/poolitem.hxx
index 46dd8b0a76f5..ce4d94e782f9 100644
--- a/include/svl/poolitem.hxx
+++ b/include/svl/poolitem.hxx
@@ -35,17 +35,8 @@
 
 class IntlWrapper;
 
-enum class SfxItemKind : sal_Int8
-{
-   NONE,
-   DeleteOnIdle,
-   StaticDefault,
-   PoolDefault
-};
-
 #define SFX_ITEMS_OLD_MAXREF                0xffef
 #define SFX_ITEMS_MAXREF                    0xfffffffe
-#define SFX_ITEMS_SPECIAL                   0xffffffff
 
 #define CONVERT_TWIPS                       0x80    // Uno conversion for measurement (for MemberId)
 
@@ -124,15 +115,51 @@ friend class SfxItemSet;
 
     mutable sal_uInt32 m_nRefCount;
     sal_uInt16  m_nWhich;
-    SfxItemKind  m_nKind;
+
+    // bitfield for flags (instead of SfxItemKind)
+    bool        m_bIsVoidItem : 1;
+    bool        m_bDeleteOnIdle : 1;
+    bool        m_bStaticDefault : 1;
+    bool        m_bPoolDefault : 1;
+#ifdef DBG_UTIL
+    // this flag will make debugging item stuff much simpler
+    bool        m_bDeleted : 1;
+#endif
 
 private:
-    inline void              SetRefCount(sal_uInt32 n);
-    inline void              SetKind( SfxItemKind n );
+    inline void SetRefCount(sal_uInt32 n)
+    {
+        m_nRefCount = n;
+        m_bStaticDefault = m_bPoolDefault = false;
+    }
+
+protected:
+    void setVoidItem() { m_bIsVoidItem = true; }
+    void setDeleteOnIdle() { m_bDeleteOnIdle = true; }
+    void setStaticDefault() { m_bStaticDefault = true; }
+    void setPoolDefault() { m_bPoolDefault = true; }
+
 public:
-    inline void              AddRef(sal_uInt32 n = 1) const;
+    inline void AddRef(sal_uInt32 n = 1) const
+    {
+        assert(m_nRefCount <= SFX_ITEMS_MAXREF && "AddRef with non-Pool-Item");
+        assert(n <= SFX_ITEMS_MAXREF - m_nRefCount && "AddRef: refcount overflow");
+        m_nRefCount += n;
+    }
+
+    bool isVoidItem() const { return m_bIsVoidItem; }
+    bool isDeleteOnIdle() const { return m_bDeleteOnIdle; }
+    bool isStaticDefault() const { return m_bStaticDefault; }
+    bool isPoolDefault() const { return m_bPoolDefault; }
+
 private:
-    inline sal_uInt32        ReleaseRef(sal_uInt32 n = 1) const;
+    inline sal_uInt32 ReleaseRef(sal_uInt32 n = 1) const
+    {
+        assert(m_nRefCount <= SFX_ITEMS_MAXREF && "ReleaseRef with non-Pool-Item");
+        assert(n <= m_nRefCount);
+        m_nRefCount -= n;
+        return m_nRefCount;
+    }
 
 protected:
                              explicit SfxPoolItem( sal_uInt16 nWhich = 0 );
@@ -226,61 +253,28 @@ public:
     }
 
     sal_uInt32               GetRefCount() const { return m_nRefCount; }
-    SfxItemKind       GetKind() const { return m_nKind; }
     virtual void dumpAsXml(xmlTextWriterPtr pWriter) const;
     virtual boost::property_tree::ptree dumpAsJSON() const;
 
-    /** Only SfxVoidItem shall and must return true for this.
-
-        This avoids costly calls to dynamic_cast<const SfxVoidItem*>()
-        specifically in SfxItemSet::GetItemState()
-     */
-    virtual bool             IsVoidItem() const;
-
 private:
     SfxPoolItem&             operator=( const SfxPoolItem& ) = delete;
 };
 
-inline void SfxPoolItem::SetRefCount(sal_uInt32 n)
-{
-    m_nRefCount = n;
-    m_nKind = SfxItemKind::NONE;
-}
 
-inline void SfxPoolItem::SetKind( SfxItemKind n )
-{
-    m_nRefCount = SFX_ITEMS_SPECIAL;
-    m_nKind = n;
-}
-
-inline void SfxPoolItem::AddRef(sal_uInt32 n) const
-{
-    assert(m_nRefCount <= SFX_ITEMS_MAXREF && "AddRef with non-Pool-Item");
-    assert(n <= SFX_ITEMS_MAXREF - m_nRefCount && "AddRef: refcount overflow");
-    m_nRefCount += n;
-}
-
-inline sal_uInt32 SfxPoolItem::ReleaseRef(sal_uInt32 n) const
-{
-    assert(m_nRefCount <= SFX_ITEMS_MAXREF && "ReleaseRef with non-Pool-Item");
-    assert(n <= m_nRefCount);
-    m_nRefCount -= n;
-    return m_nRefCount;
-}
 
 inline bool IsPoolDefaultItem(const SfxPoolItem *pItem )
 {
-    return pItem && pItem->GetKind() == SfxItemKind::PoolDefault;
+    return pItem && pItem->isPoolDefault();
 }
 
 inline bool IsStaticDefaultItem(const SfxPoolItem *pItem )
 {
-    return pItem && pItem->GetKind() == SfxItemKind::StaticDefault;
+    return pItem && pItem->isStaticDefault();
 }
 
 inline bool IsDefaultItem( const SfxPoolItem *pItem )
 {
-    return pItem && (pItem->GetKind() == SfxItemKind::StaticDefault || pItem->GetKind() == SfxItemKind::PoolDefault);
+    return pItem && (pItem->isPoolDefault() || pItem->isStaticDefault());
 }
 
 inline bool IsPooledItem( const SfxPoolItem *pItem )
diff --git a/include/svl/voiditem.hxx b/include/svl/voiditem.hxx
index b06f28e84b2e..911c691470fb 100644
--- a/include/svl/voiditem.hxx
+++ b/include/svl/voiditem.hxx
@@ -26,11 +26,12 @@ class SVL_DLLPUBLIC SfxVoidItem final : public SfxPoolItem
 {
 public:
     static SfxPoolItem* CreateDefault();
+
     explicit SfxVoidItem(sal_uInt16 nWhich);
+    SfxVoidItem(const SfxVoidItem& rCopy);
+    SfxVoidItem(SfxVoidItem&& rOrig);
     virtual ~SfxVoidItem() override;
 
-    SfxVoidItem(SfxVoidItem const&) = default;
-    SfxVoidItem(SfxVoidItem&&) = default;
     SfxVoidItem& operator=(SfxVoidItem const&) = delete; // due to SfxPoolItem
     SfxVoidItem& operator=(SfxVoidItem&&) = delete; // due to SfxPoolItem
 
@@ -43,9 +44,6 @@ public:
 
     // create a copy of itself
     virtual SfxVoidItem* Clone(SfxItemPool* pPool = nullptr) const override;
-
-    /** Always returns true as this is an SfxVoidItem. */
-    virtual bool IsVoidItem() const override;
 };
 
 #endif