loplugin, merge weakobject into weakbase

and make them support virtual bases, even though the bridge code does
not support that (yet)

Change-Id: I247e795391fa452dea2922869b15ab043eb2bdd1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/156941
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

4 files changed, 113 insertions, 128 deletions

diff --git a/compilerplugins/clang/test/weakbase.cxx b/compilerplugins/clang/test/weakbase.cxx
index a59a5372891e..3d5284ef543c 100644
--- a/compilerplugins/clang/test/weakbase.cxx
+++ b/compilerplugins/clang/test/weakbase.cxx
@@ -25,10 +25,59 @@ struct Foo2 : public tools::WeakBase
     virtual ~Foo2();
 };
 
-// expected-error@+1 {{multiple copies of WeakBase, through inheritance paths Bar->Foo1->WeakBase, Bar->Foo2->WeakBase [loplugin:weakbase]}}
+// expected-error@+1 {{found multiple copies of tools::WeakBase, through inheritance paths Bar->Foo1->WeakBase, Bar->Foo2->WeakBase [loplugin:weakbase]}}
 struct Bar : public Foo1, public Foo2
 {
     virtual ~Bar();
 };
 
+namespace cppu
+{
+class OWeakObject
+{
+};
+}
+
+namespace test2
+{
+class Foo1 : public cppu::OWeakObject
+{
+};
+class Foo2 : public cppu::OWeakObject
+{
+};
+// expected-error@+1 {{found multiple copies of cppu::OWeakObject, through inheritance paths Foo3->Foo1->OWeakObject, Foo3->Foo2->OWeakObject [loplugin:weakbase]}}
+class Foo3 : public Foo1, public Foo2
+{
+};
+}
+
+namespace test3
+{
+class Foo1 : public virtual cppu::OWeakObject
+{
+};
+class Foo2 : public virtual cppu::OWeakObject
+{
+};
+// no warning expected
+class Foo3 : public Foo1, public Foo2
+{
+};
+}
+
+namespace test4
+{
+class Foo1 : public cppu::OWeakObject
+{
+};
+class Foo2 : public virtual cppu::OWeakObject
+{
+};
+// expected-error@+1 {{found one virtual base and one or more normal bases of cppu::OWeakObject, through inheritance paths Foo3->Foo1->OWeakObject, Foo3->Foo2->OWeakObject [loplugin:weakbase]}}
+class Foo3 : public Foo1, public Foo2
+{
+};
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/test/weakobject.cxx b/compilerplugins/clang/test/weakobject.cxx
deleted file mode 100644
index 7c7da55664d2..000000000000
--- a/compilerplugins/clang/test/weakobject.cxx
+++ /dev/null
@@ -1,31 +0,0 @@
-/* -*- 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/.
- */
-
-#include "config_clang.h"
-
-namespace cppu
-{
-class OWeakObject
-{
-};
-}
-
-class Foo1 : public cppu::OWeakObject
-{
-};
-class Foo2 : public cppu::OWeakObject
-{
-};
-
-// expected-error@+1 {{more than one copy of cppu::OWeakObject inherited [loplugin:weakobject]}}
-class Foo3 : public Foo1, public Foo2
-{
-};
-
-/* 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
index 666444ff7ffb..f6f7c8db01be 100644
--- a/compilerplugins/clang/weakbase.cxx
+++ b/compilerplugins/clang/weakbase.cxx
@@ -15,6 +15,7 @@
 #include <set>
 
 #include "plugin.hxx"
+#include "check.hxx"
 #include "clang/AST/CXXInheritance.h"
 
 /**
@@ -69,7 +70,11 @@ bool WeakBase::VisitCXXRecordDecl(CXXRecordDecl const* recordDecl)
         return true;
 
     int noWeakBases = 0;
-    std::string basePaths;
+    int noWeakObjects = 0;
+    bool foundVirtualWeakBase = false;
+    bool foundVirtualOWeakObject = false;
+    std::string basePaths1;
+    std::string basePaths2;
     auto BaseMatchesCallback = [&](const CXXBaseSpecifier* cxxBaseSpecifier, CXXBasePath& Paths) {
         if (!cxxBaseSpecifier->getType().getTypePtr())
             return false;
@@ -78,9 +83,30 @@ bool WeakBase::VisitCXXRecordDecl(CXXRecordDecl const* recordDecl)
             return false;
         if (baseCXXRecordDecl->isInvalidDecl())
             return false;
-        if (baseCXXRecordDecl->getName() != "WeakBase")
+        bool isWeakBase(loplugin::DeclCheck(baseCXXRecordDecl)
+                            .Struct("WeakBase")
+                            .Namespace("tools")
+                            .GlobalNamespace());
+        bool isOWeakObject(loplugin::DeclCheck(baseCXXRecordDecl)
+                               .Class("OWeakObject")
+                               .Namespace("cppu")
+                               .GlobalNamespace());
+        if (isWeakBase)
+        {
+            if (cxxBaseSpecifier->isVirtual())
+                foundVirtualWeakBase = true;
+            else
+                ++noWeakBases;
+        }
+        else if (isOWeakObject)
+        {
+            if (cxxBaseSpecifier->isVirtual())
+                foundVirtualOWeakObject = true;
+            else
+                ++noWeakObjects;
+        }
+        else
             return false;
-        ++noWeakBases;
         std::string sPath;
         for (CXXBasePathElement const& pathElement : Paths)
         {
@@ -95,22 +121,48 @@ bool WeakBase::VisitCXXRecordDecl(CXXRecordDecl const* recordDecl)
         }
         sPath += "->";
         sPath += baseCXXRecordDecl->getNameAsString();
-        if (!basePaths.empty())
-            basePaths += ", ";
-        basePaths += sPath;
+        if (isWeakBase)
+        {
+            if (!basePaths1.empty())
+                basePaths1 += ", ";
+            basePaths1 += sPath;
+        }
+        else
+        {
+            if (!basePaths2.empty())
+                basePaths2 += ", ";
+            basePaths2 += sPath;
+        }
         return false;
     };
 
     CXXBasePaths aPaths;
     recordDecl->lookupInBases(BaseMatchesCallback, aPaths);
 
-    if (noWeakBases > 1)
-    {
+    if (foundVirtualWeakBase && noWeakBases > 0)
         report(DiagnosticsEngine::Warning,
-               "multiple copies of WeakBase, through inheritance paths %0",
+               "found one virtual base and one or more normal bases of tools::WeakBase, through "
+               "inheritance paths %0",
                recordDecl->getBeginLoc())
-            << basePaths << recordDecl->getSourceRange();
-    }
+            << basePaths1;
+    else if (!foundVirtualWeakBase && noWeakBases > 1)
+        report(DiagnosticsEngine::Warning,
+               "found multiple copies of tools::WeakBase, through inheritance paths %0",
+               recordDecl->getBeginLoc())
+            << basePaths1;
+
+    if (foundVirtualOWeakObject && noWeakObjects > 0)
+        report(DiagnosticsEngine::Warning,
+               "found one virtual base and one or more normal bases of cppu::OWeakObject, through "
+               "inheritance paths %0",
+               recordDecl->getBeginLoc())
+            << basePaths2;
+    else if (!foundVirtualOWeakObject && noWeakObjects > 1)
+        report(DiagnosticsEngine::Warning,
+               "found multiple copies of cppu::OWeakObject, through inheritance paths %0",
+               recordDecl->getBeginLoc())
+            << basePaths2;
+
     return true;
 }
 
diff --git a/compilerplugins/clang/weakobject.cxx b/compilerplugins/clang/weakobject.cxx
deleted file mode 100644
index 7f1a2986faa5..000000000000
--- a/compilerplugins/clang/weakobject.cxx
+++ /dev/null
@@ -1,85 +0,0 @@
-
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
-/*
- * This file is part of the LibreOffice project.
- *
- * Based on LLVM/Clang.
- *
- * This file is distributed under the University of Illinois Open Source
- * License. See LICENSE.TXT for details.
- *
- */
-#ifndef LO_CLANG_SHARED_PLUGINS
-
-#include <cassert>
-#include <string>
-#include <iostream>
-#include <fstream>
-#include <set>
-#include <unordered_set>
-#include "plugin.hxx"
-#include "check.hxx"
-
-/*
-Check for places where we end up with more than one copy of cppu::OweakObject in a class, which
-really should not happen - we should be using one of the AggImplInheritanceHelper classes then
-to inherit.
-*/
-
-namespace
-{
-class WeakObject : public loplugin::FilteringPlugin<WeakObject>
-{
-public:
-    explicit WeakObject(loplugin::InstantiationData const& data)
-        : FilteringPlugin(data)
-    {
-    }
-
-    virtual bool preRun() override
-    {
-        return true;
-    }
-
-    virtual void run() override
-    {
-        if (preRun())
-            TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
-    }
-
-    bool VisitCXXRecordDecl( const CXXRecordDecl* decl);
-
-};
-
-bool WeakObject::VisitCXXRecordDecl(const CXXRecordDecl* decl)
-{
-    if (ignoreLocation(decl))
-        return true;
-    if (!decl->hasDefinition())
-        return true;
-    if (decl->hasAnyDependentBases())
-        return true;
-    int cnt = 0;
-    decl->forallBases(
-        [&cnt] (const CXXRecordDecl *BaseDefinition) -> bool
-        {
-            if (loplugin::DeclCheck(BaseDefinition).Class("OWeakObject").Namespace("cppu").GlobalNamespace())
-                ++cnt;
-            return true;
-        });
-    if (cnt < 2)
-        return true;
-
-    report(DiagnosticsEngine::Warning, "more than one copy of cppu::OWeakObject inherited",
-            decl->getBeginLoc())
-        << decl->getSourceRange();
-    return true;
-}
-
-loplugin::Plugin::Registration<WeakObject> weakobject("weakobject", false);
-
-} // namespace
-
-#endif // LO_CLANG_SHARED_PLUGINS
-
-/* vim:set shiftwidth=4 softtabstop=4 expandtab: */