[Libreoffice-commits] core.git: compilerplugins/clang include/svx include/tools solenv/CompilerTest_compilerplugins_clang.mk svx/source

Noel Grandin (via logerrit) logerrit at kemper.freedesktop.org
Wed Apr 24 14:10:16 UTC 2019


 compilerplugins/clang/test/weakbase.cxx      |   34 ++++++++
 compilerplugins/clang/weakbase.cxx           |  110 +++++++++++++++++++++++++++
 include/svx/svdmodel.hxx                     |    2 
 include/svx/svdobj.hxx                       |    2 
 include/svx/svdpage.hxx                      |    2 
 include/svx/svdtext.hxx                      |    2 
 include/svx/svdview.hxx                      |    2 
 include/tools/weakbase.hxx                   |    7 -
 solenv/CompilerTest_compilerplugins_clang.mk |    1 
 svx/source/svdraw/svdpage.cxx                |    4 
 10 files changed, 156 insertions(+), 10 deletions(-)

New commits:
commit bd44b3eef62f4325a189539d6ab1b90ca63cfc28
Author:     Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Wed Apr 24 13:40:59 2019 +0200
Commit:     Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Wed Apr 24 16:09:08 2019 +0200

    tdf#89522 PERF FILEOPEN xlsx, part 1
    
    Inherit from tools::WeakBase non-virtually, so that we can use a
    static_cast in tools::WeakReference::get instead of a dynamic_cast.
    
    This takes the file-open time from 1m21 to 40s for me.
    
    Add a clang plugin to make sure we don't accidentally end up inheriting
    from tools::WeakBase more than once.
    
    Change-Id: I9c7c36403333f99094e1f9d8cce2ecd9200377f9
    Reviewed-on: https://gerrit.libreoffice.org/71231
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
    Tested-by: Noel Grandin <noel.grandin at collabora.co.uk>

