[Libreoffice-commits] core.git: compilerplugins/clang desktop/source hwpfilter/source include/comphelper l10ntools/source solenv/CompilerTest_compilerplugins_clang.mk
Noel Grandin (via logerrit)
logerrit at kemper.freedesktop.org
Thu Aug 1 07:10:09 UTC 2019
compilerplugins/clang/mapindex.cxx | 105 +++++++++++++++++++++++++++
compilerplugins/clang/test/mapindex.cxx | 42 ++++++++++
desktop/source/lib/init.cxx | 2
hwpfilter/source/hpara.cxx | 5 -
include/comphelper/IdPropArrayHelper.hxx | 5 -
l10ntools/source/xmlparse.cxx | 2
solenv/CompilerTest_compilerplugins_clang.mk | 1
7 files changed, 156 insertions(+), 6 deletions(-)
New commits:
commit 12062cb281dce9b23bf643dce7744520cf8820f7
Author: Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Mon Jul 29 21:23:08 2019 +0200
Commit: Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Thu Aug 1 09:09:34 2019 +0200
new loplugin:mapindex
Change-Id: I6b5f73b2187009e95d4d666e03e5803f522cee06
Reviewed-on: https://gerrit.libreoffice.org/76584
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/compilerplugins/clang/mapindex.cxx b/compilerplugins/clang/mapindex.cxx
new file mode 100644
index 000000000000..bf8e38919552
--- /dev/null
+++ b/compilerplugins/clang/mapindex.cxx
@@ -0,0 +1,105 @@
+/* -*- 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 "check.hxx"
+#include "plugin.hxx"
+
+/**
+ Look for places we are using map[idx] in a bool context, which allocates an entry in the map, which is sometimes a side-effect we don't want.
+*/
+
+namespace
+{
+class MapIndex : public loplugin::FilteringPlugin<MapIndex>
+{
+public:
+ explicit MapIndex(loplugin::InstantiationData const& data)
+ : FilteringPlugin(data)
+ {
+ }
+
+ virtual void run() override
+ {
+ if (preRun())
+ TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+ }
+
+ bool VisitImplicitCastExpr(const ImplicitCastExpr*);
+ bool VisitMemberExpr(const MemberExpr*);
+};
+
+bool MapIndex::VisitMemberExpr(const MemberExpr* memberExpr)
+{
+ if (ignoreLocation(memberExpr))
+ return true;
+ // operator bool conversion
+ auto conversionDecl = dyn_cast<CXXConversionDecl>(memberExpr->getMemberDecl());
+ if (!conversionDecl || !conversionDecl->getConversionType()->isBooleanType())
+ return true;
+ auto operatorCall = dyn_cast<CXXOperatorCallExpr>(memberExpr->getBase()->IgnoreCasts());
+ if (!operatorCall)
+ return true;
+ if (operatorCall->getOperator() != OverloadedOperatorKind::OO_Subscript)
+ return true;
+ auto tc = loplugin::TypeCheck(operatorCall->getArg(0)->getType());
+ if (!tc.Class("map") && !tc.Class("unordered_map"))
+ return true;
+ report(DiagnosticsEngine::Warning,
+ "will create an empty entry in the map, you sure about that, rather use count()1",
+ operatorCall->getExprLoc());
+ return true;
+}
+
+bool MapIndex::VisitImplicitCastExpr(const ImplicitCastExpr* implicitCastExpr)
+{
+ if (ignoreLocation(implicitCastExpr))
+ {
+ return true;
+ }
+
+ // first cast is some kind of "ToBoolean" cast
+ auto ck = implicitCastExpr->getCastKind();
+ if (ck != CK_MemberPointerToBoolean && ck != CK_PointerToBoolean && ck != CK_IntegralToBoolean
+ && ck != CK_FloatingToBoolean && ck != CK_FloatingComplexToBoolean
+ && ck != CK_IntegralComplexToBoolean)
+ return true;
+
+ // second cast is LValueToRValue
+ implicitCastExpr = dyn_cast<ImplicitCastExpr>(implicitCastExpr->getSubExpr());
+ if (!implicitCastExpr)
+ return true;
+
+ if (implicitCastExpr->getCastKind() != CK_LValueToRValue)
+ return true;
+ auto operatorCall = dyn_cast<CXXOperatorCallExpr>(implicitCastExpr->getSubExpr());
+ if (!operatorCall)
+ return true;
+ if (operatorCall->getOperator() != OverloadedOperatorKind::OO_Subscript)
+ return true;
+ auto tc = loplugin::TypeCheck(operatorCall->getArg(0)->getType());
+ if (!tc.Class("map") && !tc.Class("unordered_map"))
+ return true;
+ report(DiagnosticsEngine::Warning,
+ "will create an empty entry in the map, you sure about that, rather use count()2",
+ implicitCastExpr->getExprLoc());
+ return true;
+}
+
+loplugin::Plugin::Registration<MapIndex> X("mapindex");
+
+} // namespace
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/test/mapindex.cxx b/compilerplugins/clang/test/mapindex.cxx
new file mode 100644
index 000000000000..68261df500dd
--- /dev/null
+++ b/compilerplugins/clang/test/mapindex.cxx
@@ -0,0 +1,42 @@
+/* -*- 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 <map>
+#include <memory>
+
+struct CallbackFlushHandler
+{
+};
+
+struct LibLODocument_Impl
+{
+ std::map<size_t, std::shared_ptr<CallbackFlushHandler>> mpCallbackFlushHandlers;
+};
+
+void foo(LibLODocument_Impl* pDoc)
+{
+ std::map<int, int> aMap;
+ if (aMap[0]) // expected-error {{will create an empty entry in the map, you sure about that, rather use count()2 [loplugin:mapindex]}}
+ ;
+
+ // expected-error at +1 {{will create an empty entry in the map, you sure about that, rather use count()1 [loplugin:mapindex]}}
+ if (pDoc->mpCallbackFlushHandlers[0])
+ ;
+}
+
+void no_warning_expected(const std::string& payload)
+{
+ for (size_t numberPos = 0; numberPos < payload.length(); ++numberPos)
+ {
+ if (payload[numberPos] == ',')
+ break;
+ }
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/desktop/source/lib/init.cxx b/desktop/source/lib/init.cxx
index 89078209ef43..89e93d1723a1 100644
--- a/desktop/source/lib/init.cxx
+++ b/desktop/source/lib/init.cxx
@@ -3261,7 +3261,7 @@ static void doc_postUnoCommand(LibreOfficeKitDocument* pThis, const char* pComma
}
bool bResult = false;
- if (bNotifyWhenFinished && pDocument->mpCallbackFlushHandlers[nView])
+ if (bNotifyWhenFinished && pDocument->mpCallbackFlushHandlers.count(nView))
{
bResult = comphelper::dispatchCommand(aCommand, comphelper::containerToSequence(aPropertyValuesVector),
new DispatchResultListener(pCommand, pDocument->mpCallbackFlushHandlers[nView]));
diff --git a/hwpfilter/source/hpara.cxx b/hwpfilter/source/hpara.cxx
index e470135d09f2..7363a4b91cf1 100644
--- a/hwpfilter/source/hpara.cxx
+++ b/hwpfilter/source/hpara.cxx
@@ -165,9 +165,10 @@ bool HWPPara::Read(HWPFile & hwpf, unsigned char flag)
ii = 0;
while (ii < nch)
{
- hhstr[ii] = readHBox(hwpf);
- if (!hhstr[ii])
+ auto hBox = readHBox(hwpf);
+ if (!hBox)
return false;
+ hhstr[ii] = std::move(hBox);
if (hhstr[ii]->hh == CH_END_PARA)
break;
if( hhstr[ii]->hh < CH_END_PARA )
diff --git a/include/comphelper/IdPropArrayHelper.hxx b/include/comphelper/IdPropArrayHelper.hxx
index 49c324beab2e..34e403b3ab0a 100644
--- a/include/comphelper/IdPropArrayHelper.hxx
+++ b/include/comphelper/IdPropArrayHelper.hxx
@@ -100,9 +100,10 @@ namespace comphelper
OSL_ENSURE(s_nRefCount, "OIdPropertyArrayUsageHelper::getArrayHelper : suspicious call : have a refcount of 0 !");
::osl::MutexGuard aGuard(OIdPropertyArrayUsageHelperMutex<TYPE>::get());
// do we have the array already?
- if (! (*s_pMap)[nId] )
+ auto& rEntry = (*s_pMap)[nId];
+ if (!rEntry)
{
- (*s_pMap)[nId] = createArrayHelper(nId);
+ rEntry = createArrayHelper(nId);
OSL_ENSURE((*s_pMap)[nId], "OIdPropertyArrayUsageHelper::getArrayHelper : createArrayHelper returned nonsense !");
}
return (*s_pMap)[nId];
diff --git a/l10ntools/source/xmlparse.cxx b/l10ntools/source/xmlparse.cxx
index 088729aaf051..73fff95d831d 100644
--- a/l10ntools/source/xmlparse.cxx
+++ b/l10ntools/source/xmlparse.cxx
@@ -376,7 +376,7 @@ void XMLFile::InsertL10NElement( XMLElement* pElement )
else // Already there
{
pElem=pos->second;
- if ( (*pElem)[ sLanguage ] )
+ if ( pElem->count(sLanguage) )
{
fprintf(stdout,"Error: Duplicated entry. ID = %s LANG = %s in File %s\n", sId.getStr(), sLanguage.getStr(), m_sFileName.getStr() );
exit( -1 );
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk
index d70c41faead6..dfa82b5dc18f 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -36,6 +36,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
compilerplugins/clang/test/indentation \
compilerplugins/clang/test/logexceptionnicely \
compilerplugins/clang/test/loopvartoosmall \
+ compilerplugins/clang/test/mapindex \
compilerplugins/clang/test/oncevar \
compilerplugins/clang/test/oslendian-1 \
compilerplugins/clang/test/oslendian-2 \
More information about the Libreoffice-commits
mailing list