[Libreoffice-commits] core.git: compilerplugins/clang

[Noel Grandin (via logerrit)]

 compilerplugins/clang/redundantpointerops.cxx      |   84 +++++++++++++--------
 compilerplugins/clang/test/redundantpointerops.cxx |    5 +
 2 files changed, 59 insertions(+), 30 deletions(-)

New commits:
commit 57b185a972bce7371ae52f813c3408c1308c0326
Author:     Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Fri Jan 10 15:59:59 2020 +0200
Commit:     Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Sat Jan 11 11:59:46 2020 +0100

    loplugin:redundantpointerops, look for ".get()->"
    
    Change-Id: I396864b4dea3341b78634cb74555cfb78f1aea25
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/86551
    Tested-by: Jenkins
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>

diff --git a/compilerplugins/clang/redundantpointerops.cxx b/compilerplugins/clang/redundantpointerops.cxx
index e2a97bc64350..1539588c1e36 100644
--- a/compilerplugins/clang/redundantpointerops.cxx
+++ b/compilerplugins/clang/redundantpointerops.cxx
@@ -53,13 +53,16 @@ public:
     bool VisitFunctionDecl(FunctionDecl const *);
     bool VisitMemberExpr(MemberExpr const *);
     bool VisitUnaryOperator(UnaryOperator const *);
+private:
+    bool isSmartPointerType(const Expr* e);
 };
 
 bool RedundantPointerOps::VisitFunctionDecl(FunctionDecl const * functionDecl)
 {
     if (ignoreLocation(functionDecl))
         return true;
-    //functionDecl->dump();
+//    if (functionDecl->getIdentifier() && functionDecl->getName() == "function6b")
+//        functionDecl->dump();
     return true;
 }
 
@@ -95,6 +98,21 @@ bool RedundantPointerOps::VisitMemberExpr(MemberExpr const * memberExpr)
                     << memberExpr->getSourceRange();
 
         }
+        else if (auto cxxMemberCallExpr = dyn_cast<CXXMemberCallExpr>(base))
+        {
+            auto methodDecl = cxxMemberCallExpr->getMethodDecl();
+            if (methodDecl->getIdentifier() && methodDecl->getName() == "get")
+            {
+                auto const e = cxxMemberCallExpr->getImplicitObjectArgument();
+                if (isSmartPointerType(e))
+                    report(
+                        DiagnosticsEngine::Warning,
+                        "'get()' followed by '->' operating on %0, just use '->'",
+                        compat::getBeginLoc(memberExpr))
+                        << e->IgnoreImpCasts()->getType().getLocalUnqualifiedType()
+                        << memberExpr->getSourceRange();
+            }
+        }
     }
 //    else
 //    {
@@ -132,40 +150,46 @@ bool RedundantPointerOps::VisitUnaryOperator(UnaryOperator const * unaryOperator
         if (methodDecl->getIdentifier() && methodDecl->getName() == "get")
         {
             auto const e = cxxMemberCallExpr->getImplicitObjectArgument();
-            // First check the object type as written, in case the get member function is
-            // declared at a base class of std::unique_ptr or std::shared_ptr:
-            auto const t = e->IgnoreImpCasts()->getType();
-            auto const tc1 = loplugin::TypeCheck(t);
-            if (!(tc1.ClassOrStruct("unique_ptr").StdNamespace()
-                  || tc1.ClassOrStruct("shared_ptr").StdNamespace()))
-            {
-                // 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; but note that this will fail when the type-as-
-                // written is derived from std::unique_ptr or std::shared_ptr for which the get
-                // member function is declared at a base class):
-                auto const tc2 = loplugin::TypeCheck(e->getType());
-                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())))
-                {
-                    return true;
-                }
-            }
-            report(
-                DiagnosticsEngine::Warning,
-                "'*' followed by '.get()' operating on %0, just use '*'",
-                compat::getBeginLoc(unaryOperator))
-                << t.getLocalUnqualifiedType() << unaryOperator->getSourceRange();
-
+            if (isSmartPointerType(e))
+                report(
+                    DiagnosticsEngine::Warning,
+                    "'*' followed by '.get()' operating on %0, just use '*'",
+                    compat::getBeginLoc(unaryOperator))
+                    << e->IgnoreImpCasts()->getType().getLocalUnqualifiedType()
+                    << unaryOperator->getSourceRange();
         }
     }
     return true;
 }
 
+bool RedundantPointerOps::isSmartPointerType(const Expr* e)
+{
+    // First check the object type as written, in case the get member function is
+    // declared at a base class of std::unique_ptr or std::shared_ptr:
+    auto const t = e->IgnoreImpCasts()->getType();
+    auto const tc1 = loplugin::TypeCheck(t);
+    if (tc1.ClassOrStruct("unique_ptr").StdNamespace()
+          || tc1.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; but note that this will fail when the type-as-
+    // written is derived from std::unique_ptr or std::shared_ptr for which the get
+    // member function is declared at a base class):
+    auto const tc2 = loplugin::TypeCheck(e->getType());
+    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()))
+    {
+        return true;
+    }
+    return false;
+}
+
 loplugin::Plugin::Registration< RedundantPointerOps > redundantpointerops("redundantpointerops");
 
 } // namespace
diff --git a/compilerplugins/clang/test/redundantpointerops.cxx b/compilerplugins/clang/test/redundantpointerops.cxx
index 346d84bf0b71..c766099fce3a 100644
--- a/compilerplugins/clang/test/redundantpointerops.cxx
+++ b/compilerplugins/clang/test/redundantpointerops.cxx
@@ -56,6 +56,11 @@ void function6(std::shared_ptr<int> x)
     (void) *x.get(); // expected-error-re {{'*' followed by '.get()' operating on '{{.*}}shared_ptr{{.*}}', just use '*' [loplugin:redundantpointerops]}}
 }
 
+void function6b(std::shared_ptr<Struct1> x)
+{
+    x.get()->x = 1; // expected-error-re {{'get()' followed by '->' operating on '{{.*}}shared_ptr{{.*}}', just use '->' [loplugin:redundantpointerops]}}
+}
+
 void function7(rtl::Reference<css::uno::XInterface> x)
 {
     (void) *x.get(); // expected-error {{'*' followed by '.get()' operating on 'rtl::Reference<css::uno::XInterface>', just use '*' [loplugin:redundantpointerops]}}