Improve performance of loplugin:commaoperator

...by avoiding calls to parentStmt, thereby also improving the precision of
exactly which comma operators to ignore (which turned up a handful more finds).
Also added tests.

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

2 files changed, 74 insertions, 24 deletions

diff --git a/compilerplugins/clang/commaoperator.cxx b/compilerplugins/clang/commaoperator.cxx
index c4a61e772281..c9efc09092de 100644
--- a/compilerplugins/clang/commaoperator.cxx
+++ b/compilerplugins/clang/commaoperator.cxx
@@ -20,6 +20,13 @@ the comma operator is best used sparingly
 
 namespace {
 
+Stmt const * lookThroughExprWithCleanups(Stmt const * stmt) {
+    if (auto const e = dyn_cast_or_null<ExprWithCleanups>(stmt)) {
+        return e->getSubExpr();
+    }
+    return stmt;
+}
+
 class CommaOperator:
     public RecursiveASTVisitor<CommaOperator>, public loplugin::Plugin
 {
@@ -31,11 +38,49 @@ public:
         TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
     }
 
-    bool VisitBinaryOperator(const BinaryOperator* );
+    bool TraverseForStmt(ForStmt * stmt) {
+        auto const saved1 = ignore1_;
+        ignore1_ = lookThroughExprWithCleanups(stmt->getInit());
+        auto const saved2 = ignore2_;
+        ignore2_ = lookThroughExprWithCleanups(stmt->getInc());
+        auto const ret = RecursiveASTVisitor::TraverseForStmt(stmt);
+        ignore1_ = saved1;
+        ignore2_ = saved2;
+        return ret;
+    }
+
+    bool TraverseParenExpr(ParenExpr * expr) {
+        auto const saved1 = ignore1_;
+        ignore1_ = expr->getSubExpr();
+        auto const ret = RecursiveASTVisitor::TraverseParenExpr(expr);
+        ignore1_ = saved1;
+        return ret;
+    }
+
+    bool TraverseBinComma(BinaryOperator * expr) {
+        if (!WalkUpFromBinComma(expr)) {
+            return false;
+        }
+        auto const saved1 = ignore1_;
+        ignore1_ = expr->getLHS();
+        auto const ret = TraverseStmt(expr->getLHS())
+            && TraverseStmt(expr->getRHS());
+        ignore1_ = saved1;
+        return ret;
+    }
+
+    bool VisitBinComma(const BinaryOperator* );
+
+private:
+    Stmt const * ignore1_ = nullptr;
+    Stmt const * ignore2_ = nullptr;
 };
 
-bool CommaOperator::VisitBinaryOperator(const BinaryOperator* binaryOp)
+bool CommaOperator::VisitBinComma(const BinaryOperator* binaryOp)
 {
+    if (binaryOp == ignore1_ || binaryOp == ignore2_) {
+        return true;
+    }
     if (ignoreLocation(binaryOp)) {
         return true;
     }
@@ -54,28 +99,6 @@ bool CommaOperator::VisitBinaryOperator(const BinaryOperator* binaryOp)
     {
         return true;
     }
-    if (binaryOp->getOpcode() != BO_Comma) {
-        return true;
-    }
-    const Stmt* parent = parentStmt(binaryOp);
-    if (parent != nullptr) {
-        if (isa<ParenExpr>(parent)) {
-            return true;
-        }
-        if (isa<BinaryOperator>(parent)) {
-            return true;
-        }
-        if (isa<ForStmt>(parent)) {
-            return true;
-        }
-        if (isa<ExprWithCleanups>(parent)) {
-            const Stmt* parent2 = parentStmt(parent);
-            if (isa<ForStmt>(parent2)) {
-                return true;
-            }
-        }
-    }
-//    parent->dump();
     report(
         DiagnosticsEngine::Warning, "comma operator hides code",
         binaryOp->getOperatorLoc())
diff --git a/compilerplugins/clang/test/commaoperator.cxx b/compilerplugins/clang/test/commaoperator.cxx
new file mode 100644
index 000000000000..199dcf41c243
--- /dev/null
+++ b/compilerplugins/clang/test/commaoperator.cxx
@@ -0,0 +1,27 @@
+/* -*- 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/.
+ */
+
+bool f();
+
+struct S { ~S(); };
+
+int main() {
+    f(), f(), f(); // expected-error {{comma operator hides code [loplugin:commaoperator]}}
+    (f(), f());
+    for (
+        f(), f();
+        f(), f(); // expected-error {{comma operator hides code [loplugin:commaoperator]}}
+        f(), f())
+        f(), f(); // expected-error {{comma operator hides code [loplugin:commaoperator]}}
+    S s;
+    (s = S(), s = S(), s = S());
+    for (s = S(), f(); f(); s = S(), f()) {}
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */