[Libreoffice-commits] core.git: 2 commits - compilerplugins/clang
Noel Grandin
noel.grandin at collabora.co.uk
Tue Jun 20 08:03:14 UTC 2017
compilerplugins/clang/test/unusedfields.cxx | 35 +++++++++-
compilerplugins/clang/unusedfields.cxx | 96 +++++++++++++---------------
2 files changed, 78 insertions(+), 53 deletions(-)
New commits:
commit 03ee996717dcf9e20529a6a3295df69d0d86dcce
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date: Tue Jun 20 10:00:58 2017 +0200
loplugin:unusedfields fix more false +
deal with fields assigned to local variables, and some general cleanup
Change-Id: I894c74a01e9e28935ecd84308c2e92b080afafc6
diff --git a/compilerplugins/clang/test/unusedfields.cxx b/compilerplugins/clang/test/unusedfields.cxx
index d24e6551c660..1d43c81ea177 100644
--- a/compilerplugins/clang/test/unusedfields.cxx
+++ b/compilerplugins/clang/test/unusedfields.cxx
@@ -7,7 +7,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/
-#include <rtl/ustring.hxx>
+#include <vector>
struct Foo
// expected-error at -1 {{read m_foo1 [loplugin:unusedfields]}}
@@ -19,25 +19,37 @@ struct Bar
// expected-error at -1 {{read m_bar2 [loplugin:unusedfields]}}
// expected-error at -2 {{read m_bar3 [loplugin:unusedfields]}}
// expected-error at -3 {{read m_bar4 [loplugin:unusedfields]}}
-// expected-error at -4 {{read m_functionpointer [loplugin:unusedfields]}}
+// expected-error at -4 {{read m_bar5 [loplugin:unusedfields]}}
+// expected-error at -5 {{read m_bar6 [loplugin:unusedfields]}}
+// expected-error at -6 {{read m_barfunctionpointer [loplugin:unusedfields]}}
{
- int m_bar1;
- int m_bar2 = 1;
+ int m_bar1;
+ int m_bar2 = 1;
int* m_bar3;
- int m_bar4;
- void (*m_functionpointer)(int&);
+ int m_bar4;
+ void (*m_barfunctionpointer)(int&);
+ int m_bar5;
+ std::vector<int> m_bar6;
+
//check that we see reads of fields when referred to via constructor initializer
Bar(Foo const & foo) : m_bar1(foo.m_foo1) {}
- int bar1() { return m_bar2; }
+ // check that we DONT see reads here
+ int bar2() { return m_bar2; }
- // check that we see reads of fields when operated on via pointer de-ref
- void bar2() { *m_bar3 = 2; }
+ // check that we DONT see reads here
+ void bar3() { *m_bar3 = 2; }
+ void bar3b() { m_bar3 = nullptr; }
// check that we see reads of field when passed to a function pointer
// check that we see read of a field that is a function pointer
- void bar3() { m_functionpointer(m_bar4); }
+ void bar4() { m_barfunctionpointer(m_bar4); }
+
+ // check that we see reads of a field when used in variable init
+ void bar5() { int x = m_bar5; (void) x; }
+ // check that we see reads of a field when used in ranged-for
+ void bar6() { for (auto i : m_bar6) { (void)i; } }
};
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/unusedfields.cxx b/compilerplugins/clang/unusedfields.cxx
index 824efa5c41f7..d12cd19e275f 100644
--- a/compilerplugins/clang/unusedfields.cxx
+++ b/compilerplugins/clang/unusedfields.cxx
@@ -145,7 +145,7 @@ MyFieldInfo UnusedFields::niceName(const FieldDecl* fieldDecl)
}
aInfo.fieldName = fieldDecl->getNameAsString();
- // sometimes the name (if it's anonymous thing) contains the full path of the build folder, which we don't need
+ // sometimes the name (if it's an anonymous thing) contains the full path of the build folder, which we don't need
size_t idx = aInfo.fieldName.find(SRCDIR);
if (idx != std::string::npos) {
aInfo.fieldName = aInfo.fieldName.replace(idx, strlen(SRCDIR), "");
@@ -179,27 +179,6 @@ bool UnusedFields::VisitFieldDecl( const FieldDecl* fieldDecl )
return true;
}
- QualType type = fieldDecl->getType();
- // unwrap array types
- while (type->isArrayType())
- type = type->getAsArrayTypeUnsafe()->getElementType();
-/*
- if( CXXRecordDecl* recordDecl = type->getAsCXXRecordDecl() )
- {
- bool warn_unused = recordDecl->hasAttr<WarnUnusedAttr>();
- if( !warn_unused )
- {
- string n = recordDecl->getQualifiedNameAsString();
- // Check some common non-LO types.
- if( n == "std::string" || n == "std::basic_string"
- || n == "std::list" || n == "std::__debug::list"
- || n == "std::vector" || n == "std::__debug::vector" )
- warn_unused = true;
- }
- if (!warn_unused)
- return true;
- }
-*/
definitionSet.insert(niceName(fieldDecl));
return true;
}
@@ -245,23 +224,24 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
// walk up the tree until we find something interesting
bool bPotentiallyReadFrom = false;
bool bDump = false;
- do {
- if (!parent) {
- // check if we're inside a CXXCtorInitializer
+ do
+ {
+ if (!parent)
+ {
+ // check if we're inside a CXXCtorInitializer or a VarDecl
auto parentsRange = compiler.getASTContext().getParents(*child);
if ( parentsRange.begin() != parentsRange.end())
{
const Decl* decl = parentsRange.begin()->get<Decl>();
- if (decl && isa<CXXConstructorDecl>(decl))
+ if (decl && (isa<CXXConstructorDecl>(decl) || isa<VarDecl>(decl)))
bPotentiallyReadFrom = true;
}
if (!bPotentiallyReadFrom)
return true;
- else
- break;
+ break;
}
if (isa<CastExpr>(parent) || isa<MemberExpr>(parent) || isa<ParenExpr>(parent) || isa<ParenListExpr>(parent)
- || isa<ExprWithCleanups>(parent))
+ || isa<ExprWithCleanups>(parent) || isa<ArrayInitLoopExpr>(parent))
{
child = parent;
auto parentsRange = compiler.getASTContext().getParents(*parent);
@@ -283,20 +263,19 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
auto parentsRange = compiler.getASTContext().getParents(*parent);
parent = parentsRange.begin() == parentsRange.end() ? nullptr : parentsRange.begin()->get<Stmt>();
}
- else if (isa<CaseStmt>(parent))
+ else if (auto caseStmt = dyn_cast<CaseStmt>(parent))
{
- bPotentiallyReadFrom = dyn_cast<CaseStmt>(parent)->getLHS() == child
- || dyn_cast<CaseStmt>(parent)->getRHS() == child;
+ bPotentiallyReadFrom = caseStmt->getLHS() == child || caseStmt->getRHS() == child;
break;
}
- else if (isa<IfStmt>(parent))
+ else if (auto ifStmt = dyn_cast<IfStmt>(parent))
{
- bPotentiallyReadFrom = dyn_cast<IfStmt>(parent)->getCond() == child;
+ bPotentiallyReadFrom = ifStmt->getCond() == child;
break;
}
- else if (isa<DoStmt>(parent))
+ else if (auto doStmt = dyn_cast<DoStmt>(parent))
{
- bPotentiallyReadFrom = dyn_cast<DoStmt>(parent)->getCond() == child;
+ bPotentiallyReadFrom = doStmt->getCond() == child;
break;
}
else if (auto callExpr = dyn_cast<CallExpr>(parent))
@@ -312,7 +291,7 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
;
else if (name == "clear" || name == "dispose" || name == "clearAndDispose" || name == "swap")
// we're abusing the write-only analysis here to look for fields which don't have anything useful
- // being done to them, so we're ignoring things like std::vector::clear, vector::swap,
+ // being done to them, so we're ignoring things like std::vector::clear, std::vector::swap,
// and VclPtr::clearAndDispose
;
else
@@ -325,16 +304,11 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
else if (auto binaryOp = dyn_cast<BinaryOperator>(parent))
{
BinaryOperator::Opcode op = binaryOp->getOpcode();
- // If the child is on the LHS and it is an assignment, we are obviously not reading from it,
- // so walk up the tree.
- if (binaryOp->getLHS() == child && op == BO_Assign) {
- child = parent;
- auto parentsRange = compiler.getASTContext().getParents(*parent);
- parent = parentsRange.begin() == parentsRange.end() ? nullptr : parentsRange.begin()->get<Stmt>();
- } else {
+ // If the child is on the LHS and it is an assignment, we are obviously not reading from it
+ if (!(binaryOp->getLHS() == child && op == BO_Assign)) {
bPotentiallyReadFrom = true;
- break;
}
+ break;
}
else if (isa<ReturnStmt>(parent)
|| isa<CXXConstructExpr>(parent)
@@ -348,7 +322,7 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
|| isa<InitListExpr>(parent)
|| isa<CXXDependentScopeMemberExpr>(parent)
|| isa<UnresolvedMemberExpr>(parent)
- || isa<MaterializeTemporaryExpr>(parent)) //???
+ || isa<MaterializeTemporaryExpr>(parent))
{
bPotentiallyReadFrom = true;
break;
@@ -364,12 +338,14 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
{
break;
}
- else {
+ else
+ {
bPotentiallyReadFrom = true;
bDump = true;
break;
}
} while (true);
+
if (bDump)
{
report(
@@ -377,12 +353,18 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
"oh dear, what can the matter be?",
memberExpr->getLocStart())
<< memberExpr->getSourceRange();
+ report(
+ DiagnosticsEngine::Note,
+ "parent over here",
+ parent->getLocStart())
+ << parent->getSourceRange();
parent->dump();
+ memberExpr->dump();
}
+
if (bPotentiallyReadFrom)
- {
readFromSet.insert(fieldInfo);
- }
+
return true;
}
commit 9d9d024ffba8753db8a93e34d34e22818da002aa
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date: Mon Jun 19 14:16:05 2017 +0200
loplugin:unusedfields fix some more false positives
in the write-only analysis
Change-Id: Ic570416e855b8ec38d54f6f6f1adef4819ea53ee
diff --git a/compilerplugins/clang/test/unusedfields.cxx b/compilerplugins/clang/test/unusedfields.cxx
index f549a9a37673..d24e6551c660 100644
--- a/compilerplugins/clang/test/unusedfields.cxx
+++ b/compilerplugins/clang/test/unusedfields.cxx
@@ -17,12 +17,27 @@ struct Foo
struct Bar
// expected-error at -1 {{read m_bar2 [loplugin:unusedfields]}}
+// expected-error at -2 {{read m_bar3 [loplugin:unusedfields]}}
+// expected-error at -3 {{read m_bar4 [loplugin:unusedfields]}}
+// expected-error at -4 {{read m_functionpointer [loplugin:unusedfields]}}
{
int m_bar1;
int m_bar2 = 1;
+ int* m_bar3;
+ int m_bar4;
+ void (*m_functionpointer)(int&);
+ //check that we see reads of fields when referred to via constructor initializer
Bar(Foo const & foo) : m_bar1(foo.m_foo1) {}
- int bar() { return m_bar2; }
+ int bar1() { return m_bar2; }
+
+ // check that we see reads of fields when operated on via pointer de-ref
+ void bar2() { *m_bar3 = 2; }
+
+ // check that we see reads of field when passed to a function pointer
+ // check that we see read of a field that is a function pointer
+ void bar3() { m_functionpointer(m_bar4); }
+
};
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/unusedfields.cxx b/compilerplugins/clang/unusedfields.cxx
index df64c987f347..824efa5c41f7 100644
--- a/compilerplugins/clang/unusedfields.cxx
+++ b/compilerplugins/clang/unusedfields.cxx
@@ -261,12 +261,28 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
break;
}
if (isa<CastExpr>(parent) || isa<MemberExpr>(parent) || isa<ParenExpr>(parent) || isa<ParenListExpr>(parent)
- || isa<ExprWithCleanups>(parent) || isa<UnaryOperator>(parent))
+ || isa<ExprWithCleanups>(parent))
{
child = parent;
auto parentsRange = compiler.getASTContext().getParents(*parent);
parent = parentsRange.begin() == parentsRange.end() ? nullptr : parentsRange.begin()->get<Stmt>();
}
+ else if (auto unaryOperator = dyn_cast<UnaryOperator>(parent))
+ {
+ UnaryOperator::Opcode op = unaryOperator->getOpcode();
+ if (op == UO_AddrOf || op == UO_Deref
+ || op == UO_Plus || op == UO_Minus
+ || op == UO_Not || op == UO_LNot
+ || op == UO_PreInc || op == UO_PostInc
+ || op == UO_PreDec || op == UO_PostDec)
+ {
+ bPotentiallyReadFrom = true;
+ break;
+ }
+ child = parent;
+ auto parentsRange = compiler.getASTContext().getParents(*parent);
+ parent = parentsRange.begin() == parentsRange.end() ? nullptr : parentsRange.begin()->get<Stmt>();
+ }
else if (isa<CaseStmt>(parent))
{
bPotentiallyReadFrom = dyn_cast<CaseStmt>(parent)->getLHS() == child
More information about the Libreoffice-commits
mailing list