[Libreoffice-commits] core.git: compilerplugins/clang
Noel Grandin
noel.grandin at collabora.co.uk
Thu Jun 29 09:02:35 UTC 2017
compilerplugins/clang/test/unusedfields.cxx | 28 +++++++++++-----
compilerplugins/clang/unusedfields.cxx | 49 +++++++++++++++++++++++++---
compilerplugins/clang/unusedfields.py | 3 +
3 files changed, 67 insertions(+), 13 deletions(-)
New commits:
commit 4cc2fc6cef4f76c1d203666cb5e47b5d70ec7be5
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date: Thu Jun 29 10:42:17 2017 +0200
unusedfields loplugin writeonly analysis improvements
(1) ignore reads inside copy/move constructors/operator=
(2) fix false+ when assigning to array field
(3) ignore reference ("&") fields
Change-Id: I69a1a1c567a0b28a783e605982e5150811b6cc4a
diff --git a/compilerplugins/clang/test/unusedfields.cxx b/compilerplugins/clang/test/unusedfields.cxx
index 1d43c81ea177..a66b7a6e7eca 100644
--- a/compilerplugins/clang/test/unusedfields.cxx
+++ b/compilerplugins/clang/test/unusedfields.cxx
@@ -17,29 +17,39 @@ 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_bar5 [loplugin:unusedfields]}}
-// expected-error at -5 {{read m_bar6 [loplugin:unusedfields]}}
-// expected-error at -6 {{read m_barfunctionpointer [loplugin:unusedfields]}}
+// expected-error at -2 {{read m_bar4 [loplugin:unusedfields]}}
+// expected-error at -3 {{read m_bar5 [loplugin:unusedfields]}}
+// expected-error at -4 {{read m_bar6 [loplugin:unusedfields]}}
+// expected-error at -5 {{read m_barfunctionpointer [loplugin:unusedfields]}}
{
int m_bar1;
int m_bar2 = 1;
int* m_bar3;
+ int* m_bar3b;
int m_bar4;
void (*m_barfunctionpointer)(int&);
int m_bar5;
std::vector<int> m_bar6;
+ int m_bar7[5];
- //check that we see reads of fields when referred to via constructor initializer
+ // check that we see reads of fields like m_foo1 when referred to via constructor initializer
Bar(Foo const & foo) : m_bar1(foo.m_foo1) {}
+ // check that we don't see reads when inside copy/move constructor
+ Bar(Bar const & other) { m_bar3 = other.m_bar3; }
+
+ // check that we don't see reads when inside copy/move assignment operator
+ Bar& operator=(Bar const & other) { m_bar3 = other.m_bar3; return *this; }
+
// check that we DONT see reads here
int bar2() { return m_bar2; }
// check that we DONT see reads here
- void bar3() { *m_bar3 = 2; }
- void bar3b() { m_bar3 = nullptr; }
+ void bar3()
+ {
+ m_bar3 = nullptr;
+ m_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
@@ -50,6 +60,8 @@ struct Bar
// check that we see reads of a field when used in ranged-for
void bar6() { for (auto i : m_bar6) { (void)i; } }
+
+ void bar7() { m_bar7[3] = 1; }
};
/* 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 1ba09f11e303..df13a7ce266f 100644
--- a/compilerplugins/clang/unusedfields.cxx
+++ b/compilerplugins/clang/unusedfields.cxx
@@ -80,9 +80,12 @@ public:
bool VisitFieldDecl( const FieldDecl* );
bool VisitMemberExpr( const MemberExpr* );
bool VisitDeclRefExpr( const DeclRefExpr* );
+ bool TraverseCXXConstructorDecl( CXXConstructorDecl* );
+ bool TraverseCXXMethodDecl( CXXMethodDecl* );
private:
MyFieldInfo niceName(const FieldDecl*);
void checkTouched(const FieldDecl* fieldDecl, const Expr* memberExpr);
+ CXXMethodDecl * insideMoveOrCopyDecl;
};
void UnusedFields::run()
@@ -189,11 +192,30 @@ static char easytolower(char in)
return in-('Z'-'z');
return in;
}
+
bool startswith(const std::string& rStr, const char* pSubStr)
{
return rStr.compare(0, strlen(pSubStr), pSubStr) == 0;
}
+bool UnusedFields::TraverseCXXConstructorDecl(CXXConstructorDecl* cxxConstructorDecl)
+{
+ if (cxxConstructorDecl->isCopyOrMoveConstructor())
+ insideMoveOrCopyDecl = cxxConstructorDecl;
+ bool ret = RecursiveASTVisitor::TraverseCXXConstructorDecl(cxxConstructorDecl);
+ insideMoveOrCopyDecl = nullptr;
+ return ret;
+}
+
+bool UnusedFields::TraverseCXXMethodDecl(CXXMethodDecl* cxxMethodDecl)
+{
+ if (cxxMethodDecl->isCopyAssignmentOperator() || cxxMethodDecl->isMoveAssignmentOperator())
+ insideMoveOrCopyDecl = cxxMethodDecl;
+ bool ret = RecursiveASTVisitor::TraverseCXXMethodDecl(cxxMethodDecl);
+ insideMoveOrCopyDecl = nullptr;
+ return ret;
+}
+
bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
{
const ValueDecl* decl = memberExpr->getMemberDecl();
@@ -218,6 +240,15 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
// for the write-only analysis
+ // we don't care about reads from a field when inside that field parent class copy/move constructor/operator=
+ if (insideMoveOrCopyDecl)
+ {
+ auto cxxRecordDecl1 = dyn_cast<CXXRecordDecl>(fieldDecl->getDeclContext());
+ auto cxxRecordDecl2 = dyn_cast<CXXRecordDecl>(insideMoveOrCopyDecl->getDeclContext());
+ if (cxxRecordDecl1 && cxxRecordDecl1 == cxxRecordDecl2)
+ return true;
+ }
+
auto parentsRange = compiler.getASTContext().getParents(*memberExpr);
const Stmt* child = memberExpr;
const Stmt* parent = parentsRange.begin() == parentsRange.end() ? nullptr : parentsRange.begin()->get<Stmt>();
@@ -253,7 +284,11 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
else if (auto unaryOperator = dyn_cast<UnaryOperator>(parent))
{
UnaryOperator::Opcode op = unaryOperator->getOpcode();
- if (op == UO_AddrOf || op == UO_Deref
+ if (memberExpr->getType()->isArrayType() && op == UO_Deref)
+ {
+ // ignore, deref'ing an array does not count as a read
+ }
+ else 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
@@ -307,8 +342,12 @@ 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
- if (!(binaryOp->getLHS() == child && op == BO_Assign)) {
+ // If the child is on the LHS and it is an assignment op, we are obviously not reading from it
+ const bool assignmentOp = op == BO_Assign || op == BO_MulAssign
+ || op == BO_DivAssign || op == BO_RemAssign || op == BO_AddAssign
+ || op == BO_SubAssign || op == BO_ShlAssign || op == BO_ShrAssign
+ || op == BO_AndAssign || op == BO_XorAssign || op == BO_OrAssign;
+ if (!(binaryOp->getLHS() == child && assignmentOp)) {
bPotentiallyReadFrom = true;
}
break;
@@ -317,7 +356,6 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
|| isa<CXXConstructExpr>(parent)
|| isa<ConditionalOperator>(parent)
|| isa<SwitchStmt>(parent)
- || isa<ArraySubscriptExpr>(parent)
|| isa<DeclStmt>(parent)
|| isa<WhileStmt>(parent)
|| isa<CXXNewExpr>(parent)
@@ -337,6 +375,7 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
|| isa<LabelStmt>(parent)
|| isa<CXXForRangeStmt>(parent)
|| isa<CXXTypeidExpr>(parent)
+ || isa<ArraySubscriptExpr>(parent)
|| isa<DefaultStmt>(parent))
{
break;
@@ -414,7 +453,7 @@ void UnusedFields::checkTouched(const FieldDecl* fieldDecl, const Expr* memberEx
if (memberExprParentFunction->getParent() == fieldDecl->getParent()) {
touchedFromInsideSet.insert(fieldInfo);
} else {
- touchedFromOutsideSet.insert(fieldInfo);
+ touchedFromOutsideSet.insert(fieldInfo);
}
}
}
diff --git a/compilerplugins/clang/unusedfields.py b/compilerplugins/clang/unusedfields.py
index 5d15b84ba528..0baf4c738cff 100755
--- a/compilerplugins/clang/unusedfields.py
+++ b/compilerplugins/clang/unusedfields.py
@@ -125,6 +125,9 @@ for d in definitionSet:
continue
if "::sfx2::sidebar::ControllerItem" in fieldType:
continue
+ # ignore reference fields, because writing to them actually writes to another field somewhere else
+ if fieldType.endswith("&"):
+ continue
writeonlySet.add((clazz + " " + definitionToTypeMap[d], srcLoc))
More information about the Libreoffice-commits
mailing list