loplugin:unusedvariablecheck improve

to find unused smart pointer variables

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

5 files changed, 63 insertions, 0 deletions

diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx
index 552f676735d3..57fdf83b79ff 100644
--- a/compilerplugins/clang/plugin.cxx
+++ b/compilerplugins/clang/plugin.cxx
@@ -818,6 +818,42 @@ bool hasExternalLinkage(VarDecl const * decl) {
     return true;
 }
 
+bool isSmartPointerType(QualType qt)
+{
+    // First check whether the object type as written is, or is derived from, std::unique_ptr or
+    // std::shared_ptr, in case the get member function is declared at a base class of that std
+    // type:
+    if (loplugin::isDerivedFrom(
+            qt->getAsCXXRecordDecl(),
+            [](Decl const * decl) {
+                auto const dc = loplugin::DeclCheck(decl);
+                return dc.ClassOrStruct("unique_ptr").StdNamespace()
+                    || dc.ClassOrStruct("shared_ptr").StdNamespace();
+            }))
+        return true;
+
+    // Then check the object type coerced to the type of the get member function, in
+    // case the type-as-written is derived from one of these types (tools::SvRef is
+    // final, but the rest are not):
+    auto const tc2 = loplugin::TypeCheck(qt);
+    if (tc2.ClassOrStruct("unique_ptr").StdNamespace()
+           || tc2.ClassOrStruct("shared_ptr").StdNamespace()
+           || tc2.Class("Reference").Namespace("uno").Namespace("star")
+                .Namespace("sun").Namespace("com").GlobalNamespace()
+           || tc2.Class("Reference").Namespace("rtl").GlobalNamespace()
+           || tc2.Class("SvRef").Namespace("tools").GlobalNamespace()
+           || tc2.Class("WeakReference").Namespace("tools").GlobalNamespace()
+           || tc2.Class("ScopedReadAccess").Namespace("Bitmap").GlobalNamespace()
+           || tc2.Class("ScopedVclPtrInstance").GlobalNamespace()
+           || tc2.Class("VclPtr").GlobalNamespace()
+           || tc2.Class("ScopedVclPtr").GlobalNamespace()
+           || tc2.Class("intrusive_ptr").Namespace("boost").GlobalNamespace())
+    {
+        return true;
+    }
+    return false;
+}
+
 bool isSmartPointerType(const Expr* e)
 {
     // First check whether the object type as written is, or is derived from, std::unique_ptr or
diff --git a/compilerplugins/clang/plugin.hxx b/compilerplugins/clang/plugin.hxx
index 156955da4b87..540352df9868 100644
--- a/compilerplugins/clang/plugin.hxx
+++ b/compilerplugins/clang/plugin.hxx
@@ -306,6 +306,7 @@ int derivedFromCount(const CXXRecordDecl* subtypeRecord, const CXXRecordDecl* ba
 bool hasExternalLinkage(VarDecl const * decl);
 
 bool isSmartPointerType(const Expr*);
+bool isSmartPointerType(clang::QualType);
 
 const Decl* getFunctionDeclContext(ASTContext& context, const Stmt* stmt);
 
diff --git a/compilerplugins/clang/test/unusedvariablecheck.cxx b/compilerplugins/clang/test/unusedvariablecheck.cxx
index c5b2a04d89fe..e53da1be40a8 100644
--- a/compilerplugins/clang/test/unusedvariablecheck.cxx
+++ b/compilerplugins/clang/test/unusedvariablecheck.cxx
@@ -12,6 +12,7 @@
 #include <list>
 #include <string>
 #include <vector>
+#include <memory>
 
 namespace
 {
@@ -23,6 +24,8 @@ int main()
     std::list<int> v1; // expected-error {{unused variable 'v1' [loplugin:unusedvariablecheck]}}
     std::string v2; // expected-error {{unused variable 'v2' [loplugin:unusedvariablecheck]}}
     Vec<int> v3; // expected-error {{unused variable 'v3' [loplugin:unusedvariablecheck]}}
+    std::unique_ptr<int>
+        v4; // expected-error {{unused variable 'v4' [loplugin:unusedvariablecheck]}}
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/unusedvariablecheck.cxx b/compilerplugins/clang/unusedvariablecheck.cxx
index 1e09645733e0..73529e118bf4 100644
--- a/compilerplugins/clang/unusedvariablecheck.cxx
+++ b/compilerplugins/clang/unusedvariablecheck.cxx
@@ -16,6 +16,7 @@
 #include "compat.hxx"
 #include "check.hxx"
 #include "unusedvariablecheck.hxx"
+#include "plugin.hxx"
 
 namespace loplugin
 {
@@ -57,6 +58,10 @@ bool UnusedVariableCheck::VisitVarDecl( const VarDecl* var )
     auto type = var->getType();
     bool check = loplugin::isExtraWarnUnusedType(type);
 
+    if (!check)
+        check = isUnusedSmartPointer(var);
+
+
     // this chunk of logic generates false+, which is why we don't leave it on
 /*
     if (!check && type->isRecordType())
@@ -88,6 +93,22 @@ bool UnusedVariableCheck::VisitVarDecl( const VarDecl* var )
     return true;
     }
 
+bool UnusedVariableCheck::isUnusedSmartPointer( const VarDecl* var )
+    {
+        // if we have a var of smart-pointer type, and that var is both uninitialised and
+        // not referenced, then we can remove it
+        if (!isSmartPointerType(var->getType()))
+            return false;
+
+        if (!var->hasInit())
+            return true;
+
+        auto cxxConstructExpr = dyn_cast<CXXConstructExpr>(var->getInit());
+        if (!cxxConstructExpr)
+            return false;
+        return cxxConstructExpr->getConstructor()->isDefaultConstructor();
+    }
+
 static Plugin::Registration< UnusedVariableCheck > unusedvariablecheck( "unusedvariablecheck" );
 
 } // namespace
diff --git a/compilerplugins/clang/unusedvariablecheck.hxx b/compilerplugins/clang/unusedvariablecheck.hxx
index f22162e108a6..5f5edbbbaf96 100644
--- a/compilerplugins/clang/unusedvariablecheck.hxx
+++ b/compilerplugins/clang/unusedvariablecheck.hxx
@@ -23,6 +23,8 @@ class UnusedVariableCheck
         explicit UnusedVariableCheck( const InstantiationData& data );
         virtual void run() override;
         bool VisitVarDecl( const VarDecl* var );
+    private:
+        bool isUnusedSmartPointer( const VarDecl* var );
     };
 
 } // namespace