Make loplugin:simplifypointertobool handle parenthesized expressions

...as discussed as an open TODO in the commit message of
fe6cce01c88d045a1fcf09acf049c34c22299b02 "Fix loplugin:simplifypointertobool for
libstdc++ std::shared_ptr".  The necessary changes across the code base have
been done fully automatically with the rewriting plugin on Linux.  (All those
changes apparently involve uses of macro arguments wrapped in parentheses in the
macro body, but always in conditionally-converted-to-bool contexts.  In other
contexts, such automatic rewriting would add the "bool" to the macro body, which
would be wrong in general, but we apparently get away with that sloppy coding
for now.)

The parenExprs_ stack that fe6cce01c88d045a1fcf09acf049c34c22299b02 had
introduced to treat such (then-undetected, it had turned out) parenthesized
cases now turns out to not be needed after all.

Change-Id: I2021f61c2e2805be7e18b38edf8744d186cac3cb
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/95010
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

3 files changed, 17 insertions, 32 deletions

diff --git a/compilerplugins/clang/sharedvisitor/dummyplugin.hxx b/compilerplugins/clang/sharedvisitor/dummyplugin.hxx
index b24157965df2..32e0e5e560ee 100644
--- a/compilerplugins/clang/sharedvisitor/dummyplugin.hxx
+++ b/compilerplugins/clang/sharedvisitor/dummyplugin.hxx
@@ -41,7 +41,6 @@ public:
     bool TraverseWhileStmt( WhileStmt* ) { return complain(); }
     bool TraverseDoStmt( DoStmt* ) { return complain(); }
     bool TraverseForStmt( ForStmt* ) { return complain(); }
-    bool TraverseParenExpr( ParenExpr* ) { return complain(); }
     bool TraverseUnaryLNot( UnaryOperator* ) { return complain(); }
     bool TraverseBinLAnd( BinaryOperator* ) { return complain(); }
     bool TraverseBinLOr( BinaryOperator* ) { return complain(); }
diff --git a/compilerplugins/clang/simplifypointertobool.cxx b/compilerplugins/clang/simplifypointertobool.cxx
index 4fc4901c3efe..46a80a770757 100644
--- a/compilerplugins/clang/simplifypointertobool.cxx
+++ b/compilerplugins/clang/simplifypointertobool.cxx
@@ -10,7 +10,6 @@
 #include <algorithm>
 #include <cassert>
 #include <deque>
-#include <stack>
 #include <string>
 #include <iostream>
 #include <fstream>
@@ -234,28 +233,6 @@ public:
         return res;
     }
 
-    bool PreTraverseParenExpr(ParenExpr* expr)
-    {
-        parenExprs_.push(expr);
-        return true;
-    }
-
-    bool PostTraverseParenExpr(ParenExpr*, bool)
-    {
-        assert(!parenExprs_.empty());
-        parenExprs_.pop();
-        return true;
-    }
-
-    bool TraverseParenExpr(ParenExpr* expr)
-    {
-        auto res = PreTraverseParenExpr(expr);
-        assert(res);
-        res = FilteringRewritePlugin::TraverseParenExpr(expr);
-        PostTraverseParenExpr(expr, res);
-        return res;
-    }
-
 private:
     bool isContextuallyConverted(Expr const* expr) const
     {
@@ -318,8 +295,6 @@ private:
     //TODO: There are some more places where an expression is contextually converted to bool, but
     // those are probably not relevant for our needs here.
     std::deque<Expr const*> contextuallyConvertedExprs_;
-
-    std::stack<ParenExpr const*> parenExprs_;
 };
 
 bool SimplifyPointerToBool::VisitImplicitCastExpr(ImplicitCastExpr const* castExpr)
@@ -328,7 +303,7 @@ bool SimplifyPointerToBool::VisitImplicitCastExpr(ImplicitCastExpr const* castEx
         return true;
     if (castExpr->getCastKind() != CK_PointerToBoolean)
         return true;
-    auto memberCallExpr = dyn_cast<CXXMemberCallExpr>(castExpr->getSubExpr());
+    auto memberCallExpr = dyn_cast<CXXMemberCallExpr>(castExpr->getSubExpr()->IgnoreParens());
     if (!memberCallExpr)
         return true;
     auto methodDecl = memberCallExpr->getMethodDecl();
@@ -395,17 +370,27 @@ bool SimplifyPointerToBool::VisitImplicitCastExpr(ImplicitCastExpr const* castEx
         report(DiagnosticsEngine::Warning, "simplify, drop the get()", memberCallExpr->getExprLoc())
             << memberCallExpr->getSourceRange();
     }
-    else if (!parenExprs_.empty() && parenExprs_.top()->IgnoreImpCasts() == memberCallExpr)
+    else if (isa<ParenExpr>(castExpr->getSubExpr()))
     {
-        //TODO: attempt rewrite
+        if (rewriter)
+        {
+            auto const loc
+                = compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(memberCallExpr));
+            auto const range = getCallSourceRange(memberCallExpr);
+            if (loc.isValid() && range.isValid() && insertText(loc, "bool") && removeText(range))
+            {
+                //TODO: atomically only change both or neither
+                return true;
+            }
+        }
         report(DiagnosticsEngine::Warning,
                "simplify, drop the get() and turn the surrounding parentheses into a functional "
                "cast to bool",
                memberCallExpr->getExprLoc())
             << memberCallExpr->getSourceRange();
         report(DiagnosticsEngine::Note, "surrounding parentheses here",
-               parenExprs_.top()->getExprLoc())
-            << parenExprs_.top()->getSourceRange();
+               castExpr->getSubExpr()->getExprLoc())
+            << castExpr->getSubExpr()->getSourceRange();
     }
     else
     {
diff --git a/compilerplugins/clang/test/simplifypointertobool.cxx b/compilerplugins/clang/test/simplifypointertobool.cxx
index 18e1da607bb8..e4bf14c40f45 100644
--- a/compilerplugins/clang/test/simplifypointertobool.cxx
+++ b/compilerplugins/clang/test/simplifypointertobool.cxx
@@ -18,7 +18,8 @@ bool test1(std::unique_ptr<int> p2)
         foo();
     // expected-error@+1 {{simplify, drop the get() and wrap the expression in a functional cast to bool [loplugin:simplifypointertobool]}}
     bool b1 = p2.get();
-    //TODO:
+    // expected-error@+3 {{simplify, drop the get() and turn the surrounding parentheses into a functional cast to bool [loplugin:simplifypointertobool]}}
+    // expected-note@+1 {{surrounding parentheses here [loplugin:simplifypointertobool]}}
     bool b2 = ( // deliberately spread across multiple lines
         p2.get());
     return b1 && b2;