strepsirrhini/office-gobmx
9
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

loplugin:passstuffbyref make some small improvements

Browse source 
Change-Id: Ib14a2e6b41165887fcf99c3d155850faa8564822
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/176218
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
This commit is contained in:
Noel Grandin 2024-11-05 11:19:51 +02:00
parent 0fe2b2545d
commit 5fdfc074c9
10 changed files with 77 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

79
compilerplugins/clang/passstuffbyref.cxx
View file

@ -189,12 +189,20 @@ void PassStuffByRef::checkParams(const FunctionDecl * functionDecl) {
return;
}
// these functions are passed as parameters to another function
if (loplugin::DeclCheck(functionDecl).MemberFunction()
auto dc = loplugin::DeclCheck(functionDecl);
if (dc.MemberFunction()
.Class("ShapeAttributeLayer").Namespace("internal")
.Namespace("slideshow").GlobalNamespace())
{
return;
}
// not sure why, but changing these causes problems
if (dc.Function("lcl_createColorMapFromShapeProps")
|| dc.Function("getAPIAnglesFrom3DProperties").Class("Scene3DHelper")
|| dc.Function("FillSeriesSimple").Class("ScTable")
|| dc.Function("FillAutoSimple").Class("ScTable"))
return;
unsigned n = functionDecl->getNumParams();
assert(!functionDecls_.empty());
functionDecls_.back().check = true;
@ -282,28 +290,51 @@ void PassStuffByRef::checkReturnValue(const FunctionDecl * functionDecl, const C
|| dc.Function("parseSingleValue").AnonymousNamespace().Namespace("configmgr").GlobalNamespace()
|| dc.Function("Create").Class("HandlerComponentBase").Namespace("pcr").GlobalNamespace()
|| dc.Function("toAny").Struct("Convert").Namespace("detail").Namespace("comphelper").GlobalNamespace()
|| dc.Function("makeAny").Namespace("utl").GlobalNamespace()) {
|| dc.Function("makeAny").Namespace("utl").GlobalNamespace()
|| dc.Operator(OO_Call).Struct("makeAny") // in chart2
|| dc.Function("toString").Struct("assertion_traits"))
return;
}
if (startswith(type.getAsString(), "struct o3tl::strong_int")) {
// these are sometimes dummy classes
if (dc.MemberFunction().Class("DdeService")
|| dc.MemberFunction().Class("DdeConnection")
|| dc.MemberFunction().Class("DdeTopic")
|| dc.MemberFunction().Class("CrashReporter"))
return;
}
// function has its address taken and is used as a function pointer value
if (dc.Function("xforms_bool"))
return;
if (startswith(type.getAsString(), "struct o3tl::strong_int"))
return;
// false positive
if (dc.Function("FindFieldSep").Namespace("mark").Namespace("sw"))
return;
// false positive
if (dc.Function("GetTime").Class("SwDateTimeField"))
return;
// false positive
if (dc.Function("Create").Class("StyleFamilyEntry"))
return;
if (dc.Function("getManualMax").Class("SparklineAttributes"))
return;
if (dc.Function("getManualMin").Class("SparklineAttributes"))
return;
if (dc.Function("ReadEmbeddedData").Class("XclImpHyperlink"))
return;
auto tc = loplugin::TypeCheck(functionDecl->getReturnType());
// these functions are passed by function-pointer
if (functionDecl->getIdentifier() && functionDecl->getName() == "GetRanges"
&& tc.Struct("WhichRangesContainer").GlobalNamespace())
return;
// extremely simple class, might as well pass by value
if (tc.Class("Color")) {
// extremely simple classes, might as well pass by value
if (tc.Class("Color")
|| tc.Struct("TranslateId")
|| tc.Class("span").StdNamespace()
|| tc.Class("strong_ordering").StdNamespace()
|| tc.Struct("TokenId"))
return;
}
// extremely simple class, might as well pass by value
if (tc.Struct("TranslateId")) {
return;
}
if (tc.Class("span").Namespace("o3tl")) {
return;
}
// functionDecl->dump();
@ -318,7 +349,7 @@ void PassStuffByRef::checkReturnValue(const FunctionDecl * functionDecl, const C
report( DiagnosticsEngine::Warning,
"rather return %0 by const& than by value, to avoid unnecessary copying",
functionDecl->getSourceRange().getBegin())
<< type.getAsString() << functionDecl->getSourceRange();
<< type.getAsString();
// display the location of the class member declaration so I don't have to search for it by hand
auto canonicalDecl = functionDecl->getCanonicalDecl();
@ -337,7 +368,12 @@ bool PassStuffByRef::VisitReturnStmt(const ReturnStmt * returnStmt)
{
if (!mbInsideFunctionDecl)
return true;
const Expr* expr = dyn_cast<Expr>(*returnStmt->child_begin())->IgnoreParenImpCasts();
if (returnStmt->child_begin() == returnStmt->child_end())
return true;
auto expr = dyn_cast<Expr>(*returnStmt->child_begin());
if (!expr)
return true;
expr = expr->IgnoreParenImpCasts();
if (isReturnExprDisqualified(expr))
mbFoundReturnValueDisqualifier = true;
@ -379,7 +415,13 @@ bool PassStuffByRef::isReturnExprDisqualified(const Expr* expr)
if (isa<MaterializeTemporaryExpr>(expr)) {
return true;
}
if (isa<CXXBindTemporaryExpr>(expr)) {
if (auto bindExpr = dyn_cast<CXXBindTemporaryExpr>(expr)) {
// treat "return OUString();" as fine, because that is easy to convert
// to use EMPTY_OUSTRING.
if (loplugin::TypeCheck(expr->getType()).Class("OUString"))
if (auto tempExpr = dyn_cast<CXXTemporaryObjectExpr>(bindExpr->getSubExpr()))
if (tempExpr->getNumArgs() == 0)
return false;
return true;
}
if (isa<InitListExpr>(expr)) {
@ -474,6 +516,7 @@ bool PassStuffByRef::VisitVarDecl(const VarDecl * varDecl)
dc.Class("SolarMutexGuard").GlobalNamespace() ||
dc.Class("SfxModelGuard").GlobalNamespace() ||
dc.Class("ReadWriteGuard").Namespace("utl").GlobalNamespace() ||
dc.Class("LifeTimeGuard").Namespace("apphelper").GlobalNamespace() || // in chart2
dc.Class("unique_lock").StdNamespace() ||
dc.Class("lock_guard").StdNamespace() ||
dc.Class("scoped_lock").StdNamespace())

2
compilerplugins/clang/test/passstuffbyref.cxx
View file

@ -45,6 +45,8 @@ struct S2 {
OUString get11() const { return mxCow->get(); } // expected-error {{rather return class rtl::OUString by const& than by value, to avoid unnecessary copying [loplugin:passstuffbyref]}}
OUString get12() { return child.get2(false); } // expected-error {{rather return class rtl::OUString by const& than by value, to avoid unnecessary copying [loplugin:passstuffbyref]}}
OUString get13() { return OUString(); } // expected-error {{rather return class rtl::OUString by const& than by value, to avoid unnecessary copying [loplugin:passstuffbyref]}}
// no warning expected
OUString set1() { return OUString("xxx"); }
OUString set2() { OUString v1("xxx"); return v1; }

2
configmgr/source/node.hxx
View file

@ -54,7 +54,7 @@ public:
int getFinalized() const { return finalized_;}
void setDescription(OUString const& description) { description_ = description; };
OUString getDescription() { return description_; }
const OUString & getDescription() { return description_; }
rtl::Reference< Node > getMember(OUString const & name);

4
extensions/source/abpilot/datasourcehandling.cxx
View file

@ -441,10 +441,10 @@ namespace abp
}
OUString ODataSource::getName() const
const OUString & ODataSource::getName() const
{
if ( !isValid() )
return OUString();
return EMPTY_OUSTRING;
return m_pImpl->sName;
}

2
extensions/source/abpilot/datasourcehandling.hxx
View file

@ -123,7 +123,7 @@ namespace abp
// TODO: put this into the context class
/// returns the name of the data source
OUString
const OUString &
getName() const;
/// renames the data source

2
include/drawinglayer/primitive2d/structuretagprimitive2d.hxx
View file

@ -74,7 +74,7 @@ namespace drawinglayer::primitive2d
bool isDecorative() const { return mbIsDecorative; }
bool isTaggedSdrObject() const;
void const* GetAnchorStructureElementKey() const { return m_pAnchorStructureElementKey; }
::std::vector<sal_Int32> GetAnnotIds() const { return m_AnnotIds; }
const ::std::vector<sal_Int32> & GetAnnotIds() const { return m_AnnotIds; }
/// compare operator
virtual bool operator==(const BasePrimitive2D& rPrimitive) const override;

2
include/test/a11y/accessibletestbase.hxx
View file

@ -212,7 +212,7 @@ protected:
void setAutoClose(bool bAutoClose) { mbAutoClose = bAutoClose; }
css::uno::Reference<css::accessibility::XAccessible> getAccessible() const
const css::uno::Reference<css::accessibility::XAccessible>& getAccessible() const
{
return mxAccessible;
}

12
sot/source/sdstor/ucbstorage.cxx
View file

@ -569,9 +569,9 @@ struct UCBStorageElement_Impl
::ucbhelper::Content* GetContent();
bool IsModified() const;
OUString GetContentType() const;
const OUString & GetContentType() const;
void SetContentType( const OUString& );
OUString GetOriginalContentType() const;
const OUString & GetOriginalContentType() const;
bool IsLoaded() const
{ return m_xStream.is() || m_xStorage.is(); }
};
@ -586,7 +586,7 @@ struct UCBStorageElement_Impl
return nullptr;
}
OUString UCBStorageElement_Impl::GetContentType() const
const OUString & UCBStorageElement_Impl::GetContentType() const
{
if ( m_xStream.is() )
return m_xStream->m_aContentType;
@ -595,7 +595,7 @@ OUString UCBStorageElement_Impl::GetContentType() const
else
{
OSL_FAIL("Element not loaded!");
return OUString();
return EMPTY_OUSTRING;
}
}
@ -612,14 +612,14 @@ void UCBStorageElement_Impl::SetContentType( const OUString& rType )
}
}
OUString UCBStorageElement_Impl::GetOriginalContentType() const
const OUString & UCBStorageElement_Impl::GetOriginalContentType() const
{
if ( m_xStream.is() )
return m_xStream->m_aOriginalContentType;
else if ( m_xStorage.is() )
return m_xStorage->m_aOriginalContentType;
else
return OUString();
return EMPTY_OUSTRING;
}
bool UCBStorageElement_Impl::IsModified() const

2
toolkit/source/controls/table/tablecontrol_impl.cxx
View file

@ -2368,7 +2368,7 @@ namespace svt::table
}
rtl::Reference<vcl::table::IAccessibleTableControl> TableControl_Impl::getAccessible( vcl::Window& i_parentWindow )
const rtl::Reference<vcl::table::IAccessibleTableControl> & TableControl_Impl::getAccessible( vcl::Window& i_parentWindow )
{
if (m_pAccessibleTable)
return m_pAccessibleTable;

2
toolkit/source/controls/table/tablecontrol_impl.hxx
View file

@ -280,7 +280,7 @@ namespace svt::table
tools::Rectangle calcCellRect( sal_Int32 nRow, sal_Int32 nCol ) const;
// A11Y
rtl::Reference<vcl::table::IAccessibleTableControl>
const rtl::Reference<vcl::table::IAccessibleTableControl> &
getAccessible( vcl::Window& i_parentWindow );
void disposeAccessible();