Fix detection of std::unique_ptr/shared_ptr in loplugin:redundantpointerops

...when the get member function is implemented in a base class, as happens for
std::shared_ptr in libstdc++ (where it is implemented in base __shared_ptr; see
also 7d361e96c9ea822790db21806e9fc05279423833 "loplugin:redundantpointerops").
And while at it, check for precisely the classes we are interested in (for which
we know the semantics of get and operator*), rather than any classes whose
unqualified names happen to match.

Change-Id: I0c85ba46f191a2ee038c8175d979aa0c1be765cd
Reviewed-on: https://gerrit.libreoffice.org/80585
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

4 files changed, 84 insertions, 20 deletions

diff --git a/compilerplugins/clang/check.hxx b/compilerplugins/clang/check.hxx
index e027a5ca709f..7aa56cb8523d 100644
--- a/compilerplugins/clang/check.hxx
+++ b/compilerplugins/clang/check.hxx
@@ -65,6 +65,8 @@ public:
 
     inline ContextCheck Struct(llvm::StringRef id) const;
 
+    inline ContextCheck ClassOrStruct(llvm::StringRef id) const;
+
     TypeCheck Typedef() const;
 
     inline ContextCheck Typedef(llvm::StringRef id) const;
@@ -189,6 +191,15 @@ ContextCheck TypeCheck::Struct(llvm::StringRef id) const
     return ContextCheck();
 }
 
+ContextCheck TypeCheck::ClassOrStruct(llvm::StringRef id) const
+{
+    auto const c1 = Class(id);
+    if (c1) {
+        return c1;
+    }
+    return Struct(id);
+}
+
 ContextCheck TypeCheck::Typedef(llvm::StringRef id) const
 {
     if (!type_.isNull()) {
diff --git a/compilerplugins/clang/compat.hxx b/compilerplugins/clang/compat.hxx
index c091c51601f7..cb13f44cfa66 100644
--- a/compilerplugins/clang/compat.hxx
+++ b/compilerplugins/clang/compat.hxx
@@ -240,19 +240,6 @@ inline const clang::Expr *getSubExprAsWritten(const clang::CastExpr *This) {
   return getSubExprAsWritten(const_cast<clang::CastExpr *>(This));
 }
 
-inline clang::QualType getObjectType(clang::CXXMemberCallExpr const * expr) {
-#if CLANG_VERSION >= 100000
-    return expr->getObjectType();
-#else
-    // <https://github.com/llvm/llvm-project/commit/88559637641e993895337e1047a0bd787fecc647>
-    // "[OpenCL] Improve destructor support in C++ for OpenCL":
-    clang::QualType Ty = expr->getImplicitObjectArgument()->getType();
-    if (Ty->isPointerType())
-        Ty = Ty->getPointeeType();
-    return Ty;
-#endif
-}
-
 inline bool isExplicitSpecified(clang::CXXConstructorDecl const * decl) {
 #if CLANG_VERSION >= 90000
     return decl->getExplicitSpecifier().isExplicit();
diff --git a/compilerplugins/clang/redundantpointerops.cxx b/compilerplugins/clang/redundantpointerops.cxx
index dbb3ef7882fd..e2a97bc64350 100644
--- a/compilerplugins/clang/redundantpointerops.cxx
+++ b/compilerplugins/clang/redundantpointerops.cxx
@@ -15,6 +15,8 @@
 #include <set>
 
 #include <clang/AST/CXXInheritance.h>
+
+#include "check.hxx"
 #include "compat.hxx"
 #include "plugin.hxx"
 
@@ -129,13 +131,35 @@ bool RedundantPointerOps::VisitUnaryOperator(UnaryOperator const * unaryOperator
         auto methodDecl = cxxMemberCallExpr->getMethodDecl();
         if (methodDecl->getIdentifier() && methodDecl->getName() == "get")
         {
-            auto className = cxxMemberCallExpr->getRecordDecl()->getName();
-            if (className == "unique_ptr" || className == "shared_ptr" || className == "Reference" || className == "SvRef")
-                report(
-                    DiagnosticsEngine::Warning,
-                    "'*' followed by '.get()' operating on %0, just use '*'",
-                    compat::getBeginLoc(unaryOperator))
-                    << compat::getObjectType(cxxMemberCallExpr) << unaryOperator->getSourceRange();
+            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();
 
         }
     }
diff --git a/compilerplugins/clang/test/redundantpointerops.cxx b/compilerplugins/clang/test/redundantpointerops.cxx
index 0a1f7b3afad5..346d84bf0b71 100644
--- a/compilerplugins/clang/test/redundantpointerops.cxx
+++ b/compilerplugins/clang/test/redundantpointerops.cxx
@@ -7,8 +7,16 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
+#include <sal/config.h>
+
 #include <memory>
 
+#include <com/sun/star/uno/Reference.hxx>
+#include <com/sun/star/uno/XInterface.hpp>
+#include <rtl/ref.hxx>
+#include <sal/types.h>
+#include <tools/ref.hxx>
+
 struct Struct1 {
     int x;
 };
@@ -43,6 +51,40 @@ int function5(std::unique_ptr<int> x)
     return *x.get(); // expected-error-re {{'*' followed by '.get()' operating on '{{.*}}unique_ptr{{.*}}', just use '*' [loplugin:redundantpointerops]}}
 };
 
+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 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]}}
+}
+
+void function8(css::uno::Reference<css::uno::XInterface> x)
+{
+    (void) *x.get(); // expected-error {{'*' followed by '.get()' operating on 'css::uno::Reference<css::uno::XInterface>', just use '*' [loplugin:redundantpointerops]}}
+}
+
+void function9(tools::SvRef<SvRefBase> x)
+{
+    (void) *x.get(); // expected-error {{'*' followed by '.get()' operating on 'tools::SvRef<SvRefBase>', just use '*' [loplugin:redundantpointerops]}}
+}
+
+struct DerivedRtlReference: public rtl::Reference<css::uno::XInterface> {};
+
+void function10(DerivedRtlReference x)
+{
+    (void) *x.get(); // expected-error {{'*' followed by '.get()' operating on 'DerivedRtlReference', just use '*' [loplugin:redundantpointerops]}}
+}
+
+struct DerivedUnoReference: public css::uno::Reference<css::uno::XInterface> {};
+
+void function11(DerivedUnoReference x)
+{
+    (void) *x.get(); // expected-error {{'*' followed by '.get()' operating on 'DerivedUnoReference', just use '*' [loplugin:redundantpointerops]}}
+}
 
+// tools::SvRef is final
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */