Skip to content

C
core
Kaydet (Commit) 1ff0f0ba authored tarafından Noel Grandin's avatar Noel Grandin
Dosyalara gözat
Seçenekler

improve unusedfields loplugin 

(*) IsPassedByNonConst was completely wrong, not even sure why it worked
before.
(*) treat a field passed to operator>>= as being written to, but not
read

Change-Id: Id3a5f2f35222986fe5edba3f5a58215a1815d401
üst afeff910
Expand all Show whitespace changes
Inline Side-by-side
Showing with 58 additions and 25 deletions
compilerplugins/clang/test/unusedfields.cxx
Dosyayı görüntüle @ 1ff0f0ba
......@@ -9,6 +9,7 @@
#include <vector>
#include <ostream>
#include <com/sun/star/uno/Any.hxx>
struct Foo
// expected-error@-1 {{read m_foo1 [loplugin:unusedfields]}}
......@@ -23,13 +24,15 @@ struct Bar
// expected-error@-4 {{read m_bar6 [loplugin:unusedfields]}}
// expected-error@-5 {{read m_barfunctionpointer [loplugin:unusedfields]}}
// expected-error@-6 {{read m_bar8 [loplugin:unusedfields]}}
// expected-error@-7 {{write m_bar1 [loplugin:unusedfields]}}
// expected-error@-8 {{write m_bar2 [loplugin:unusedfields]}}
// expected-error@-9 {{write m_bar3 [loplugin:unusedfields]}}
// expected-error@-10 {{write m_bar3b [loplugin:unusedfields]}}
// expected-error@-11 {{write m_bar4 [loplugin:unusedfields]}}
// expected-error@-12 {{write m_bar7 [loplugin:unusedfields]}}
// expected-error@-13 {{write m_barfunctionpointer [loplugin:unusedfields]}}
// expected-error@-7 {{read m_bar10 [loplugin:unusedfields]}}
// expected-error@-8 {{write m_bar1 [loplugin:unusedfields]}}
// expected-error@-9 {{write m_bar2 [loplugin:unusedfields]}}
// expected-error@-10 {{write m_bar3 [loplugin:unusedfields]}}
// expected-error@-11 {{write m_bar3b [loplugin:unusedfields]}}
// expected-error@-12 {{write m_bar4 [loplugin:unusedfields]}}
// expected-error@-13 {{write m_bar7 [loplugin:unusedfields]}}
// expected-error@-14 {{write m_barfunctionpointer [loplugin:unusedfields]}}
// expected-error@-15 {{write m_bar9 [loplugin:unusedfields]}}
{
int m_bar1;
int m_bar2 = 1;
......@@ -42,6 +45,8 @@ struct Bar
int m_bar7[5];
int m_bar8;
int m_barstream;
int m_bar9;
int m_bar10;
// 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) {}
......@@ -81,6 +86,20 @@ struct Bar
char tmp[5];
return tmp[m_bar8];
}
// check that we don't see reads when calling operator>>=
void bar9()
{
css::uno::Any any;
any >>= m_bar9;
}
// check that we see don't see writes when calling operator<<=
void bar10()
{
css::uno::Any any;
any <<= m_bar10;
}
};
// check that we __dont__ see a read of m_barstream
......
compilerplugins/clang/unusedfields.cxx
Dosyayı görüntüle @ 1ff0f0ba
......@@ -493,11 +493,16 @@ void UnusedFields::checkWriteOnly(const FieldDecl* fieldDecl, const Expr* member
{
// check for calls to ReadXXX() type methods and the operator>>= methods on Any.
const FunctionDecl * calleeFunctionDecl = callExpr->getDirectCallee();
if (calleeFunctionDecl && calleeFunctionDecl->getIdentifier())
if (calleeFunctionDecl)
{
// FIXME perhaps a better solution here would be some kind of SAL_PARAM_WRITEONLY attribute
// which we could scatter around.
std::string name = calleeFunctionDecl->getNameAsString();
std::transform(name.begin(), name.end(), name.begin(), easytolower);
if (startswith(name, "read") || name.find(">>=") != std::string::npos)
if (startswith(name, "read"))
// this is a write-only call
;
else if (name.find(">>=") != std::string::npos && callExpr->getArg(1) == child)
// this is a write-only call
;
else if (name == "clear" || name == "dispose" || name == "disposeAndClear" || name == "swap")
......@@ -650,8 +655,10 @@ void UnusedFields::checkReadOnly(const FieldDecl* fieldDecl, const Expr* memberE
bPotentiallyWrittenTo = true;
}
else if (IsPassedByNonConst(fieldDecl, child, operatorCallExpr, calleeFunctionDecl))
{
bPotentiallyWrittenTo = true;
}
}
else
bPotentiallyWrittenTo = true; // conservative, could improve
break;
......@@ -787,14 +794,14 @@ bool UnusedFields::IsPassedByNonConst(const FieldDecl* fieldDecl, const Stmt * c
{
for (unsigned i = 0; i < len; ++i)
if (callExpr.getArg(i) == child)
if (loplugin::TypeCheck(calleeFunctionDecl.getParamDecl(i)->getType()).NonConst().Pointer())
if (loplugin::TypeCheck(calleeFunctionDecl.getParamDecl(i)->getType()).Pointer().NonConst())
return true;
}
else
{
for (unsigned i = 0; i < len; ++i)
if (callExpr.getArg(i) == child)
if (loplugin::TypeCheck(calleeFunctionDecl.getParamDecl(i)->getType()).NonConst().LvalueReference())
if (loplugin::TypeCheck(calleeFunctionDecl.getParamDecl(i)->getType()).LvalueReference().NonConst())
return true;
}
return false;
......
compilerplugins/clang/unusedfields.py
Dosyayı görüntüle @ 1ff0f0ba
......@@ -69,6 +69,7 @@ for k, definitions in sourceLocationToDefinitionMap.iteritems():
# Calculate untouched or untouched-except-for-in-constructor
untouchedSet = set()
untouchedSetD = set()
for d in definitionSet:
if d in touchedFromOutsideSet or d in touchedFromInsideSet:
continue
......@@ -100,11 +101,12 @@ for d in definitionSet:
if "::sfx2::sidebar::ControllerItem" in fieldType:
continue
untouchedSet.add((d[0] + " " + d[1] + " " + fieldType, srcLoc))
untouchedSetD.add(d)
writeonlySet = set()
for d in definitionSet:
parentClazz = d[0];
if d in readFromSet:
if d in readFromSet or d in untouchedSetD:
continue
srcLoc = definitionToSourceLocationMap[d];
# this is all representations of on-disk data structures
......@@ -138,9 +140,12 @@ for d in definitionSet:
if "Guard" in fieldType:
continue
# these are just all model classes
if (srcLoc.startswith("oox/") or srcLoc.startswith("lotuswordpro/")
or srcLoc.startswith("include/oox/") or srcLoc.startswith("include/filter/")
or srcLoc.startswith("hwpfilter/") or srcLoc.startswith("filter/")):
if (srcLoc.startswith("oox/")
or srcLoc.startswith("lotuswordpro/")
or srcLoc.startswith("include/oox/")
or srcLoc.startswith("include/filter/")
or srcLoc.startswith("hwpfilter/")
or srcLoc.startswith("filter/")):
continue
writeonlySet.add((d[0] + " " + d[1] + " " + definitionToTypeMap[d], srcLoc))
......@@ -149,7 +154,7 @@ for d in definitionSet:
readonlySet = set()
for d in definitionSet:
parentClazz = d[0];
if d in writeToSet:
if d in writeToSet or d in untouchedSetD:
continue
fieldType = definitionToTypeMap[d]
srcLoc = definitionToSourceLocationMap[d];
......
compilerplugins/clang/unusedfields.untouched.results
Dosyayı görüntüle @ 1ff0f0ba
......@@ -24,14 +24,6 @@ connectivity/source/drivers/evoab2/EApi.h:126
(anonymous) ext char *
connectivity/source/drivers/mork/MDatabaseMetaData.hxx:29
connectivity::mork::ODatabaseMetaData m_pMetaDataHelper std::unique_ptr<MDatabaseMetaDataHelper>
connectivity/source/inc/OTypeInfo.hxx:31
connectivity::OTypeInfo aTypeName class rtl::OUString
connectivity/source/inc/OTypeInfo.hxx:32
connectivity::OTypeInfo aLocalTypeName class rtl::OUString
connectivity/source/inc/OTypeInfo.hxx:34
connectivity::OTypeInfo nPrecision sal_Int32
connectivity/source/inc/OTypeInfo.hxx:36
connectivity::OTypeInfo nMaximumScale sal_Int16
cppu/source/threadpool/threadpool.cxx:377
_uno_ThreadPool dummy sal_Int32
cppu/source/typelib/typelib.cxx:61
......@@ -52,6 +44,8 @@ include/LibreOfficeKit/LibreOfficeKitGtk.h:33
_LOKDocView aDrawingArea GtkDrawingArea
include/LibreOfficeKit/LibreOfficeKitGtk.h:38
_LOKDocViewClass parent_class GtkDrawingAreaClass
include/oox/vml/vmlshapecontext.hxx:116
oox::vml::ShapeTypeContext m_pShapeType std::shared_ptr<ShapeType>
include/sfx2/msg.hxx:117
SfxType0 createSfxPoolItemFunc std::function<SfxPoolItem *(void)>
include/sfx2/msg.hxx:119
......@@ -169,7 +163,7 @@ svl/source/crypto/cryptosign.cxx:280
svl/source/crypto/cryptosign.cxx:281
(anonymous namespace)::(anonymous) failInfo SECItem
svtools/source/svhtml/htmlkywd.cxx:558
HTML_OptionEntry union HTML_OptionEntry::(anonymous at /home/noel/libo3/svtools/source/svhtml/htmlkywd.cxx:558:5)
HTML_OptionEntry union HTML_OptionEntry::(anonymous at /home/noel/libo2/svtools/source/svhtml/htmlkywd.cxx:558:5)
svtools/source/svhtml/htmlkywd.cxx:560
HTML_OptionEntry::(anonymous) sToken const sal_Char *
svtools/source/svhtml/htmlkywd.cxx:561
......@@ -192,6 +186,14 @@ unoidl/source/unoidlprovider.cxx:673
unoidl::detail::(anonymous namespace)::UnoidlCursor reference2_ rtl::Reference<UnoidlModuleEntity>
vcl/inc/opengl/zone.hxx:46
OpenGLVCLContextZone aZone class OpenGLZone
vcl/inc/unx/cpdmgr.hxx:60
psp::CPDPrinterOption name class rtl::OUString
vcl/inc/unx/cpdmgr.hxx:61
psp::CPDPrinterOption default_value class rtl::OUString
vcl/inc/unx/cpdmgr.hxx:62
psp::CPDPrinterOption num_supported_values int
vcl/inc/unx/cpdmgr.hxx:63
psp::CPDPrinterOption supported_values std::vector<OUString>
vcl/source/gdi/jobset.cxx:34
ImplOldJobSetupData cDeviceName char [32]
vcl/source/gdi/jobset.cxx:35
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment