From 4ff66e42bfd7d03020d9fd09bc24aef92d92ecd0 Mon Sep 17 00:00:00 2001
From: Stephan Bergmann <sbergman@redhat.com>
Date: Wed, 15 Nov 2017 17:41:22 +0100
Subject: Make checkIdenticalDefaultArguments more precise

...by structurally comparing complex constexpr exprs that use template functions
that happen to not have been instantiated, so Expr::EvaluateAsRValue et al would
fail.  (Which happened with SFX_PRINTER_ALL in SfxViewShell::SetPrinter,
include/sfx2/viewsh.hxx.)

Now all of the LO code base should compile without causing
checkIdenticalDefaultArguments to return Maybe.

Change-Id: I2b103418c2c68f6d2242535c9cca3222a2508778
Reviewed-on: https://gerrit.libreoffice.org/44773
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
---
 compilerplugins/clang/plugin.cxx                   | 76 ++++++++++++++++++++++
 compilerplugins/clang/test/unnecessaryoverride.cxx | 21 ++++++
 2 files changed, 97 insertions(+)

diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx
index ed179d9b2b65..3c716112cd47 100644
--- a/compilerplugins/clang/plugin.cxx
+++ b/compilerplugins/clang/plugin.cxx
@@ -38,6 +38,76 @@ Expr const * skipImplicit(Expr const * expr) {
     return expr;
 }
 
+bool structurallyIdentical(Stmt const * stmt1, Stmt const * stmt2) {
+    if (stmt1->getStmtClass() != stmt2->getStmtClass()) {
+        return false;
+    }
+    switch (stmt1->getStmtClass()) {
+    case Stmt::CXXConstructExprClass:
+        if (cast<CXXConstructExpr>(stmt1)->getConstructor()->getCanonicalDecl()
+            != cast<CXXConstructExpr>(stmt2)->getConstructor()->getCanonicalDecl())
+        {
+            return false;
+        }
+        break;
+    case Stmt::DeclRefExprClass:
+        if (cast<DeclRefExpr>(stmt1)->getDecl()->getCanonicalDecl()
+            != cast<DeclRefExpr>(stmt2)->getDecl()->getCanonicalDecl())
+        {
+            return false;
+        }
+        break;
+    case Stmt::ImplicitCastExprClass:
+        {
+            auto const e1 = cast<ImplicitCastExpr>(stmt1);
+            auto const e2 = cast<ImplicitCastExpr>(stmt2);
+            if (e1->getCastKind() != e2->getCastKind()
+                || e1->getType().getCanonicalType() != e2->getType().getCanonicalType())
+            {
+                return false;
+            }
+            break;
+        }
+    case Stmt::MemberExprClass:
+        {
+            auto const e1 = cast<MemberExpr>(stmt1);
+            auto const e2 = cast<MemberExpr>(stmt2);
+            if (e1->isArrow() != e2->isArrow()
+                || e1->getType().getCanonicalType() != e2->getType().getCanonicalType())
+            {
+                return false;
+            }
+            break;
+        }
+    case Stmt::CXXMemberCallExprClass:
+    case Stmt::CXXOperatorCallExprClass:
+        if (cast<Expr>(stmt1)->getType().getCanonicalType()
+            != cast<Expr>(stmt2)->getType().getCanonicalType())
+        {
+            return false;
+        }
+        break;
+    case Stmt::MaterializeTemporaryExprClass:
+    case Stmt::ParenExprClass:
+        break;
+    default:
+        // Conservatively assume non-identical for expressions that don't happen for us in practice
+        // when compiling the LO code base (and for which the above set of supported classes would
+        // need to be extended):
+        return false;
+    }
+    auto i1 = stmt1->child_begin();
+    auto e1 = stmt1->child_end();
+    auto i2 = stmt2->child_begin();
+    auto e2 = stmt2->child_end();
+    for (; i1 != e1; ++i1, ++i2) {
+        if (i2 == e2 || !structurallyIdentical(*i1, *i2)) {
+            return false;
+        }
+    }
+    return i2 == e2;
+}
+
 }
 
 Plugin::Plugin( const InstantiationData& data )
@@ -294,6 +364,12 @@ Plugin::IdenticalDefaultArgumentsResult Plugin::checkIdenticalDefaultArguments(
         }
         break;
     }
+    // If the EvaluateAsRValue derivatives above failed because the arguments use e.g. (constexpr)
+    // function template specializations that happen to not have been instantiated in this TU, try a
+    // structural comparison of the arguments:
+    if (structurallyIdentical(argument1, argument2)) {
+        return IdenticalDefaultArgumentsResult::Yes;
+    }
     return IdenticalDefaultArgumentsResult::Maybe;
 }
 
diff --git a/compilerplugins/clang/test/unnecessaryoverride.cxx b/compilerplugins/clang/test/unnecessaryoverride.cxx
index 1f5c1f9fb4a7..7caeab3b7cec 100644
--- a/compilerplugins/clang/test/unnecessaryoverride.cxx
+++ b/compilerplugins/clang/test/unnecessaryoverride.cxx
@@ -10,6 +10,7 @@
 #include <sal/config.h>
 
 #include <config_clang.h>
+#include <o3tl/typed_flags_set.hxx>
 
 struct Base
 {
@@ -90,11 +91,25 @@ struct DerivedSlightlyDifferent : Base
     }
 };
 
+enum class E
+{
+    E1 = 1,
+    E2 = 2,
+    E3 = 4
+};
+namespace o3tl
+{
+template <> struct typed_flags<E> : is_typed_flags<E, 7>
+{
+};
+}
+
 struct Base2
 {
     void default1(Base const& = SimpleDerived());
     void default2(Base const& = SimpleDerived());
     void default3(Base = Base());
+    void default4(E = (E::E1 | E::E2 | E::E3));
 };
 
 struct Derived2 : Base2
@@ -112,6 +127,12 @@ struct Derived2 : Base2
     {
         (Base2::default3(x));
     }
+    void
+    default4( // expected-error {{public function just calls public parent [loplugin:unnecessaryoverride]}}
+        E x = (E::E1 | E::E2 | E::E3))
+    {
+        Base2::default4(x);
+    }
 };
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
-- 
cgit