[Libreoffice-commits] core.git: compilerplugins/clang solenv/CompilerTest_compilerplugins_clang.mk sw/source vcl/source
Stephan Bergmann
sbergman at redhat.com
Fri Oct 6 08:05:27 UTC 2017
compilerplugins/clang/commaoperator.cxx | 71 +++++++++++++++++----------
compilerplugins/clang/test/commaoperator.cxx | 27 ++++++++++
solenv/CompilerTest_compilerplugins_clang.mk | 1
sw/source/core/doc/number.cxx | 20 ++++++-
vcl/source/gdi/svmconverter.cxx | 5 +
5 files changed, 95 insertions(+), 29 deletions(-)
New commits:
commit 4fc52078f6afa4368b2f4de3cd700e474b7a417f
Author: Stephan Bergmann <sbergman at redhat.com>
Date: Fri Oct 6 08:55:26 2017 +0200
Improve performance of loplugin:commaoperator
...by avoiding calls to parentStmt, thereby also improving the precision of
exactly which comma operators to ignore (which turned up a handful more finds).
Also added tests.
Change-Id: Ie74f824fd7f54131aab09b59086452fb4f3ff827
Reviewed-on: https://gerrit.libreoffice.org/43181
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman at redhat.com>
diff --git a/compilerplugins/clang/commaoperator.cxx b/compilerplugins/clang/commaoperator.cxx
index c4a61e772281..c9efc09092de 100644
--- a/compilerplugins/clang/commaoperator.cxx
+++ b/compilerplugins/clang/commaoperator.cxx
@@ -20,6 +20,13 @@ the comma operator is best used sparingly
namespace {
+Stmt const * lookThroughExprWithCleanups(Stmt const * stmt) {
+ if (auto const e = dyn_cast_or_null<ExprWithCleanups>(stmt)) {
+ return e->getSubExpr();
+ }
+ return stmt;
+}
+
class CommaOperator:
public RecursiveASTVisitor<CommaOperator>, public loplugin::Plugin
{
@@ -31,11 +38,49 @@ public:
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
}
- bool VisitBinaryOperator(const BinaryOperator* );
+ bool TraverseForStmt(ForStmt * stmt) {
+ auto const saved1 = ignore1_;
+ ignore1_ = lookThroughExprWithCleanups(stmt->getInit());
+ auto const saved2 = ignore2_;
+ ignore2_ = lookThroughExprWithCleanups(stmt->getInc());
+ auto const ret = RecursiveASTVisitor::TraverseForStmt(stmt);
+ ignore1_ = saved1;
+ ignore2_ = saved2;
+ return ret;
+ }
+
+ bool TraverseParenExpr(ParenExpr * expr) {
+ auto const saved1 = ignore1_;
+ ignore1_ = expr->getSubExpr();
+ auto const ret = RecursiveASTVisitor::TraverseParenExpr(expr);
+ ignore1_ = saved1;
+ return ret;
+ }
+
+ bool TraverseBinComma(BinaryOperator * expr) {
+ if (!WalkUpFromBinComma(expr)) {
+ return false;
+ }
+ auto const saved1 = ignore1_;
+ ignore1_ = expr->getLHS();
+ auto const ret = TraverseStmt(expr->getLHS())
+ && TraverseStmt(expr->getRHS());
+ ignore1_ = saved1;
+ return ret;
+ }
+
+ bool VisitBinComma(const BinaryOperator* );
+
+private:
+ Stmt const * ignore1_ = nullptr;
+ Stmt const * ignore2_ = nullptr;
};
-bool CommaOperator::VisitBinaryOperator(const BinaryOperator* binaryOp)
+bool CommaOperator::VisitBinComma(const BinaryOperator* binaryOp)
{
+ if (binaryOp == ignore1_ || binaryOp == ignore2_) {
+ return true;
+ }
if (ignoreLocation(binaryOp)) {
return true;
}
@@ -54,28 +99,6 @@ bool CommaOperator::VisitBinaryOperator(const BinaryOperator* binaryOp)
{
return true;
}
- if (binaryOp->getOpcode() != BO_Comma) {
- return true;
- }
- const Stmt* parent = parentStmt(binaryOp);
- if (parent != nullptr) {
- if (isa<ParenExpr>(parent)) {
- return true;
- }
- if (isa<BinaryOperator>(parent)) {
- return true;
- }
- if (isa<ForStmt>(parent)) {
- return true;
- }
- if (isa<ExprWithCleanups>(parent)) {
- const Stmt* parent2 = parentStmt(parent);
- if (isa<ForStmt>(parent2)) {
- return true;
- }
- }
- }
-// parent->dump();
report(
DiagnosticsEngine::Warning, "comma operator hides code",
binaryOp->getOperatorLoc())
diff --git a/compilerplugins/clang/test/commaoperator.cxx b/compilerplugins/clang/test/commaoperator.cxx
new file mode 100644
index 000000000000..199dcf41c243
--- /dev/null
+++ b/compilerplugins/clang/test/commaoperator.cxx
@@ -0,0 +1,27 @@
+/* -*- 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/.
+ */
+
+bool f();
+
+struct S { ~S(); };
+
+int main() {
+ f(), f(), f(); // expected-error {{comma operator hides code [loplugin:commaoperator]}}
+ (f(), f());
+ for (
+ f(), f();
+ f(), f(); // expected-error {{comma operator hides code [loplugin:commaoperator]}}
+ f(), f())
+ f(), f(); // expected-error {{comma operator hides code [loplugin:commaoperator]}}
+ S s;
+ (s = S(), s = S(), s = S());
+ for (s = S(), f(); f(); s = S(), f()) {}
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk
index 0a1fc0f2ebfe..26306dccc983 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -13,6 +13,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
compilerplugins/clang/test/badstatics \
compilerplugins/clang/test/blockblock \
compilerplugins/clang/test/casttovoid \
+ compilerplugins/clang/test/commaoperator \
compilerplugins/clang/test/constparams \
$(if $(filter-out INTEL,$(CPU)),compilerplugins/clang/test/convertuintptr) \
compilerplugins/clang/test/cppunitassertequals \
diff --git a/sw/source/core/doc/number.cxx b/sw/source/core/doc/number.cxx
index 415e3ce10752..fabadb97b065 100644
--- a/sw/source/core/doc/number.cxx
+++ b/sw/source/core/doc/number.cxx
@@ -487,17 +487,29 @@ SwNumRule::~SwNumRule()
int n;
for( n = 0; n < MAXLEVEL; ++n, ++ppFormats )
- delete *ppFormats, *ppFormats = nullptr;
+ {
+ delete *ppFormats;
+ *ppFormats = nullptr;
+ }
// Outline:
for( n = 0; n < MAXLEVEL; ++n, ++ppFormats )
- delete *ppFormats, *ppFormats = nullptr;
+ {
+ delete *ppFormats;
+ *ppFormats = nullptr;
+ }
ppFormats = &SwNumRule::maLabelAlignmentBaseFormats[0][0];
for( n = 0; n < MAXLEVEL; ++n, ++ppFormats )
- delete *ppFormats, *ppFormats = nullptr;
+ {
+ delete *ppFormats;
+ *ppFormats = nullptr;
+ }
for( n = 0; n < MAXLEVEL; ++n, ++ppFormats )
- delete *ppFormats, *ppFormats = nullptr;
+ {
+ delete *ppFormats;
+ *ppFormats = nullptr;
+ }
}
maTextNodeList.clear();
diff --git a/vcl/source/gdi/svmconverter.cxx b/vcl/source/gdi/svmconverter.cxx
index 5a07927216f4..57c47fd3d85c 100644
--- a/vcl/source/gdi/svmconverter.cxx
+++ b/vcl/source/gdi/svmconverter.cxx
@@ -988,7 +988,10 @@ void SVMConverter::ImplConvertFromSVM1( SvStream& rIStm, GDIMetaFile& rMtf )
pDXAry.reset(new long[nDXAryLen]);
for (sal_Int32 j = 0; j < nAryLen; ++j)
- rIStm.ReadInt32( nTmp ), pDXAry[ j ] = nTmp;
+ {
+ rIStm.ReadInt32( nTmp );
+ pDXAry[ j ] = nTmp;
+ }
// #106172# Add last DX array elem, if missing
if( nAryLen != nStrLen )
More information about the Libreoffice-commits
mailing list