[Libreoffice-commits] core.git: compilerplugins/clang
Noel Grandin
noel.grandin at collabora.co.uk
Wed Mar 28 11:01:16 UTC 2018
compilerplugins/clang/virtualdown.cxx | 223 ++++++++++++++++++++++++++++++++++
compilerplugins/clang/virtualdown.py | 49 +++++++
2 files changed, 272 insertions(+)
New commits:
commit 7f7653c11b4312e89b7ad3cc0e5fb0b6a9b94c5d
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date: Wed Mar 28 10:00:11 2018 +0200
new loplugin virtualdown
Look for virtual methods where we can push their definition "down" i.e.
the base virtual method does not need to exist at all, because all the
call-sites are calling the more specific overrides.
Change-Id: Ib8e82637bfb6bc2a06df45de0e289d27344fb3ab
Reviewed-on: https://gerrit.libreoffice.org/51986
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/compilerplugins/clang/virtualdown.cxx b/compilerplugins/clang/virtualdown.cxx
new file mode 100644
index 000000000000..edb81617de2b
--- /dev/null
+++ b/compilerplugins/clang/virtualdown.cxx
@@ -0,0 +1,223 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <cassert>
+#include <string>
+#include <iostream>
+#include <set>
+#include "plugin.hxx"
+#include "compat.hxx"
+#include <fstream>
+
+/**
+Look for virtual methods where we never call the defining virtual method, and only call the overriding virtual
+methods, which indicates a places where the virtual-ness is unwarranted, normally a result of premature abstraction.
+
+The process goes something like this:
+ $ make check
+ $ make FORCE_COMPILE_ALL=1 COMPILER_PLUGIN_TOOL='VirtualDown' check
+ $ ./compilerplugins/clang/VirtualDown.py
+
+ at TODO for some reason, we get false+ for operator== methods
+ at TODO some templates confuse it and we get false+
+
+*/
+
+namespace
+{
+struct MyFuncInfo
+{
+ std::string name;
+ std::string sourceLocation;
+};
+bool operator<(const MyFuncInfo& lhs, const MyFuncInfo& rhs) { return lhs.name < rhs.name; }
+
+// try to limit the voluminous output a little
+static std::set<MyFuncInfo> definitionSet;
+static std::set<std::string> callSet;
+
+class VirtualDown : public RecursiveASTVisitor<VirtualDown>, public loplugin::Plugin
+{
+public:
+ explicit VirtualDown(loplugin::InstantiationData const& data)
+ : Plugin(data)
+ {
+ }
+
+ virtual void run() override
+ {
+ TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+
+ // dump all our output in one write call - this is to try and limit IO "crosstalk" between multiple processes
+ // writing to the same logfile
+ std::string output;
+ for (const MyFuncInfo& s : definitionSet)
+ output += "definition:\t" + s.name + "\t" + s.sourceLocation + "\n";
+ for (const std::string& s : callSet)
+ output += "call:\t" + s + "\n";
+ std::ofstream myfile;
+ myfile.open(SRCDIR "/loplugin.virtualdown.log", std::ios::app | std::ios::out);
+ myfile << output;
+ myfile.close();
+ }
+ bool shouldVisitTemplateInstantiations() const { return true; }
+ bool shouldVisitImplicitCode() const { return true; }
+
+ bool VisitCXXMethodDecl(CXXMethodDecl const*);
+ bool VisitCXXMemberCallExpr(CXXMemberCallExpr const*);
+ bool TraverseFunctionDecl(FunctionDecl*);
+ bool TraverseCXXMethodDecl(CXXMethodDecl*);
+ bool TraverseCXXConversionDecl(CXXConversionDecl*);
+#if CLANG_VERSION >= 50000
+ bool TraverseCXXDeductionGuideDecl(CXXDeductionGuideDecl*);
+#endif
+private:
+ std::string toString(SourceLocation loc);
+ std::string niceName(const CXXMethodDecl* functionDecl);
+ FunctionDecl const* currentFunctionDecl = nullptr;
+};
+
+bool VirtualDown::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
+{
+ if (ignoreLocation(methodDecl))
+ {
+ return true;
+ }
+ if (!methodDecl->isThisDeclarationADefinition() || !methodDecl->isVirtual()
+ || methodDecl->isDeleted())
+ {
+ return true;
+ }
+ methodDecl = methodDecl->getCanonicalDecl();
+ // ignore stuff that forms part of the stable URE interface
+ if (isInUnoIncludeFile(methodDecl))
+ {
+ return true;
+ }
+
+ std::string aNiceName = niceName(methodDecl);
+
+ if (isa<CXXDestructorDecl>(methodDecl))
+ return true;
+ if (isa<CXXConstructorDecl>(methodDecl))
+ return true;
+
+ if (methodDecl->size_overridden_methods() == 0)
+ definitionSet.insert({ aNiceName, toString(methodDecl->getLocation()) });
+
+ return true;
+}
+
+bool VirtualDown::VisitCXXMemberCallExpr(CXXMemberCallExpr const* 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 const* calleeFunctionDecl = expr->getDirectCallee();
+ if (calleeFunctionDecl == nullptr)
+ {
+ Expr const* callee = expr->getCallee()->IgnoreParenImpCasts();
+ DeclRefExpr const* dr = dyn_cast<DeclRefExpr>(callee);
+ if (dr)
+ {
+ calleeFunctionDecl = dyn_cast<FunctionDecl>(dr->getDecl());
+ if (calleeFunctionDecl)
+ goto gotfunc;
+ }
+ return true;
+ }
+
+gotfunc:
+
+ // ignore recursive calls
+ if (currentFunctionDecl == calleeFunctionDecl)
+ return true;
+
+ auto cxxMethodDecl = dyn_cast<CXXMethodDecl>(calleeFunctionDecl);
+ if (!cxxMethodDecl)
+ return true;
+
+ while (cxxMethodDecl->getTemplateInstantiationPattern())
+ cxxMethodDecl = dyn_cast<CXXMethodDecl>(cxxMethodDecl->getTemplateInstantiationPattern());
+ while (cxxMethodDecl->getInstantiatedFromMemberFunction())
+ cxxMethodDecl = dyn_cast<CXXMethodDecl>(cxxMethodDecl->getInstantiatedFromMemberFunction());
+ if (cxxMethodDecl->getLocation().isValid() && !ignoreLocation(cxxMethodDecl))
+ callSet.insert(niceName(cxxMethodDecl));
+
+ return true;
+}
+
+bool VirtualDown::TraverseFunctionDecl(FunctionDecl* f)
+{
+ auto copy = currentFunctionDecl;
+ currentFunctionDecl = f;
+ bool ret = RecursiveASTVisitor::TraverseFunctionDecl(f);
+ currentFunctionDecl = copy;
+ return ret;
+}
+bool VirtualDown::TraverseCXXMethodDecl(CXXMethodDecl* f)
+{
+ auto copy = currentFunctionDecl;
+ currentFunctionDecl = f;
+ bool ret = RecursiveASTVisitor::TraverseCXXMethodDecl(f);
+ currentFunctionDecl = copy;
+ return ret;
+}
+bool VirtualDown::TraverseCXXConversionDecl(CXXConversionDecl* f)
+{
+ auto copy = currentFunctionDecl;
+ currentFunctionDecl = f;
+ bool ret = RecursiveASTVisitor::TraverseCXXConversionDecl(f);
+ currentFunctionDecl = copy;
+ return ret;
+}
+#if CLANG_VERSION >= 50000
+bool VirtualDown::TraverseCXXDeductionGuideDecl(CXXDeductionGuideDecl* f)
+{
+ auto copy = currentFunctionDecl;
+ currentFunctionDecl = f;
+ bool ret = RecursiveASTVisitor::TraverseCXXDeductionGuideDecl(f);
+ currentFunctionDecl = copy;
+ return ret;
+}
+#endif
+
+std::string VirtualDown::niceName(const CXXMethodDecl* functionDecl)
+{
+ // return type not necessary, since we cannot have two otherwise identical functions
+ // with different return types
+ std::string s = functionDecl->getQualifiedNameAsString() + "(";
+ for (const ParmVarDecl* pParmVarDecl : compat::parameters(*functionDecl))
+ {
+ s += pParmVarDecl->getType().getCanonicalType().getAsString();
+ s += ",";
+ }
+ s += ")";
+ if (functionDecl->isConst())
+ {
+ s += "const";
+ }
+ return s;
+}
+
+std::string VirtualDown::toString(SourceLocation loc)
+{
+ SourceLocation expansionLoc = compiler.getSourceManager().getExpansionLoc(loc);
+ StringRef name = compiler.getSourceManager().getFilename(expansionLoc);
+ std::string sourceLocation
+ = std::string(name.substr(strlen(SRCDIR) + 1)) + ":"
+ + std::to_string(compiler.getSourceManager().getSpellingLineNumber(expansionLoc));
+ loplugin::normalizeDotDotInFilePath(sourceLocation);
+ return sourceLocation;
+}
+
+loplugin::Plugin::Registration<VirtualDown> X("virtualdown", false);
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/virtualdown.py b/compilerplugins/clang/virtualdown.py
new file mode 100755
index 000000000000..f2121c07a29b
--- /dev/null
+++ b/compilerplugins/clang/virtualdown.py
@@ -0,0 +1,49 @@
+#!/usr/bin/python
+
+import io
+import re
+import sys
+
+definitionSet = set()
+definitionToSourceLocationMap = dict()
+callSet = set()
+
+
+with io.open("loplugin.virtualdown.log", "rb", buffering=1024*1024) as txt:
+ for line in txt:
+ tokens = line.strip().split("\t")
+ if tokens[0] == "definition:":
+ fullMethodName = tokens[1]
+ sourceLocation = tokens[2]
+ definitionSet.add(fullMethodName)
+ definitionToSourceLocationMap[fullMethodName] = sourceLocation
+ elif tokens[0] == "call:":
+ fullMethodName = tokens[1]
+ callSet.add(fullMethodName)
+
+unnecessaryVirtualSet = set()
+
+for clazz in (definitionSet - callSet):
+# if clazz.startswith("canvas::"): continue
+# if clazz == "basegfx::unotools::UnoPolyPolygon::void-modifying()const": continue
+ # ignore external code
+ if definitionToSourceLocationMap[clazz].startswith("external/"): continue
+
+ unnecessaryVirtualSet.add((clazz,definitionToSourceLocationMap[clazz] ))
+
+
+# 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()
+ for text in re.split(_nsre, s)]
+
+# sort results by name and line number
+tmp1list = sorted(unnecessaryVirtualSet, key=lambda v: natural_sort_key(v[1]))
+
+with open("compilerplugins/clang/virtualdown.results", "wt") as f:
+ for t in tmp1list:
+ f.write( t[1] + "\n" )
+ f.write( " " + t[0] + "\n" )
+ # add an empty line at the end to make it easier for the removevirtuals plugin to mmap() the output file
+ f.write("\n")
+
More information about the Libreoffice-commits
mailing list