[Libreoffice-commits] core.git: compilerplugins/clang connectivity/source editeng/source forms/source svx/source ucb/source xmloff/source
Noel (via logerrit)
logerrit at kemper.freedesktop.org
Tue Feb 9 06:42:55 UTC 2021
compilerplugins/clang/referencecasting.cxx | 98 +++++++++++++++++++++
compilerplugins/clang/test/referencecasting.cxx | 2
connectivity/source/drivers/file/FDriver.cxx | 2
editeng/source/uno/unotext2.cxx | 2
forms/source/helper/controlfeatureinterception.cxx | 2
svx/source/fmcomp/fmgridif.cxx | 2
ucb/source/cacher/dynamicresultsetwrapper.cxx | 4
ucb/source/sorter/sortdynres.cxx | 2
xmloff/source/draw/ximpstyl.cxx | 2
9 files changed, 108 insertions(+), 8 deletions(-)
New commits:
commit 066799b4a162aa0a4bc6aa28339f1f943a13971e
Author: Noel <noel.grandin at collabora.co.uk>
AuthorDate: Mon Feb 8 15:50:13 2021 +0200
Commit: Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Tue Feb 9 07:42:11 2021 +0100
loplugin:referencecasting check for Reference::query
Change-Id: I008d16d933c70df132699872ac4c39a5c1f87b34
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/110592
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/compilerplugins/clang/referencecasting.cxx b/compilerplugins/clang/referencecasting.cxx
index f23e2f6811cd..aa11bc0738d7 100644
--- a/compilerplugins/clang/referencecasting.cxx
+++ b/compilerplugins/clang/referencecasting.cxx
@@ -60,6 +60,7 @@ public:
bool VisitCXXConstructExpr(const CXXConstructExpr* cce);
bool VisitCXXMemberCallExpr(const CXXMemberCallExpr* mce);
+ bool VisitCallExpr(const CallExpr*);
private:
bool CheckForUnnecessaryGet(const Expr*);
@@ -290,6 +291,103 @@ bool ReferenceCasting::VisitCXXMemberCallExpr(const CXXMemberCallExpr* mce)
return true;
}
+bool ReferenceCasting::VisitCallExpr(const CallExpr* ce)
+{
+ // don't bother processing anything in the Reference.h file. Makes my life easier when debugging this.
+ StringRef aFileName = getFilenameOfLocation(
+ compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(ce)));
+ if (loplugin::isSamePathname(aFileName, SRCDIR "/include/com/sun/star/uno/Reference.h"))
+ return true;
+ if (loplugin::isSamePathname(aFileName, SRCDIR "/include/com/sun/star/uno/Reference.hxx"))
+ return true;
+
+ // look for calls to Reference<T>::query(x)
+ auto method = dyn_cast_or_null<CXXMethodDecl>(ce->getDirectCallee());
+ if (!method || !method->getIdentifier() || method->getName() != "query")
+ return true;
+ if (ce->getNumArgs() != 1)
+ return true;
+
+ auto methodRecordDecl = dyn_cast<ClassTemplateSpecializationDecl>(method->getParent());
+ if (!methodRecordDecl || !methodRecordDecl->getIdentifier()
+ || methodRecordDecl->getName() != "Reference")
+ return true;
+
+ if (CheckForUnnecessaryGet(ce->getArg(0)))
+ report(DiagnosticsEngine::Warning, "unnecessary get() call", compat::getBeginLoc(ce))
+ << ce->getSourceRange();
+
+ // extract the type parameter passed to the template
+ const RecordType* templateParamType
+ = dyn_cast<RecordType>(methodRecordDecl->getTemplateArgs()[0].getAsType());
+ if (!templateParamType)
+ return true;
+
+ // extract the type of the first parameter passed to the method
+ const Expr* arg0 = ce->getArg(0);
+ if (!arg0)
+ return true;
+
+ // drill down the expression tree till we hit the bottom, because at the top, the type is BaseReference
+ const clang::Type* argType;
+ for (;;)
+ {
+ if (auto castExpr = dyn_cast<CastExpr>(arg0))
+ {
+ arg0 = castExpr->getSubExpr();
+ continue;
+ }
+ if (auto matTempExpr = dyn_cast<MaterializeTemporaryExpr>(arg0))
+ {
+ arg0 = compat::getSubExpr(matTempExpr);
+ continue;
+ }
+ if (auto bindTempExpr = dyn_cast<CXXBindTemporaryExpr>(arg0))
+ {
+ arg0 = bindTempExpr->getSubExpr();
+ continue;
+ }
+ if (auto tempObjExpr = dyn_cast<CXXTemporaryObjectExpr>(arg0))
+ {
+ arg0 = tempObjExpr->getArg(0);
+ continue;
+ }
+ if (auto parenExpr = dyn_cast<ParenExpr>(arg0))
+ {
+ arg0 = parenExpr->getSubExpr();
+ continue;
+ }
+ argType = arg0->getType().getTypePtr();
+ break;
+ }
+
+ const RecordType* argTemplateType = extractTemplateType(argType);
+ if (!argTemplateType)
+ return true;
+
+ CXXRecordDecl* templateParamRD = dyn_cast<CXXRecordDecl>(templateParamType->getDecl());
+ CXXRecordDecl* methodArgRD = dyn_cast<CXXRecordDecl>(argTemplateType->getDecl());
+
+ // querying for XInterface (instead of doing an upcast) has special semantics,
+ // to check for UNO object equivalence.
+ if (templateParamRD->getName() == "XInterface")
+ return true;
+
+ // XShape is used in UNO aggregates in very "entertaining" ways, which means an UNO_QUERY
+ // can return a completely different object, e.g. see SwXShape::queryInterface
+ if (templateParamRD->getName() == "XShape")
+ return true;
+
+ if (methodArgRD->Equals(templateParamRD) || isDerivedFrom(methodArgRD, templateParamRD))
+ {
+ report(DiagnosticsEngine::Warning,
+ "the source reference is already a subtype of the destination reference, just use =",
+ compat::getBeginLoc(ce))
+ << ce->getSourceRange();
+ }
+ return true;
+}
+
/**
Check for
Reference<T>(x.get(), UNO_QUERY)
diff --git a/compilerplugins/clang/test/referencecasting.cxx b/compilerplugins/clang/test/referencecasting.cxx
index 0272bc89cc98..0864aec0f697 100644
--- a/compilerplugins/clang/test/referencecasting.cxx
+++ b/compilerplugins/clang/test/referencecasting.cxx
@@ -19,6 +19,8 @@ void test1(const css::uno::Reference<css::io::XStreamListener>& a)
{
// expected-error at +1 {{the source reference is already a subtype of the destination reference, just use = [loplugin:referencecasting]}}
css::uno::Reference<css::lang::XEventListener> b(a, css::uno::UNO_QUERY);
+ // expected-error at +1 {{the source reference is already a subtype of the destination reference, just use = [loplugin:referencecasting]}}
+ auto c = css::uno::Reference<css::lang::XEventListener>::query(a);
}
namespace test2
diff --git a/connectivity/source/drivers/file/FDriver.cxx b/connectivity/source/drivers/file/FDriver.cxx
index 16cf60e02cb4..54be5f952429 100644
--- a/connectivity/source/drivers/file/FDriver.cxx
+++ b/connectivity/source/drivers/file/FDriver.cxx
@@ -181,7 +181,7 @@ Reference< XTablesSupplier > SAL_CALL OFileDriver::getDataDefinitionByConnection
OConnection* pConnection = nullptr;
for (auto const& elem : m_xConnections)
{
- if (static_cast<OConnection*>( Reference< XConnection >::query(elem.get().get()).get() ) == pSearchConnection)
+ if (static_cast<OConnection*>( Reference< XConnection >::query(elem.get()).get() ) == pSearchConnection)
{
pConnection = pSearchConnection;
break;
diff --git a/editeng/source/uno/unotext2.cxx b/editeng/source/uno/unotext2.cxx
index 8aa040c97637..1e61337f5471 100644
--- a/editeng/source/uno/unotext2.cxx
+++ b/editeng/source/uno/unotext2.cxx
@@ -234,7 +234,7 @@ void SAL_CALL SvxUnoTextContent::attach( const uno::Reference< text::XTextRange
uno::Reference< text::XTextRange > SAL_CALL SvxUnoTextContent::getAnchor()
{
- return uno::Reference< text::XTextRange >::query( mxParentText );
+ return mxParentText;
}
// XComponent
diff --git a/forms/source/helper/controlfeatureinterception.cxx b/forms/source/helper/controlfeatureinterception.cxx
index 1e0d0fbcd03d..091af550cc29 100644
--- a/forms/source/helper/controlfeatureinterception.cxx
+++ b/forms/source/helper/controlfeatureinterception.cxx
@@ -93,7 +93,7 @@ namespace frm
// reconnect the chain
if ( xMaster.is() )
{
- xMaster->setSlaveDispatchProvider( Reference< XDispatchProvider >::query( xSlave ) );
+ xMaster->setSlaveDispatchProvider( xSlave );
}
// if somebody has registered the same interceptor twice, then we will remove
diff --git a/svx/source/fmcomp/fmgridif.cxx b/svx/source/fmcomp/fmgridif.cxx
index fc8d52735ea6..96f6fc4c2526 100644
--- a/svx/source/fmcomp/fmgridif.cxx
+++ b/svx/source/fmcomp/fmgridif.cxx
@@ -2482,7 +2482,7 @@ void FmXGridPeer::releaseDispatchProviderInterceptor(const Reference< css::frame
if (xMaster.is())
{
if (xSlave.is())
- xMaster->setSlaveDispatchProvider(Reference< css::frame::XDispatchProvider >::query(xSlave));
+ xMaster->setSlaveDispatchProvider(xSlave);
else
// it's the first interceptor of the chain, set ourself as slave
xMaster->setSlaveDispatchProvider(static_cast<css::frame::XDispatchProvider*>(this));
diff --git a/ucb/source/cacher/dynamicresultsetwrapper.cxx b/ucb/source/cacher/dynamicresultsetwrapper.cxx
index 28dc828944a2..3aa765da316a 100644
--- a/ucb/source/cacher/dynamicresultsetwrapper.cxx
+++ b/ucb/source/cacher/dynamicresultsetwrapper.cxx
@@ -301,7 +301,7 @@ void SAL_CALL DynamicResultSetWrapper::setSource( const Reference< XInterface >
else if( bStatic )
{
Reference< XComponent > xSourceComponent( Source, UNO_QUERY );
- xSourceComponent->addEventListener( Reference< XEventListener > ::query( xMyListenerImpl ) );
+ xSourceComponent->addEventListener( xMyListenerImpl );
}
m_aSourceSet.set();
}
@@ -354,7 +354,7 @@ void SAL_CALL DynamicResultSetWrapper::setListener( const Reference< XDynamicRes
throw ListenerAlreadySetException();
m_xListener = Listener;
- addEventListener( Reference< XEventListener >::query( Listener ) );
+ addEventListener( Listener );
xSource = m_xSource;
xMyListenerImpl = m_xMyListenerImpl.get();
diff --git a/ucb/source/sorter/sortdynres.cxx b/ucb/source/sorter/sortdynres.cxx
index dc7ad5ea64fc..d2455ca11ec0 100644
--- a/ucb/source/sorter/sortdynres.cxx
+++ b/ucb/source/sorter/sortdynres.cxx
@@ -169,7 +169,7 @@ SortedDynamicResultSet::setListener( const Reference< XDynamicResultSetListener
if ( mxListener.is() )
throw ListenerAlreadySetException();
- addEventListener( Reference< XEventListener >::query( Listener ) );
+ addEventListener( Listener );
mxListener = Listener;
diff --git a/xmloff/source/draw/ximpstyl.cxx b/xmloff/source/draw/ximpstyl.cxx
index 05ecaf377eec..00d47c5c6f3a 100644
--- a/xmloff/source/draw/ximpstyl.cxx
+++ b/xmloff/source/draw/ximpstyl.cxx
@@ -1288,7 +1288,7 @@ uno::Reference< container::XNameAccess > SdXMLStylesContext::getPageLayouts() co
}
}
- return uno::Reference< container::XNameAccess >::query( xLayouts );
+ return xLayouts;
}
More information about the Libreoffice-commits
mailing list