Enable loplugin:cstylecast for some more cases

...mostly of C-style casts among arithmetic types, and automatically rewrite
those into either static_cast or a functional cast (which should have identical
semantics, but where the latter probably looks better for simple cases like
casting a literal to a specific type, as in "sal_Int32(0)" vs.
"static_cast<sal_Int32>(0)").

The main benefit of reducing the amount of C-style casts across the code base
further is so that other plugins (that have not been taught about the complex
semantics of C-style cast) can pick those up (cf. the various recent
"loplugin:redundantcast" commits, which address those findings after this
improved loplugin:cstylecast has been run).  Also, I found some places where
a C-style cast has probably been applied only to the first part of a larger
expression in error (because it's easy to forget parentheses in cases like
"(sal_uInt16)VOPT_CLIPMARKS+1"); I'll follow up on those individually.

The improved loplugin:cstylecast is careful to output either "(performs:
static_cast)" or "(performs: functional cast)", so that
compilerplugins/clang/test/cstylecast.cxx can check that the plugin would
automatically rewrite to one or the other form.

To allow fully-automatic rewriting, this also required loplugin:unnecessaryparen
to become a rewriting plugin, at least for the parens-around-cast case (where
"((foo)bar)" first gets rewritten to "(static_cast<foo>(bar))", then to
"static_cast<foo>(bar)".  Rewriting could probably be added to other cases of
loplugin:unnecessaryparen in the future, too.

(The final version of this patch would even have been able to cope with
361dd2576a09fbda83f3ce9a26ecb590c38f74e3 "Replace some C-style casts in ugly
macros with static_cast", so that manual change would not have been necessary
after all.)

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

1 files changed, 98 insertions, 6 deletions

diff --git a/compilerplugins/clang/unnecessaryparen.cxx b/compilerplugins/clang/unnecessaryparen.cxx
index ba6c62a78e43..d4ebd6ed0b14 100644
--- a/compilerplugins/clang/unnecessaryparen.cxx
+++ b/compilerplugins/clang/unnecessaryparen.cxx
@@ -59,11 +59,11 @@ Expr const * ignoreAllImplicit(Expr const * expr) {
 }
 
 class UnnecessaryParen:
-    public RecursiveASTVisitor<UnnecessaryParen>, public loplugin::Plugin
+    public RecursiveASTVisitor<UnnecessaryParen>, public loplugin::RewritePlugin
 {
 public:
     explicit UnnecessaryParen(loplugin::InstantiationData const & data):
-        Plugin(data) {}
+        RewritePlugin(data) {}
 
     virtual void run() override
     {
@@ -103,6 +103,10 @@ private:
     // SwNode::dumpAsXml (sw/source/core/docnode/node.cxx):
     bool isPrecededBy_BAD_CAST(Expr const * expr);
 
+    bool badCombination(SourceLocation loc, int prevOffset, int nextOffset);
+
+    bool removeParens(ParenExpr const * expr);
+
     std::unordered_set<ParenExpr const *> handled_;
 };
 
@@ -200,10 +204,12 @@ bool UnnecessaryParen::VisitParenExpr(const ParenExpr* parenExpr)
             }
         }
     } else if (isa<CXXNamedCastExpr>(subExpr)) {
-        report(
-            DiagnosticsEngine::Warning, "unnecessary parentheses around cast",
-            parenExpr->getLocStart())
-            << parenExpr->getSourceRange();
+        if (!removeParens(parenExpr)) {
+            report(
+                DiagnosticsEngine::Warning, "unnecessary parentheses around cast",
+                parenExpr->getLocStart())
+                << parenExpr->getSourceRange();
+        }
         handled_.insert(parenExpr);
     }
 
@@ -473,6 +479,92 @@ bool UnnecessaryParen::isPrecededBy_BAD_CAST(Expr const * expr) {
     return std::string(p1, p2 - p1).find("BAD_CAST") != std::string::npos;
 }
 
+namespace {
+
+bool badCombinationChar(char c) {
+    return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '_'
+        || c == '+' || c == '-' || c == '\'' || c == '"';
+}
+
+}
+
+bool UnnecessaryParen::badCombination(SourceLocation loc, int prevOffset, int nextOffset) {
+    //TODO: check for start/end of file; take backslash-newline line concatentation into account
+    auto const c1
+        = compiler.getSourceManager().getCharacterData(loc.getLocWithOffset(prevOffset))[0];
+    auto const c2
+        = compiler.getSourceManager().getCharacterData(loc.getLocWithOffset(nextOffset))[0];
+    // An approximation of avoiding whatever combinations that would cause two ajacent tokens to be
+    // lexed differently, using, for now, letters (TODO: non-ASCII ones) and digits and '_'; '+' and
+    // '-' (to avoid ++, etc.); '\'' and '"' (to avoid u'x' or "foo"bar, etc.):
+    return badCombinationChar(c1) && badCombinationChar(c2);
+}
+
+bool UnnecessaryParen::removeParens(ParenExpr const * expr) {
+    if (rewriter == nullptr) {
+        return false;
+    }
+    auto const firstBegin = expr->getLocStart();
+    auto secondBegin = expr->getLocEnd();
+    if (firstBegin.isMacroID() || secondBegin.isMacroID()) {
+        return false;
+    }
+    unsigned firstLen = Lexer::MeasureTokenLength(
+        firstBegin, compiler.getSourceManager(), compiler.getLangOpts());
+    for (auto l = firstBegin.getLocWithOffset(std::max<unsigned>(firstLen, 1));;
+         l = l.getLocWithOffset(1))
+    {
+        unsigned n = Lexer::MeasureTokenLength(
+            l, compiler.getSourceManager(), compiler.getLangOpts());
+        if (n != 0) {
+            break;
+        }
+        ++firstLen;
+    }
+    unsigned secondLen = Lexer::MeasureTokenLength(
+        secondBegin, compiler.getSourceManager(), compiler.getLangOpts());
+    for (;;) {
+        auto l = secondBegin.getLocWithOffset(-1);
+        auto const c = compiler.getSourceManager().getCharacterData(l)[0];
+        if (c == '\n') {
+            if (compiler.getSourceManager().getCharacterData(l.getLocWithOffset(-1))[0] == '\\') {
+                break;
+            }
+        } else if (!(c == ' ' || c == '\t' || c == '\v' || c == '\f')) {
+            break;
+        }
+        secondBegin = l;
+        ++secondLen;
+    }
+    if (!replaceText(firstBegin, firstLen, badCombination(firstBegin, -1, firstLen) ? " " : "")) {
+        if (isDebugMode()) {
+            report(
+                DiagnosticsEngine::Fatal,
+                "TODO: cannot rewrite opening parenthesis, needs investigation",
+                firstBegin);
+            report(
+                DiagnosticsEngine::Note, "when removing these parentheses", expr->getExprLoc())
+                << expr->getSourceRange();
+        }
+        return false;
+    }
+    if (!replaceText(secondBegin, secondLen, badCombination(secondBegin, -1, secondLen) ? " " : ""))
+    {
+        //TODO: roll back first change
+        if (isDebugMode()) {
+            report(
+                DiagnosticsEngine::Fatal,
+                "TODO: cannot rewrite closing parenthesis, needs investigation",
+                secondBegin);
+            report(
+                DiagnosticsEngine::Note, "when removing these parentheses", expr->getExprLoc())
+                << expr->getSourceRange();
+        }
+        return false;
+    }
+    return true;
+}
+
 loplugin::Plugin::Registration< UnnecessaryParen > X("unnecessaryparen", true);
 
 }