diff --git a/compilerplugins/clang/test/weakbase.cxx b/compilerplugins/clang/test/weakbase.cxx
new file mode 100644
index 000000000000..a59a5372891e
--- /dev/null
+++ b/compilerplugins/clang/test/weakbase.cxx
@@ -0,0 +1,34 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * 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/.
+ */
+
+namespace tools
+{
+struct WeakBase
+{
+    virtual ~WeakBase();
+};
+}
+
+struct Foo1 : public tools::WeakBase
+{
+    virtual ~Foo1();
+};
+
+struct Foo2 : public tools::WeakBase
+{
+    virtual ~Foo2();
+};
+
+// expected-error at +1 {{multiple copies of WeakBase, through inheritance paths Bar->Foo1->WeakBase, Bar->Foo2->WeakBase [loplugin:weakbase]}}
+struct Bar : public Foo1, public Foo2
+{
+    virtual ~Bar();
+};
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/weakbase.cxx b/compilerplugins/clang/weakbase.cxx
new file mode 100644
index 000000000000..fbd72955c0ba
--- /dev/null
+++ b/compilerplugins/clang/weakbase.cxx
@@ -0,0 +1,110 @@
+/* -*- 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/.
+ */
+
+#include <string>
+#include <iostream>
+#include <map>
+#include <set>
+
+#include "plugin.hxx"
+#include "clang/AST/CXXInheritance.h"
+
+/**
+ * Check for multiple copies of WeakBase in base classes
+ */
+namespace
+{
+class WeakBase : public loplugin::FilteringPlugin<WeakBase>
+{
+public:
+    explicit WeakBase(loplugin::InstantiationData const& data)
+        : FilteringPlugin(data)
+    {
+    }
+
+    virtual void run() override { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
+
+    bool VisitCXXRecordDecl(CXXRecordDecl const*);
+};
+
+bool WeakBase::VisitCXXRecordDecl(CXXRecordDecl const* recordDecl)
+{
+    if (ignoreLocation(recordDecl))
+    {
+        return true;
+    }
+    //    StringRef aFileName = getFileNameOfSpellingLoc(
+    //        compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(fieldDecl)));
+
+    //    if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/chart2/source/"))
+    //        return true;
+    //    if (loplugin::isSamePathname(aFileName, SRCDIR "/include/sfx2/recentdocsview.hxx"))
+    //        return true;
+    //    if (loplugin::isSamePathname(aFileName, SRCDIR "/include/sfx2/templatelocalview.hxx"))
+    //        return true;
+    //    if (loplugin::isSamePathname(aFileName, SRCDIR "/store/source/stortree.hxx")
+    //        || loplugin::isSamePathname(aFileName, SRCDIR "/store/source/stordata.hxx"))
+    //        return true;
+    //    if (loplugin::isSamePathname(aFileName, SRCDIR "/sw/source/uibase/inc/dbtree.hxx"))
+    //        return true;
+
+    recordDecl = recordDecl->getCanonicalDecl();
+    if (!recordDecl->hasDefinition())
+        return true;
+
+    int noWeakBases = 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")
+            return false;
+        ++noWeakBases;
+        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);
+
+    if (noWeakBases > 1)
+    {
+        report(DiagnosticsEngine::Warning,
+               "multiple copies of WeakBase, through inheritance paths %0",
+               compat::getBeginLoc(recordDecl))
+            << basePaths << recordDecl->getSourceRange();
+    }
+    return true;
+}
+
+loplugin::Plugin::Registration<WeakBase> WeakBase("weakbase", true);
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/include/svx/svdmodel.hxx b/include/svx/svdmodel.hxx
index 9dad1baa8cc1..e2dc48461c78 100644
--- a/include/svx/svdmodel.hxx
+++ b/include/svx/svdmodel.hxx
@@ -143,7 +143,7 @@ public:
 
 struct SdrModelImpl;
 
-class SVX_DLLPUBLIC SdrModel : public SfxBroadcaster, public virtual tools::WeakBase
+class SVX_DLLPUBLIC SdrModel : public SfxBroadcaster, public tools::WeakBase
 {
 private:
 #ifdef DBG_UTIL
diff --git a/include/svx/svdobj.hxx b/include/svx/svdobj.hxx
index 9de436daa7e8..0d2d836f8efc 100644
--- a/include/svx/svdobj.hxx
+++ b/include/svx/svdobj.hxx
@@ -304,7 +304,7 @@ public:
 //      SwFlyDrawObj
 
 /// Abstract DrawObject
-class SVX_DLLPUBLIC SdrObject : public SfxListener, public virtual tools::WeakBase
+class SVX_DLLPUBLIC SdrObject : public SfxListener, public tools::WeakBase
 {
 private:
     friend class                SdrObjListIter;
diff --git a/include/svx/svdpage.hxx b/include/svx/svdpage.hxx
index 24747bc823a9..d23957dcd906 100644
--- a/include/svx/svdpage.hxx
+++ b/include/svx/svdpage.hxx
@@ -364,7 +364,7 @@ public:
 //          SwDPage
 //      OReportPage
 
-class SVX_DLLPUBLIC SdrPage : public SdrObjList, public virtual tools::WeakBase
+class SVX_DLLPUBLIC SdrPage : public SdrObjList, public tools::WeakBase
 {
     // #i9076#
     friend class SdrModel;
diff --git a/include/svx/svdtext.hxx b/include/svx/svdtext.hxx
index 1f96268c7e88..a02d9390ef3e 100644
--- a/include/svx/svdtext.hxx
+++ b/include/svx/svdtext.hxx
@@ -41,7 +41,7 @@ namespace sdr { namespace properties {
 */
 
 class SfxStyleSheet;
-class SVX_DLLPUBLIC SdrText : public virtual tools::WeakBase
+class SVX_DLLPUBLIC SdrText : public tools::WeakBase
 {
 public:
     explicit SdrText( SdrTextObj& rObject );
diff --git a/include/svx/svdview.hxx b/include/svx/svdview.hxx
index c0d3b21a8f42..176d6e6146b5 100644
--- a/include/svx/svdview.hxx
+++ b/include/svx/svdview.hxx
@@ -146,7 +146,7 @@ public:
 };
 
 
-class SVX_DLLPUBLIC SdrView : public SdrCreateView, public virtual tools::WeakBase
+class SVX_DLLPUBLIC SdrView : public SdrCreateView, public tools::WeakBase
 {
     friend class                SdrPageView;
 
diff --git a/include/tools/weakbase.hxx b/include/tools/weakbase.hxx
index 9fe3f32775c3..fa16edd2f147 100644
--- a/include/tools/weakbase.hxx
+++ b/include/tools/weakbase.hxx
@@ -61,9 +61,10 @@ template< class reference_type >
 inline reference_type * WeakReference< reference_type >::get() const
 {
     auto pWeakBase = mpWeakConnection->mpReference;
-    auto pRet = dynamic_cast<reference_type *>(pWeakBase);
-    assert((pWeakBase && pRet) || (!pWeakBase && !pRet));
-    return pRet;
+    if (!pWeakBase)
+        return nullptr;
+    assert(dynamic_cast<reference_type *>(pWeakBase));
+    return static_cast<reference_type *>(pWeakBase);
 }
 
 template< class reference_type >
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk
index ff71625bc3a5..fbc2d8a8d452 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -79,6 +79,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
     compilerplugins/clang/test/unusedvariablemore \
     compilerplugins/clang/test/useuniqueptr \
     compilerplugins/clang/test/vclwidgets \
+    compilerplugins/clang/test/weakbase \
     compilerplugins/clang/test/writeonlyvars \
 ))
 
diff --git a/svx/source/svdraw/svdpage.cxx b/svx/source/svdraw/svdpage.cxx
index 85b9e79b62b4..49c3355bac9c 100644
--- a/svx/source/svdraw/svdpage.cxx
+++ b/svx/source/svdraw/svdpage.cxx
@@ -1113,8 +1113,8 @@ void SdrPageProperties::SetStyleSheet(SfxStyleSheet* pStyleSheet)
 
 
 SdrPage::SdrPage(SdrModel& rModel, bool bMasterPage)
-:   tools::WeakBase(),
-    SdrObjList(),
+:   SdrObjList(),
+    tools::WeakBase(),
     maPageUsers(),
     mrSdrModelFromSdrPage(rModel),
     mnWidth(10),


More information about the Libreoffice-commits mailing list