new loplugin:makeshared

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

3 files changed, 181 insertions, 0 deletions

diff --git a/compilerplugins/clang/makeshared.cxx b/compilerplugins/clang/makeshared.cxx
new file mode 100644
index 000000000000..93aac06666ba
--- /dev/null
+++ b/compilerplugins/clang/makeshared.cxx
@@ -0,0 +1,137 @@
+/* -*- 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 <cassert>
+#include <string>
+#include <iostream>
+#include <fstream>
+#include <set>
+
+#include <clang/AST/CXXInheritance.h>
+
+#include "check.hxx"
+#include "compat.hxx"
+#include "plugin.hxx"
+
+/**
+ * look for places we can use std::make_shared
+ */
+
+namespace
+{
+class MakeShared : public loplugin::FilteringPlugin<MakeShared>
+{
+public:
+    explicit MakeShared(loplugin::InstantiationData const& data)
+        : FilteringPlugin(data)
+    {
+    }
+
+    virtual bool preRun() override
+    {
+        StringRef fn(handler.getMainFileName());
+        // uses boost::shared_ptr and we trigger because we're not looking specifically for std::shared_ptr
+        if (loplugin::isSamePathname(fn, SRCDIR "/ucb/source/ucp/cmis/cmis_repo_content.cxx"))
+            return false;
+        if (loplugin::isSamePathname(fn, SRCDIR "/ucb/source/ucp/cmis/cmis_content.cxx"))
+            return false;
+        // TODO something weird with protected base classes going on here
+        if (loplugin::isSamePathname(fn, SRCDIR "/sc/source/filter/excel/xeextlst.cxx"))
+            return false;
+        if (loplugin::isSamePathname(fn, SRCDIR "/sc/source/filter/excel/xecontent.cxx"))
+            return false;
+        return true;
+    }
+
+    virtual void run() override
+    {
+        if (preRun())
+            TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+    }
+
+    bool shouldVisitTemplateInstantiations() const { return true; }
+
+    bool VisitCXXConstructExpr(CXXConstructExpr const*);
+    bool VisitCXXMemberCallExpr(CXXMemberCallExpr const*);
+};
+
+bool MakeShared::VisitCXXConstructExpr(CXXConstructExpr const* constructExpr)
+{
+    if (ignoreLocation(constructExpr))
+        return true;
+    if (!loplugin::TypeCheck(constructExpr->getType()).ClassOrStruct("shared_ptr").StdNamespace())
+        return true;
+    if (constructExpr->getNumArgs() != 1)
+        return true;
+    auto cxxNewExpr = dyn_cast<CXXNewExpr>(constructExpr->getArg(0)->IgnoreParenImpCasts());
+    if (!cxxNewExpr)
+        return true;
+    auto construct2 = cxxNewExpr->getConstructExpr();
+    if (construct2)
+    {
+        if (construct2->getConstructor()->getAccess() != AS_public)
+            return true;
+        if (construct2->getNumArgs() == 1 && isa<CXXStdInitializerListExpr>(construct2->getArg(0)))
+            return true;
+    }
+
+    StringRef fn = getFilenameOfLocation(
+        compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(constructExpr)));
+    if (loplugin::isSamePathname(fn, SRCDIR "/include/o3tl/make_shared.hxx"))
+        return true;
+
+    report(DiagnosticsEngine::Warning, "rather use make_shared", compat::getBeginLoc(cxxNewExpr))
+        << cxxNewExpr->getSourceRange();
+    return true;
+}
+
+bool MakeShared::VisitCXXMemberCallExpr(CXXMemberCallExpr const* cxxMemberCallExpr)
+{
+    if (ignoreLocation(cxxMemberCallExpr))
+        return true;
+    if (cxxMemberCallExpr->getNumArgs() != 1)
+        return true;
+
+    // cannot find a way to use the loplugin::DeclCheck stuff here
+    auto templateDecl
+        = dyn_cast<ClassTemplateSpecializationDecl>(cxxMemberCallExpr->getRecordDecl());
+    if (!templateDecl)
+        return true;
+    auto cxxRecordDecl = templateDecl->getSpecializedTemplate()->getTemplatedDecl();
+    if (!cxxRecordDecl->getName().contains("shared_ptr"))
+        return true;
+
+    if (cxxMemberCallExpr->getMethodDecl()->getName() != "reset")
+        return true;
+    auto cxxNewExpr = dyn_cast<CXXNewExpr>(cxxMemberCallExpr->getArg(0)->IgnoreParenImpCasts());
+    if (!cxxNewExpr)
+        return true;
+    if (cxxNewExpr->getConstructExpr()
+        && cxxNewExpr->getConstructExpr()->getConstructor()->getAccess() != AS_public)
+        return true;
+
+    StringRef fn = getFilenameOfLocation(
+        compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(cxxMemberCallExpr)));
+    if (loplugin::isSamePathname(fn, SRCDIR "/include/o3tl/make_shared.hxx"))
+        return true;
+
+    report(DiagnosticsEngine::Warning, "rather use make_shared", compat::getBeginLoc(cxxNewExpr))
+        << cxxNewExpr->getSourceRange();
+
+    return true;
+}
+
+loplugin::Plugin::Registration<MakeShared> makeshared("makeshared", false);
+
+} // namespace
+
+#endif // LO_CLANG_SHARED_PLUGINS
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/test/makeshared.cxx b/compilerplugins/clang/test/makeshared.cxx
new file mode 100644
index 000000000000..6e388428f8f1
--- /dev/null
+++ b/compilerplugins/clang/test/makeshared.cxx
@@ -0,0 +1,43 @@
+/* -*- 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 <sal/config.h>
+
+#include <memory>
+#include <o3tl/deleter.hxx>
+#include <o3tl/sorted_vector.hxx>
+
+struct S1
+{
+    friend void test1();
+
+private:
+    S1() {}
+};
+
+void test1()
+{
+    std::shared_ptr<int> x(
+        new int); // expected-error {{rather use make_shared [loplugin:makeshared]}}
+    x.reset(new int); // expected-error {{rather use make_shared [loplugin:makeshared]}}
+    x = std::shared_ptr<int>(
+        new int); // expected-error {{rather use make_shared [loplugin:makeshared]}}
+
+    // no warning expected
+    std::shared_ptr<int> y(new int, o3tl::default_delete<int>());
+    y.reset(new int, o3tl::default_delete<int>());
+    // no warning expected, no public constructor
+    std::shared_ptr<S1> z(new S1);
+    z.reset(new S1);
+
+    // no warning expected - this constructor takes an initializer-list, which maked_shared does not support
+    auto a = std::shared_ptr<o3tl::sorted_vector<int>>(new o3tl::sorted_vector<int>({ 1, 2 }));
+};
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk
index a65969a0078e..05c470df005d 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -47,6 +47,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
     compilerplugins/clang/test/logexceptionnicely \
     compilerplugins/clang/test/loopvartoosmall \
     compilerplugins/clang/test/mapindex \
+    compilerplugins/clang/test/makeshared \
     compilerplugins/clang/test/noexceptmove \
     compilerplugins/clang/test/nullptr \
     compilerplugins/clang/test/oncevar \