new loplugin: convertlong

merge the droplong and convertuintptr into one new plugin.
Limit the analysis to looking at var decl's, since that seems to be
safest proposition, even if that too needs some careful analysis.

Change-Id: Id005baaf05cfb157ce44a06a1c81f08559a07d1f
Reviewed-on: https://gerrit.libreoffice.org/46851
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

6 files changed, 183 insertions, 343 deletions

diff --git a/compilerplugins/clang/convertlong.cxx b/compilerplugins/clang/convertlong.cxx
new file mode 100644
index 000000000000..4d713aeecf2b
--- /dev/null
+++ b/compilerplugins/clang/convertlong.cxx
@@ -0,0 +1,142 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * 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/.
+ */
+
+#include <memory>
+#include <cassert>
+#include <string>
+#include <iostream>
+#include <fstream>
+#include <set>
+#include "plugin.hxx"
+#include "check.hxx"
+
+/**
+  plugin to help to when converting code from
+
+     sal_uIntPtr/sal_uLong/sal_Long/long/unsigned long
+
+  to something more precise.
+ */
+namespace
+{
+class ConvertLong : public RecursiveASTVisitor<ConvertLong>, public loplugin::Plugin
+{
+public:
+    explicit ConvertLong(loplugin::InstantiationData const& data)
+        : Plugin(data)
+    {
+    }
+
+    virtual void run() override
+    {
+        std::string fn(compiler.getSourceManager()
+                           .getFileEntryForID(compiler.getSourceManager().getMainFileID())
+                           ->getName());
+        loplugin::normalizeDotDotInFilePath(fn);
+        // using sal_uIntPtr as in-between type when converting void* to rtl_TextEncoding
+        if (fn == SRCDIR "/sal/osl/unx/thread.cxx")
+            return;
+        // too much magic
+        if (fn == SRCDIR "/sal/rtl/alloc_arena.cxx")
+            return;
+        if (fn == SRCDIR "/sal/rtl/alloc_cache.cxx")
+            return;
+        // TODO not sure what is going on here
+        if (fn == SRCDIR "/tools/source/generic/bigint.cxx")
+            return;
+        TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+    }
+
+    bool VisitVarDecl(VarDecl const*);
+    bool TraverseFunctionDecl(FunctionDecl*);
+
+private:
+    bool isInterestingType(QualType qt);
+};
+
+bool ConvertLong::TraverseFunctionDecl(FunctionDecl* functionDecl)
+{
+    // ignore template stuff
+    if (functionDecl->getTemplatedKind() != FunctionDecl::TK_NonTemplate)
+    {
+        return true;
+    }
+    return RecursiveASTVisitor::TraverseFunctionDecl(functionDecl);
+}
+
+bool ConvertLong::VisitVarDecl(VarDecl const* varDecl)
+{
+    if (ignoreLocation(varDecl))
+        return true;
+    StringRef fileName{ compiler.getSourceManager().getFilename(varDecl->getLocation()) };
+    if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/tools/bigint.hxx"))
+        return true;
+    if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/tools/solar.h"))
+        return true;
+    if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/o3tl/string_view.hxx"))
+        return true;
+    if (!varDecl->hasInit())
+        return true;
+    if (isa<IntegerLiteral>(varDecl->getInit()->IgnoreParenImpCasts()))
+        return true;
+    // ignore int x = -1;
+    if (isa<UnaryOperator>(varDecl->getInit()->IgnoreParenImpCasts()))
+        return true;
+    auto lhsType = varDecl->getType();
+    auto rhsType = varDecl->getInit()->IgnoreParenImpCasts()->getType();
+    if (lhsType.getLocalUnqualifiedType() == rhsType)
+        return true;
+    if (!rhsType.getTypePtrOrNull())
+        return true;
+    if (isInterestingType(rhsType))
+        return true;
+    if (!isInterestingType(lhsType))
+        return true;
+    if (rhsType->isFloatingType()) // TODO
+        return true;
+    report(DiagnosticsEngine::Warning, "rather replace type of decl %0 with %1",
+           varDecl->getLocation())
+        << lhsType << rhsType << varDecl->getSourceRange();
+    //lhsType->dump();
+    //varDecl->dump();
+    return true;
+}
+
+bool ConvertLong::isInterestingType(QualType qt)
+{
+    auto tc = loplugin::TypeCheck(qt);
+    if (tc.Typedef())
+    {
+        TypedefType const* typedefType = qt->getAs<TypedefType>();
+        auto name = typedefType->getDecl()->getName();
+        if (name == "sal_uLong")
+            return true;
+        // because this is a typedef to long on 64-bit Linux
+        if (name == "sal_Int64" || name == "sal_uInt64" || name.find("size_t") != StringRef::npos)
+            return false;
+    }
+    if (isa<AutoType>(qt.getTypePtr()))
+        return false;
+    auto unqual = qt.getUnqualifiedType();
+    if (unqual->isSpecificBuiltinType(BuiltinType::Kind::Long)
+        || unqual->isSpecificBuiltinType(BuiltinType::Kind::ULong))
+    {
+        return true;
+    }
+    if (!tc.Typedef())
+        return false;
+    TypedefType const* typedefType = qt->getAs<TypedefType>();
+    auto name = typedefType->getDecl()->getName();
+    return name == "sal_uIntPtr" || name == "sal_IntPtr";
+}
+
+loplugin::Plugin::Registration<ConvertLong> X("convertlong", false);
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/convertuintptr.cxx b/compilerplugins/clang/convertuintptr.cxx
deleted file mode 100644
index d2cc93b458f6..000000000000
--- a/compilerplugins/clang/convertuintptr.cxx
+++ /dev/null
@@ -1,106 +0,0 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
-/*
- * 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/.
- */
-
-#include <memory>
-#include <cassert>
-#include <string>
-#include <iostream>
-#include <fstream>
-#include <set>
-#include "plugin.hxx"
-#include "check.hxx"
-
-/**
-  plugin to help to when converting code from sal_uIntPtr to something more precise.
- */
-namespace {
-
-class ConvertUIntPtr:
-    public RecursiveASTVisitor<ConvertUIntPtr>, public loplugin::Plugin
-{
-public:
-    explicit ConvertUIntPtr(loplugin::InstantiationData const & data):
-        Plugin(data) {}
-
-    virtual void run() override
-    {
-        std::string fn( compiler.getSourceManager().getFileEntryForID(
-                        compiler.getSourceManager().getMainFileID())->getName() );
-        loplugin::normalizeDotDotInFilePath(fn);
-        // using sal_uIntPtr as in-between type when converting void* to rtl_TextEncoding
-        if (fn == SRCDIR "/sal/osl/unx/thread.cxx")
-            return;
-        // too much magic
-        if (fn == SRCDIR "/sal/rtl/alloc_arena.cxx")
-            return;
-        if (fn == SRCDIR "/sal/rtl/alloc_cache.cxx")
-            return;
-        // TODO not sure what is going on here
-        if (fn == SRCDIR "/tools/source/generic/bigint.cxx")
-            return;
-        TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
-    }
-
-    bool VisitImplicitCastExpr(ImplicitCastExpr const *);
-private:
-    bool isIntPtr(QualType qt);
-};
-
-bool ConvertUIntPtr::VisitImplicitCastExpr(ImplicitCastExpr const * castExpr)
-{
-    if (ignoreLocation(castExpr))
-        return true;
-
-    if (castExpr->getCastKind() == CK_LValueToRValue)
-        return true;
-    if (isa<IntegerLiteral>(castExpr->IgnoreCasts()))
-        return true;
-    // ignore literals like "-123"
-    if (isa<UnaryOperator>(castExpr->IgnoreCasts()))
-        return true;
-
-    bool isSrcIntPtr = isIntPtr(castExpr->getSubExpr()->getType());
-    bool isDestIntPtr = isIntPtr(castExpr->getType());
-
-    if (!isSrcIntPtr && !isDestIntPtr)
-        return true;
-
-    // exclude casting between sal_uIntPtr <-> sal_IntPtr
-    if (isSrcIntPtr && isDestIntPtr)
-        return true;
-
-    if (isSrcIntPtr && loplugin::TypeCheck(castExpr->getType()).AnyBoolean())
-        return true;
-
-    report(
-        DiagnosticsEngine::Warning,
-        "cast from %0 to %1",
-        castExpr->getExprLoc())
-      << castExpr->getSubExpr()->getType()
-      << castExpr->getType()
-      << castExpr->getSourceRange();
-
-    return true;
-}
-
-bool ConvertUIntPtr::isIntPtr(QualType qt)
-{
-    auto tc = loplugin::TypeCheck(qt);
-    if (!tc.Typedef())
-        return false;
-    TypedefType const * typedefType = qt->getAs<TypedefType>();
-    auto name = typedefType->getDecl()->getName();
-    return name == "sal_uIntPtr" || name == "sal_IntPtr";
-}
-
-loplugin::Plugin::Registration< ConvertUIntPtr > X("convertuintptr", false);
-
-}
-
-/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/droplong.cxx b/compilerplugins/clang/droplong.cxx
deleted file mode 100644
index 1efcf9bf9efb..000000000000
--- a/compilerplugins/clang/droplong.cxx
+++ /dev/null
@@ -1,190 +0,0 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
-/*
- * 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/.
- */
-
-#include <memory>
-#include <cassert>
-#include <string>
-#include <iostream>
-#include <fstream>
-#include <set>
-#include "plugin.hxx"
-#include "check.hxx"
-
-/**
-The types 'long' and 'unsigned long' are different sizes on different platforms, making them wholly unsuitable
-for portable code.
-And when I mean different sizes, I mean 64bit Linux and 64bit Windows have different sizes.
- */
-namespace {
-
-static bool startswith(const std::string& rStr, const char* pSubStr) {
-    return rStr.compare(0, strlen(pSubStr), pSubStr) == 0;
-}
-
-class DropLong:
-    public RecursiveASTVisitor<DropLong>, public loplugin::Plugin
-{
-public:
-    explicit DropLong(loplugin::InstantiationData const & data): Plugin(data) {}
-
-    virtual void run() override
-    {
-        std::string fn( compiler.getSourceManager().getFileEntryForID(
-                        compiler.getSourceManager().getMainFileID())->getName() );
-        loplugin::normalizeDotDotInFilePath(fn);
-        if (startswith(fn, SRCDIR "/sal/"))
-            return;
-        if (startswith(fn, SRCDIR "/desktop/unx/"))
-            return;
-        if (startswith(fn, SRCDIR "/bridges/"))
-            return;
-        if (startswith(fn, SRCDIR "/registry/"))
-            return;
-        if (startswith(fn, SRCDIR "/tools/source/generic/fract.cxx"))
-            return;
-        if (startswith(fn, SRCDIR "/tools/source/generic/bigint.cxx"))
-            return;
-        // TODO figure out how to cope with iterators
-        if (startswith(fn, SRCDIR "/cppu/source/threadpool/jobqueue.cxx"))
-            return;
-
-        TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
-    }
-
-    bool VisitBinAssign(BinaryOperator const *);
-    bool VisitVarDecl(VarDecl const *);
-    bool VisitCastExpr(CastExpr const *);
-private:
-    bool isOK(QualType lhs, QualType rhs);
-};
-
-bool DropLong::VisitBinAssign(BinaryOperator const * expr)
-{
-    if (ignoreLocation(expr))
-        return true;
-    StringRef fileName { compiler.getSourceManager().getFilename(expr->getExprLoc()) };
-    if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/tools/bigint.hxx"))
-        return true;
-
-    auto lhsType = expr->getLHS()->getType();
-    auto rhsType = expr->getRHS()->IgnoreCasts()->getType();
-    if (!isOK(lhsType, rhsType))
-    {
-        report(
-            DiagnosticsEngine::Warning,
-            "rather replace %0 with %1",
-            expr->getExprLoc())
-          << lhsType
-          << rhsType
-          << expr->getSourceRange();
-//        lhsType->dump();
-    }
-    return true;
-}
-
-bool DropLong::VisitVarDecl(VarDecl const * varDecl)
-{
-    if (ignoreLocation(varDecl))
-        return true;
-    StringRef fileName { compiler.getSourceManager().getFilename(varDecl->getLocation()) };
-    if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/tools/bigint.hxx"))
-        return true;
-    if (!varDecl->hasInit())
-        return true;
-    auto lhsType = varDecl->getType();
-    auto rhsType = varDecl->getInit()->IgnoreCasts()->getType();
-    if (!isOK(lhsType, rhsType))
-    {
-        report(
-            DiagnosticsEngine::Warning,
-            "rather replace %0 with %1",
-            varDecl->getLocation())
-          << lhsType
-          << rhsType
-          << varDecl->getSourceRange();
-//        lhsType->dump();
-    }
-    return true;
-}
-
-bool DropLong::VisitCastExpr(CastExpr const * castExpr)
-{
-    if (ignoreLocation(castExpr))
-        return true;
-    StringRef fileName { compiler.getSourceManager().getFilename(castExpr->getExprLoc()) };
-    if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/tools/bigint.hxx"))
-        return true;
-    if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/sal/types.h"))
-        return true;
-    if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/rtl/math.hxx"))
-        return true;
-    // TODO
-    if (loplugin::hasPathnamePrefix(fileName, SRCDIR "/include/tools/helpers.hxx"))
-        return true;
-    if (isa<ImplicitCastExpr>(castExpr))
-        return true;
-
-    auto type = castExpr->getType();
-    if (loplugin::TypeCheck(type).Typedef())
-    {
-        TypedefType const * typedefType = type->getAs<TypedefType>();
-        if (typedefType->getDecl()->getName() == "sal_uLong")
-            report(
-                DiagnosticsEngine::Warning,
-                "sal_uLong cast from %0",
-                castExpr->getExprLoc())
-              << castExpr->getSubExpr()->getType()
-              << castExpr->getSourceRange();
-    }
-    else if (type->isSpecificBuiltinType(BuiltinType::Kind::Long)
-         || type->isSpecificBuiltinType(BuiltinType::Kind::ULong))
-    {
-        report(
-            DiagnosticsEngine::Warning,
-            "long cast from %0",
-            castExpr->getExprLoc())
-          << castExpr->getSubExpr()->getType()
-          << castExpr->getSourceRange();
-    }
-    return true;
-}
-
-bool DropLong::isOK(QualType lhs, QualType rhs)
-{
-    if (loplugin::TypeCheck(lhs).Typedef())
-    {
-        TypedefType const * typedefType = lhs->getAs<TypedefType>();
-        // Lots of stuff in the standard library and in sal/types.h is
-        // 'long' on Linux, so just ignore all typedefs.
-        if (typedefType->getDecl()->getName() != "sal_uLong")
-            return true;
-    }
-    else if (lhs->isSpecificBuiltinType(BuiltinType::Kind::Long)
-         || lhs->isSpecificBuiltinType(BuiltinType::Kind::ULong))
-    {
-        if (rhs->isSpecificBuiltinType(BuiltinType::Kind::Long)
-            || rhs->isSpecificBuiltinType(BuiltinType::Kind::ULong))
-            return true;
-    }
-    else
-        return true;
-
-    if (isa<SubstTemplateTypeParmType>(lhs))
-        return true;
-    if (isa<AutoType>(lhs))
-        return true;
-
-    return false;
-}
-
-loplugin::Plugin::Registration< DropLong > X("droplong", false);
-
-}
-
-/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/test/convertlong.cxx b/compilerplugins/clang/test/convertlong.cxx
new file mode 100644
index 000000000000..850da3edebf1
--- /dev/null
+++ b/compilerplugins/clang/test/convertlong.cxx
@@ -0,0 +1,41 @@
+/* -*- 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/.
+ */
+
+#include <tools/solar.h>
+
+int main()
+{
+    sal_uIntPtr x = 1;
+    sal_uInt32 y = x;
+    y = x;
+    (void)y;
+}
+
+void main2()
+{
+    int x = 1;
+    int y = 1;
+    long tmp = x + y;
+    // expected-error@-1 {{rather replace type of decl 'long' with 'int' [loplugin:convertlong]}}
+    (void)tmp;
+    tmp = x + y;
+
+    sal_uLong tmp1 = x + y;
+    // expected-error-re@-1 {{rather replace type of decl 'sal_uLong' (aka 'unsigned {{.+}}') with 'int' [loplugin:convertlong]}}
+    (void)tmp1;
+
+    int tmp2 = (sal_uLong)1;
+    tmp2 = (long)1;
+
+    sal_uIntPtr tmp3 = x + y;
+    // expected-error-re@-1 {{rather replace type of decl 'sal_uIntPtr' (aka 'unsigned {{.+}}') with 'int' [loplugin:convertlong]}}
+    (void)tmp3;
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/test/convertuintptr.cxx b/compilerplugins/clang/test/convertuintptr.cxx
deleted file mode 100644
index f1c484d8f730..000000000000
--- a/compilerplugins/clang/test/convertuintptr.cxx
+++ /dev/null
@@ -1,20 +0,0 @@
-/* -*- 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/.
- */
-
-#include <tools/solar.h>
-
-int main()
-{
-    sal_uIntPtr x = 1;
-    sal_uInt32 y = x; // expected-error-re {{cast from 'sal_uIntPtr' (aka 'unsigned {{.+}}') to 'sal_uInt32' (aka 'unsigned {{.+}}') [loplugin:convertuintptr]}}
-    y = x; // expected-error-re {{cast from 'sal_uIntPtr' (aka 'unsigned {{.+}}') to 'sal_uInt32' (aka 'unsigned {{.+}}') [loplugin:convertuintptr]}}
-    (void)y;
-}
-
-/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/test/droplong.cxx b/compilerplugins/clang/test/droplong.cxx
deleted file mode 100644
index 6b9271a20103..000000000000
--- a/compilerplugins/clang/test/droplong.cxx
+++ /dev/null
@@ -1,27 +0,0 @@
-/* -*- 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/.
- */
-
-#include <tools/solar.h>
-
-int main()
-{
-    int x = 1;
-    int y = 1;
-    long tmp = x + y; // expected-error {{rather replace 'long' with 'int' [loplugin:droplong]}}
-    (void)tmp;
-    tmp = x + y; // expected-error {{rather replace 'long' with 'int' [loplugin:droplong]}}
-
-    sal_uLong tmp1 = x + y; // expected-error-re {{rather replace 'sal_uLong' (aka 'unsigned {{.+}}') with 'int' [loplugin:droplong]}}
-    (void)tmp1;
-
-    int tmp2 = (sal_uLong)1; // expected-error-re {{sal_uLong cast from 'sal_uLong' (aka 'unsigned {{.+}}') [loplugin:droplong]}}
-    tmp2 = (long)1; // expected-error {{long cast from 'long' [loplugin:droplong]}}
-}
-
-/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */