mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

loplugin:unusedfields fix more false +

Browse Source 
deal with fields assigned to local variables, and some general cleanup

Change-Id: I894c74a01e9e28935ecd84308c2e92b080afafc6
This commit is contained in:
Noel Grandin
2017-06-20 10:00:58 +02:00
parent 9d9d024ffb
commit 03ee996717
2 changed files with 53 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
compilerplugins/clang/test/unusedfields.cxx
View File

@@ -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@-1 {{read m_foo1 [loplugin:unusedfields]}}
@@ -19,25 +19,37 @@ struct Bar
// expected-error@-1 {{read m_bar2 [loplugin:unusedfields]}}
// expected-error@-2 {{read m_bar3 [loplugin:unusedfields]}}
// expected-error@-3 {{read m_bar4 [loplugin:unusedfields]}}
// expected-error@-4 {{read m_functionpointer [loplugin:unusedfields]}}
// expected-error@-4 {{read m_bar5 [loplugin:unusedfields]}}
// expected-error@-5 {{read m_bar6 [loplugin:unusedfields]}}
// expected-error@-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: */

80
compilerplugins/clang/unusedfields.cxx
View File

@@ -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;
}