summaryrefslogtreecommitdiff
path: root/compilerplugins/clang
diff options
context:
space:
mode:
authorNoel Grandin <noel.grandin@collabora.co.uk>2018-05-10 09:08:07 +0200
committerNoel Grandin <noel.grandin@collabora.co.uk>2018-05-10 11:34:18 +0200
commit0e384e1080381e894b590fd0a6d453568715e8fa (patch)
tree1f91b14d4140219c6bfcaacf85d755facdc34d40 /compilerplugins/clang
parent6ede622ab6d2393c3ec90fcaa6e2487232b8c1a8 (diff)
loplugin:unnecessaryvirtual improvements
look for virtual methods where all of the overrides of the method are empty Change-Id: I87d99a0b647700a8d53498e0ab5f0437d3508553 Reviewed-on: https://gerrit.libreoffice.org/54060 Tested-by: Jenkins <ci@libreoffice.org> Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Diffstat (limited to 'compilerplugins/clang')
-rw-r--r--compilerplugins/clang/unnecessaryvirtual-dead.results34
-rw-r--r--compilerplugins/clang/unnecessaryvirtual.cxx44
-rwxr-xr-xcompilerplugins/clang/unnecessaryvirtual.py29
-rw-r--r--compilerplugins/clang/unnecessaryvirtual.results40
4 files changed, 119 insertions, 28 deletions
diff --git a/compilerplugins/clang/unnecessaryvirtual-dead.results b/compilerplugins/clang/unnecessaryvirtual-dead.results
new file mode 100644
index 000000000000..dc3fa6cd6d5a
--- /dev/null
+++ b/compilerplugins/clang/unnecessaryvirtual-dead.results
@@ -0,0 +1,34 @@
+basic/source/comp/codegen.cxx:462
+ void OffSetAccumulator::start(const unsigned char *,)
+forms/source/component/entrylisthelper.hxx:138
+ void frm::OEntryListHelper::connectedExternalListSource()
+forms/source/component/entrylisthelper.hxx:142
+ void frm::OEntryListHelper::disconnectedExternalListSource()
+include/basegfx/utils/unopolypolygon.hxx:97
+ void basegfx::unotools::UnoPolyPolygon::modifying()const
+include/canvas/base/bufferedgraphicdevicebase.hxx:108
+ void canvas::BufferedGraphicDeviceBase::destroyBuffers()
+include/canvas/base/graphicdevicebase.hxx:302
+ void canvas::GraphicDeviceBase::removePropertyChangeListener(const class rtl::OUString &,const class com::sun::star::uno::Reference<class com::sun::star::beans::XPropertyChangeListener> &,)
+include/canvas/base/graphicdevicebase.hxx:315
+ void canvas::GraphicDeviceBase::removeVetoableChangeListener(const class rtl::OUString &,const class com::sun::star::uno::Reference<class com::sun::star::beans::XVetoableChangeListener> &,)
+slideshow/source/engine/animationfactory.cxx:442
+ void slideshow::internal::(anonymous namespace)::GenericAnimation::prefetch(const class std::shared_ptr<class slideshow::internal::AnimatableShape> &,const class std::shared_ptr<class slideshow::internal::ShapeAttributeLayer> &,)
+vcl/inc/salframe.hxx:135
+ void SalFrame::SetRepresentedURL(const class rtl::OUString &,)
+vcl/inc/salmenu.hxx:80
+ void SalMenu::RemoveMenuBarButton(unsigned short,)
+vcl/inc/salobj.hxx:46
+ void SalObject::Enable(_Bool,)
+vcl/inc/unx/saldata.hxx:68
+ void X11SalData::initNWF()
+vcl/inc/unx/saldata.hxx:69
+ void X11SalData::deInitNWF()
+writerfilter/source/ooxml/OOXMLFactory.hxx:71
+ void writerfilter::ooxml::OOXMLFactory_ns::startAction(class writerfilter::ooxml::OOXMLFastContextHandler *,)
+writerfilter/source/ooxml/OOXMLFactory.hxx:72
+ void writerfilter::ooxml::OOXMLFactory_ns::charactersAction(class writerfilter::ooxml::OOXMLFastContextHandler *,const class rtl::OUString &,)
+writerfilter/source/ooxml/OOXMLFactory.hxx:73
+ void writerfilter::ooxml::OOXMLFactory_ns::endAction(class writerfilter::ooxml::OOXMLFastContextHandler *,)
+writerfilter/source/ooxml/OOXMLFactory.hxx:74
+ void writerfilter::ooxml::OOXMLFactory_ns::attributeAction(class writerfilter::ooxml::OOXMLFastContextHandler *,int,const class std::shared_ptr<class writerfilter::ooxml::OOXMLValue> &,)
diff --git a/compilerplugins/clang/unnecessaryvirtual.cxx b/compilerplugins/clang/unnecessaryvirtual.cxx
index e2af4bec6dc3..afc324cee156 100644
--- a/compilerplugins/clang/unnecessaryvirtual.cxx
+++ b/compilerplugins/clang/unnecessaryvirtual.cxx
@@ -11,6 +11,7 @@
#include <string>
#include <iostream>
#include <set>
+#include <unordered_set>
#include "plugin.hxx"
#include "compat.hxx"
#include <fstream>
@@ -19,6 +20,9 @@
Dump a list of virtual methods and a list of methods overriding virtual methods.
Then we will post-process the 2 lists and find the set of virtual methods which don't need to be virtual.
+Also, we look for virtual methods where the bodies of all the overrides are empty i.e. this is leftover code
+that no longer has a purpose.
+
The process goes something like this:
$ make check
$ make FORCE_COMPILE_ALL=1 COMPILER_PLUGIN_TOOL='unnecessaryvirtual' check
@@ -46,7 +50,8 @@ bool operator < (const MyFuncInfo &lhs, const MyFuncInfo &rhs)
// try to limit the voluminous output a little
static std::set<MyFuncInfo> definitionSet;
-static std::set<std::string> overridingSet;
+static std::unordered_set<std::string> overridingSet;
+static std::unordered_set<std::string> nonEmptySet;
class UnnecessaryVirtual:
public RecursiveASTVisitor<UnnecessaryVirtual>, public loplugin::Plugin
@@ -66,6 +71,8 @@ public:
output += "definition:\t" + s.name + "\t" + s.sourceLocation + "\n";
for (const std::string & s : overridingSet)
output += "overriding:\t" + s + "\n";
+ for (const std::string & s : nonEmptySet)
+ output += "nonempty:\t" + s + "\n";
std::ofstream myfile;
myfile.open( WORKDIR "/loplugin.unnecessaryvirtual.log", std::ios::app | std::ios::out);
myfile << output;
@@ -76,6 +83,7 @@ public:
bool VisitCXXMethodDecl( const CXXMethodDecl* decl );
private:
+ void MarkRootOverridesNonEmpty( const CXXMethodDecl* methodDecl );
std::string toString(SourceLocation loc);
};
@@ -103,18 +111,27 @@ bool UnnecessaryVirtual::VisitCXXMethodDecl( const CXXMethodDecl* methodDecl )
if (ignoreLocation(methodDecl)) {
return true;
}
- if (!methodDecl->isThisDeclarationADefinition() ||
- !methodDecl->isVirtual() ||
- methodDecl->isDeleted())
- {
+ if (!methodDecl->isVirtual() || methodDecl->isDeleted()) {
return true;
}
- methodDecl = methodDecl->getCanonicalDecl();
// ignore stuff that forms part of the stable URE interface
- if (isInUnoIncludeFile(methodDecl)) {
+ if (isInUnoIncludeFile(methodDecl->getCanonicalDecl())) {
return true;
}
+ auto body = methodDecl->getBody();
+ if (body) {
+ auto compoundStmt = dyn_cast<CompoundStmt>(body);
+ if (!compoundStmt)
+ MarkRootOverridesNonEmpty(methodDecl->getCanonicalDecl());
+ else if (compoundStmt->size() > 0)
+ MarkRootOverridesNonEmpty(methodDecl->getCanonicalDecl());
+ }
+
+ if (!methodDecl->isThisDeclarationADefinition())
+ return true;
+
+ methodDecl = methodDecl->getCanonicalDecl();
std::string aNiceName = niceName(methodDecl);
// for destructors, we need to check if any of the superclass' destructors are virtual
@@ -155,6 +172,19 @@ bool UnnecessaryVirtual::VisitCXXMethodDecl( const CXXMethodDecl* methodDecl )
return true;
}
+void UnnecessaryVirtual::MarkRootOverridesNonEmpty( const CXXMethodDecl* methodDecl )
+{
+ if (methodDecl->size_overridden_methods() == 0) {
+ nonEmptySet.insert(niceName(methodDecl));
+ return;
+ }
+ for (auto iter = methodDecl->begin_overridden_methods();
+ iter != methodDecl->end_overridden_methods(); ++iter)
+ {
+ MarkRootOverridesNonEmpty(*iter);
+ }
+}
+
std::string UnnecessaryVirtual::toString(SourceLocation loc)
{
SourceLocation expansionLoc = compiler.getSourceManager().getExpansionLoc( loc );
diff --git a/compilerplugins/clang/unnecessaryvirtual.py b/compilerplugins/clang/unnecessaryvirtual.py
index 7ae6e6435907..187aafcbb2d8 100755
--- a/compilerplugins/clang/unnecessaryvirtual.py
+++ b/compilerplugins/clang/unnecessaryvirtual.py
@@ -7,6 +7,7 @@ import sys
definitionSet = set()
definitionToSourceLocationMap = dict()
overridingSet = set()
+nonEmptySet = set()
with io.open("workdir/loplugin.unnecessaryvirtual.log", "rb", buffering=1024*1024) as txt:
@@ -20,7 +21,12 @@ with io.open("workdir/loplugin.unnecessaryvirtual.log", "rb", buffering=1024*102
elif tokens[0] == "overriding:":
fullMethodName = tokens[1]
overridingSet.add(fullMethodName)
-
+ elif tokens[0] == "nonempty:":
+ fullMethodName = tokens[1]
+ nonEmptySet.add(fullMethodName)
+ else:
+ print( "unknown line: " + line)
+
unnecessaryVirtualSet = set()
for clazz in (definitionSet - overridingSet):
@@ -56,6 +62,21 @@ for clazz in (definitionSet - overridingSet):
unnecessaryVirtualSet.add( (clazz,loc) )
+deadSet = set()
+
+for clazz in (definitionSet - nonEmptySet):
+
+ # ignore destructors
+ if "::~" in clazz: continue
+
+ loc = definitionToSourceLocationMap[clazz]
+
+ # ignore external code
+ if loc.startswith("external/"): continue
+
+ deadSet.add( (clazz,loc) )
+
+
# 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()
@@ -63,6 +84,7 @@ def natural_sort_key(s, _nsre=re.compile('([0-9]+)')):
# sort results by name and line number
tmp1list = sorted(unnecessaryVirtualSet, key=lambda v: natural_sort_key(v[1]))
+tmp2list = sorted(deadSet, key=lambda v: natural_sort_key(v[1]))
with open("compilerplugins/clang/unnecessaryvirtual.results", "wt") as f:
for t in tmp1list:
@@ -71,3 +93,8 @@ with open("compilerplugins/clang/unnecessaryvirtual.results", "wt") as f:
# add an empty line at the end to make it easier for the removevirtuals plugin to mmap() the output file
f.write("\n")
+with open("compilerplugins/clang/unnecessaryvirtual-dead.results", "wt") as f:
+ for t in tmp2list:
+ f.write( t[1] + "\n" )
+ f.write( " " + t[0] + "\n" )
+
diff --git a/compilerplugins/clang/unnecessaryvirtual.results b/compilerplugins/clang/unnecessaryvirtual.results
index 8e8c84672159..13d089a6dc94 100644
--- a/compilerplugins/clang/unnecessaryvirtual.results
+++ b/compilerplugins/clang/unnecessaryvirtual.results
@@ -1,27 +1,23 @@
-basic/source/comp/codegen.cxx:464
+basic/source/comp/codegen.cxx:462
void OffSetAccumulator::start(const unsigned char *,)
-basic/source/comp/codegen.cxx:465
+basic/source/comp/codegen.cxx:463
void OffSetAccumulator::processOpCode0(enum SbiOpcode,)
-basic/source/comp/codegen.cxx:466
+basic/source/comp/codegen.cxx:464
void OffSetAccumulator::processOpCode1(enum SbiOpcode,type-parameter-0-0,)
-basic/source/comp/codegen.cxx:467
+basic/source/comp/codegen.cxx:465
void OffSetAccumulator::processOpCode2(enum SbiOpcode,type-parameter-0-0,type-parameter-0-0,)
-basic/source/comp/codegen.cxx:468
- void OffSetAccumulator::end()
-basic/source/comp/codegen.cxx:477
+basic/source/comp/codegen.cxx:474
_Bool OffSetAccumulator::processParams()
-basic/source/comp/codegen.cxx:488
+basic/source/comp/codegen.cxx:485
void BufferTransformer::start(const unsigned char *,)
-basic/source/comp/codegen.cxx:489
+basic/source/comp/codegen.cxx:486
void BufferTransformer::processOpCode0(enum SbiOpcode,)
-basic/source/comp/codegen.cxx:493
+basic/source/comp/codegen.cxx:490
void BufferTransformer::processOpCode1(enum SbiOpcode,type-parameter-0-0,)
-basic/source/comp/codegen.cxx:518
+basic/source/comp/codegen.cxx:515
void BufferTransformer::processOpCode2(enum SbiOpcode,type-parameter-0-0,type-parameter-0-0,)
-basic/source/comp/codegen.cxx:528
+basic/source/comp/codegen.cxx:525
_Bool BufferTransformer::processParams()
-basic/source/comp/codegen.cxx:529
- void BufferTransformer::end()
chart2/source/inc/WeakListenerAdapter.hxx:58
void chart::WeakListenerAdapter::disposing(const struct com::sun::star::lang::EventObject &,)
extensions/source/dbpilots/unoautopilot.hxx:98
@@ -106,6 +102,8 @@ include/sfx2/itemwrapper.hxx:156
const type-parameter-0-0 & sfx::IdentItemWrapper::GetItemValue(const type-parameter-0-0 &,)const
include/sfx2/itemwrapper.hxx:158
void sfx::IdentItemWrapper::SetItemValue(type-parameter-0-0 &,const type-parameter-0-0 &,)const
+include/sfx2/tabdlg.hxx:254
+ void SfxTabDialogController::RefreshInputSet()
include/svl/svdde.hxx:244
class DdeData * DdeTopic::Get(enum SotClipboardFormatId,)
include/svl/svdde.hxx:245
@@ -120,8 +118,6 @@ include/svl/svdde.hxx:307
void DdeService::~DdeService()
include/svtools/treelistbox.hxx:722
void SvTreeListBox::SelectAll(_Bool,_Bool,)
-include/svx/svdpage.hxx:94
- class SdrObjList * SdrObjList::CloneSdrObjList(class SdrModel *,)const
include/toolkit/controls/geometrycontrolmodel.hxx:191
void OGeometryControlModel::fillProperties(class com::sun::star::uno::Sequence<struct com::sun::star::beans::Property> &,class com::sun::star::uno::Sequence<struct com::sun::star::beans::Property> &,)const
include/vbahelper/vbacollectionimpl.hxx:290
@@ -146,9 +142,9 @@ include/vbahelper/vbareturntypes.hxx:40
void ooo::vba::DefaultReturnHelper::setValue(type-parameter-0-0,)
include/vbahelper/vbareturntypes.hxx:41
type-parameter-0-0 ooo::vba::DefaultReturnHelper::getValue()
-include/vcl/weld.hxx:267
+include/vcl/weld.hxx:288
void weld::TreeView::append_text(const class rtl::OUString &,)
-include/vcl/weld.hxx:270
+include/vcl/weld.hxx:291
void weld::TreeView::append(const class rtl::OUString &,const class rtl::OUString &,const class rtl::OUString &,)
sc/source/core/opencl/formulagroupcl.cxx:870
void sc::opencl::DynamicKernelMixedArgument::GenNumDeclRef(class std::basic_stringstream<char> &,)const
@@ -242,7 +238,9 @@ vcl/inc/salframe.hxx:135
void SalFrame::SetRepresentedURL(const class rtl::OUString &,)
vcl/inc/salframe.hxx:179
void SalFrame::Flush(const class tools::Rectangle &,)
-vcl/inc/sallayout.hxx:181
+vcl/inc/salinst.hxx:167
+ class weld::Builder * SalInstance::CreateInterimBuilder(class vcl::Window *,const class rtl::OUString &,const class rtl::OUString &,)
+vcl/inc/sallayout.hxx:179
_Bool SalLayout::GetBoundRect(class SalGraphics &,class tools::Rectangle &,)const
vcl/inc/salmenu.hxx:79
_Bool SalMenu::AddMenuBarButton(const struct SalMenuButtonItem &,)
@@ -252,8 +250,10 @@ vcl/inc/salmenu.hxx:92
class tools::Rectangle SalMenu::GetMenuBarButtonRectPixel(unsigned short,class SalFrame *,)
vcl/inc/salobj.hxx:46
void SalObject::Enable(_Bool,)
-vcl/inc/salprn.hxx:111
+vcl/inc/salprn.hxx:112
enum SalPrinterError SalPrinter::GetErrorCode()
+vcl/inc/unx/glyphcache.hxx:121
+ void FreetypeFont::~FreetypeFont()
vcl/inc/unx/gtk/gtkdata.hxx:164
int GtkSalDisplay::CaptureMouse(class SalFrame *,)
vcl/inc/unx/saldata.hxx:65