diff options
author | Noel Grandin <noel.grandin@collabora.co.uk> | 2018-07-25 13:55:35 +0200 |
---|---|---|
committer | Noel Grandin <noel.grandin@collabora.co.uk> | 2018-07-25 15:17:31 +0200 |
commit | fdc50b0a1b9741c5610a2b6c793c8fcdf5573c76 (patch) | |
tree | 7419f98a38cda7fe79a6660ec16f449f868f606b /compilerplugins | |
parent | d459f4c92e7bbb7e031b8cbba395d4767ecf344e (diff) |
new loplugin:returnconstant
look for methods that always return the same value
Change-Id: I5f1c5b58a985ecf5f7765a2d4ebd464eae5ef76f
Reviewed-on: https://gerrit.libreoffice.org/57973
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Diffstat (limited to 'compilerplugins')
-rw-r--r-- | compilerplugins/clang/pluginhandler.cxx | 2 | ||||
-rw-r--r-- | compilerplugins/clang/returnconstant.cxx | 666 |
2 files changed, 667 insertions, 1 deletions
diff --git a/compilerplugins/clang/pluginhandler.cxx b/compilerplugins/clang/pluginhandler.cxx index a4637cd78e6d..5a31368bdee0 100644 --- a/compilerplugins/clang/pluginhandler.cxx +++ b/compilerplugins/clang/pluginhandler.cxx @@ -49,7 +49,7 @@ struct PluginData bool byDefault; }; -const int MAX_PLUGINS = 100; +const int MAX_PLUGINS = 200; static PluginData plugins[ MAX_PLUGINS ]; static int pluginCount = 0; static bool bPluginObjectsCreated = false; diff --git a/compilerplugins/clang/returnconstant.cxx b/compilerplugins/clang/returnconstant.cxx new file mode 100644 index 000000000000..0c6e747bc348 --- /dev/null +++ b/compilerplugins/clang/returnconstant.cxx @@ -0,0 +1,666 @@ +/* -*- 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 "plugin.hxx" +#include "check.hxx" +#include <iostream> +#include <set> +#include <string> + +/* + Look for member functions that merely return a compile-time constant, or they are empty, and can thus + be either removed, or converted into a constant. + + This mostly tends to happen as a side-effect of other cleanups. +*/ +namespace +{ +class ReturnConstant : public RecursiveASTVisitor<ReturnConstant>, public loplugin::Plugin +{ +public: + explicit ReturnConstant(loplugin::InstantiationData const& data) + : Plugin(data) + { + } + + void run() override + { + // these files crash clang-3.5 somewhere in the isEvaluatable/EvaluateAsXXX stuff + /* FileID mainFileID = compiler.getSourceManager().getMainFileID(); + if (strstr(compiler.getSourceManager().getFileEntryForID(mainFileID)->getName(), "bootstrapfixture.cxx") != 0) { + return; + } + if (strstr(compiler.getSourceManager().getFileEntryForID(mainFileID)->getName(), "gtk3gtkinst.cxx") != 0) { + return; + }*/ + + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + } + + bool TraverseCXXMethodDecl(CXXMethodDecl*); + bool VisitReturnStmt(ReturnStmt const*); + +private: + StringRef getFilename(FunctionDecl const* functionDecl); + std::string getExprValue(Expr const* arg); + + struct Context + { + bool ignore = false; + std::set<std::string> values; + }; + std::vector<Context> m_functionStack; +}; + +StringRef ReturnConstant::getFilename(FunctionDecl const* functionDecl) +{ + SourceLocation spellingLocation = compiler.getSourceManager().getSpellingLoc( + functionDecl->getCanonicalDecl()->getNameInfo().getLoc()); + StringRef name{ compiler.getSourceManager().getFilename(spellingLocation) }; + return name; +} + +static bool startsWith(std::string const& rStr, char const* pSubStr) +{ + return rStr.compare(0, strlen(pSubStr), pSubStr) == 0; +} + +bool ReturnConstant::TraverseCXXMethodDecl(CXXMethodDecl* functionDecl) +{ + if (ignoreLocation(functionDecl)) + { + return true; + } + if (!functionDecl->hasBody()) + { + return true; + } + if (!functionDecl->isThisDeclarationADefinition()) + { + return true; + } + // stuff declared extern-C is almost always used as a some kind of callback + if (functionDecl->isExternC()) + { + return true; + } + if (functionDecl->isConstexpr()) + { + return true; + } + if (functionDecl->isMain()) + { + return true; + } + + StringRef aFileName = getFilename(functionDecl); + + // various tests in here are empty stubs under Linux + if (aFileName.startswith(SRCDIR "/sal/qa/")) + { + return true; + } + // lots of empty stuff here where it looks like someone is still going to "fill in the blanks" + if (aFileName.startswith(SRCDIR "/basegfx/test/")) + { + return true; + } + // some stuff is just stubs under Linux, although this appears to be a SOLARIS-specific hack, so it + // should probably not even be compiling under Linux. + if (aFileName == SRCDIR "/setup_native/scripts/source/getuid.c") + { + return true; + } + // bridges has some weird stuff in it.... + if (aFileName.startswith(SRCDIR "/bridges/")) + { + return true; + } + // dummy implementation of DDE, since it is only active on Windows + if (aFileName == SRCDIR "/svl/unx/source/svdde/ddedummy.cxx" + || aFileName == SRCDIR "/include/svl/svdde.hxx") + { + return true; + } + // fancy templates at work here + if (aFileName == SRCDIR "/vcl/source/gdi/bmpfast.cxx") + { + return true; + } + // bunch of stuff used as callbacks here + if (aFileName == SRCDIR "/vcl/generic/glyphs/gcach_layout.cxx") + { + return true; + } + // salplug runtime-loading mechanism at work + if (aFileName == SRCDIR "/vcl/inc/salinst.hxx") + { + return true; + } + // lots of callbacks here + if (aFileName == SRCDIR "/extensions/source/plugin/unx/npnapi.cxx") + { + return true; + } + // template magic + if (aFileName == SRCDIR "/filter/source/svg/svgreader.cxx") + { + return true; + } + // vcl/unx/gtk3 re-using vcl/unx/gtk: + if (aFileName.find("/../../gtk/") != std::string::npos) + { + return true; + } + // used by code generated by python + if (aFileName == SRCDIR "/writerfilter/source/ooxml/OOXMLFastContextHandler.hxx") + { + return true; + } + // this test just test the include of some headers + if (aFileName == SRCDIR "/officecfg/qa/cppheader.cxx") + { + return true; + } + // just ignore this for now, people furiously hacking in there + if (startsWith(aFileName, SRCDIR "/libreofficekit")) + { + return true; + } + // templates + if (startsWith(aFileName, SRCDIR "/store")) + return true; + + // not worth it + if (startsWith(aFileName, SRCDIR "/registry")) + return true; + if (startsWith(aFileName, SRCDIR "/idlc")) + return true; + if (startsWith(aFileName, SRCDIR "/include/unotools")) + return true; + if (startsWith(aFileName, SRCDIR "/xmlreader/")) + return true; + if (startsWith(aFileName, SRCDIR "/include/xmlreader/")) + return true; + if (startsWith(aFileName, SRCDIR "/jvmfwk")) + return true; + + const CXXMethodDecl* pCXXMethodDecl = dyn_cast<CXXMethodDecl>(functionDecl); + if (pCXXMethodDecl) + { + if (pCXXMethodDecl->isVirtual()) + { + return true; + } + // static with inline body will be optimised at compile-time to a constant anyway + if (pCXXMethodDecl->isStatic() + && (pCXXMethodDecl->hasInlineBody() || pCXXMethodDecl->isInlineSpecified())) + { + return true; + } + // this catches some stuff in templates + if (functionDecl->hasAttr<OverrideAttr>()) + { + return true; + } + } + // a free function with an inline body will be optimised at compile-time to a constant anyway + if (!pCXXMethodDecl && functionDecl->isInlineSpecified()) + { + return true; + } + if (isa<CXXConstructorDecl>(functionDecl) || isa<CXXDestructorDecl>(functionDecl) + || isa<CXXConversionDecl>(functionDecl)) + { + return true; + } + if (isInUnoIncludeFile(functionDecl)) + { + return true; + } + + switch (functionDecl->getOverloadedOperator()) + { + case OO_Delete: + case OO_EqualEqual: + case OO_Call: + return true; + default: + break; + } + + std::string aFunctionName = functionDecl->getQualifiedNameAsString(); + + // something to do with dynamic loading in sal/textenc/textenc.cxx + if (aFunctionName == "thisModule") + { + return true; + } + // an empty stub under certain conditions, sal/osl/unx/thread.cxx + if (aFunctionName == "osl_thread_priority_init_Impl") + { + return true; + } + // a pointer to this function is taken and passed to an underlying API, shell/source/unix/sysshell/recently_used_file_handler.cxx + if (aFunctionName == "(anonymous namespace)::recently_used_item::set_nothing") + { + return true; + } + // a pointer to this function is taken and passed to an underlying API, cppu/source/uno/lbenv.cxx + if (aFunctionName == "defenv_dispose") + { + return true; + } + // a pointer to this function is taken and passed to an underlying API, cppuhelper/source/exc_thrower.cxx + if (aFunctionName == "ExceptionThrower_acquire_release_nop") + { + return true; + } + // different hook function is called on different platforms, /vcl/source/app/svmainhook.cxx + if (aFunctionName == "ImplSVMainHook") + { + return true; + } + // used as a callback, /vcl/source/filter/jpeg/JpegReader.cxx + if (aFunctionName == "term_source") + { + return true; + } + // only valid for windows, extensions/source/update/check/updatecheck.cxx + if (aFunctionName == "(anonymous namespace)::UpdateCheckThread::hasInternetConnection") + { + return true; + } + // used as callback, extensions/source/plugin/unx/npwrap.cxx + if (aFunctionName == "plugin_x_error_handler" || aFunctionName == "noClosure") + { + return true; + } + // used as callback, sax/source/expatwrap/sax_expat.cxx + if (aFunctionName == "(anonymous namespace)::SaxExpatParser_Impl::callbackUnknownEncoding") + { + return true; + } + // used as callback, i18npool/source/textconversion/textconversion.cxx + if (aFunctionName == "com::sun::star::i18n::nullFunc") + { + return true; + } + // used as callback, xmloff/source/text/txtparae.cxx + if (aFunctionName == "(anonymous namespace)::lcl_TextContentsUnfiltered") + { + return true; + } + // template magic, include/canvas/verifyinput.hxx + if (aFunctionName == "canvas::tools::verifyInput") + { + return true; + } + // template magic, cppcanvas/source/mtfrenderer/implrenderer.cxx + if (aFunctionName == "cppcanvas::internal::(anonymous namespace)::AreaQuery::result") + { + return true; + } + // callback, drawinglayer/source/dumper/XShapeDumper. + if (aFunctionName == "(anonymous namespace)::closeCallback") + { + return true; + } + // callback, basic/source/runtime/runtime.cxx + if (aFunctionName == "SbiRuntime::StepNOP") + { + return true; + } + // DLL stuff, only used on windows, basic/source/runtime/dllmgr.hxx + if (aFunctionName == "SbiDllMgr::FreeDll") + { + return true; + } + // only used on Windows, basic/source/sbx/sbxdec.cxx + if (aFunctionName == "SbxDecimal::neg" || aFunctionName == "SbxDecimal::isZero") + { + return true; + } + // used as a callback, include/sfx2/shell.hxx + if (aFunctionName == "SfxShell::EmptyExecStub" || aFunctionName == "SfxShell::EmptyStateStub" + || aFunctionName == "SfxShell::VerbState") + { + return true; + } + // SFX_IMPL_POS_CHILDWINDOW_WITHID macro + if (aFunctionName.find("GetChildWindowId") != std::string::npos) + { + return true; + } + // SFX_IMPL_SUPERCLASS_INTERFACE macro + if (aFunctionName.find("InitInterface_Impl") != std::string::npos) + { + return true; + } + // callback, vcl/unx/generic/app/sm.cxx + if (aFunctionName == "IgnoreIceIOErrors" || aFunctionName == "IgnoreIceErrors") + { + return true; + } + // callback, vcl/unx/gtk/a11y/atkcomponent.cxx + if (aFunctionName == "component_wrapper_get_mdi_zorder") + { + return true; + } + // callback, vcl/unx/gtk/a11y/atkaction.cxx + if (aFunctionName == "action_wrapper_set_description") + { + return true; + } + // callback, vcl/unx/gtk/a11y/atkutil.cxx + if (aFunctionName == "ooo_atk_util_get_toolkit_version" + || aFunctionName == "ooo_atk_util_get_toolkit_name") + { + return true; + } + // callback, vcl/unx/gtk/a11y/atktextattributes.cxx + if (aFunctionName == "InvalidValue") + { + return true; + } + // callback, vcl/unx/gtk/a11y/atktable.cxx + if (aFunctionName == "table_wrapper_set_summary" + || aFunctionName == "table_wrapper_set_row_header" + || aFunctionName == "table_wrapper_set_row_description" + || aFunctionName == "table_wrapper_set_column_header" + || aFunctionName == "table_wrapper_set_column_description" + || aFunctionName == "table_wrapper_set_caption") + { + return true; + } + // callbacks, vcl/unx/gtk/window/gtksalframe.cxx + if (startsWith(aFunctionName, "GtkSalFrame::IMHandler::signal")) + { + return true; + } + // callbacks, vcl/unx/gtk/window/glomenu.cxx + if (startsWith(aFunctionName, "g_lo_menu_is_mutable")) + { + return true; + } + // only contains code for certain versions of GTK, /vcl/unx/gtk/window/gtksalframe.cx + if (aFunctionName == "GtkSalFrame::AllocateFrame") + { + return true; + } + // only valid for Windows, embeddedobj/source/msole/olemisc.cxx + if (aFunctionName == "OleEmbeddedObject::GetRidOfComponent") + { + return true; + } + // callback, svx/source/accessibility/ShapeTypeHandler.cxx + if (aFunctionName == "accessibility::CreateEmptyShapeReference") + { + return true; + } + // chart2/source/view/main/AbstractShapeFactory.cxx + if (aFunctionName == "chart::(anonymous namespace)::thisModule") + { + return true; + } + // chart2/source/tools/InternalData.cxx + if (aFunctionName == "chart::InternalData::dump") + { + return true; + } + // hwpfilter/ + if (aFunctionName == "debug" || aFunctionName == "token_debug") + { + return true; + } + // callback, sdext/source/presenter/PresenterFrameworkObserver.cxx + if (aFunctionName == "sdext::presenter::PresenterFrameworkObserver::True") + { + return true; + } + // callback, sw/source/core/doc/tblrwcl.cxx + if (aFunctionName == "lcl_DelOtherBox") + { + return true; + } + // callback, sw/source/filter/ww8/ww8par.cxx + if (aFunctionName == "SwWW8ImplReader::Read_Majority") + { + return true; + } + // callback, sw/source/filter/ww8/ww8par5.cxx + if (aFunctionName == "SwWW8ImplReader::Read_F_Shape") + { + return true; + } + // called from SDI file, I don't know what that stuff is about, sd/source/ui/slidesorter/shell/SlideSorterViewShell.cx + if (aFunctionName == "sd::slidesorter::SlideSorterViewShell::ExecStatusBar" + || aFunctionName == "sd::OutlineViewShell::ExecStatusBar") + { + return true; + } + // only used in debug mode, sd/source/filter/ppt/pptinanimations.cxx + if (startsWith(aFunctionName, "ppt::AnimationImporter::dump")) + { + return true; + } + // only used in ENABLE_SDREMOTE_BLUETOOTH mode, sd/source/ui/dlg/tpoption.cx + if (aFunctionName == "SdTpOptionsMisc::SetImpressMode") + { + return true; + } + // template magic, sc/source/ui/docshell/datastream.cxx + if (startsWith(aFunctionName, "sc::(anonymous namespace)::CSVHandler::")) + { + return true; + } + // called from SDI file, I don't know what that stuff is about, sc/source/ui/view/cellsh4.cxx + if (aFunctionName == "ScCellShell::GetStateCursor") + { + return true; + } + // template magic, sc/source/filter/excel/xepivot.cxx + if (aFunctionName == "XclExpPivotCache::SaveXml") + { + return true; + } + // template magic, sc/source/filter/html/htmlpars.cxx + if (startsWith(aFunctionName, "(anonymous namespace)::CSSHandler::")) + { + return true; + } + // callbacks, sc/source/filter/oox/formulaparser.cxx + if (startsWith(aFunctionName, "oox::xls::BiffFormulaParserImpl::import")) + { + return true; + } + // template magic, sc/qa/unit/helper/csv_handler.hxx + if (startsWith(aFunctionName, "csv_handler::") + || startsWith(aFunctionName, "conditional_format_handler::")) + { + return true; + } + // template magic, slideshow/source/inc/listenercontainer.hxx + if (startsWith(aFunctionName, "slideshow::internal::EmptyBase::EmptyClearableGuard::")) + { + return true; + } + // callback, scripting/source/vbaevents/eventhelper.cxx + if (aFunctionName == "ApproveAll") + { + return true; + } + // only on WNT, basic/qa/cppunit/test_vba.cx + if (aFunctionName == "(anonymous namespace)::VBATest::testMiscOLEStuff") + { + return true; + } + // GtkSalFrame::TriggerPaintEvent() is only compiled under certain versions of GTK + if (aFunctionName == "GtkSalFrame::TriggerPaintEvent") + { + return true; + } + if (aFunctionName == "SwVectorModifyBase::dumpAsXml") + { + return true; + } + // vcl/unx/gtk3 re-using vcl/unx/gtk: + if (aFunctionName == "DeInitAtkBridge" || aFunctionName == "GtkData::initNWF" + || aFunctionName == "GtkSalFrame::EnsureAppMenuWatch" || aFunctionName == "InitAtkBridge") + { + return true; + } + if (aFunctionName == "sc::AlignedAllocator::operator!=") + { + return true; + } + if (aFunctionName == "clipboard_owner_init") + { + return true; + } + // returns sizeof(struct) vcl/source/gdi/dibtools.cxx + if (aFunctionName == "getDIBV5HeaderSize") + { + return true; + } + // windows only + if (aFunctionName == "InitAccessBridge") + { + return true; + } + // callbacks + if (aFunctionName == "disabled_initSystray" || aFunctionName == "disabled_deInitSystray") + { + return true; + } + // behind a BREAKPAD option + if (aFunctionName == "desktop::(anonymous namespace)::crashReportInfoExists") + { + return true; + } + // LOK stuff + if (aFunctionName == "doc_getTileMode") + { + return true; + } + + // ignore LINK macro stuff + std::string aImmediateMacro = ""; + if (compiler.getSourceManager().isMacroBodyExpansion(functionDecl->getLocStart())) + { + StringRef name{ Lexer::getImmediateMacroName( + functionDecl->getLocStart(), compiler.getSourceManager(), compiler.getLangOpts()) }; + aImmediateMacro = name; + if (name.startswith("IMPL_LINK_")) + { + return true; + } + } + + m_functionStack.emplace_back(); + bool ret = RecursiveASTVisitor<ReturnConstant>::TraverseCXXMethodDecl(functionDecl); + Context& rContext = m_functionStack.back(); + if (!rContext.ignore && rContext.values.size() == 1 + && rContext.values.find("unknown") == rContext.values.end()) + { + report(DiagnosticsEngine::Warning, + "Method only returns a single constant value %0, does it make sense?", + functionDecl->getLocStart()) + << *rContext.values.begin() << functionDecl->getSourceRange(); + if (functionDecl != functionDecl->getCanonicalDecl()) + report(DiagnosticsEngine::Note, "decl here", + functionDecl->getCanonicalDecl()->getLocStart()) + << functionDecl->getCanonicalDecl()->getSourceRange(); + } + m_functionStack.pop_back(); + return ret; +} + +bool ReturnConstant::VisitReturnStmt(ReturnStmt const* returnStmt) +{ + if (ignoreLocation(returnStmt)) + return true; + if (m_functionStack.empty()) + return true; + Context& rContext = m_functionStack.back(); + + if (!returnStmt->getRetValue()) + return true; + if (returnStmt->getRetValue()->isTypeDependent()) + { + rContext.ignore = true; + return true; + } + if (const UnaryOperator* unaryOp = dyn_cast<UnaryOperator>(returnStmt->getRetValue())) + { + if (unaryOp->getOpcode() == UO_AddrOf) + { + rContext.ignore = true; + return true; + } + } + rContext.values.insert(getExprValue(returnStmt->getRetValue())); + return true; +} + +std::string ReturnConstant::getExprValue(Expr const* arg) +{ + arg = arg->IgnoreParenCasts(); + if (isa<CXXDefaultArgExpr>(arg)) + { + arg = dyn_cast<CXXDefaultArgExpr>(arg)->getExpr(); + } + arg = arg->IgnoreParenCasts(); + // ignore this, it seems to trigger an infinite recursion + if (isa<UnaryExprOrTypeTraitExpr>(arg)) + { + return "unknown"; + } + APSInt x1; + if (arg->EvaluateAsInt(x1, compiler.getASTContext())) + { + return x1.toString(10); + } + if (isa<CXXNullPtrLiteralExpr>(arg)) + { + return "0"; + } + if (isa<MaterializeTemporaryExpr>(arg)) + { + const CXXBindTemporaryExpr* strippedArg + = dyn_cast_or_null<CXXBindTemporaryExpr>(arg->IgnoreParenCasts()); + if (strippedArg && strippedArg->getSubExpr()) + { + auto temp = dyn_cast<CXXTemporaryObjectExpr>(strippedArg->getSubExpr()); + if (temp->getNumArgs() == 0) + { + if (loplugin::TypeCheck(temp->getType()) + .Class("OUString") + .Namespace("rtl") + .GlobalNamespace()) + { + return "\"\""; + } + if (loplugin::TypeCheck(temp->getType()) + .Class("OString") + .Namespace("rtl") + .GlobalNamespace()) + { + return "\"\""; + } + } + } + } + return "unknown"; +} + +loplugin::Plugin::Registration<ReturnConstant> X("returnconstant", false); +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ |