diff options
-rw-r--r-- | compilerplugins/clang/referencecasting.cxx | 98 | ||||
-rw-r--r-- | compilerplugins/clang/test/referencecasting.cxx | 2 | ||||
-rw-r--r-- | connectivity/source/drivers/file/FDriver.cxx | 2 | ||||
-rw-r--r-- | editeng/source/uno/unotext2.cxx | 2 | ||||
-rw-r--r-- | forms/source/helper/controlfeatureinterception.cxx | 2 | ||||
-rw-r--r-- | svx/source/fmcomp/fmgridif.cxx | 2 | ||||
-rw-r--r-- | ucb/source/cacher/dynamicresultsetwrapper.cxx | 4 | ||||
-rw-r--r-- | ucb/source/sorter/sortdynres.cxx | 2 | ||||
-rw-r--r-- | xmloff/source/draw/ximpstyl.cxx | 2 |
9 files changed, 108 insertions, 8 deletions
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@+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@+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; } |