compilerplugins/clang/refcountingbase.cxx | 141 ++++++++++++++++++++++
include/svx/svdotext.hxx | 2
include/svx/svdtext.hxx | 3
svx/inc/sdr/primitive2d/sdrtextprimitive2d.hxx | 3
svx/source/inc/cell.hxx | 3
svx/source/sdr/primitive2d/sdrtextprimitive2d.cxx | 4
svx/source/svdraw/svdotext.cxx | 12 -
svx/source/svdraw/svdtext.cxx | 1
svx/source/table/cell.cxx | 4
9 files changed, 156 insertions(+), 17 deletions(-)
New commits:
commit 3a2176ae256643a3b8c42bca33d333aecad61d19
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Mon Sep 12 17:01:43 2022 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Wed Sep 14 11:22:49 2022 +0200
new loplugin refcountingbase
Look for classes that have more than one ref-counting base class.
A situation which is going to cause trouble.
Which reveals that sdr::table::Cell has two different ref-counting bases,
so rather make SdrText extends OWeakObject, which means
that Cell can just have one ref-counting base,
Change-Id: I8d968270f7b449cff2f29da0bd48fa17181c68c5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/139807
Tested-by: Noel Grandin <noel.gran...@collabora.co.uk>
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/compilerplugins/clang/refcountingbase.cxx
b/compilerplugins/clang/refcountingbase.cxx
new file mode 100644
index 000000000000..dddc7335e8d5
--- /dev/null
+++ b/compilerplugins/clang/refcountingbase.cxx
@@ -0,0 +1,141 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#ifndef LO_CLANG_SHARED_PLUGINS
+
+#include <string>
+#include <iostream>
+#include <map>
+#include <set>
+
+#include "plugin.hxx"
+#include "check.hxx"
+#include "clang/AST/CXXInheritance.h"
+
+/**
+ * Make sure a class does not have multiple reference-counting base classes
+ */
+namespace
+{
+class RefCountingBase : public loplugin::FilteringPlugin<RefCountingBase>
+{
+public:
+ explicit RefCountingBase(loplugin::InstantiationData const& data)
+ : FilteringPlugin(data)
+ {
+ }
+
+ bool preRun() override { return compiler.getLangOpts().CPlusPlus; }
+
+ void run() override
+ {
+ if (preRun())
+ {
+ TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+ }
+ }
+
+ bool VisitCXXRecordDecl(CXXRecordDecl const*);
+};
+
+bool RefCountingBase::VisitCXXRecordDecl(CXXRecordDecl const* recordDecl)
+{
+ if (ignoreLocation(recordDecl))
+ return true;
+ if (!recordDecl->isThisDeclarationADefinition())
+ return true;
+
+ int virtualWeakBase = 0;
+ int virtualOWeakObject = 0;
+ int virtualSimpleReferenceObject = 0;
+ int virtualSvRefBase = 0;
+ int virtualXmlImportContenxt = 0;
+ int virtualVclReferenceBase = 0;
+ int noRefCountingBases = 0;
+ std::string basePaths;
+ auto BaseMatchesCallback = [&](const CXXBaseSpecifier* cxxBaseSpecifier,
CXXBasePath& Paths) {
+ if (!cxxBaseSpecifier->getType().getTypePtr())
+ return false;
+ const CXXRecordDecl* baseCXXRecordDecl =
cxxBaseSpecifier->getType()->getAsCXXRecordDecl();
+ if (!baseCXXRecordDecl)
+ return false;
+ if (baseCXXRecordDecl->isInvalidDecl())
+ return false;
+
+ if (baseCXXRecordDecl->getName() != "WeakBase" // tools::WeakBase
+ && baseCXXRecordDecl->getName() != "OWeakObject" // cppu::WeakBase
+ && baseCXXRecordDecl->getName()
+ != "SimpleReferenceObject" //
salhelper::SimpleReferenceObject
+ && baseCXXRecordDecl->getName() != "SvRefBase" // tool::SvRefBase
+ && baseCXXRecordDecl->getName() != "SvXMLImportContext" // in
xmloff
+ && baseCXXRecordDecl->getName() != "VclReferenceBase") // in vcl
+ return false;
+ if (cxxBaseSpecifier->isVirtual())
+ {
+ if (baseCXXRecordDecl->getName() == "WeakBase")
+ virtualWeakBase = 1;
+ else if (baseCXXRecordDecl->getName() != "OWeakObject")
+ virtualOWeakObject = 1;
+ else if (baseCXXRecordDecl->getName() != "SimpleReferenceObject")
+ virtualSimpleReferenceObject = 1;
+ else if (baseCXXRecordDecl->getName() != "SvRefBase")
+ virtualSvRefBase = 1;
+ else if (baseCXXRecordDecl->getName() != "SvXMLImportContext")
+ virtualXmlImportContenxt = 1;
+ else if (baseCXXRecordDecl->getName() != "VclReferenceBase")
+ virtualVclReferenceBase = 1;
+ else
+ assert(false);
+ }
+ else
+ ++noRefCountingBases;
+ std::string sPath;
+ for (CXXBasePathElement const& pathElement : Paths)
+ {
+ if (!sPath.empty())
+ {
+ sPath += "->";
+ }
+ if (pathElement.Class->hasDefinition())
+ sPath += pathElement.Class->getNameAsString();
+ else
+ sPath += "???";
+ }
+ sPath += "->";
+ sPath += baseCXXRecordDecl->getNameAsString();
+ if (!basePaths.empty())
+ basePaths += ", ";
+ basePaths += sPath;
+ return false;
+ };
+
+ CXXBasePaths aPaths;
+ recordDecl->lookupInBases(BaseMatchesCallback, aPaths);
+
+ int total = virtualWeakBase + virtualOWeakObject +
virtualSimpleReferenceObject
+ + virtualSvRefBase + virtualXmlImportContenxt +
virtualVclReferenceBase
+ + noRefCountingBases;
+ if (total > 1)
+ {
+ report(DiagnosticsEngine::Warning,
+ "this class has multiple copies of a reference-counting base
class, through "
+ "inheritance paths %0",
+ recordDecl->getBeginLoc())
+ << basePaths << recordDecl->getSourceRange();
+ }
+ return true;
+}
+
+loplugin::Plugin::Registration<RefCountingBase>
refcountingbase("refcountingbase", true);
+
+} // namespace
+
+#endif // LO_CLANG_SHARED_PLUGINS
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/include/svx/svdotext.hxx b/include/svx/svdotext.hxx
index edb061ca0802..6b1a61e08a69 100644
--- a/include/svx/svdotext.hxx
+++ b/include/svx/svdotext.hxx
@@ -171,7 +171,7 @@ protected:
GeoStat maGeo;
// this is the active text
- std::unique_ptr<SdrText> mpText;
+ rtl::Reference<SdrText> mxText;
// This contains the dimensions of the text
Size maTextSize;
diff --git a/include/svx/svdtext.hxx b/include/svx/svdtext.hxx
index f8e6a40da8b7..8a76e435b8e6 100644
--- a/include/svx/svdtext.hxx
+++ b/include/svx/svdtext.hxx
@@ -22,7 +22,6 @@
#include <editeng/outlobj.hxx>
#include <svx/sdr/properties/defaultproperties.hxx>
#include <svx/svxdllapi.h>
-#include <tools/weakbase.hxx>
class OutlinerParaObject;
class SdrOutliner;
@@ -40,7 +39,7 @@ class TextProperties;
*/
class SfxStyleSheet;
-class SVXCORE_DLLPUBLIC SdrText : public tools::WeakBase
+class SVXCORE_DLLPUBLIC SdrText : public ::cppu::OWeakObject
{
public:
explicit SdrText(SdrTextObj& rObject);
diff --git a/svx/inc/sdr/primitive2d/sdrtextprimitive2d.hxx
b/svx/inc/sdr/primitive2d/sdrtextprimitive2d.hxx
index bc44ca0096ee..dfe66d3b601a 100644
--- a/svx/inc/sdr/primitive2d/sdrtextprimitive2d.hxx
+++ b/svx/inc/sdr/primitive2d/sdrtextprimitive2d.hxx
@@ -30,6 +30,7 @@
#include <tools/weakbase.h>
#include <svx/sdtaitm.hxx>
#include <rtl/ref.hxx>
+#include <unotools/weakref.hxx>
// predefines
@@ -43,7 +44,7 @@ namespace drawinglayer::primitive2d
private:
// The text model data; this should later just be the
OutlinerParaObject or
// something equal
- ::tools::WeakReference< SdrText > mrSdrText;
+ ::unotools::WeakReference< SdrText > mxSdrText;
// #i97628#
// The text content; now as local OutlinerParaObject copy
(internally RefCounted and
diff --git a/svx/source/inc/cell.hxx b/svx/source/inc/cell.hxx
index 36ce254d1aa7..331465f4eaa3 100644
--- a/svx/source/inc/cell.hxx
+++ b/svx/source/inc/cell.hxx
@@ -46,8 +46,7 @@ class UNLESS_MERGELIBS(SVXCORE_DLLPUBLIC) Cell final : public
SdrText,
public SvxUnoTextBase,
public css::table::XMergeableCell,
public css::awt::XLayoutConstrains,
- public css::lang::XEventListener,
- public ::cppu::OWeakObject
+ public css::lang::XEventListener
{
friend class CellUndo;
diff --git a/svx/source/sdr/primitive2d/sdrtextprimitive2d.cxx
b/svx/source/sdr/primitive2d/sdrtextprimitive2d.cxx
index 1d44b8fbf7db..8e67f32b1e6b 100644
--- a/svx/source/sdr/primitive2d/sdrtextprimitive2d.cxx
+++ b/svx/source/sdr/primitive2d/sdrtextprimitive2d.cxx
@@ -97,7 +97,7 @@ namespace drawinglayer::primitive2d
SdrTextPrimitive2D::SdrTextPrimitive2D(
const SdrText* pSdrText,
OutlinerParaObject aOutlinerParaObject)
- : mrSdrText(const_cast< SdrText* >(pSdrText)),
+ : mxSdrText(const_cast< SdrText* >(pSdrText)),
maOutlinerParaObject(std::move(aOutlinerParaObject)),
mnLastPageNumber(0),
mnLastPageCount(0),
@@ -115,7 +115,7 @@ namespace drawinglayer::primitive2d
|| rETO.HasField(SvxAuthorField::CLASS_ID);
}
- const SdrText* SdrTextPrimitive2D::getSdrText() const { return
mrSdrText.get(); }
+ const SdrText* SdrTextPrimitive2D::getSdrText() const { return
mxSdrText.get().get(); }
bool SdrTextPrimitive2D::operator==(const BasePrimitive2D& rPrimitive)
const
{
diff --git a/svx/source/svdraw/svdotext.cxx b/svx/source/svdraw/svdotext.cxx
index 24223aaee738..53b32ddf28b2 100644
--- a/svx/source/svdraw/svdotext.cxx
+++ b/svx/source/svdraw/svdotext.cxx
@@ -193,7 +193,7 @@ SdrTextObj::SdrTextObj(SdrModel& rSdrModel, SdrObjKind
eNewTextKind,
SdrTextObj::~SdrTextObj()
{
- mpText.reset();
+ mxText.clear();
ImpDeregisterLink();
}
@@ -2047,10 +2047,10 @@ rtl::Reference<SdrObject>
SdrTextObj::getFullDragClone() const
/** returns the currently active text. */
SdrText* SdrTextObj::getActiveText() const
{
- if( !mpText )
+ if( !mxText )
return getText( 0 );
else
- return mpText.get();
+ return mxText.get();
}
/** returns the nth available text. */
@@ -2058,9 +2058,9 @@ SdrText* SdrTextObj::getText( sal_Int32 nIndex ) const
{
if( nIndex == 0 )
{
- if( !mpText )
- const_cast< SdrTextObj* >(this)->mpText.reset( new SdrText(
*const_cast< SdrTextObj* >(this) ) );
- return mpText.get();
+ if( !mxText )
+ const_cast< SdrTextObj* >(this)->mxText = new SdrText(
*const_cast< SdrTextObj* >(this) );
+ return mxText.get();
}
else
{
diff --git a/svx/source/svdraw/svdtext.cxx b/svx/source/svdraw/svdtext.cxx
index 72591ef67d92..e013620b7d06 100644
--- a/svx/source/svdraw/svdtext.cxx
+++ b/svx/source/svdraw/svdtext.cxx
@@ -36,7 +36,6 @@ SdrText::SdrText( SdrTextObj& rObject )
SdrText::~SdrText()
{
- clearWeak();
}
void SdrText::CheckPortionInfo( const SdrOutliner& rOutliner )
diff --git a/svx/source/table/cell.cxx b/svx/source/table/cell.cxx
index 9922952fc780..a358865334d9 100644
--- a/svx/source/table/cell.cxx
+++ b/svx/source/table/cell.cxx
@@ -866,13 +866,13 @@ Any SAL_CALL Cell::queryInterface( const Type & rType )
void SAL_CALL Cell::acquire() noexcept
{
- ::cppu::OWeakObject::acquire();
+ SdrText::acquire();
}
void SAL_CALL Cell::release() noexcept
{
- ::cppu::OWeakObject::release();
+ SdrText::release();
}
