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
361dd257 "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/47798Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

compilerplugins/clang/test/cstylecast.cxx

+/* -*- 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/.
+ */
+
+namespace N
+{
+enum E
+{
+    E1
+};
+
+using T = unsigned int;
+}
+
+static const int C
+    = (int)0; // expected-error {{C-style cast from 'int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+
+int main()
+{
+    constexpr int c1 = 0;
+    int const c2 = 0;
+    int n
+        = (int)0; // expected-error {{C-style cast from 'int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+    n = (signed int)0; // expected-error {{C-style cast from 'int' to 'int' (performs: static_cast) (NoOp) [loplugin:cstylecast]}}
+    n = (int)~0; // expected-error {{C-style cast from 'int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+    n = (int)-0; // expected-error {{C-style cast from 'int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+    n = (int)+0; // expected-error {{C-style cast from 'int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+    n = (int)!0; // expected-error {{C-style cast from 'bool' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+    n = (int) // expected-error {{C-style cast from 'int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+        (0 << 0);
+    n = (int) // expected-error {{C-style cast from 'const int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+        c1;
+    n = (int) // expected-error {{C-style cast from 'const int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+        c2;
+    n = (int) // expected-error {{C-style cast from 'const int' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+        C;
+    n = (int) // expected-error {{C-style cast from 'N::E' to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+        N::E1;
+    n = (N::E) // expected-error {{C-style cast from 'N::E' to 'N::E' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+        N::E1;
+    n = (enum // expected-error {{C-style cast from 'N::E' to 'enum N::E' (performs: static_cast) (NoOp) [loplugin:cstylecast]}}
+         N::E)N::E1;
+    n = (N::T)0; // expected-error {{C-style cast from 'int' to 'N::T' (aka 'unsigned int') (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+    n = (int) // expected-error-re {{C-style cast from {{.*}} to 'int' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+        sizeof(int);
+    n = (int) // expected-error {{C-style cast from 'int' to 'int' (performs: static_cast) (NoOp) [loplugin:cstylecast]}}
+        n;
+    n = (int)~n; // expected-error {{C-style cast from 'int' to 'int' (performs: static_cast) (NoOp) [loplugin:cstylecast]}}
+    n = (int)-n; // expected-error {{C-style cast from 'int' to 'int' (performs: static_cast) (NoOp) [loplugin:cstylecast]}}
+    n = (int)+n; // expected-error {{C-style cast from 'int' to 'int' (performs: static_cast) (NoOp) [loplugin:cstylecast]}}
+    n = (int)!n; // expected-error {{C-style cast from 'bool' to 'int' (performs: static_cast) (NoOp) [loplugin:cstylecast]}}
+    n = (int) // expected-error {{C-style cast from 'int' to 'int' (performs: static_cast) (NoOp) [loplugin:cstylecast]}}
+        (0 << n);
+    n = (double)0; // expected-error {{C-style cast from 'int' to 'double' (performs: functional cast) (NoOp) [loplugin:cstylecast]}}
+    (void)n;
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

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);
 
 }

solenv/CompilerTest_compilerplugins_clang.mk

@@ -17,6 +17,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
     compilerplugins/clang/test/constparams \
     compilerplugins/clang/test/convertlong \
     compilerplugins/clang/test/cppunitassertequals \
+    compilerplugins/clang/test/cstylecast \
     compilerplugins/clang/test/datamembershadow \
     compilerplugins/clang/test/dodgyswitch \
     compilerplugins/clang/test/externvar \