Kaydet (Commit) 7ceee0f1 authored tarafından Stephan Bergmann's avatar Stephan Bergmann

Extend loplugin:redundantinline to catch inline functions w/o external linkage

...where "inline" (in its meaning of "this function can be defined in multiple
translation units") thus doesn't make much sense.  (As discussed in
compilerplugins/clang/redundantinline.cxx, exempt such "static inline" functions
in include files for now.)

All the rewriting has been done automatically by the plugin, except for one
instance in sw/source/ui/frmdlg/column.cxx that used to involve an #if), plus
some subsequent solenv/clang-format/reformat-formatted-files.

Change-Id: Ib8b996b651aeafc03bbdc8890faa05ed50517224
Reviewed-on: https://gerrit.libreoffice.org/61573
Tested-by: Jenkins
Reviewed-by: 's avatarStephan Bergmann <sbergman@redhat.com>
üst 664db0d9
......@@ -70,7 +70,7 @@ using namespace ::svt::table;
namespace {
inline bool hasFloatingChild(vcl::Window *pWindow)
bool hasFloatingChild(vcl::Window *pWindow)
{
vcl::Window * pChild = pWindow->GetAccessibleChildWindow(0);
return pChild && pChild->GetType() == WindowType::FLOATINGWINDOW;
......
......@@ -59,7 +59,7 @@ namespace basegfx { namespace utils
return BColor(h,s,l);
}
static inline double hsl2rgbHelper( double nValue1, double nValue2, double nHue )
static double hsl2rgbHelper( double nValue1, double nValue2, double nHue )
{
// clamp hue to [0,360]
nHue = fmod( nHue, 360.0 );
......
......@@ -907,7 +907,7 @@ namespace basegfx
namespace
{
inline void impCheckExtremumResult(double fCandidate, std::vector< double >& rResult)
void impCheckExtremumResult(double fCandidate, std::vector< double >& rResult)
{
// check for range ]0.0 .. 1.0[ with excluding 1.0 and 0.0 clearly
// by using the equalZero test, NOT ::more or ::less which will use the
......
......@@ -2161,7 +2161,7 @@ namespace basegfx
namespace
{
/// return 0 for input of 0, -1 for negative and 1 for positive input
inline int lcl_sgn( const double n )
int lcl_sgn( const double n )
{
return n == 0.0 ? 0 : 1 - 2*int(rtl::math::isSignBitSet(n));
}
......
......@@ -223,7 +223,7 @@ namespace basegfx
// helper for getting the 3D Point from given cartesian coordinates. fHor is defined from
// [F_PI2 .. -F_PI2], fVer from [0.0 .. F_2PI]
static inline B3DPoint getPointFromCartesian(double fHor, double fVer)
static B3DPoint getPointFromCartesian(double fHor, double fVer)
{
const double fCosVer(cos(fVer));
return B3DPoint(fCosVer * cos(fHor), sin(fVer), fCosVer * -sin(fHor));
......
......@@ -680,7 +680,7 @@ namespace basegfx
aNewEdges );
}
inline bool isSameRect(ActiveEdge const & rEdge,
bool isSameRect(ActiveEdge const & rEdge,
basegfx::B2DRange const & rRect)
{
return &rEdge.getRect() == &rRect;
......@@ -689,12 +689,12 @@ namespace basegfx
// wow what a hack. necessary because stl's list::erase does
// not eat reverse_iterator
template<typename Cont, typename Iter> Iter eraseFromList(Cont&, const Iter&);
template<> inline ListOfEdges::iterator eraseFromList(
template<> ListOfEdges::iterator eraseFromList(
ListOfEdges& rList, const ListOfEdges::iterator& aIter)
{
return rList.erase(aIter);
}
template<> inline ListOfEdges::reverse_iterator eraseFromList(
template<> ListOfEdges::reverse_iterator eraseFromList(
ListOfEdges& rList, const ListOfEdges::reverse_iterator& aIter)
{
return ListOfEdges::reverse_iterator(
......@@ -702,7 +702,7 @@ namespace basegfx
}
template<int bPerformErase,
typename Iterator> inline void processActiveEdges(
typename Iterator> void processActiveEdges(
Iterator first,
Iterator last,
ListOfEdges& rActiveEdgeList,
......@@ -761,7 +761,7 @@ namespace basegfx
}
}
template<int bPerformErase> inline void processActiveEdgesTopDown(
template<int bPerformErase> void processActiveEdgesTopDown(
SweepLineEvent& rCurrEvent,
ListOfEdges& rActiveEdgeList,
VectorOfPolygons& rPolygonPool,
......@@ -776,7 +776,7 @@ namespace basegfx
rRes);
}
template<int bPerformErase> inline void processActiveEdgesBottomUp(
template<int bPerformErase> void processActiveEdgesBottomUp(
SweepLineEvent& rCurrEvent,
ListOfEdges& rActiveEdgeList,
VectorOfPolygons& rPolygonPool,
......@@ -824,7 +824,7 @@ namespace basegfx
rCurrEvent, rActiveEdgeList, rPolygonPool, rRes);
}
inline void handleSweepLineEvent( SweepLineEvent& rCurrEvent,
void handleSweepLineEvent( SweepLineEvent& rCurrEvent,
ListOfEdges& rActiveEdgeList,
VectorOfPolygons& rPolygonPool,
B2DPolyPolygon& rRes)
......
......@@ -28,7 +28,7 @@ namespace basegfx
{
namespace
{
inline double distance( const double& nX,
double distance( const double& nX,
const double& nY,
const ::basegfx::B2DVector& rNormal,
const double& nC )
......
......@@ -49,7 +49,7 @@ double getRandomOrdinal( const std::size_t n )
return comphelper::rng::uniform_size_distribution(0, n-1);
}
static inline bool compare(const B2DPoint& left, const B2DPoint& right)
static bool compare(const B2DPoint& left, const B2DPoint& right)
{
return left.getX()<right.getX()
|| (rtl::math::approxEqual(left.getX(),right.getX()) && left.getY()<right.getY());
......
......@@ -140,7 +140,7 @@ static const CharClass& GetCharClass()
return aCharClass;
}
static inline bool isFolder( FileStatus::Type aType )
static bool isFolder( FileStatus::Type aType )
{
return ( aType == FileStatus::Directory || aType == FileStatus::Volume );
}
......@@ -2587,7 +2587,7 @@ static OUString implSetupWildcard( const OUString& rFileParam, SbiRTLData* pRTLD
return aPathStr;
}
static inline bool implCheckWildcard( const OUString& rName, SbiRTLData const * pRTLData )
static bool implCheckWildcard( const OUString& rName, SbiRTLData const * pRTLData )
{
bool bMatch = true;
......
......@@ -1828,7 +1828,7 @@ static IntervalInfo const * getIntervalInfo( const OUString& rStringCode )
return nullptr;
}
static inline void implGetDayMonthYear( sal_Int16& rnYear, sal_Int16& rnMonth, sal_Int16& rnDay, double dDate )
static void implGetDayMonthYear( sal_Int16& rnYear, sal_Int16& rnMonth, sal_Int16& rnDay, double dDate )
{
rnDay = implGetDateDay( dDate );
rnMonth = implGetDateMonth( dDate );
......@@ -1840,7 +1840,7 @@ static inline void implGetDayMonthYear( sal_Int16& rnYear, sal_Int16& rnMonth, s
@return the year number, truncated if necessary and in that case also
rMonth and rDay adjusted.
*/
static inline sal_Int16 limitDate( sal_Int32 n32Year, sal_Int16& rMonth, sal_Int16& rDay )
static sal_Int16 limitDate( sal_Int32 n32Year, sal_Int16& rMonth, sal_Int16& rDay )
{
if( n32Year > SAL_MAX_INT16 )
{
......@@ -1957,7 +1957,7 @@ void SbRtl_DateAdd(StarBASIC *, SbxArray & rPar, bool)
rPar.Get(0)->PutDate( dNewDate );
}
static inline double RoundImpl( double d )
static double RoundImpl( double d )
{
return ( d >= 0 ) ? floor( d + 0.5 ) : -floor( -d + 0.5 );
}
......
......@@ -1568,7 +1568,7 @@ void SbiRuntime::StepGET()
}
// #67607 copy Uno-Structs
static inline bool checkUnoStructCopy( bool bVBA, SbxVariableRef const & refVal, SbxVariableRef const & refVar )
static bool checkUnoStructCopy( bool bVBA, SbxVariableRef const & refVal, SbxVariableRef const & refVar )
{
SbxDataType eVarType = refVar->GetType();
SbxDataType eValType = refVal->GetType();
......
......@@ -31,7 +31,7 @@ namespace jni_uno
{
static inline std::unique_ptr<rtl_mem> seq_allocate(
static std::unique_ptr<rtl_mem> seq_allocate(
sal_Int32 nElements, sal_Int32 nSize )
{
std::unique_ptr< rtl_mem > seq(
......
......@@ -45,7 +45,7 @@ using namespace canvas;
namespace
{
inline uno::Sequence< double > color2Sequence( sal_Int32 nColor )
uno::Sequence< double > color2Sequence( sal_Int32 nColor )
{
// TODO(F3): Color management
uno::Sequence< double > aRes( 4 );
......@@ -58,7 +58,7 @@ namespace
return aRes;
}
inline uno::Reference< rendering::XPolyPolygon2D > rect2Poly( uno::Reference<rendering::XGraphicDevice> const& xDevice,
uno::Reference< rendering::XPolyPolygon2D > rect2Poly( uno::Reference<rendering::XGraphicDevice> const& xDevice,
geometry::RealRectangle2D const& rRect )
{
uno::Sequence< geometry::RealPoint2D > rectSequence( 4 );
......
......@@ -1860,7 +1860,7 @@ std::shared_ptr< DrawModelWrapper > ChartView::getDrawModelWrapper()
namespace
{
inline sal_Int32 lcl_getDiagramTitleSpace()
sal_Int32 lcl_getDiagramTitleSpace()
{
return 200; //=0,2 cm spacing
}
......@@ -2049,7 +2049,7 @@ awt::Rectangle ExplicitValueProvider::AddSubtractAxisTitleSizes(
namespace {
inline double lcl_getPageLayoutDistancePercentage()
double lcl_getPageLayoutDistancePercentage()
{
return 0.02;
}
......
......@@ -609,13 +609,13 @@ awt::Size lcl_placeLegendEntries(
}
// #i109336# Improve auto positioning in chart
inline sal_Int32 lcl_getLegendLeftRightMargin()
sal_Int32 lcl_getLegendLeftRightMargin()
{
return 210; // 1/100 mm
}
// #i109336# Improve auto positioning in chart
inline sal_Int32 lcl_getLegendTopBottomMargin()
sal_Int32 lcl_getLegendTopBottomMargin()
{
return 185; // 1/100 mm
}
......
......@@ -32,7 +32,7 @@ namespace {
namespace detail {
template <typename T>
inline void extract(
void extract(
::com::sun::star::uno::Sequence< ::com::sun::star::uno::Any> const& seq,
sal_Int32 nArg, T & v,
::com::sun::star::uno::Reference< ::com::sun::star::uno::XInterface>
......@@ -54,7 +54,7 @@ inline void extract(
}
template <typename T>
inline void extract(
void extract(
::com::sun::star::uno::Sequence< ::com::sun::star::uno::Any> const& seq,
sal_Int32 nArg, ::boost::optional<T> & v,
::com::sun::star::uno::Reference< ::com::sun::star::uno::XInterface>
......@@ -70,7 +70,7 @@ inline void extract(
} // namespace detail
template < typename T0, typename T1, typename T2, typename T3, typename T4 >
inline void unwrapArgsBaseline(
void unwrapArgsBaseline(
::com::sun::star::uno::Sequence< ::com::sun::star::uno::Any > const& seq,
T0& v0, T1& v1, T2& v2, T3& v3, T4& v4,
::com::sun::star::uno::Reference<
......
......@@ -36,7 +36,7 @@ void appendTypeError(
buf.append( '>' );
}
inline void appendChar( OUStringBuffer & buf, sal_Unicode c )
void appendChar( OUStringBuffer & buf, sal_Unicode c )
{
if (c < ' ' || c > '~') {
buf.append( "\\X" );
......
......@@ -20,27 +20,24 @@ public:
explicit RedundantInline(loplugin::InstantiationData const & data):
FilteringRewritePlugin(data) {}
void run() override {
if (compiler.getLangOpts().CPlusPlus) {
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
}
}
void run() override { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
bool VisitFunctionDecl(FunctionDecl const * decl) {
if (ignoreLocation(decl) || !decl->isInlineSpecified()
|| !(decl->doesThisDeclarationHaveABody()
|| decl->isExplicitlyDefaulted())
|| !(decl->getLexicalDeclContext()->isRecord()
|| decl->isConstexpr()))
{
if (ignoreLocation(decl)) {
return true;
}
auto l1 = unwindToQObject(compat::getBeginLoc(decl));
if (l1.isValid() && l1 == unwindToQObject(compat::getEndLoc(decl))) {
if (!decl->isInlineSpecified()) {
return true;
}
SourceLocation inlineLoc;
unsigned n;
handleImplicitInline(decl) || handleNonExternalLinkage(decl);
return true;
}
private:
bool removeInline(FunctionDecl const * decl, SourceLocation * inlineLoc) {
assert(inlineLoc != nullptr);
assert(inlineLoc->isInvalid());
unsigned n = {}; // avoid -Werror=maybe-uninitialized
auto end = Lexer::getLocForEndOfToken(
compiler.getSourceManager().getExpansionLoc(compat::getEndLoc(decl)), 0,
compiler.getSourceManager(), compiler.getLangOpts());
......@@ -58,7 +55,7 @@ public:
}
if (s == "inline") {
if (!compiler.getSourceManager().isMacroArgExpansion(loc)) {
inlineLoc = loc;
*inlineLoc = loc;
}
break;
} else if (s == "#") {
......@@ -75,8 +72,8 @@ public:
break;
}
}
if (rewriter != nullptr && inlineLoc.isValid()) {
for (auto loc = inlineLoc.getLocWithOffset(
if (rewriter != nullptr && inlineLoc->isValid()) {
for (auto loc = inlineLoc->getLocWithOffset(
std::max<unsigned>(n, 1));;)
{
assert(loc != end);
......@@ -95,20 +92,14 @@ public:
n += n2;
loc = loc.getLocWithOffset(n2);
}
if (removeText(inlineLoc, n, RewriteOptions(RemoveLineIfEmpty))) {
if (removeText(*inlineLoc, n, RewriteOptions(RemoveLineIfEmpty))) {
return true;
}
}
report(
DiagnosticsEngine::Warning,
"function definition redundantly declared 'inline'",
inlineLoc.isValid() ? inlineLoc : compat::getBeginLoc(decl))
<< decl->getSourceRange();
return true;
return false;
}
private:
SourceLocation unwindToQObject(SourceLocation const & loc) {
SourceLocation unwindTo(SourceLocation const & loc, StringRef name) {
if (!loc.isMacroID()) {
return SourceLocation();
}
......@@ -116,8 +107,67 @@ private:
return
(Lexer::getImmediateMacroName(
loc, compiler.getSourceManager(), compiler.getLangOpts())
== "Q_OBJECT")
? l : unwindToQObject(l);
== name)
? l : unwindTo(l, name);
}
bool isInMacroExpansion(FunctionDecl const * decl, StringRef name) {
auto loc = unwindTo(compat::getBeginLoc(decl), name);
return loc.isValid() && loc == unwindTo(compat::getEndLoc(decl), name);
}
bool handleImplicitInline(FunctionDecl const * decl) {
if (!(decl->doesThisDeclarationHaveABody() || decl->isExplicitlyDefaulted())
|| !(decl->getLexicalDeclContext()->isRecord() || decl->isConstexpr()))
{
return false;
}
if (isInMacroExpansion(decl, "Q_OBJECT")) {
return true;
}
SourceLocation inlineLoc;
if (!removeInline(decl, &inlineLoc)) {
report(
DiagnosticsEngine::Warning,
"function definition redundantly declared 'inline'",
inlineLoc.isValid() ? inlineLoc : compat::getBeginLoc(decl))
<< decl->getSourceRange();
}
return true;
}
bool handleNonExternalLinkage(FunctionDecl const * decl) {
if (decl->getLinkageInternal() >=
#if CLANG_VERSION >= 40000
ModuleLinkage
#else
ExternalLinkage
#endif
)
{
return false;
}
if (!compiler.getSourceManager().isInMainFile(decl->getLocation())) {
// There *may* be benefit to "static inline" in include files (esp. in C code, where an
// inline function with external linkage still requires an external definition), so
// just ignore those for now:
return true;
}
if (isInMacroExpansion(decl, "G_DEFINE_TYPE")
|| isInMacroExpansion(decl, "G_DEFINE_TYPE_WITH_CODE")
|| isInMacroExpansion(decl, "G_DEFINE_TYPE_WITH_PRIVATE"))
{
return true;
}
SourceLocation inlineLoc;
if (!removeInline(decl, &inlineLoc)) {
report(
DiagnosticsEngine::Warning,
"function has no external linkage but is explicitly declared 'inline'",
inlineLoc.isValid() ? inlineLoc : compat::getBeginLoc(decl))
<< decl->getSourceRange();
}
return true;
}
};
......
......@@ -11,4 +11,14 @@
S1::~S1() = default;
static inline int f8() { return 0; } // expected-error {{function has no external linkage but is explicitly declared 'inline' [loplugin:redundantinline]}}
namespace {
static inline int f9() { return 0; } // expected-error {{function has no external linkage but is explicitly declared 'inline' [loplugin:redundantinline]}}
}
int main() { return f8() + f9(); }
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
......@@ -52,27 +52,29 @@ void f3() {}
S3::operator int() { return 0; }
struct S4 {
inline S4() {} // expected-error {{[loplugin:redundantinline]}}
inline ~S4() { f1(); } // expected-error {{[loplugin:redundantinline]}}
inline S4() {} // expected-error {{function definition redundantly declared 'inline' [loplugin:redundantinline]}}
inline ~S4() { f1(); } // expected-error {{function definition redundantly declared 'inline' [loplugin:redundantinline]}}
inline void f1() { (void)this; } // expected-error {{[loplugin:redundantinline]}}
inline void f1() { (void)this; } // expected-error {{function definition redundantly declared 'inline' [loplugin:redundantinline]}}
static inline void f2() {} // expected-error {{[loplugin:redundantinline]}}
static inline void f2() {} // expected-error {{function definition redundantly declared 'inline' [loplugin:redundantinline]}}
inline void operator +() {} // expected-error {{[loplugin:redundantinline]}}
inline void operator +() {} // expected-error {{function definition redundantly declared 'inline' [loplugin:redundantinline]}}
inline operator int() { return 0; } // expected-error {{[loplugin:redundantinline]}}
inline operator int() { return 0; } // expected-error {{function definition redundantly declared 'inline' [loplugin:redundantinline]}}
friend inline void f4() {} // expected-error {{[loplugin:redundantinline]}}
friend inline void f4() {} // expected-error {{function definition redundantly declared 'inline' [loplugin:redundantinline]}}
static constexpr int f5() { return 0; }
static constexpr inline int f6() { return 0; } // expected-error {{[loplugin:redundantinline]}}
static constexpr inline int f6() { return 0; } // expected-error {{function definition redundantly declared 'inline' [loplugin:redundantinline]}}
};
constexpr int f5() { return 0; }
constexpr inline int f6() { return 0; } // expected-error {{[loplugin:redundantinline]}}
constexpr inline int f6() { return 0; } // expected-error {{function definition redundantly declared 'inline' [loplugin:redundantinline]}}
static inline int f7() { return 0; }
#endif
......
......@@ -36,7 +36,7 @@
#include <rtl/process.h>
using namespace ::comphelper;
static inline sal_Unicode rtl_ascii_toUpperCase( sal_Unicode ch )
static sal_Unicode rtl_ascii_toUpperCase( sal_Unicode ch )
{
return ch >= 0x0061 && ch <= 0x007a ? ch + 0x20 : ch;
}
......
......@@ -1338,7 +1338,7 @@ sal_Int32 typeNameToDataType( const OUString &typeName, const OUString &typtype
}
namespace {
inline bool isSystemColumn( sal_Int16 attnum )
bool isSystemColumn( sal_Int16 attnum )
{
return attnum <= 0;
}
......
......@@ -95,7 +95,7 @@ OUString concatQualified( const OUString & a, const OUString &b)
return buf.makeStringAndClear();
}
static inline OString iOUStringToOString( const OUString& str, ConnectionSettings const *settings) {
static OString iOUStringToOString( const OUString& str, ConnectionSettings const *settings) {
OSL_ENSURE(settings, "pgsql-sdbc: OUStringToOString got NULL settings");
return OUStringToOString( str, ConnectionSettings::encoding );
}
......@@ -131,7 +131,7 @@ void bufferEscapeConstant( OUStringBuffer & buf, const OUString & value, Connect
buf.append( OStringToOUString( strbuf.makeStringAndClear(), RTL_TEXTENCODING_UTF8 ) );
}
static inline void ibufferQuoteConstant( OUStringBuffer & buf, const OUString & value, ConnectionSettings *settings )
static void ibufferQuoteConstant( OUStringBuffer & buf, const OUString & value, ConnectionSettings *settings )
{
buf.append( "'" );
bufferEscapeConstant( buf, value, settings );
......@@ -155,7 +155,7 @@ void bufferQuoteAnyConstant( OUStringBuffer & buf, const Any &val, ConnectionSet
buf.append( "NULL" );
}
static inline void ibufferQuoteIdentifier( OUStringBuffer & buf, const OUString &toQuote, ConnectionSettings *settings )
static void ibufferQuoteIdentifier( OUStringBuffer & buf, const OUString &toQuote, ConnectionSettings *settings )
{
OSL_ENSURE(settings, "pgsql-sdbc: bufferQuoteIdentifier got NULL settings");
......
......@@ -35,7 +35,7 @@ using namespace ::osl;
using namespace ::cppu;
static inline void createLocalId( sal_Sequence **ppThreadId )
static void createLocalId( sal_Sequence **ppThreadId )
{
rtl_byte_sequence_constructNoDefault( ppThreadId , 4 + 16 );
sal_uInt32 id = osl::Thread::getCurrentIdentifier();
......
......@@ -72,7 +72,7 @@ struct AlignSize_Impl
// the value of the maximal alignment
static sal_Int32 nMaxAlignment = static_cast<sal_Int32>( reinterpret_cast<sal_Size>(&reinterpret_cast<AlignSize_Impl *>(16)->dDouble) - 16);
static inline sal_Int32 adjustAlignment( sal_Int32 nRequestedAlignment )
static sal_Int32 adjustAlignment( sal_Int32 nRequestedAlignment )
{
if( nRequestedAlignment > nMaxAlignment )
nRequestedAlignment = nMaxAlignment;
......@@ -82,7 +82,7 @@ static inline sal_Int32 adjustAlignment( sal_Int32 nRequestedAlignment )
/**
* Calculate the new size of the struktur.
*/
static inline sal_Int32 newAlignedSize(
static sal_Int32 newAlignedSize(
sal_Int32 OldSize, sal_Int32 ElementSize, sal_Int32 NeededAlignment )
{