loplugin:simplifybool for negation of comparison operator

Change-Id: Ie56daf560185274754afbc7a09c432b5c2793791
Reviewed-on: https://gerrit.libreoffice.org/45068
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 53 insertions, 17 deletions

diff --git a/compilerplugins/clang/simplifybool.cxx b/compilerplugins/clang/simplifybool.cxx
index cf5570b60008..091591f30988 100644
--- a/compilerplugins/clang/simplifybool.cxx
+++ b/compilerplugins/clang/simplifybool.cxx
@@ -87,25 +87,43 @@ bool SimplifyBool::VisitUnaryLNot(UnaryOperator const * expr) {
         return true;
     }
     auto e = getSubExprOfLogicalNegation(expr->getSubExpr());
-    if (e == nullptr) {
+    if (e) {
+        // Ignore macros, otherwise
+        //    OSL_ENSURE(!b, ...);
+        // triggers.
+        if (e->getLocStart().isMacroID())
+            return true;
+        // double logical not of an int is an idiom to convert to bool
+        if (!e->IgnoreImpCasts()->getType()->isBooleanType())
+            return true;
+        report(
+            DiagnosticsEngine::Warning,
+            ("double logical negation expression of the form '!!A' (with A of type"
+             " %0) can %select{logically|literally}1 be simplified as 'A'"),
+            expr->getLocStart())
+            << e->IgnoreImpCasts()->getType()
+            << e->IgnoreImpCasts()->getType()->isBooleanType()
+            << expr->getSourceRange();
         return true;
     }
-    // Ignore macros, otherwise
-    //    OSL_ENSURE(!b, ...);
-    // triggers.
-    if (e->getLocStart().isMacroID())
-        return true;
-    // double logical not of an int is an idiom to convert to bool
-    if (!e->IgnoreImpCasts()->getType()->isBooleanType())
-        return true;
-    report(
-        DiagnosticsEngine::Warning,
-        ("double logical negation expression of the form '!!A' (with A of type"
-         " %0) can %select{logically|literally}1 be simplified as 'A'"),
-        expr->getLocStart())
-        << e->IgnoreImpCasts()->getType()
-        << e->IgnoreImpCasts()->getType()->isBooleanType()
-        << expr->getSourceRange();
+    if (auto binaryOp = dyn_cast<BinaryOperator>(expr->getSubExpr()->IgnoreParenImpCasts())) {
+        // Ignore macros, otherwise
+        //    OSL_ENSURE(!b, ...);
+        // triggers.
+        if (binaryOp->getLocStart().isMacroID())
+            return true;
+        auto t = binaryOp->getLHS()->IgnoreImpCasts()->getType()->getUnqualifiedDesugaredType();
+        // RecordType would require more smarts - we'd need to verify that an inverted operator actually existed
+        if (t->isTemplateTypeParmType() || t->isRecordType() || t->isDependentType())
+            return true;
+        if (!binaryOp->isComparisonOp())
+            return true;
+        report(
+            DiagnosticsEngine::Warning,
+            ("logical negation of comparison operator, can be simplified by inverting operator"),
+            expr->getLocStart())
+            << expr->getSourceRange();
+    }
     return true;
 }
 
diff --git a/compilerplugins/clang/test/simplifybool.cxx b/compilerplugins/clang/test/simplifybool.cxx
new file mode 100644
index 000000000000..383e513b25e2
--- /dev/null
+++ b/compilerplugins/clang/test/simplifybool.cxx
@@ -0,0 +1,18 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+void f1(int a, int b)
+{
+    if (!(a < b))
+    { // expected-error@-1 {{logical negation of comparison operator, can be simplified by inverting operator [loplugin:simplifybool]}}
+        a = b;
+    }
+};
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */