Make checkIdenticalDefaultArguments more precise

...when creating objects of the same derived type

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

2 files changed, 43 insertions, 15 deletions

diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx
index 6697f94b3ee8..717d88b23091 100644
--- a/compilerplugins/clang/plugin.cxx
+++ b/compilerplugins/clang/plugin.cxx
@@ -26,6 +26,20 @@ Base classes for plugin actions.
 namespace loplugin
 {
 
+namespace {
+
+Expr const * skipImplicit(Expr const * expr) {
+    if (auto const e = dyn_cast<MaterializeTemporaryExpr>(expr)) {
+        expr = e->GetTemporaryExpr();
+    }
+    if (auto const e = dyn_cast<CXXBindTemporaryExpr>(expr)) {
+        expr = e->getSubExpr();
+    }
+    return expr;
+}
+
+}
+
 Plugin::Plugin( const InstantiationData& data )
     : compiler( data.compiler ), handler( data.handler ), name( data.name )
 {
@@ -247,27 +261,29 @@ Plugin::IdenticalDefaultArgumentsResult Plugin::checkIdenticalDefaultArguments(
             : IdenticalDefaultArgumentsResult::No;
     }
 #endif
-    if (auto const lit1 = dyn_cast<clang::StringLiteral>(
-            argument1->IgnoreParenImpCasts()))
-    {
-        if (auto const lit2 = dyn_cast<clang::StringLiteral>(
-                argument2->IgnoreParenImpCasts()))
-        {
+    auto const desugared1 = argument1->IgnoreParenImpCasts();
+    auto const desugared2 = argument2->IgnoreParenImpCasts();
+    if (auto const lit1 = dyn_cast<clang::StringLiteral>(desugared1)) {
+        if (auto const lit2 = dyn_cast<clang::StringLiteral>(desugared2)) {
             return lit1->getBytes() == lit2->getBytes()
                 ? IdenticalDefaultArgumentsResult::Yes
                 : IdenticalDefaultArgumentsResult::No;
         }
     }
     // catch params with defaults like "= OUString()"
-    if (isa<MaterializeTemporaryExpr>(argument1)
-        && isa<MaterializeTemporaryExpr>(argument2))
-    {
-        return IdenticalDefaultArgumentsResult::Yes;
-    }
-    if (isa<CXXBindTemporaryExpr>(argument1)
-        && isa<CXXBindTemporaryExpr>(argument2))
-    {
-        return IdenticalDefaultArgumentsResult::Yes;
+    if (auto const e1 = dyn_cast<CXXConstructExpr>(skipImplicit(desugared1))) {
+        if (auto const e2 = dyn_cast<CXXConstructExpr>(
+                skipImplicit(desugared2)))
+        {
+            if ((e1->getConstructor()->getCanonicalDecl()
+                 != e2->getConstructor()->getCanonicalDecl()))
+            {
+                return IdenticalDefaultArgumentsResult::No;
+            }
+            if (e1->getNumArgs() == 0 && e2->getNumArgs() == 0) {
+                return IdenticalDefaultArgumentsResult::Yes;
+            }
+        }
     }
     return IdenticalDefaultArgumentsResult::Maybe;
 }
diff --git a/compilerplugins/clang/test/unnecessaryoverride.cxx b/compilerplugins/clang/test/unnecessaryoverride.cxx
index 816feb9d6af4..b3733a7e192d 100644
--- a/compilerplugins/clang/test/unnecessaryoverride.cxx
+++ b/compilerplugins/clang/test/unnecessaryoverride.cxx
@@ -90,4 +90,16 @@ struct DerivedSlightlyDifferent : Base
     }
 };
 
+struct Base2
+{
+    void default1(Base const& = SimpleDerived());
+    void default2(Base const& = SimpleDerived());
+};
+
+struct Derived2 : Base2
+{
+    void default1(Base const& x = Intermediate1()) { Base2::default1(x); } // no warning
+    void default2(Base const& x = SimpleDerived()) { Base2::default2(x); } // expected-error {{public function just calls public parent [loplugin:unnecessaryoverride]}}
+};
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */