Improve loplugin:unnecessaryoverride

<sberg> thorsten, remember what that "TODO" in
SvxAccessibleTextPropertySet::getSupportedServiceNames was to be about exactly,
in a909acb7009acadffa53e74ea05ddb88803490f1 ?
<thorsten> sberg: that's a nonsense, prolly copy'n'pasted, or a 'please review
me'
<sberg> thorsten, OK, thanks (that override will eventually go away with
loplugin:unnecessaryoverride, and the TODO comment be lost)

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

1 files changed, 55 insertions, 7 deletions

diff --git a/compilerplugins/clang/unnecessaryoverride.cxx b/compilerplugins/clang/unnecessaryoverride.cxx
index 8a882af9c362..06a468c0f61d 100644
--- a/compilerplugins/clang/unnecessaryoverride.cxx
+++ b/compilerplugins/clang/unnecessaryoverride.cxx
@@ -12,6 +12,8 @@
 #include <iostream>
 #include <fstream>
 #include <set>
+
+#include "compat.hxx"
 #include "plugin.hxx"
 
 /**
@@ -67,16 +69,61 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
 
     const CXXMethodDecl* overriddenMethodDecl = *methodDecl->begin_overridden_methods();
 
+    if (compat::getReturnType(*methodDecl).getCanonicalType()
+        != compat::getReturnType(*overriddenMethodDecl).getCanonicalType())
+    {
+        return true;
+    }
+    //TODO: check for identical exception specifications
+
     const CompoundStmt* compoundStmt = dyn_cast<CompoundStmt>(methodDecl->getBody());
     if (!compoundStmt || compoundStmt->size() != 1)
         return true;
-    const Stmt* firstStmt = compoundStmt->body_front();
-    if (const ReturnStmt* returnStmt = dyn_cast<ReturnStmt>(firstStmt)) {
-        firstStmt = returnStmt->getRetValue();
+    auto returnStmt = dyn_cast<ReturnStmt>(compoundStmt->body_front());
+    if (returnStmt == nullptr) {
+        return true;
     }
-    if (!firstStmt)
+    auto returnExpr = returnStmt->getRetValue();
+    if (returnExpr == nullptr) {
         return true;
-    const CXXMemberCallExpr* callExpr = dyn_cast<CXXMemberCallExpr>(firstStmt);
+    }
+    returnExpr = returnExpr->IgnoreImplicit();
+
+    // In something like
+    //
+    //  Reference< XResultSet > SAL_CALL OPreparedStatement::executeQuery(
+    //      const rtl::OUString& sql)
+    //      throw(SQLException, RuntimeException, std::exception)
+    //  {
+    //      return OCommonStatement::executeQuery( sql );
+    //  }
+    //
+    // look down through all the
+    //
+    //   ReturnStmt
+    //   `-ExprWithCleanups
+    //     `-CXXConstructExpr
+    //      `-MaterializeTemporaryExpr
+    //       `-ImplicitCastExpr
+    //        `-CXXBindTemporaryExpr
+    //         `-CXXMemberCallExpr
+    //
+    // where the fact that the overriding and overridden function have identical
+    // return types makes us confident that all we need to check here is whether
+    // there's an (arbitrary, one-argument) CXXConstructorExpr and
+    // CXXBindTemporaryExpr in between:
+    if (auto ctorExpr = dyn_cast<CXXConstructExpr>(returnExpr)) {
+        if (ctorExpr->getNumArgs() == 1) {
+            if (auto tempExpr = dyn_cast<CXXBindTemporaryExpr>(
+                    ctorExpr->getArg(0)->IgnoreImplicit()))
+            {
+                returnExpr = tempExpr->getSubExpr();
+            }
+        }
+    }
+
+    const CXXMemberCallExpr* callExpr = dyn_cast<CXXMemberCallExpr>(
+        returnExpr->IgnoreParenImpCasts());
     if (!callExpr || callExpr->getMethodDecl() != overriddenMethodDecl)
         return true;
     const ImplicitCastExpr* expr1 = dyn_cast_or_null<ImplicitCastExpr>(callExpr->getImplicitObjectArgument());
@@ -92,9 +139,10 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
     }
 
     report(
-        DiagnosticsEngine::Warning, "method just calls parent method",
+        DiagnosticsEngine::Warning, "%0 virtual function just calls %1 parent",
         methodDecl->getSourceRange().getBegin())
-      << methodDecl->getSourceRange();
+        << methodDecl->getAccess() << overriddenMethodDecl->getAccess()
+        << methodDecl->getSourceRange();
     if (methodDecl->getCanonicalDecl()->getLocation() != methodDecl->getLocation()) {
         const CXXMethodDecl* pOther = methodDecl->getCanonicalDecl();
         report(