Improved loplugin:redundantcast static_cast handling

Change-Id: I74e4ebda40f95661c5ae344132fcabbbf08ab0a4

5 files changed, 257 insertions, 16 deletions

diff --git a/compilerplugins/clang/redundantcast.cxx b/compilerplugins/clang/redundantcast.cxx
index de3353b295c1..93fb8a880ad4 100644
--- a/compilerplugins/clang/redundantcast.cxx
+++ b/compilerplugins/clang/redundantcast.cxx
@@ -66,6 +66,26 @@ bool isArithmeticOp(Expr const * expr) {
     return isa<UnaryOperator>(expr) || isa<AbstractConditionalOperator>(expr);
 }
 
+bool canConstCastFromTo(Expr const * from, Expr const * to) {
+    auto const k1 = from->getValueKind();
+    auto const k2 = to->getValueKind();
+    return (k2 == VK_LValue && k1 == VK_LValue)
+        || (k2 == VK_XValue
+            && (k1 != VK_RValue || from->getType()->isRecordType()));
+}
+
+char const * printExprValueKind(ExprValueKind k) {
+    switch (k) {
+    case VK_RValue:
+        return "prvalue";
+    case VK_LValue:
+        return "lvalue";
+    case VK_XValue:
+        return "xvalue";
+    };
+    llvm_unreachable("unknown ExprValueKind");
+}
+
 class RedundantCast:
     public RecursiveASTVisitor<RedundantCast>, public loplugin::RewritePlugin
 {
@@ -333,9 +353,46 @@ bool RedundantCast::VisitCXXStaticCastExpr(CXXStaticCastExpr const * expr) {
     if (ignoreLocation(expr)) {
         return true;
     }
-    auto t1 = compat::getSubExprAsWritten(expr)->getType();
-    auto t2 = expr->getTypeAsWritten();
-    if (t1.getCanonicalType() != t2.getCanonicalType()) {
+    auto const sub = compat::getSubExprAsWritten(expr);
+    auto const t1 = sub->getType();
+    auto const t2 = expr->getTypeAsWritten();
+    auto const nonClassObjectType = t2->isObjectType()
+        && !(t2->isRecordType() || t2->isArrayType());
+    if (nonClassObjectType && t2.hasLocalQualifiers()) {
+        report(
+            DiagnosticsEngine::Warning,
+            ("in static_cast from %0 %1 to %2 %3, remove redundant top-level"
+             " %select{const qualifier|volatile qualifer|const volatile"
+             " qualifiers}4"),
+            expr->getExprLoc())
+            << t1 << printExprValueKind(sub->getValueKind())
+            << t2 << printExprValueKind(expr->getValueKind())
+            << ((t2.isLocalConstQualified() ? 1 : 0)
+                + (t2.isLocalVolatileQualified() ? 2 : 0) - 1)
+            << expr->getSourceRange();
+        return true;
+    }
+    auto const t3 = expr->getType();
+    auto const c1 = t1.getCanonicalType();
+    auto const c3 = t3.getCanonicalType();
+    if (nonClassObjectType || !canConstCastFromTo(sub, expr)
+        ? c1.getTypePtr() != c3.getTypePtr() : c1 != c3)
+    {
+        bool ObjCLifetimeConversion;
+        if (nonClassObjectType
+            || (c1.getTypePtr() != c3.getTypePtr()
+                && !compiler.getSema().IsQualificationConversion(
+                    c1, c3, false, ObjCLifetimeConversion)))
+        {
+            return true;
+        }
+        report(
+            DiagnosticsEngine::Warning,
+            "static_cast from %0 %1 to %2 %3 should be written as const_cast",
+            expr->getExprLoc())
+            << t1 << printExprValueKind(sub->getValueKind())
+            << t2 << printExprValueKind(expr->getValueKind())
+            << expr->getSourceRange();
         return true;
     }
     if (!isOkToRemoveArithmeticCast(t1, t2, expr->getSubExpr())) {
@@ -372,10 +429,21 @@ bool RedundantCast::VisitCXXStaticCastExpr(CXXStaticCastExpr const * expr) {
             }
         }
     }
+    auto const k1 = sub->getValueKind();
+    auto const k3 = expr->getValueKind();
+    if ((k3 == VK_XValue && k1 != VK_XValue)
+        || (k3 == VK_LValue && k1 == VK_XValue))
+    {
+        return true;
+    }
     report(
         DiagnosticsEngine::Warning,
-        "redundant static_cast from %0 to %1", expr->getExprLoc())
-        << t1 << t2 << expr->getSourceRange();
+        ("static_cast from %0 %1 to %2 %3 is redundant%select{| or should be"
+         " written as an explicit construction of a temporary}4"),
+        expr->getExprLoc())
+        << t1 << printExprValueKind(k1) << t2 << printExprValueKind(k3)
+        << (k3 == VK_RValue && (k1 != VK_RValue || t1->isRecordType()))
+        << expr->getSourceRange();
     return true;
 }
 
