From 6f2edd4514dc59f5cbac8c653c6e4b1a96ec28a2 Mon Sep 17 00:00:00 2001 From: Noel Grandin Date: Tue, 3 Jun 2014 13:54:07 +0200 Subject: [PATCH] new loplugin: inlinesimpleaccessmethods Create a rewriting plugin for finding methods that simply return object fields, and should therefore be declared in the header, so that the compiler can reduce method calls into a simple fixed-offset load instruction. Change-Id: I7a620fc54250b79681918dc31ed9a8f21118c037 Signed-off-by: Stephan Bergmann --- .../clang/inlinesimplememberfunctions.cxx | 282 ++++++++++++++++++ include/svtools/fmtfield.hxx | 2 +- include/svtools/ruler.hxx | 2 +- include/svtools/treelistentry.hxx | 4 +- include/svtools/viewdataentry.hxx | 12 +- svtools/source/config/optionsdrawinglayer.cxx | 150 ++-------- svtools/source/contnr/treelistentry.cxx | 8 - svtools/source/contnr/viewdataentry.cxx | 24 -- svtools/source/control/ctrlbox.cxx | 12 +- svtools/source/control/fmtfield.cxx | 4 - svtools/source/control/ruler.cxx | 4 - svtools/source/table/tablecontrol_impl.cxx | 8 - svtools/source/table/tablecontrol_impl.hxx | 4 +- 13 files changed, 321 insertions(+), 195 deletions(-) create mode 100644 compilerplugins/clang/inlinesimplememberfunctions.cxx diff --git a/compilerplugins/clang/inlinesimplememberfunctions.cxx b/compilerplugins/clang/inlinesimplememberfunctions.cxx new file mode 100644 index 000000000000..4bee14c25200 --- /dev/null +++ b/compilerplugins/clang/inlinesimplememberfunctions.cxx @@ -0,0 +1,282 @@ +/* -*- 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 + +#include "plugin.hxx" + +// Methods that purely return a local field should be declared in the header and be declared inline. +// So that the compiler can elide the function call and turn it into a simple fixed-offset-load instruction. + +namespace { + +class InlineSimpleMemberFunctions: + public RecursiveASTVisitor, public loplugin::RewritePlugin +{ +public: + explicit InlineSimpleMemberFunctions(InstantiationData const & data): RewritePlugin(data) {} + + virtual void run() override { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); } + + bool VisitCXXMethodDecl(const CXXMethodDecl * decl); +private: + bool isInUnoIncludeFile(SourceLocation spellingLocation) const; + bool isInMainFile(SourceLocation spellingLocation) const; + bool rewrite(const CXXMethodDecl * functionDecl); +}; + +static bool oneAndOnlyOne(clang::Stmt::const_child_range range) { + if (range.empty()) { + return false; + } + if ((++range.first) != range.second) { + return false; + } + return true; +} + + +bool InlineSimpleMemberFunctions::VisitCXXMethodDecl(const CXXMethodDecl * functionDecl) { + if (ignoreLocation(functionDecl)) { + return true; + } + // no point in doing virtual methods, the compiler always has to generate a vtable entry and a method + if (functionDecl->isVirtual()) { + return true; + } + if (functionDecl->getTemplatedKind() != FunctionDecl::TK_NonTemplate) { + return true; + } + if (!functionDecl->isInstance()) { + return true; + } + if (!functionDecl->isOutOfLine()) { + return true; + } + if( !functionDecl->hasBody()) { + return true; + } + if( functionDecl->isInlineSpecified()) { + return true; + } + // ignore stuff that forms part of the stable URE interface + if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc( + functionDecl->getCanonicalDecl()->getNameInfo().getLoc()))) { + return true; + } + // ignore stuff like: + // template E * Sequence::begin() { return getArray(); } + if( functionDecl->getParent()->getDescribedClassTemplate() != nullptr ) { + return true; + } + + /* + The chain here looks like + CompoundStmt + ReturnStmt + other stuff + CXXThisExpr + */ + + const CompoundStmt* compoundStmt = dyn_cast< CompoundStmt >( functionDecl->getBody() ); + if (compoundStmt == nullptr) { + return true; + } + if (compoundStmt->body_begin() == compoundStmt->body_end()) { + return true; + } + + + const Stmt* childStmt = *compoundStmt->child_begin(); + + if (dyn_cast( childStmt ) == nullptr) { + return true; + } + if (!oneAndOnlyOne(childStmt->children())) { + return true; + } + + + /* Don't warn if we see a method definition like + X X::a() { + return *this; + } + which translates to: + CompoundStmt + ReturnStmt + ImplicitCastExpr + UnaryOperator + CXXThisExpr + or: + CompoundStmt + ReturnStmt + UnaryOperator + CXXThisExpr + */ + childStmt = *childStmt->child_begin(); + if (dyn_cast( childStmt ) != nullptr + && oneAndOnlyOne( childStmt->children() )) + { + const Stmt* childStmt2 = *childStmt->child_begin(); + if (dyn_cast( childStmt2 ) != nullptr + && oneAndOnlyOne(childStmt2->children())) + { + childStmt2 = *childStmt2->child_begin(); + if (dyn_cast( childStmt2 ) != nullptr + && childStmt2->children().empty()) + { + return true; + } + } + } + if (dyn_cast( childStmt ) != nullptr + && oneAndOnlyOne( childStmt->children() )) + { + const Stmt* childStmt2 = *childStmt->child_begin(); + if (dyn_cast( childStmt2 ) != nullptr + && childStmt2->children().empty()) + { + return true; + } + } + + /* look for a chain like: + CompoundStmt + ReturnStmt + ImplicitCastExpr + MemberExpr + CXXThisExpr + */ + if (dyn_cast( childStmt ) != nullptr + && oneAndOnlyOne( childStmt->children() )) + { + const Stmt* childStmt2 = *childStmt->child_begin(); + if (dyn_cast( childStmt2 ) != nullptr + && oneAndOnlyOne(childStmt2->children())) + { + childStmt2 = *childStmt2->child_begin(); + if (dyn_cast( childStmt2 ) != nullptr + && childStmt2->children().empty()) + { + if (!rewrite(functionDecl)) + { + report( + DiagnosticsEngine::Warning, + "inlinesimpleaccessmethods", + functionDecl->getSourceRange().getBegin()) + << functionDecl->getSourceRange(); + } + return true; + } + } + } + + return true; +} + +bool InlineSimpleMemberFunctions::isInUnoIncludeFile(SourceLocation spellingLocation) const { + StringRef name { + compiler.getSourceManager().getFilename(spellingLocation) }; + return isInMainFile(spellingLocation) + ? (name == SRCDIR "/cppu/source/cppu/compat.cxx" + || name == SRCDIR "/cppuhelper/source/compat.cxx" + || name == SRCDIR "/sal/osl/all/compat.cxx") + : (name.startswith(SRCDIR "/include/com/") + || name.startswith(SRCDIR "/include/cppu/") + || name.startswith(SRCDIR "/include/cppuhelper/") + || name.startswith(SRCDIR "/include/osl/") + || name.startswith(SRCDIR "/include/rtl/") + || name.startswith(SRCDIR "/include/sal/") + || name.startswith(SRCDIR "/include/salhelper/") + || name.startswith(SRCDIR "/include/systools/") + || name.startswith(SRCDIR "/include/typelib/") + || name.startswith(SRCDIR "/include/uno/") + || name == SRCDIR "/include/comphelper/implbase_var.hxx"); +} + +bool InlineSimpleMemberFunctions::isInMainFile(SourceLocation spellingLocation) const { +#if (__clang_major__ == 3 && __clang_minor__ >= 4) || __clang_major__ > 3 + return compiler.getSourceManager().isInMainFile(spellingLocation); +#else + return compiler.getSourceManager().isFromMainFile(spellingLocation); +#endif +} + +static std::string ReplaceString(std::string subject, const std::string& search, + const std::string& replace) { + size_t pos = 0; + while ((pos = subject.find(search, pos)) != std::string::npos) { + subject.replace(pos, search.length(), replace); + pos += replace.length(); + } + return subject; +} + +bool InlineSimpleMemberFunctions::rewrite(const CXXMethodDecl * functionDecl) { + if (rewriter == nullptr) { + return false; + } + // Only rewrite declarations in include files if a + // definition is also seen, to avoid compilation of a + // definition (in a main file only processed later) to fail + // with a "mismatch" error before the rewriter had a chance + // to act upon the definition. + if (!isInMainFile( + compiler.getSourceManager().getSpellingLoc( + functionDecl->getNameInfo().getLoc()))) { + return false; + } + + const char *p1, *p2; + + // get the function body contents + p1 = compiler.getSourceManager().getCharacterData( functionDecl->getBody()->getLocStart() ); + p2 = compiler.getSourceManager().getCharacterData( functionDecl->getBody()->getLocEnd() ); + std::string s1( p1, p2 - p1 + 1); + // strip linefeeds and any double-spaces, so we have a max of one space between tokens + s1 = ReplaceString(s1, "\r", ""); + s1 = ReplaceString(s1, "\n", ""); + s1 = ReplaceString(s1, "\t", " "); + s1 = ReplaceString(s1, " ", " "); + s1 = ReplaceString(s1, " ", " "); + s1 = ReplaceString(s1, " ", " "); + s1 = " " + s1; + + // scan from the end of the functions body through the whitespace, so we can do a nice clean remove +// commented out because for some reason it will sometimes chomp an extra token +// SourceLocation endOfRemoveLoc = functionDecl->getBody()->getLocEnd(); +// for (;;) { +// endOfRemoveLoc = endOfRemoveLoc.getLocWithOffset(1); +// p1 = compiler.getSourceManager().getCharacterData( endOfRemoveLoc ); +// if (*p1 != ' ' && *p1 != '\r' && *p1 != '\n' && *p1 != '\t') +// break; +// } + + // remove the function's out of line body and declaration + RewriteOptions opts; + opts.RemoveLineIfEmpty = true; + if (!removeText(SourceRange(functionDecl->getLocStart(), functionDecl->getBody()->getLocEnd()), opts)) { + return false; + } + + // scan forward until we find the semicolon + const FunctionDecl * canonicalDecl = functionDecl->getCanonicalDecl(); + p1 = compiler.getSourceManager().getCharacterData( canonicalDecl->getLocEnd() ); + p2 = ++p1; + while (*p2 != 0 && *p2 != ';') p2++; + + // insert the function body into the inline function definition (i.e. the one inside the class definition) + return replaceText(canonicalDecl->getLocEnd().getLocWithOffset(p2 - p1 + 1), 1, s1); +} + +loplugin::Plugin::Registration< InlineSimpleMemberFunctions > X("inlinesimplememberfunctions"); + +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/include/svtools/fmtfield.hxx b/include/svtools/fmtfield.hxx index 792465f371ef..d7bd1f1f1eac 100644 --- a/include/svtools/fmtfield.hxx +++ b/include/svtools/fmtfield.hxx @@ -231,7 +231,7 @@ public: using F2 */ void UseInputStringForFormatting( bool bUseInputStr = true ); - bool IsUsingInputStringForFormatting() const; + bool IsUsingInputStringForFormatting() const { return m_bUseInputStringForFormatting;} protected: virtual bool Notify(NotifyEvent& rNEvt) SAL_OVERRIDE; diff --git a/include/svtools/ruler.hxx b/include/svtools/ruler.hxx index 1468ec452619..67eb55d84035 100644 --- a/include/svtools/ruler.hxx +++ b/include/svtools/ruler.hxx @@ -707,7 +707,7 @@ private: Ruler& operator= (const Ruler &); protected: - long GetRulerVirHeight() const; + long GetRulerVirHeight() const { return mnVirHeight;} MapMode GetCurrentMapMode() const { return maMapMode; } RulerUnitData GetCurrentRulerUnit() const; diff --git a/include/svtools/treelistentry.hxx b/include/svtools/treelistentry.hxx index d5cdf05eb06d..0f74d9757f8b 100644 --- a/include/svtools/treelistentry.hxx +++ b/include/svtools/treelistentry.hxx @@ -90,13 +90,13 @@ public: const SvLBoxItem* GetFirstItem( sal_uInt16 nId ) const; SvLBoxItem* GetFirstItem( sal_uInt16 nId ); size_t GetPos( const SvLBoxItem* pItem ) const; - void* GetUserData() const; + void* GetUserData() const { return pUserData;} void SetUserData( void* pPtr ); void EnableChildrenOnDemand( bool bEnable=true ); bool HasChildrenOnDemand() const; bool HasInUseEmphasis() const; - sal_uInt16 GetFlags() const; + sal_uInt16 GetFlags() const { return nEntryFlags;} void SetFlags( sal_uInt16 nFlags ); bool GetIsMarked() const { return bIsMarked; } diff --git a/include/svtools/viewdataentry.hxx b/include/svtools/viewdataentry.hxx index 31681af4dc19..b24910b7ce72 100644 --- a/include/svtools/viewdataentry.hxx +++ b/include/svtools/viewdataentry.hxx @@ -58,12 +58,12 @@ public: SvViewDataEntry( const SvViewDataEntry& ); ~SvViewDataEntry(); - bool IsSelected() const; - bool IsHighlighted() const; - bool IsExpanded() const; - bool HasFocus() const; - bool IsCursored() const; - bool IsSelectable() const; + bool IsSelected() const { return mbSelected;} + bool IsHighlighted() const { return mbHighlighted;} + bool IsExpanded() const { return mbExpanded;} + bool HasFocus() const { return mbFocused;} + bool IsCursored() const { return mbCursored;} + bool IsSelectable() const { return mbSelectable;} void SetFocus( bool bFocus ); void SetSelected( bool bSelected ); void SetHighlighted( bool bHighlighted ); diff --git a/svtools/source/config/optionsdrawinglayer.cxx b/svtools/source/config/optionsdrawinglayer.cxx index d0cc92b027b4..07fa25c54f6c 100644 --- a/svtools/source/config/optionsdrawinglayer.cxx +++ b/svtools/source/config/optionsdrawinglayer.cxx @@ -181,48 +181,48 @@ public: // public interface - bool IsOverlayBuffer() const; - bool IsPaintBuffer() const; + bool IsOverlayBuffer() const { return m_bOverlayBuffer;} + bool IsPaintBuffer() const { return m_bPaintBuffer;} Color GetStripeColorA() const; Color GetStripeColorB() const; - sal_uInt16 GetStripeLength() const; + sal_uInt16 GetStripeLength() const { return m_nStripeLength;} // #i73602# - bool IsOverlayBuffer_Calc() const; - bool IsOverlayBuffer_Writer() const; - bool IsOverlayBuffer_DrawImpress() const; + bool IsOverlayBuffer_Calc() const { return m_bOverlayBuffer_Calc;} + bool IsOverlayBuffer_Writer() const { return m_bOverlayBuffer_Writer;} + bool IsOverlayBuffer_DrawImpress() const { return m_bOverlayBuffer_DrawImpress;} // #i74769#, #i75172# - bool IsPaintBuffer_Calc() const; - bool IsPaintBuffer_Writer() const; - bool IsPaintBuffer_DrawImpress() const; + bool IsPaintBuffer_Calc() const { return m_bPaintBuffer_Calc;} + bool IsPaintBuffer_Writer() const { return m_bPaintBuffer_Writer;} + bool IsPaintBuffer_DrawImpress() const { return m_bPaintBuffer_DrawImpress;} // #i4219# - sal_uInt32 GetMaximumPaperWidth() const; - sal_uInt32 GetMaximumPaperHeight() const; - sal_uInt32 GetMaximumPaperLeftMargin() const; - sal_uInt32 GetMaximumPaperRightMargin() const; - sal_uInt32 GetMaximumPaperTopMargin() const; - sal_uInt32 GetMaximumPaperBottomMargin() const; + sal_uInt32 GetMaximumPaperWidth() const { return m_nMaximumPaperWidth;} + sal_uInt32 GetMaximumPaperHeight() const { return m_nMaximumPaperHeight;} + sal_uInt32 GetMaximumPaperLeftMargin() const { return m_nMaximumPaperLeftMargin;} + sal_uInt32 GetMaximumPaperRightMargin() const { return m_nMaximumPaperRightMargin;} + sal_uInt32 GetMaximumPaperTopMargin() const { return m_nMaximumPaperTopMargin;} + sal_uInt32 GetMaximumPaperBottomMargin() const { return m_nMaximumPaperBottomMargin;} // helper bool IsAAPossibleOnThisSystem() const; // primitives - bool IsAntiAliasing() const; - bool IsSnapHorVerLinesToDiscrete() const; - bool IsSolidDragCreate() const; - bool IsRenderDecoratedTextDirect() const; - bool IsRenderSimpleTextDirect() const; - sal_uInt32 GetQuadratic3DRenderLimit() const; - sal_uInt32 GetQuadraticFormControlRenderLimit() const; + bool IsAntiAliasing() const { return m_bAntiAliasing;} + bool IsSnapHorVerLinesToDiscrete() const { return m_bSnapHorVerLinesToDiscrete;} + bool IsSolidDragCreate() const { return m_bSolidDragCreate;} + bool IsRenderDecoratedTextDirect() const { return m_bRenderDecoratedTextDirect;} + bool IsRenderSimpleTextDirect() const { return m_bRenderSimpleTextDirect;} + sal_uInt32 GetQuadratic3DRenderLimit() const { return m_nQuadratic3DRenderLimit;} + sal_uInt32 GetQuadraticFormControlRenderLimit() const { return m_nQuadraticFormControlRenderLimit;} void SetAntiAliasing( bool bState ); // #i97672# selection settings - bool IsTransparentSelection() const; - sal_uInt16 GetTransparentSelectionPercent() const; - sal_uInt16 GetSelectionMaximumLuminancePercent() const; + bool IsTransparentSelection() const { return m_bTransparentSelection;} + sal_uInt16 GetTransparentSelectionPercent() const { return m_nTransparentSelectionPercent;} + sal_uInt16 GetSelectionMaximumLuminancePercent() const { return m_nSelectionMaximumLuminancePercent;} void SetTransparentSelection( bool bState ); void SetTransparentSelectionPercent( sal_uInt16 nPercent ); @@ -693,18 +693,10 @@ void SvtOptionsDrawinglayer_Impl::Notify( const com::sun::star::uno::SequencenMargin2; } -long Ruler::GetRulerVirHeight() const -{ - return mnVirHeight; -} bool Ruler::GetTextRTL() { diff --git a/svtools/source/table/tablecontrol_impl.cxx b/svtools/source/table/tablecontrol_impl.cxx index 344a2f2eb999..f2b1b1533f91 100644 --- a/svtools/source/table/tablecontrol_impl.cxx +++ b/svtools/source/table/tablecontrol_impl.cxx @@ -2453,16 +2453,8 @@ namespace svt { namespace table } - ScrollBar* TableControl_Impl::getHorzScrollbar() - { - return m_pHScroll; - } - ScrollBar* TableControl_Impl::getVertScrollbar() - { - return m_pVScroll; - } bool TableControl_Impl::isRowSelected( RowPos i_row ) const diff --git a/svtools/source/table/tablecontrol_impl.hxx b/svtools/source/table/tablecontrol_impl.hxx index db3ff9f3f5d1..7a6191f4e4eb 100644 --- a/svtools/source/table/tablecontrol_impl.hxx +++ b/svtools/source/table/tablecontrol_impl.hxx @@ -300,8 +300,8 @@ namespace svt { namespace table TableDataWindow& getDataWindow() { return *m_pDataWindow; } const TableDataWindow& getDataWindow() const { return *m_pDataWindow; } - ScrollBar* getHorzScrollbar(); - ScrollBar* getVertScrollbar(); + ScrollBar* getHorzScrollbar() { return m_pHScroll; } + ScrollBar* getVertScrollbar() { return m_pVScroll; } Rectangle calcHeaderRect( bool bColHeader ); Rectangle calcHeaderCellRect( bool bColHeader, sal_Int32 nPos );