[Libreoffice-commits] core.git: compilerplugins/clang xmlscript/source
Noel Grandin
noel.grandin at collabora.co.uk
Thu Nov 17 06:41:12 UTC 2016
compilerplugins/clang/unusedfields.cxx | 143 +++++++++++------------
compilerplugins/clang/unusedfields.py | 97 +++++++--------
xmlscript/source/xml_helper/xml_impctx.cxx | 3
xmlscript/source/xmldlg_imexp/imp_share.hxx | 14 +-
xmlscript/source/xmlflat_imexp/xmlbas_import.hxx | 2
xmlscript/source/xmllib_imexp/imp_share.hxx | 2
xmlscript/source/xmlmod_imexp/imp_share.hxx | 2
7 files changed, 129 insertions(+), 134 deletions(-)
New commits:
commit 2f7ccd102a6280750ce920e4c4eab66f9a01b9d3
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date: Thu Nov 17 08:39:50 2016 +0200
extend unusedfields loplugin to find fields that can be private
and apply the results in xmlscript
Change-Id: Ib126f6e1576639abfd171e99d9561be9715ece2f
diff --git a/compilerplugins/clang/unusedfields.cxx b/compilerplugins/clang/unusedfields.cxx
index 6e4da40..1bf71cd 100644
--- a/compilerplugins/clang/unusedfields.cxx
+++ b/compilerplugins/clang/unusedfields.cxx
@@ -47,6 +47,7 @@ struct MyFieldInfo
std::string fieldName;
std::string fieldType;
std::string sourceLocation;
+ std::string access;
};
bool operator < (const MyFieldInfo &lhs, const MyFieldInfo &rhs)
{
@@ -57,6 +58,7 @@ bool operator < (const MyFieldInfo &lhs, const MyFieldInfo &rhs)
// try to limit the voluminous output a little
static std::set<MyFieldInfo> touchedSet;
+static std::set<MyFieldInfo> touchedFromOutsideSet;
static std::set<MyFieldInfo> readFromSet;
static std::set<MyFieldInfo> definitionSet;
@@ -76,11 +78,13 @@ public:
std::string output;
for (const MyFieldInfo & s : touchedSet)
output += "touch:\t" + s.parentClass + "\t" + s.fieldName + "\n";
+ for (const MyFieldInfo & s : touchedFromOutsideSet)
+ output += "outside:\t" + s.parentClass + "\t" + s.fieldName + "\n";
for (const MyFieldInfo & s : readFromSet)
output += "read:\t" + s.parentClass + "\t" + s.fieldName + "\n";
for (const MyFieldInfo & s : definitionSet)
{
- output += "definition:\t" + s.parentClass + "\t" + s.fieldName + "\t" + s.fieldType + "\t" + s.sourceLocation + "\n";
+ output += "definition:\t" + s.access + "\t" + s.parentClass + "\t" + s.fieldName + "\t" + s.fieldType + "\t" + s.sourceLocation + "\n";
}
ofstream myfile;
myfile.open( SRCDIR "/loplugin.unusedfields.log", ios::app | ios::out);
@@ -89,14 +93,14 @@ public:
}
bool shouldVisitTemplateInstantiations () const { return true; }
+ bool shouldVisitImplicitCode() const { return true; }
- bool VisitCallExpr(CallExpr* );
bool VisitFieldDecl( const FieldDecl* );
bool VisitMemberExpr( const MemberExpr* );
bool VisitDeclRefExpr( const DeclRefExpr* );
private:
MyFieldInfo niceName(const FieldDecl*);
- std::string fullyQualifiedName(const FunctionDecl*);
+ void checkForTouchedFromOutside(const FieldDecl* fieldDecl, const Expr* memberExpr, const MyFieldInfo& fieldInfo);
};
MyFieldInfo UnusedFields::niceName(const FieldDecl* fieldDecl)
@@ -111,73 +115,27 @@ MyFieldInfo UnusedFields::niceName(const FieldDecl* fieldDecl)
aInfo.sourceLocation = std::string(name.substr(strlen(SRCDIR)+1)) + ":" + std::to_string(compiler.getSourceManager().getSpellingLineNumber(expansionLoc));
normalizeDotDotInFilePath(aInfo.sourceLocation);
- return aInfo;
-}
-
-std::string UnusedFields::fullyQualifiedName(const FunctionDecl* functionDecl)
-{
- std::string ret = compat::getReturnType(*functionDecl).getCanonicalType().getAsString();
- ret += " ";
- if (isa<CXXMethodDecl>(functionDecl)) {
- const CXXRecordDecl* recordDecl = dyn_cast<CXXMethodDecl>(functionDecl)->getParent();
- ret += recordDecl->getQualifiedNameAsString();
- ret += "::";
- }
- ret += functionDecl->getNameAsString() + "(";
- bool bFirst = true;
- for (const ParmVarDecl *pParmVarDecl : compat::parameters(*functionDecl)) {
- if (bFirst)
- bFirst = false;
- else
- ret += ",";
- ret += pParmVarDecl->getType().getCanonicalType().getAsString();
- }
- ret += ")";
- if (isa<CXXMethodDecl>(functionDecl) && dyn_cast<CXXMethodDecl>(functionDecl)->isConst()) {
- ret += " const";
- }
-
- return ret;
-}
-
-// prevent recursive templates from blowing up the stack
-static std::set<std::string> traversedFunctionSet;
-
-bool UnusedFields::VisitCallExpr(CallExpr* expr)
-{
- // Note that I don't ignore ANYTHING here, because I want to get calls to my code that result
- // from template instantiation deep inside the STL and other external code
-
- FunctionDecl* calleeFunctionDecl = expr->getDirectCallee();
- if (calleeFunctionDecl == nullptr) {
- Expr* callee = expr->getCallee()->IgnoreParenImpCasts();
- DeclRefExpr* dr = dyn_cast<DeclRefExpr>(callee);
- if (dr) {
- calleeFunctionDecl = dyn_cast<FunctionDecl>(dr->getDecl());
- if (calleeFunctionDecl)
- goto gotfunc;
- }
- return true;
+ switch (fieldDecl->getAccess())
+ {
+ case AS_public: aInfo.access = "public"; break;
+ case AS_private: aInfo.access = "private"; break;
+ case AS_protected: aInfo.access = "protected"; break;
+ default: aInfo.access = "unknown"; break;
}
-gotfunc:
- // if we see a call to a function, it may effectively create new code,
- // if the function is templated. However, if we are inside a template function,
- // calling another function on the same template, the same problem occurs.
- // Rather than tracking all of that, just traverse anything we have not already traversed.
- if (traversedFunctionSet.insert(fullyQualifiedName(calleeFunctionDecl)).second)
- TraverseFunctionDecl(calleeFunctionDecl);
-
- return true;
+ return aInfo;
}
bool UnusedFields::VisitFieldDecl( const FieldDecl* fieldDecl )
{
fieldDecl = fieldDecl->getCanonicalDecl();
- const FieldDecl* canonicalDecl = fieldDecl;
-
- if( ignoreLocation( fieldDecl ))
+ if (ignoreLocation( fieldDecl )) {
return true;
+ }
+ // ignore stuff that forms part of the stable URE interface
+ if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) {
+ return true;
+ }
QualType type = fieldDecl->getType();
// unwrap array types
@@ -200,7 +158,7 @@ bool UnusedFields::VisitFieldDecl( const FieldDecl* fieldDecl )
return true;
}
*/
- definitionSet.insert(niceName(canonicalDecl));
+ definitionSet.insert(niceName(fieldDecl));
return true;
}
@@ -211,20 +169,22 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
if (!fieldDecl) {
return true;
}
+ fieldDecl = fieldDecl->getCanonicalDecl();
+ if (ignoreLocation(fieldDecl)) {
+ return true;
+ }
+ // ignore stuff that forms part of the stable URE interface
+ if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) {
+ return true;
+ }
MyFieldInfo fieldInfo = niceName(fieldDecl);
- // ignore move/copy operator, it's self->self
- const FunctionDecl* parentFunction = parentFunctionDecl(memberExpr);
- const CXXMethodDecl* methodDecl = dyn_cast_or_null<CXXMethodDecl>(parentFunction);
- if (!methodDecl || !(methodDecl->isCopyAssignmentOperator() || methodDecl->isMoveAssignmentOperator())) {
- touchedSet.insert(fieldInfo);
- }
+ // for the touched-from-outside analysis
- // for the write-only analysis
+ checkForTouchedFromOutside(fieldDecl, memberExpr, fieldInfo);
- if (ignoreLocation(memberExpr))
- return true;
+ // for the write-only analysis
const Stmt* child = memberExpr;
const Stmt* parent = parentStmt(memberExpr);
@@ -298,13 +258,48 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
bool UnusedFields::VisitDeclRefExpr( const DeclRefExpr* declRefExpr )
{
const Decl* decl = declRefExpr->getDecl();
- if (!isa<FieldDecl>(decl)) {
+ const FieldDecl* fieldDecl = dyn_cast<FieldDecl>(decl);
+ if (!fieldDecl) {
+ return true;
+ }
+ fieldDecl = fieldDecl->getCanonicalDecl();
+ if (ignoreLocation(fieldDecl)) {
return true;
}
- touchedSet.insert(niceName(dyn_cast<FieldDecl>(decl)));
+ // ignore stuff that forms part of the stable URE interface
+ if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocation()))) {
+ return true;
+ }
+ MyFieldInfo fieldInfo = niceName(fieldDecl);
+ touchedSet.insert(fieldInfo);
+ checkForTouchedFromOutside(fieldDecl, declRefExpr, fieldInfo);
return true;
}
+void UnusedFields::checkForTouchedFromOutside(const FieldDecl* fieldDecl, const Expr* memberExpr, const MyFieldInfo& fieldInfo) {
+ const FunctionDecl* memberExprParentFunction = parentFunctionDecl(memberExpr);
+ const CXXMethodDecl* methodDecl = dyn_cast_or_null<CXXMethodDecl>(memberExprParentFunction);
+
+ // it's touched from somewhere outside a class
+ if (!methodDecl) {
+ touchedSet.insert(fieldInfo);
+ touchedFromOutsideSet.insert(fieldInfo);
+ return;
+ }
+
+ auto constructorDecl = dyn_cast<CXXConstructorDecl>(methodDecl);
+ if (methodDecl->isCopyAssignmentOperator() || methodDecl->isMoveAssignmentOperator()) {
+ // ignore move/copy operator, it's self->self
+ } else if (constructorDecl && (constructorDecl->isCopyConstructor() || constructorDecl->isMoveConstructor())) {
+ // ignore move/copy constructor, it's self->self
+ } else {
+ touchedSet.insert(fieldInfo);
+ if (memberExprParentFunction->getParent() != fieldDecl->getParent()) {
+ touchedFromOutsideSet.insert(fieldInfo);
+ }
+ }
+}
+
loplugin::Plugin::Registration< UnusedFields > X("unusedfields", false);
}
diff --git a/compilerplugins/clang/unusedfields.py b/compilerplugins/clang/unusedfields.py
index 785b7a9..7bf910f 100755
--- a/compilerplugins/clang/unusedfields.py
+++ b/compilerplugins/clang/unusedfields.py
@@ -5,11 +5,13 @@ import re
import io
definitionSet = set()
+protectedAndPublicDefinitionSet = set() # set of tuple(type, name)
definitionToSourceLocationMap = dict()
definitionToTypeMap = dict()
callSet = set()
readFromSet = set()
sourceLocationSet = set()
+touchedFromOutsideSet = set()
# clang does not always use exactly the same numbers in the type-parameter vars it generates
# so I need to substitute them to ensure we can match correctly.
@@ -17,30 +19,43 @@ normalizeTypeParamsRegex = re.compile(r"type-parameter-\d+-\d+")
def normalizeTypeParams( line ):
return normalizeTypeParamsRegex.sub("type-parameter-?-?", line)
+def parseFieldInfo( tokens ):
+ if len(tokens) == 3:
+ return (normalizeTypeParams(tokens[1]), tokens[2])
+ else:
+ return (normalizeTypeParams(tokens[1]), "")
+
# The parsing here is designed to avoid grabbing stuff which is mixed in from gbuild.
# I have not yet found a way of suppressing the gbuild output.
with io.open("loplugin.unusedfields.log", "rb", buffering=1024*1024) as txt:
for line in txt:
tokens = line.strip().split("\t")
if tokens[0] == "definition:":
- fieldInfo = (normalizeTypeParams(tokens[1]), tokens[2])
+ access = tokens[1]
+ fieldInfo = (normalizeTypeParams(tokens[2]), tokens[3])
+ srcLoc = tokens[5]
+ # ignore external source code
+ if (srcLoc.startswith("external/")):
+ continue
+ # ignore build folder
+ if (srcLoc.startswith("workdir/")):
+ continue
definitionSet.add(fieldInfo)
- definitionToTypeMap[fieldInfo] = tokens[3]
- definitionToSourceLocationMap[fieldInfo] = tokens[4]
+ definitionToTypeMap[fieldInfo] = tokens[4]
+ if access == "protected" or access == "public":
+ protectedAndPublicDefinitionSet.add(fieldInfo)
+ definitionToSourceLocationMap[fieldInfo] = tokens[5]
elif tokens[0] == "touch:":
- if len(tokens) == 3:
- callInfo = (normalizeTypeParams(tokens[1]), tokens[2])
- callSet.add(callInfo)
- else:
- callInfo = (normalizeTypeParams(tokens[1]), "")
- callSet.add(callInfo)
+ callSet.add(parseFieldInfo(tokens))
+ elif tokens[0] == "read:":
+ readFromSet.add(parseFieldInfo(tokens))
elif tokens[0] == "read:":
- if len(tokens) == 3:
- readInfo = (normalizeTypeParams(tokens[1]), tokens[2])
- readFromSet.add(readInfo)
- else:
- readInfo = (normalizeTypeParams(tokens[1]), "")
- readFromSet.add(readInfo)
+ readFromSet.add(parseFieldInfo(tokens))
+ elif tokens[0] == "outside:":
+ touchedFromOutsideSet.add(parseFieldInfo(tokens))
+ else:
+ print( "unknown line: " + line)
+
# Invert the definitionToSourceLocationMap
# If we see more than one method at the same sourceLocation, it's being autogenerated as part of a template
# and we should just ignore
@@ -58,24 +73,6 @@ for d in definitionSet:
if d in callSet:
continue
srcLoc = definitionToSourceLocationMap[d];
- # ignore external source code
- if (srcLoc.startswith("external/")):
- continue
- # ignore build folder
- if (srcLoc.startswith("workdir/")):
- continue
- # ignore our stable/URE/UNO api
- if (srcLoc.startswith("include/com/")
- or srcLoc.startswith("include/cppu/")
- or srcLoc.startswith("include/cppuhelper/")
- or srcLoc.startswith("include/osl/")
- or srcLoc.startswith("include/rtl/")
- or srcLoc.startswith("include/sal/")
- or srcLoc.startswith("include/salhelper/")
- or srcLoc.startswith("include/systools/")
- or srcLoc.startswith("include/typelib/")
- or srcLoc.startswith("include/uno/")):
- continue
# this is all representations of on-disk data structures
if (srcLoc.startswith("sc/source/filter/inc/scflt.hxx")
or srcLoc.startswith("sw/source/filter/ww8/")
@@ -108,24 +105,6 @@ for d in definitionSet:
if d in readFromSet:
continue
srcLoc = definitionToSourceLocationMap[d];
- # ignore external source code
- if (srcLoc.startswith("external/")):
- continue
- # ignore build folder
- if (srcLoc.startswith("workdir/")):
- continue
- # ignore our stable/URE/UNO api
- if (srcLoc.startswith("include/com/")
- or srcLoc.startswith("include/cppu/")
- or srcLoc.startswith("include/cppuhelper/")
- or srcLoc.startswith("include/osl/")
- or srcLoc.startswith("include/rtl/")
- or srcLoc.startswith("include/sal/")
- or srcLoc.startswith("include/salhelper/")
- or srcLoc.startswith("include/systools/")
- or srcLoc.startswith("include/typelib/")
- or srcLoc.startswith("include/uno/")):
- continue
# this is all representations of on-disk data structures
if (srcLoc.startswith("sc/source/filter/inc/scflt.hxx")
or srcLoc.startswith("sw/source/filter/ww8/")
@@ -140,6 +119,17 @@ for d in definitionSet:
writeonlySet.add((clazz + " " + definitionToTypeMap[d], srcLoc))
+
+canBePrivateSet = set()
+for d in protectedAndPublicDefinitionSet:
+ clazz = d[0] + " " + d[1]
+ if d in touchedFromOutsideSet:
+ continue
+ srcLoc = definitionToSourceLocationMap[d];
+
+ canBePrivateSet.add((clazz + " " + definitionToTypeMap[d], srcLoc))
+
+
# sort the results using a "natural order" so sequences like [item1,item2,item10] sort nicely
def natural_sort_key(s, _nsre=re.compile('([0-9]+)')):
return [int(text) if text.isdigit() else text.lower()
@@ -148,6 +138,7 @@ def natural_sort_key(s, _nsre=re.compile('([0-9]+)')):
# sort results by name and line number
tmp1list = sorted(untouchedSet, key=lambda v: natural_sort_key(v[1]))
tmp2list = sorted(writeonlySet, key=lambda v: natural_sort_key(v[1]))
+tmp3list = sorted(canBePrivateSet, key=lambda v: natural_sort_key(v[1]))
# print out the results
with open("loplugin.unusedfields.report-untouched", "wt") as f:
@@ -158,5 +149,9 @@ with open("loplugin.unusedfields.report-writeonly", "wt") as f:
for t in tmp2list:
f.write( t[1] + "\n" )
f.write( " " + t[0] + "\n" )
+with open("loplugin.unusedfields.report-can-be-private", "wt") as f:
+ for t in tmp3list:
+ f.write( t[1] + "\n" )
+ f.write( " " + t[0] + "\n" )
diff --git a/xmlscript/source/xml_helper/xml_impctx.cxx b/xmlscript/source/xml_helper/xml_impctx.cxx
index c79e8d5..52999fa 100644
--- a/xmlscript/source/xml_helper/xml_impctx.cxx
+++ b/xmlscript/source/xml_helper/xml_impctx.cxx
@@ -79,9 +79,10 @@ struct ElementEntry
class ExtendedAttributes;
-struct MGuard
+class MGuard
{
Mutex * m_pMutex;
+public:
explicit MGuard( Mutex * pMutex )
: m_pMutex( pMutex )
{ if (m_pMutex) m_pMutex->acquire(); }
diff --git a/xmlscript/source/xmldlg_imexp/imp_share.hxx b/xmlscript/source/xmldlg_imexp/imp_share.hxx
index f185fcd..30c3f94 100644
--- a/xmlscript/source/xmldlg_imexp/imp_share.hxx
+++ b/xmlscript/source/xmldlg_imexp/imp_share.hxx
@@ -114,20 +114,20 @@ struct DialogImport
: public ::cppu::WeakImplHelper< css::xml::input::XRoot >
{
friend class ImportContext;
-
+private:
css::uno::Reference< css::uno::XComponentContext > _xContext;
css::uno::Reference< css::util::XNumberFormatsSupplier > _xSupplier;
std::shared_ptr< std::vector< OUString > > _pStyleNames;
std::shared_ptr< std::vector< css::uno::Reference< css::xml::input::XElement > > > _pStyles;
+ css::uno::Reference< css::frame::XModel > _xDoc;
+public:
css::uno::Reference< css::container::XNameContainer > _xDialogModel;
css::uno::Reference< css::lang::XMultiServiceFactory > _xDialogModelFactory;
- css::uno::Reference< css::frame::XModel > _xDoc;
sal_Int32 XMLNS_DIALOGS_UID, XMLNS_SCRIPT_UID;
-public:
inline bool isEventElement(
sal_Int32 nUid, OUString const & rLocalName )
{
@@ -156,8 +156,9 @@ public:
: _xContext( xContext )
, _pStyleNames( pStyleNames )
, _pStyles( pStyles )
+ , _xDoc( xDoc )
, _xDialogModel( xDialogModel )
- , _xDialogModelFactory( xDialogModel, css::uno::UNO_QUERY_THROW ), _xDoc( xDoc )
+ , _xDialogModelFactory( xDialogModel, css::uno::UNO_QUERY_THROW )
, XMLNS_DIALOGS_UID( 0 )
, XMLNS_SCRIPT_UID( 0 )
{ OSL_ASSERT( _xDialogModel.is() && _xDialogModelFactory.is() &&
@@ -168,9 +169,9 @@ public:
, _xSupplier( rOther._xSupplier )
, _pStyleNames( rOther._pStyleNames )
, _pStyles( rOther._pStyles )
+ , _xDoc( rOther._xDoc )
, _xDialogModel( rOther._xDialogModel )
, _xDialogModelFactory( rOther._xDialogModelFactory )
- , _xDoc( rOther._xDoc )
, XMLNS_DIALOGS_UID( rOther.XMLNS_DIALOGS_UID )
, XMLNS_SCRIPT_UID( rOther.XMLNS_SCRIPT_UID ) {}
@@ -204,9 +205,10 @@ class ElementBase
protected:
DialogImport * const _pImport;
ElementBase * const _pParent;
-
+private:
const sal_Int32 _nUid;
const OUString _aLocalName;
+protected:
const css::uno::Reference< css::xml::input::XAttributes > _xAttributes;
public:
diff --git a/xmlscript/source/xmlflat_imexp/xmlbas_import.hxx b/xmlscript/source/xmlflat_imexp/xmlbas_import.hxx
index 5a913c6..194283c 100644
--- a/xmlscript/source/xmlflat_imexp/xmlbas_import.hxx
+++ b/xmlscript/source/xmlflat_imexp/xmlbas_import.hxx
@@ -45,10 +45,12 @@ namespace xmlscript
{
protected:
rtl::Reference<BasicImport> m_xImport;
+ private:
rtl::Reference<BasicElementBase> m_xParent;
OUString m_aLocalName;
css::uno::Reference< css::xml::input::XAttributes > m_xAttributes;
+ protected:
static bool getBoolAttr( bool* pRet, const OUString& rAttrName,
const css::uno::Reference< css::xml::input::XAttributes >& xAttributes,
sal_Int32 nUid );
diff --git a/xmlscript/source/xmllib_imexp/imp_share.hxx b/xmlscript/source/xmllib_imexp/imp_share.hxx
index 74560ec..71d7d94 100644
--- a/xmlscript/source/xmllib_imexp/imp_share.hxx
+++ b/xmlscript/source/xmllib_imexp/imp_share.hxx
@@ -158,7 +158,7 @@ class LibElementBase
protected:
rtl::Reference<LibraryImport> mxImport;
rtl::Reference<LibElementBase> mxParent;
-
+private:
OUString _aLocalName;
css::uno::Reference< css::xml::input::XAttributes > _xAttributes;
diff --git a/xmlscript/source/xmlmod_imexp/imp_share.hxx b/xmlscript/source/xmlmod_imexp/imp_share.hxx
index b7ae02c..1f170a0 100644
--- a/xmlscript/source/xmlmod_imexp/imp_share.hxx
+++ b/xmlscript/source/xmlmod_imexp/imp_share.hxx
@@ -50,6 +50,7 @@ struct ModuleImport
ModuleDescriptor& mrModuleDesc;
sal_Int32 XMLNS_SCRIPT_UID;
+private:
sal_Int32 XMLNS_LIBRARY_UID;
sal_Int32 XMLNS_XLINK_UID;
@@ -85,7 +86,6 @@ public:
class ModuleElement
: public ::cppu::WeakImplHelper< css::xml::input::XElement >
{
-protected:
rtl::Reference<ModuleImport> mxImport;
OUString _aLocalName;
More information about the Libreoffice-commits
mailing list