diff --git a/compilerplugins/clang/salunicodeliteral.cxx b/compilerplugins/clang/salunicodeliteral.cxx
index 6b03156b55e7..410a1aba45c9 100644
--- a/compilerplugins/clang/salunicodeliteral.cxx
+++ b/compilerplugins/clang/salunicodeliteral.cxx
@@ -52,13 +52,23 @@ private:
 
     void check(ExplicitCastExpr const * expr) {
         if (ignoreLocation(expr)
-            || isInUnoIncludeFile(expr->getExprLoc())
+            || isInUnoIncludeFile(expr->getExprLoc()))
                 //TODO: '#ifdef LIBO_INTERNAL_ONLY' within UNO include files
-            || !(loplugin::TypeCheck(expr->getTypeAsWritten())
-                 .Typedef("sal_Unicode").GlobalNamespace()))
         {
             return;
         }
+        for (auto t = expr->getTypeAsWritten();;) {
+            auto const tt = t->getAs<TypedefType>();
+            if (tt == nullptr) {
+                return;
+            }
+            if (loplugin::TypeCheck(t).Typedef("sal_Unicode")
+                .GlobalNamespace())
+            {
+                break;
+            }
+            t = tt->desugar();
+        }
         auto const e1 = expr->getSubExprAsWritten();
         auto const loc = e1->getLocStart();
         if (loc.isMacroID()
diff --git a/compilerplugins/clang/test/redundantcast.cxx b/compilerplugins/clang/test/redundantcast.cxx
index b6754391e7ca..12fb26c826bf 100644
--- a/compilerplugins/clang/test/redundantcast.cxx
+++ b/compilerplugins/clang/test/redundantcast.cxx
@@ -7,18 +7,13 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
+#include "redundantcast.hxx"
+
 void f1(char *) {}
 void f2(char const *) {}
 
 enum Enum1 { X };
 
-struct S {
-    void f1() { (void)*this; };
-    void f2() const { (void)*this; };
-    void f3() { (void)*this; }
-    void f3() const { (void)*this; };
-};
-
 int main() {
     char * p1;
     char const * p2;
@@ -45,6 +40,138 @@ int main() {
     const_cast<S &>(s).f2(); // expected-error {{redundant const_cast from 'const S' to 'S', result is implicitly cast to 'const S' [loplugin:redundantcast]}}
     const_cast<S &>(s).f3();
     s.f3();
+
+    // non-class lvalue, non-const:
+    int ni{};
+    (void) static_cast<int>(ni); // expected-error {{static_cast from 'int' lvalue to 'int' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) int(ni); //TODO: expected-error {{redundant functional cast from/to 'int' [loplugin:redundantcast]}}
+    (void) static_cast<int &>(ni); // expected-error {{static_cast from 'int' lvalue to 'int &' lvalue is redundant [loplugin:redundantcast]}}
+    (void) static_cast<int &&>(ni);
+    (void) static_cast<int const>(ni); // expected-error {{in static_cast from 'int' lvalue to 'const int' prvalue, remove redundant top-level const qualifier [loplugin:redundantcast]}}
+    /* => */ (void) static_cast<int>(ni); // expected-error {{static_cast from 'int' lvalue to 'int' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) int(ni); //TODO: expected-error {{redundant functional cast from/to 'int' [loplugin:redundantcast]}}
+    (void) static_cast<int const &>(ni); // expected-error {{static_cast from 'int' lvalue to 'const int &' lvalue should be written as const_cast [loplugin:redundantcast]}}
+    /* => */ (void) const_cast<int const &>(ni);
+    (void) static_cast<int const &&>(ni); // expected-error {{static_cast from 'int' lvalue to 'const int &&' xvalue should be written as const_cast [loplugin:redundantcast]}}
+    /* => */ (void) const_cast<int const &&>(ni);
+
+    // non-class lvalue, const:
+    int const ci{};
+    (void) static_cast<int>(ci); // expected-error {{static_cast from 'const int' lvalue to 'int' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) int(ci);
+//  (void) static_cast<int &>(ci);
+//  (void) static_cast<int &&>(ci);
+    (void) static_cast<int const>(ci); // expected-error {{in static_cast from 'const int' lvalue to 'const int' prvalue, remove redundant top-level const qualifier [loplugin:redundantcast]}}
+    /* => */ (void) static_cast<int>(ci); // expected-error {{static_cast from 'const int' lvalue to 'int' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) int(ci);
+    (void) static_cast<int const &>(ci); // expected-error {{static_cast from 'const int' lvalue to 'const int &' lvalue is redundant [loplugin:redundantcast]}}
+    (void) static_cast<int const &&>(ci);
+
+    // non-class xvalue, non-const:
+    (void) static_cast<int>(nix()); // expected-error {{static_cast from 'int' xvalue to 'int' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) int(nix()); //TODO: expected-error {{redundant functional cast from/to 'int' [loplugin:redundantcast]}}
+//  (void) static_cast<int &>(nix());
+    (void) static_cast<int &&>(nix()); // expected-error {{static_cast from 'int' xvalue to 'int &&' xvalue is redundant [loplugin:redundantcast]}}
+    (void) static_cast<int const>(nix()); // expected-error {{in static_cast from 'int' xvalue to 'const int' prvalue, remove redundant top-level const qualifier [loplugin:redundantcast]}}
+    /* => */ (void) static_cast<int>(nix()); // expected-error {{static_cast from 'int' xvalue to 'int' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    (void) static_cast<int const &>(nix());
+    (void) static_cast<int const &&>(nix()); // expected-error {{static_cast from 'int' xvalue to 'const int &&' xvalue should be written as const_cast [loplugin:redundantcast]}}
+    /* => */ (void) const_cast<int const &&>(nix());
+
+    // non-class xvalue, const:
+    (void) static_cast<int>(cix()); // expected-error {{static_cast from 'const int' xvalue to 'int' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) int(cix());
+//  (void) static_cast<int &>(cix());
+//  (void) static_cast<int &&>(cix());
+    (void) static_cast<int const>(cix()); // expected-error {{in static_cast from 'const int' xvalue to 'const int' prvalue, remove redundant top-level const qualifier [loplugin:redundantcast]}}
+    /* => */ (void) static_cast<int>(cix()); // expected-error {{static_cast from 'const int' xvalue to 'int' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) int(cix());
+    (void) static_cast<int const &>(cix());
+    (void) static_cast<int const &&>(cix()); // expected-error {{static_cast from 'const int' xvalue to 'const int &&' xvalue is redundant [loplugin:redundantcast]}}
+
+    // non-class prvalue, non-const:
+    (void) static_cast<int>(nir()); // expected-error {{static_cast from 'int' prvalue to 'int' prvalue is redundant [loplugin:redundantcast]}}
+//  (void) static_cast<int &>(nir());
+    (void) static_cast<int &&>(nir());
+    (void) static_cast<int const>(nir()); // expected-error {{in static_cast from 'int' prvalue to 'const int' prvalue, remove redundant top-level const qualifier [loplugin:redundantcast]}}
+    /* => */ (void) static_cast<int>(nir()); // expected-error {{static_cast from 'int' prvalue to 'int' prvalue is redundant [loplugin:redundantcast]}}
+    (void) static_cast<int const &>(nir()); // expected-error {{static_cast from 'int' prvalue to 'const int &' lvalue is redundant [loplugin:redundantcast]}}
+    (void) static_cast<int const &&>(nir());
+
+    // non-class prvalue, const:
+    (void) static_cast<int>(cir()); // expected-error {{static_cast from 'int' prvalue to 'int' prvalue is redundant [loplugin:redundantcast]}}
+//  (void) static_cast<int &>(cir());
+    (void) static_cast<int &&>(cir());
+    (void) static_cast<int const>(cir()); // expected-error {{in static_cast from 'int' prvalue to 'const int' prvalue, remove redundant top-level const qualifier [loplugin:redundantcast]}}
+    /* => */ (void) static_cast<int>(cir()); // expected-error {{static_cast from 'int' prvalue to 'int' prvalue is redundant [loplugin:redundantcast]}}
+    (void) static_cast<int const &>(cir()); // expected-error {{static_cast from 'int' prvalue to 'const int &' lvalue is redundant [loplugin:redundantcast]}}
+    (void) static_cast<int const &&>(cir());
+
+    // class lvalue, non-const:
+    S ns{};
+    (void) static_cast<S>(ns); // expected-error {{static_cast from 'S' lvalue to 'S' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) S(ns); //TODO: expected-error {{redundant functional cast from/to 'S' [loplugin:redundantcast]}}
+    (void) static_cast<S &>(ns); // expected-error {{static_cast from 'S' lvalue to 'S &' lvalue is redundant [loplugin:redundantcast]}}
+    (void) static_cast<S &&>(ns);
+    (void) static_cast<S const>(ns); // expected-error {{static_cast from 'S' lvalue to 'const S' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ using CS = const S; (void) CS(ns);
+    (void) static_cast<S const &>(ns); // expected-error {{static_cast from 'S' lvalue to 'const S &' lvalue should be written as const_cast [loplugin:redundantcast]}}
+    /* => */ (void) const_cast<S const &>(ns);
+    (void) static_cast<S const &&>(ns); // expected-error {{static_cast from 'S' lvalue to 'const S &&' xvalue should be written as const_cast [loplugin:redundantcast]}}
+    /* => */ (void) const_cast<S const &&>(ns);
+
+    // class lvalue, const:
+    S const cs{};
+    (void) static_cast<S>(cs); // expected-error {{static_cast from 'const S' lvalue to 'S' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) S(cs);
+//  (void) static_cast<S &>(cs);
+//  (void) static_cast<S &&>(cs);
+    (void) static_cast<S const>(cs); // expected-error {{static_cast from 'const S' lvalue to 'const S' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) CS(cs);
+    (void) static_cast<S const &>(cs); // expected-error {{static_cast from 'const S' lvalue to 'const S &' lvalue is redundant [loplugin:redundantcast]}}
+    (void) static_cast<S const &&>(cs);
+
+    // class xvalue, non-const:
+    (void) static_cast<S>(nsx()); // expected-error {{static_cast from 'S' xvalue to 'S' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) S(nsx()); //TODO: expected-error {{redundant functional cast from/to 'S' [loplugin:redundantcast]}}
+//  (void) static_cast<S &>(nsx());
+    (void) static_cast<S &&>(nsx()); // expected-error {{static_cast from 'S' xvalue to 'S &&' xvalue is redundant [loplugin:redundantcast]}}
+    (void) static_cast<S const>(nsx()); // expected-error {{static_cast from 'S' xvalue to 'const S' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) CS(nsx());
+    (void) static_cast<S const &>(nsx());
+    (void) static_cast<S const &&>(nsx()); // expected-error {{static_cast from 'S' xvalue to 'const S &&' xvalue should be written as const_cast [loplugin:redundantcast]}}
+    /* => */ (void) const_cast<S const &&>(nsx());
+
+    // class xvalue, const:
+    (void) static_cast<S>(csx()); // expected-error {{static_cast from 'const S' xvalue to 'S' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) S(csx());
+//  (void) static_cast<S &>(csx());
+//  (void) static_cast<S &&>(csx());
+    (void) static_cast<S const>(csx()); // expected-error {{static_cast from 'const S' xvalue to 'const S' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) CS(csx());
+    (void) static_cast<S const &>(csx());
+    (void) static_cast<S const &&>(csx()); // expected-error {{static_cast from 'const S' xvalue to 'const S &&' xvalue is redundant [loplugin:redundantcast]}}
+
+    // class prvalue, non-const:
+    (void) static_cast<S>(nsr()); // expected-error {{static_cast from 'S' prvalue to 'S' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) S(nsr()); //TODO: expected-error {{redundant functional cast from/to 'S' [loplugin:redundantcast]}}
+//  (void) static_cast<S &>(nsr());
+    (void) static_cast<S &&>(nsr());
+    (void) static_cast<S const>(nsr()); // expected-error {{static_cast from 'S' prvalue to 'const S' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) CS(nsr());
+    (void) static_cast<S const &>(nsr()); // expected-error {{static_cast from 'S' prvalue to 'const S &' lvalue is redundant [loplugin:redundantcast]}}
+    (void) static_cast<S const &&>(nsr()); // expected-error {{static_cast from 'S' prvalue to 'const S &&' xvalue should be written as const_cast [loplugin:redundantcast]}}
+    /* => */ (void) const_cast<S const &&>(nsr());
+
+    // class prvalue, const:
+    (void) static_cast<S>(csr()); // expected-error {{static_cast from 'const S' prvalue to 'S' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) S(csr());
+//  (void) static_cast<S &>(csr());
+//  (void) static_cast<S &&>(csr());
+    (void) static_cast<S const>(csr()); // expected-error {{static_cast from 'const S' prvalue to 'const S' prvalue is redundant or should be written as an explicit construction of a temporary [loplugin:redundantcast]}}
+    /* => */ (void) CS(csr());
+    (void) static_cast<S const &>(csr()); // expected-error {{static_cast from 'const S' prvalue to 'const S &' lvalue is redundant [loplugin:redundantcast]}}
+    (void) static_cast<S const &&>(csr());
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/test/redundantcast.hxx b/compilerplugins/clang/test/redundantcast.hxx
new file mode 100644
index 000000000000..e87da7a55fe4
--- /dev/null
+++ b/compilerplugins/clang/test/redundantcast.hxx
@@ -0,0 +1,35 @@
+/* -*- 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/.
+ */
+
+#ifndef INCLUDED_COMPILERPLUGINS_CLANG_TEST_REDUNDANTCAST_HXX
+#define INCLUDED_COMPILERPLUGINS_CLANG_TEST_REDUNDANTCAST_HXX
+
+struct S {
+    void f1();
+    void f2() const;
+    void f3();
+    void f3() const;
+};
+
+int && nix();
+int const && cix();
+int nir();
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wignored-qualifiers"
+int const cir();
+#pragma clang diagnostic pop
+
+S && nsx();
+S const && csx();
+S nsr();
+S const csr();
+
+#endif
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/test/salunicodeliteral.cxx b/compilerplugins/clang/test/salunicodeliteral.cxx
index 1daf9df0acb9..bf14e4843970 100644
--- a/compilerplugins/clang/test/salunicodeliteral.cxx
+++ b/compilerplugins/clang/test/salunicodeliteral.cxx
@@ -25,7 +25,8 @@ void f(sal_Unicode) {}
 void test() {
     f(sal_Unicode('x')); // expected-error {{in LIBO_INTERNAL_ONLY code, replace literal cast to 'sal_Unicode' (aka 'char16_t') with a u'...' char16_t character literal [loplugin:salunicodeliteral]}}
     f(static_cast<sal_Unicode>('x')); // expected-error {{in LIBO_INTERNAL_ONLY code, replace literal cast to 'sal_Unicode' (aka 'char16_t') with a u'...' char16_t character literal [loplugin:salunicodeliteral]}}
-    f(static_cast<sal_Unicode const>('x')); // expected-error {{in LIBO_INTERNAL_ONLY code, replace literal cast to 'const sal_Unicode' (aka 'const char16_t') with a u'...' char16_t character literal [loplugin:salunicodeliteral]}}
+    using T = sal_Unicode const;
+    f(static_cast<T>('x')); // expected-error {{in LIBO_INTERNAL_ONLY code, replace literal cast to 'T' (aka 'const char16_t') with a u'...' char16_t character literal [loplugin:salunicodeliteral]}}
     f((sal_Unicode) 'x'); // expected-error {{in LIBO_INTERNAL_ONLY code, replace literal cast to 'sal_Unicode' (aka 'char16_t') with a u'...' char16_t character literal [loplugin:salunicodeliteral]}}
     f(sal_Unicode(('x'))); // expected-error {{in LIBO_INTERNAL_ONLY code, replace literal cast to 'sal_Unicode' (aka 'char16_t') with a u'...' char16_t character literal [loplugin:salunicodeliteral]}}
     f(sal_Unicode(120)); // expected-error {{in LIBO_INTERNAL_ONLY code, replace literal cast to 'sal_Unicode' (aka 'char16_t') with a u'...' char16_t character literal [loplugin:salunicodeliteral]}}