[Libreoffice-commits] core.git: compilerplugins/clang i18npool/inc svx/qa svx/source xmloff/qa
Noel (via logerrit)
logerrit at kemper.freedesktop.org
Wed Feb 10 08:19:32 UTC 2021
compilerplugins/clang/refcounting.cxx | 131 +++++++++++++++++++++--------
compilerplugins/clang/test/refcounting.cxx | 25 +++--
i18npool/inc/cclass_unicode.hxx | 3
svx/qa/unit/XTableImportExportTest.cxx | 12 +-
svx/source/tbxctrls/fillctrl.cxx | 20 ++--
xmloff/qa/unit/uxmloff.cxx | 8 -
6 files changed, 137 insertions(+), 62 deletions(-)
New commits:
commit dcb1022362acb2e794bae79d8827557702dcffd7
Author: Noel <noel.grandin at collabora.co.uk>
AuthorDate: Wed Feb 10 08:52:43 2021 +0200
Commit: Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Wed Feb 10 09:18:53 2021 +0100
loplugin:refcounting also check OWeakObject subclasses
Change-Id: I2d89085a22d7424c6f8f7662307433ce50fc61d2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/110666
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/compilerplugins/clang/refcounting.cxx b/compilerplugins/clang/refcounting.cxx
index 31b1540d391e..ae8a2b217bb5 100644
--- a/compilerplugins/clang/refcounting.cxx
+++ b/compilerplugins/clang/refcounting.cxx
@@ -201,6 +201,38 @@ bool containsXInterfaceSubclass(const clang::Type* pType0) {
}
}
+bool containsOWeakObjectSubclass(const clang::Type* pType0);
+
+bool containsOWeakObjectSubclass(const QualType& qType) {
+ return containsOWeakObjectSubclass(qType.getTypePtr());
+}
+
+bool containsOWeakObjectSubclass(const clang::Type* pType0) {
+ if (!pType0)
+ return false;
+ const clang::Type* pType = pType0->getUnqualifiedDesugaredType();
+ if (!pType)
+ return false;
+ const CXXRecordDecl* pRecordDecl = pType->getAsCXXRecordDecl();
+ if (pRecordDecl) {
+ // because dbaccess just has to be special...
+ loplugin::DeclCheck dc(pRecordDecl);
+ if ((dc.Class("DocumentEvents").Namespace("dbaccess")
+ .GlobalNamespace()))
+ return false;
+ }
+ if (pType->isPointerType()) {
+ // ignore
+ return false;
+ } else if (pType->isArrayType()) {
+ const clang::ArrayType* pArrayType = dyn_cast<clang::ArrayType>(pType);
+ QualType elementType = pArrayType->getElementType();
+ return containsOWeakObjectSubclass(elementType);
+ } else {
+ return loplugin::isDerivedFrom(pRecordDecl, [](Decl const * decl) -> bool { return bool(loplugin::DeclCheck(decl).Class("OWeakObject").Namespace("cppu").GlobalNamespace()); });
+ }
+}
+
bool containsSvRefBaseSubclass(const clang::Type* pType0) {
if (!pType0)
return false;
@@ -361,6 +393,14 @@ bool RefCounting::visitTemporaryObjectExpr(Expr const * expr) {
" css::uno::Reference"),
compat::getBeginLoc(expr))
<< t.getUnqualifiedType() << expr->getSourceRange();
+ } else if (containsOWeakObjectSubclass(t)) {
+ report(
+ DiagnosticsEngine::Warning,
+ ("Temporary object of cppu::OWeakObject subclass %0 being"
+ " directly stack managed, should be managed via"
+ " css::uno::Reference"),
+ compat::getBeginLoc(expr))
+ << t.getUnqualifiedType() << expr->getSourceRange();
}
return true;
}
@@ -384,9 +424,9 @@ bool RefCounting::VisitFieldDecl(const FieldDecl * fieldDecl) {
QualType firstTemplateParamType;
if (auto recordType = fieldDecl->getType()->getUnqualifiedDesugaredType()->getAs<RecordType>()) {
auto const tc = loplugin::TypeCheck(fieldDecl->getType());
- if (tc.Class("unique_ptr").StdNamespace()
- || tc.Class("shared_ptr").StdNamespace()
- || tc.Class("intrusive_ptr").Namespace("boost").GlobalNamespace())
+ if (tc.ClassOrStruct("unique_ptr").StdNamespace()
+ || tc.ClassOrStruct("shared_ptr").StdNamespace()
+ || tc.ClassOrStruct("intrusive_ptr").Namespace("boost").GlobalNamespace())
{
auto templateDecl = dyn_cast<ClassTemplateSpecializationDecl>(recordType->getDecl());
if (templateDecl && templateDecl->getTemplateArgs().size() > 0)
@@ -478,6 +518,18 @@ bool RefCounting::VisitFieldDecl(const FieldDecl * fieldDecl) {
}
#endif
+ if (!firstTemplateParamType.isNull() && containsOWeakObjectSubclass(firstTemplateParamType.getTypePtr()))
+ {
+ report(
+ DiagnosticsEngine::Warning,
+ "cppu::OWeakObject subclass %0 being managed via smart pointer, should be managed via rtl::Reference, "
+ "parent is %1",
+ fieldDecl->getLocation())
+ << firstTemplateParamType
+ << fieldDecl->getParent()
+ << fieldDecl->getSourceRange();
+ }
+
checkUnoReference(
fieldDecl->getType(), fieldDecl,
fieldDecl->getParent(), "field");
@@ -490,38 +542,49 @@ bool RefCounting::VisitVarDecl(const VarDecl * varDecl) {
if (ignoreLocation(varDecl)) {
return true;
}
- if (!isa<ParmVarDecl>(varDecl)) {
- if (containsSvRefBaseSubclass(varDecl->getType().getTypePtr())) {
- report(
- DiagnosticsEngine::Warning,
- "SvRefBase subclass being directly stack managed, should be managed via tools::SvRef, "
- + varDecl->getType().getAsString(),
- varDecl->getLocation())
- << varDecl->getSourceRange();
- }
- if (containsSalhelperReferenceObjectSubclass(varDecl->getType().getTypePtr())) {
- StringRef name { getFilenameOfLocation(
- compiler.getSourceManager().getSpellingLoc(varDecl->getLocation())) };
- // this is playing games that it believes is safe
- if (loplugin::isSamePathname(name, SRCDIR "/stoc/source/security/permissions.cxx"))
- return true;
- report(
- DiagnosticsEngine::Warning,
- "salhelper::SimpleReferenceObject subclass being directly stack managed, should be managed via rtl::Reference, "
- + varDecl->getType().getAsString(),
- varDecl->getLocation())
- << varDecl->getSourceRange();
- }
- if (containsXInterfaceSubclass(varDecl->getType())) {
- report(
- DiagnosticsEngine::Warning,
- "XInterface subclass being directly stack managed, should be managed via uno::Reference, "
- + varDecl->getType().getAsString(),
- varDecl->getLocation())
- << varDecl->getSourceRange();
- }
- }
+
checkUnoReference(varDecl->getType(), varDecl, nullptr, "var");
+
+ if (isa<ParmVarDecl>(varDecl))
+ return true;
+
+ if (containsSvRefBaseSubclass(varDecl->getType().getTypePtr())) {
+ report(
+ DiagnosticsEngine::Warning,
+ "SvRefBase subclass being directly stack managed, should be managed via tools::SvRef, "
+ + varDecl->getType().getAsString(),
+ varDecl->getLocation())
+ << varDecl->getSourceRange();
+ }
+ if (containsSalhelperReferenceObjectSubclass(varDecl->getType().getTypePtr())) {
+ StringRef name { getFilenameOfLocation(
+ compiler.getSourceManager().getSpellingLoc(varDecl->getLocation())) };
+ // this is playing games that it believes is safe
+ if (loplugin::isSamePathname(name, SRCDIR "/stoc/source/security/permissions.cxx"))
+ return true;
+ report(
+ DiagnosticsEngine::Warning,
+ "salhelper::SimpleReferenceObject subclass being directly stack managed, should be managed via rtl::Reference, "
+ + varDecl->getType().getAsString(),
+ varDecl->getLocation())
+ << varDecl->getSourceRange();
+ }
+ if (containsXInterfaceSubclass(varDecl->getType())) {
+ report(
+ DiagnosticsEngine::Warning,
+ "XInterface subclass being directly stack managed, should be managed via uno::Reference, "
+ + varDecl->getType().getAsString(),
+ varDecl->getLocation())
+ << varDecl->getSourceRange();
+ }
+ if (containsOWeakObjectSubclass(varDecl->getType())) {
+ report(
+ DiagnosticsEngine::Warning,
+ "cppu::OWeakObject subclass being directly stack managed, should be managed via uno::Reference, "
+ + varDecl->getType().getAsString(),
+ varDecl->getLocation())
+ << varDecl->getSourceRange();
+ }
return true;
}
diff --git a/compilerplugins/clang/test/refcounting.cxx b/compilerplugins/clang/test/refcounting.cxx
index 4bcb03e2eef6..911d0461dd41 100644
--- a/compilerplugins/clang/test/refcounting.cxx
+++ b/compilerplugins/clang/test/refcounting.cxx
@@ -10,19 +10,30 @@
#include <sal/config.h>
#include <memory>
+#include <rtl/ref.hxx>
#include <boost/intrusive_ptr.hpp>
#include <com/sun/star/uno/XInterface.hpp>
-// expected-no-diagnostics
+namespace cppu
+{
+class OWeakObject
+{
+};
+}
+
+struct UnoObject : public cppu::OWeakObject
+{
+};
struct Foo
{
-// Not in general (dbaccess::DocumentEvents, dbaccess/source/core/dataaccess/databasedocument.hxx):
-#if 0
- std::unique_ptr<css::uno::XInterface> m_foo1; // expected-error {{XInterface subclass 'com::sun::star::uno::XInterface' being managed via smart pointer, should be managed via uno::Reference, parent is 'Foo' [loplugin:refcounting]}}
- std::shared_ptr<css::uno::XInterface> m_foo2; // expected-error {{XInterface subclass 'com::sun::star::uno::XInterface' being managed via smart pointer, should be managed via uno::Reference, parent is 'Foo' [loplugin:refcounting]}}
- boost::intrusive_ptr<css::uno::XInterface> m_foo3; // expected-error {{XInterface subclass 'com::sun::star::uno::XInterface' being managed via smart pointer, should be managed via uno::Reference, parent is 'Foo' [loplugin:refcounting]}}
-#endif
+ std::unique_ptr<UnoObject>
+ m_foo1; // expected-error {{cppu::OWeakObject subclass 'UnoObject' being managed via smart pointer, should be managed via rtl::Reference, parent is 'Foo' [loplugin:refcounting]}}
+ std::shared_ptr<UnoObject>
+ m_foo2; // expected-error {{cppu::OWeakObject subclass 'UnoObject' being managed via smart pointer, should be managed via rtl::Reference, parent is 'Foo' [loplugin:refcounting]}}
+ boost::intrusive_ptr<UnoObject>
+ m_foo3; // expected-error {{cppu::OWeakObject subclass 'UnoObject' being managed via smart pointer, should be managed via rtl::Reference, parent is 'Foo' [loplugin:refcounting]}}
+ rtl::Reference<UnoObject> m_foo4; // no warning expected
};
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/i18npool/inc/cclass_unicode.hxx b/i18npool/inc/cclass_unicode.hxx
index 03a084b01c5e..704deabc46b5 100644
--- a/i18npool/inc/cclass_unicode.hxx
+++ b/i18npool/inc/cclass_unicode.hxx
@@ -22,6 +22,7 @@
#include <com/sun/star/i18n/XCharacterClassification.hpp>
#include <cppuhelper/implbase.hxx>
#include <com/sun/star/lang/XServiceInfo.hpp>
+#include <rtl/ref.hxx>
#include <o3tl/typed_flags_set.hxx>
#include <memory>
@@ -94,7 +95,7 @@ public:
virtual css::uno::Sequence< OUString > SAL_CALL getSupportedServiceNames() override;
private:
- std::unique_ptr<Transliteration_casemapping> trans;
+ rtl::Reference<Transliteration_casemapping> trans;
// --- parser specific (implemented in cclass_unicode_parser.cxx) ---
diff --git a/svx/qa/unit/XTableImportExportTest.cxx b/svx/qa/unit/XTableImportExportTest.cxx
index 31962a52be52..b4c071204005 100644
--- a/svx/qa/unit/XTableImportExportTest.cxx
+++ b/svx/qa/unit/XTableImportExportTest.cxx
@@ -41,8 +41,8 @@ CPPUNIT_TEST_FIXTURE(XTableImportExportTest, testImportExport)
BitmapChecksum aChecksum(0);
{
- XBitmapList xBitmapList(aTempURL, "REF");
- uno::Reference<container::XNameContainer> xNameContainer(xBitmapList.createInstance());
+ rtl::Reference<XBitmapList> xBitmapList = new XBitmapList(aTempURL, "REF");
+ uno::Reference<container::XNameContainer> xNameContainer(xBitmapList->createInstance());
CPPUNIT_ASSERT(xNameContainer.is());
Bitmap aBitmap(Size(5, 5), 24);
@@ -52,16 +52,16 @@ CPPUNIT_TEST_FIXTURE(XTableImportExportTest, testImportExport)
uno::Reference<awt::XBitmap> xBitmap(aGraphic.GetXGraphic(), css::uno::UNO_QUERY);
xNameContainer->insertByName("SomeBitmap", uno::makeAny(xBitmap));
- xBitmapList.Save();
+ xBitmapList->Save();
aChecksum = aBitmap.GetChecksum();
}
{
- XBitmapList xBitmapList(aTempURL, "REF");
- bool bResult = xBitmapList.Load();
+ rtl::Reference<XBitmapList> xBitmapList = new XBitmapList(aTempURL, "REF");
+ bool bResult = xBitmapList->Load();
CPPUNIT_ASSERT(bResult);
- uno::Reference<container::XNameContainer> xNameContainer(xBitmapList.createInstance());
+ uno::Reference<container::XNameContainer> xNameContainer(xBitmapList->createInstance());
CPPUNIT_ASSERT(xNameContainer.is());
uno::Any aAny = xNameContainer->getByName("SomeBitmap");
diff --git a/svx/source/tbxctrls/fillctrl.cxx b/svx/source/tbxctrls/fillctrl.cxx
index 7a6cf136d99d..94555daae594 100644
--- a/svx/source/tbxctrls/fillctrl.cxx
+++ b/svx/source/tbxctrls/fillctrl.cxx
@@ -384,10 +384,10 @@ void SvxFillToolBoxControl::Update()
}
aTmpStr = TMP_STR_BEGIN + aString + TMP_STR_END;
- XGradientList aGradientList( "", ""/*TODO?*/ );
- aGradientList.Insert(std::make_unique<XGradientEntry>(mpFillGradientItem->GetGradientValue(), aTmpStr));
- aGradientList.SetDirty( false );
- const BitmapEx aBmp = aGradientList.GetUiBitmap( 0 );
+ rtl::Reference<XGradientList> xGradientList = new XGradientList( "", ""/*TODO?*/ );
+ xGradientList->Insert(std::make_unique<XGradientEntry>(mpFillGradientItem->GetGradientValue(), aTmpStr));
+ xGradientList->SetDirty( false );
+ const BitmapEx aBmp = xGradientList->GetUiBitmap( 0 );
if (!aBmp.IsEmpty())
{
@@ -395,7 +395,7 @@ void SvxFillToolBoxControl::Update()
const Size aBmpSize(aBmp.GetSizePixel());
pVD->SetOutputSizePixel(aBmpSize, false);
pVD->DrawBitmapEx(Point(), aBmp);
- mpLbFillAttr->append("", aGradientList.Get(0)->GetName(), *pVD);
+ mpLbFillAttr->append("", xGradientList->Get(0)->GetName(), *pVD);
mpLbFillAttr->set_active(mpLbFillAttr->get_count() - 1);
}
}
@@ -447,10 +447,10 @@ void SvxFillToolBoxControl::Update()
}
aTmpStr = TMP_STR_BEGIN + aString + TMP_STR_END;
- XHatchList aHatchList( "", ""/*TODO?*/ );
- aHatchList.Insert(std::make_unique<XHatchEntry>(mpHatchItem->GetHatchValue(), aTmpStr));
- aHatchList.SetDirty( false );
- const BitmapEx & aBmp = aHatchList.GetUiBitmap( 0 );
+ rtl::Reference<XHatchList> xHatchList = new XHatchList( "", ""/*TODO?*/ );
+ xHatchList->Insert(std::make_unique<XHatchEntry>(mpHatchItem->GetHatchValue(), aTmpStr));
+ xHatchList->SetDirty( false );
+ const BitmapEx & aBmp = xHatchList->GetUiBitmap( 0 );
if( !aBmp.IsEmpty() )
{
@@ -458,7 +458,7 @@ void SvxFillToolBoxControl::Update()
const Size aBmpSize(aBmp.GetSizePixel());
pVD->SetOutputSizePixel(aBmpSize, false);
pVD->DrawBitmapEx(Point(), aBmp);
- mpLbFillAttr->append("", aHatchList.GetHatch(0)->GetName(), *pVD);
+ mpLbFillAttr->append("", xHatchList->GetHatch(0)->GetName(), *pVD);
mpLbFillAttr->set_active(mpLbFillAttr->get_count() - 1);
}
}
diff --git a/xmloff/qa/unit/uxmloff.cxx b/xmloff/qa/unit/uxmloff.cxx
index 273eed259afe..b5aff93d02be 100644
--- a/xmloff/qa/unit/uxmloff.cxx
+++ b/xmloff/qa/unit/uxmloff.cxx
@@ -44,7 +44,7 @@ public:
CPPUNIT_TEST(testMetaGenerator);
CPPUNIT_TEST_SUITE_END();
private:
- std::unique_ptr<SvXMLExport> pExport;
+ rtl::Reference<SvXMLExport> pExport;
};
Test::Test()
@@ -55,14 +55,14 @@ void Test::setUp()
{
BootstrapFixture::setUp();
- pExport.reset(new SchXMLExport(
+ pExport = new SchXMLExport(
comphelper::getProcessComponentContext(), "SchXMLExport.Compact",
- SvXMLExportFlags::ALL));
+ SvXMLExportFlags::ALL);
}
void Test::tearDown()
{
- pExport.reset();
+ pExport.clear();
BootstrapFixture::tearDown();
}
More information about the Libreoffice-commits
mailing list