From 447e4209fa16e765d9cba9f1c80bf10e8901204c Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Thu, 30 Jan 2020 09:21:55 +0200 Subject: new (but unused) refassign loplugin which I wrote to check if there were any other cases where assigning to a ref local var looks dodgy, after my fixes in 0b113d6ebbaf923e11ba576bed2691bb68e95ae6, but I didn't find anything, so just leave this one in store Change-Id: Ib820924c5e8aa85206730afeb06972ef48231ec5 --- compilerplugins/clang/store/refassign.cxx | 152 ++++++++++++++++++++++++++++++ compilerplugins/clang/test/refassign.cxx | 31 ++++++ 2 files changed, 183 insertions(+) create mode 100644 compilerplugins/clang/store/refassign.cxx create mode 100644 compilerplugins/clang/test/refassign.cxx (limited to 'compilerplugins/clang') diff --git a/compilerplugins/clang/store/refassign.cxx b/compilerplugins/clang/store/refassign.cxx new file mode 100644 index 000000000000..27471a6ae643 --- /dev/null +++ b/compilerplugins/clang/store/refassign.cxx @@ -0,0 +1,152 @@ +/* -*- 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/. + */ +#ifndef LO_CLANG_SHARED_PLUGINS + +#include +#include +#include +#include +#include + +#include + +#include "check.hxx" +#include "compat.hxx" +#include "plugin.hxx" + +/** + * Look for a mistake I made (a lot) at one point where we assign a reference to a reference var, which + * does not do at all what I thought. + */ + +namespace +{ +class RefAssign : public loplugin::FilteringPlugin +{ +public: + explicit RefAssign(loplugin::InstantiationData const& data) + : FilteringPlugin(data) + { + } + + virtual bool preRun() override + { + StringRef fn(handler.getMainFileName()); + if (loplugin::isSamePathname(fn, SRCDIR "/comphelper/source/misc/syntaxhighlight.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/svl/source/numbers/zformat.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/tools/source/memtools/multisel.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/tools/source/generic/point.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/vcl/source/control/edit.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/vcl/source/control/fmtfield.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/vcl/source/control/field.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/vcl/source/control/field2.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/vcl/source/edit/textview.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/vcl/source/edit/vclmedit.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/editeng/source/editeng/editdoc.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/editeng/source/editeng/impedit2.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/svx/source/dialog/svxruler.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/sw/source/uibase/ribbar/inputwin.cxx")) + return false; + if (loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/text/txtftn.cxx")) + return false; + + return true; + } + + virtual void run() override + { + if (preRun()) + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + } + + bool VisitBinaryOperator(BinaryOperator const*); + // causes crashes in CallExpr::getReturnType + bool TraverseFunctionTemplateDecl(FunctionTemplateDecl*) { return true; } + bool TraverseClassTemplateDecl(ClassTemplateDecl*) { return true; } + +private: + clang::QualType ExtractType(Expr const*); +}; + +bool RefAssign::VisitBinaryOperator(BinaryOperator const* binaryOp) +{ + if (ignoreLocation(binaryOp)) + return true; + if (binaryOp->getOpcode() != BO_Assign) + return true; + + // ignore assigning to/from an element of a collection + if (isa(binaryOp->getLHS()->IgnoreParenImpCasts())) + return true; + if (isa(binaryOp->getRHS()->IgnoreParenImpCasts())) + return true; + + // if we are assigning to a parameter we probably mean it + if (auto declRefExpr = dyn_cast(binaryOp->getLHS()->IgnoreParenImpCasts())) + if (declRefExpr->getDecl() && isa(declRefExpr->getDecl())) + return true; + + if (auto callExpr = dyn_cast(binaryOp->getRHS()->IgnoreParenImpCasts())) + if (auto functionDecl = dyn_cast_or_null(callExpr->getCalleeDecl())) + if (functionDecl->getIdentifier() + && (functionDecl->getName() == "min" || functionDecl->getName() == "max")) + return true; + + auto lhsType = ExtractType(binaryOp->getLHS()); + auto rhsType = ExtractType(binaryOp->getRHS()); + if (!loplugin::TypeCheck(lhsType).LvalueReference()) + return true; + if (!loplugin::TypeCheck(rhsType).LvalueReference()) + return true; + binaryOp->dump(); + report(DiagnosticsEngine::Warning, + "assigning a %0 to a var of type %1 probably does not do what you think", + compat::getBeginLoc(binaryOp)) + << rhsType << lhsType << binaryOp->getSourceRange(); + return true; +} + +clang::QualType RefAssign::ExtractType(Expr const* expr) +{ + expr = expr->IgnoreParenImpCasts(); + if (auto declReflExpr = dyn_cast(expr)) + { + if (auto varDecl = dyn_cast(declReflExpr->getDecl())) + return varDecl->getType(); + } + else if (auto callExpr = dyn_cast(expr)) + { + if (callExpr->isTypeDependent()) + return {}; + // callExpr->dump(); + return callExpr->getCallReturnType(compiler.getASTContext()); + } + return expr->getType(); +} + +loplugin::Plugin::Registration refassign("refassign"); + +} // namespace + +#endif // LO_CLANG_SHARED_PLUGINS + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/compilerplugins/clang/test/refassign.cxx b/compilerplugins/clang/test/refassign.cxx new file mode 100644 index 000000000000..7fc20cbe2318 --- /dev/null +++ b/compilerplugins/clang/test/refassign.cxx @@ -0,0 +1,31 @@ +/* -*- 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 +#include + +int& f1(); +int& f2(); + +void test1() +{ + int& v1 = f1(); + v1 = f2(); // expected-error {{assigning a 'int &' to a var of type 'int &' probably does not do what you think [loplugin:refassign]}} + f1() + = f2(); // expected-error {{assigning a 'int &' to a var of type 'int &' probably does not do what you think [loplugin:refassign]}} + + // no warning expected + int x = 1; + x = f1(); + std::vector v; + v[0] = f1(); + v1 = std::min(1, 2); +}; + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ -- cgit