mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

improve loplugin:unnecessarylocking

Browse Source 
to find more locking we can remove

Change-Id: Ief7bc5ec2a1ff31f22a0ad366910b7fcc4725818
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/148599
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
This commit is contained in:
Noel Grandin 2023-03-09 14:28:47 +02:00
parent 8ceb85cafb
commit 50b68c341f
20 changed files with 202 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
accessibility/source/extended/accessibleiconchoicectrlentry.cxx
View File

@ -626,8 +626,6 @@ namespace accessibility
sal_Int32 SAL_CALL AccessibleIconChoiceCtrlEntry::getAccessibleActionCount( ) sal_Int32 SAL_CALL AccessibleIconChoiceCtrlEntry::getAccessibleActionCount( )
{ {
::osl::MutexGuard aGuard( m_aMutex );
// three actions supported // three actions supported
return ACCESSIBLE_ACTION_COUNT; return ACCESSIBLE_ACTION_COUNT;
} }
@ -665,8 +663,6 @@ namespace accessibility
Reference< XAccessibleKeyBinding > AccessibleIconChoiceCtrlEntry::getAccessibleActionKeyBinding( sal_Int32 nIndex ) Reference< XAccessibleKeyBinding > AccessibleIconChoiceCtrlEntry::getAccessibleActionKeyBinding( sal_Int32 nIndex )
{ {
::osl::MutexGuard aGuard( m_aMutex );
Reference< XAccessibleKeyBinding > xRet; Reference< XAccessibleKeyBinding > xRet;
checkActionIndex_Impl( nIndex ); checkActionIndex_Impl( nIndex );
// ... which key? // ... which key?

2
accessibility/source/extended/accessiblelistboxentry.cxx
View File

@ -808,8 +808,6 @@ namespace accessibility
Reference< XAccessibleKeyBinding > AccessibleListBoxEntry::getAccessibleActionKeyBinding( sal_Int32 nIndex ) Reference< XAccessibleKeyBinding > AccessibleListBoxEntry::getAccessibleActionKeyBinding( sal_Int32 nIndex )
{ {
::osl::MutexGuard aGuard( m_aMutex );
Reference< XAccessibleKeyBinding > xRet; Reference< XAccessibleKeyBinding > xRet;
checkActionIndex_Impl( nIndex ); checkActionIndex_Impl( nIndex );
// ... which key? // ... which key?

2
accessibility/source/standard/vclxaccessibletextfield.cxx
View File

@ -74,8 +74,6 @@ Reference<XAccessible> SAL_CALL VCLXAccessibleTextField::getAccessibleChild (sal
sal_Int16 SAL_CALL VCLXAccessibleTextField::getAccessibleRole() sal_Int16 SAL_CALL VCLXAccessibleTextField::getAccessibleRole()
{ {
::osl::Guard< ::osl::Mutex > aGuard( GetMutex() );
return AccessibleRole::TEXT; return AccessibleRole::TEXT;
} }

53
compilerplugins/clang/test/unnecessarylocking.cxx
View File

@ -8,14 +8,17 @@
*/ */
#include <mutex> #include <mutex>
#include <osl/mutex.hxx>
static std::mutex gSolarMutex; static std::mutex gSolarMutex;
class SolarMutexGuard : public std::unique_lock<std::mutex> class SolarMutexGuard
{ {
std::unique_lock<std::mutex> lock;
public: public:
SolarMutexGuard() SolarMutexGuard()
: std::unique_lock<std::mutex>(gSolarMutex) : lock(gSolarMutex)
{ {
} }
}; };
@ -25,8 +28,8 @@ namespace test1
struct Foo struct Foo
{ {
int m_foo; int m_foo;
// expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
int bar1() int bar1()
// expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
{ {
SolarMutexGuard guard; SolarMutexGuard guard;
return 1; return 1;
@ -42,19 +45,22 @@ struct Foo
namespace test2 namespace test2
{ {
int free_function() { return 1; }
struct Foo struct Foo
{ {
std::mutex m_aMutex; std::mutex m_aMutex;
osl::Mutex m_aOslMutex;
int m_foo; int m_foo;
// expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
int bar1() int bar1()
// expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
{ {
std::unique_lock guard(m_aMutex); std::unique_lock guard(m_aMutex);
return 1; return 1;
} }
// expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
int bar2() int bar2()
// expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
{ {
std::scoped_lock guard(m_aMutex); std::scoped_lock guard(m_aMutex);
return 1; return 1;
@ -65,7 +71,44 @@ struct Foo
std::scoped_lock guard(m_aMutex); std::scoped_lock guard(m_aMutex);
return m_foo; return m_foo;
} }
int bar4()
// expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
{
::osl::Guard<::osl::Mutex> aGuard(m_aOslMutex);
return 1;
}
int bar5()
{
// expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
{
std::unique_lock guard(m_aMutex);
return free_function();
}
}
osl::Mutex& getOslMutex() { return m_aOslMutex; }
int bar6()
// expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
{
::osl::Guard<::osl::Mutex> aGuard(getOslMutex());
return 1;
}
}; };
} }
// Calling anything on VCLUnoHelper means we need the SolarMutex
class VCLUnoHelper
{
public:
static int CreateToolkit();
};
namespace test4
{
// no warning expected
void bar1()
{
SolarMutexGuard guard;
VCLUnoHelper::CreateToolkit();
}
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

197
compilerplugins/clang/unnecessarylocking.cxx
View File

@ -54,34 +54,28 @@ public:
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
} }
bool TraverseCXXMethodDecl(CXXMethodDecl*); bool VisitCompoundStmt(const CompoundStmt*);
bool VisitCXXThisExpr(const CXXThisExpr*); bool VisitCXXThisExpr(const CXXThisExpr*);
bool VisitCallExpr(const CallExpr*);
private: private:
bool isSolarMutexLockGuardStmt(const Stmt*); bool isSolarMutexLockGuardStmt(const Stmt*);
const Stmt* isOtherMutexLockGuardStmt(const Stmt*); const CXXThisExpr* isOtherMutexLockGuardStmt(const Stmt*);
std::vector<bool> m_TouchesThis; std::vector<bool> m_TouchesThis;
// so we ignore the CxxThisEpxr that references the maMutex in the guard expression // so we ignore the CxxThisEpxr that references the maMutex in the guard expression
std::vector<const Stmt*> m_IgnoreThis; std::vector<const CXXThisExpr*> m_IgnoreThis;
}; };
bool UnnecessaryLocking::TraverseCXXMethodDecl(CXXMethodDecl* cxxMethodDecl) bool UnnecessaryLocking::VisitCompoundStmt(const CompoundStmt* compoundStmt)
{ {
if (ignoreLocation(cxxMethodDecl)) if (ignoreLocation(compoundStmt))
return true; return true;
if (compoundStmt->size() < 1)
if (!cxxMethodDecl->isInstance())
return true;
if (!cxxMethodDecl->isThisDeclarationADefinition())
return true;
auto compoundStmt = dyn_cast_or_null<CompoundStmt>(cxxMethodDecl->getBody());
if (!compoundStmt || compoundStmt->size() < 1)
return true; return true;
const Stmt* firstStmt = *compoundStmt->body_begin(); const Stmt* firstStmt = *compoundStmt->body_begin();
bool solarMutex = isSolarMutexLockGuardStmt(firstStmt); bool solarMutex = isSolarMutexLockGuardStmt(firstStmt);
const Stmt* ignoreThisStmt = nullptr; const CXXThisExpr* ignoreThisStmt = nullptr;
if (!solarMutex) if (!solarMutex)
ignoreThisStmt = isOtherMutexLockGuardStmt(firstStmt); ignoreThisStmt = isOtherMutexLockGuardStmt(firstStmt);
if (!solarMutex && ignoreThisStmt == nullptr) if (!solarMutex && ignoreThisStmt == nullptr)
@ -90,48 +84,133 @@ bool UnnecessaryLocking::TraverseCXXMethodDecl(CXXMethodDecl* cxxMethodDecl)
m_TouchesThis.push_back(false); m_TouchesThis.push_back(false);
m_IgnoreThis.push_back(ignoreThisStmt); m_IgnoreThis.push_back(ignoreThisStmt);
bool rv = FilteringPlugin::TraverseCXXMethodDecl(cxxMethodDecl); for (const Stmt* stmt : compoundStmt->body())
FilteringPlugin::TraverseStmt(const_cast<Stmt*>(stmt));
if (!m_TouchesThis.back()) if (!m_TouchesThis.back())
{ {
StringRef fn = getFilenameOfLocation( StringRef fn = getFilenameOfLocation(
compiler.getSourceManager().getSpellingLoc(cxxMethodDecl->getBeginLoc())); compiler.getSourceManager().getSpellingLoc(compoundStmt->getBeginLoc()));
if ( if (
// template magic // template magic
!loplugin::isSamePathname(fn, SRCDIR "/include/comphelper/unique_disposing_ptr.hxx") !loplugin::isSamePathname(fn, SRCDIR "/include/comphelper/unique_disposing_ptr.hxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/inc/unobaseclass.hxx") && !loplugin::isSamePathname(fn, SRCDIR "/sw/inc/unobaseclass.hxx")
// toolkit needs to lock around access to static methods in vcl
// false+
&& !loplugin::isSamePathname(fn, SRCDIR "/cppuhelper/source/component_context.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/toolkit/source/controls/tree/treecontrol.cxx")
&& !loplugin::isSamePathname(fn,
SRCDIR "/toolkit/source/helper/listenermultiplexer.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/include/toolkit/helper/macros.hxx")
&& !loplugin::isSamePathname(fn, SRCDIR
"/chart2/source/controller/main/CommandDispatch.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/chart2/source/controller/main/ChartView.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR
"/chart2/source/controller/main/SelectionHelper.cxx")
&& !loplugin::isSamePathname(
fn, SRCDIR "/chart2/source/controller/accessibility/AccessibleChartView.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/desktop/source/offacc/acceptor.cxx")
&& !loplugin::isSamePathname(
fn, SRCDIR "/desktop/source/deployment/registry/component/dp_component.cxx")
&& !loplugin::isSamePathname(fn,
SRCDIR "/desktop/source/deployment/gui/dp_gui_dialog2.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/desktop/source/lib/init.cxx")
&& !loplugin::isSamePathname(
fn, SRCDIR "/sd/source/ui/slidesorter/cache/SlsCacheConfiguration.cxx")
// needs to lock around access to methods in vcl
&& !loplugin::isSamePathname(fn, SRCDIR "/basctl/source/basicide/unomodel.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/cui/source/dialogs/AdditionsDialog.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/cui/source/dialogs/cuigaldlg.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/dbaccess/source/ui/browser/unodatbr.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/dbaccess/source/ui/uno/dbinteraction.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/dbaccess/source/ui/dlg/DbAdminImpl.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/dbaccess/source/ui/misc/UITools.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/desktop/source/lib/lokclipboard.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/editeng/source/misc/unolingu.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR
"/framework/source/uielement/popuptoolbarcontroller.cxx")
&& !loplugin::isSamePathname(fn,
SRCDIR "/framework/source/uielement/newmenucontroller.cxx")
&& !loplugin::isSamePathname(fn,
SRCDIR "/framework/source/uielement/menubarwrapper.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/framework/source/services/desktop.cxx")
&& !loplugin::isSamePathname(fn,
SRCDIR "/framework/source/layoutmanager/layoutmanager.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR
"/framework/source/layoutmanager/toolbarlayoutmanager.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR
"/framework/source/fwe/helper/actiontriggerhelper.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/unoobj/unodoc.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/unoobj/filtuno.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/unoobj/funcuno.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/vba/vbaapplication.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sd/source/ui/unoidl/unodoc.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sd/source/ui/unoidl/unomodule.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sd/source/ui/remotecontrol/Receiver.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR
"/sd/source/ui/slidesorter/controller/SlsClipboard.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sfx2/source/appl/fwkhelper.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sfx2/source/appl/appinit.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sfx2/source/appl/shutdownicon.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sfx2/source/dialog/dockwin.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sfx2/source/statbar/stbitem.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sfx2/source/toolbox/tbxitem.cxx")
&& !loplugin::isSamePathname(fn,
SRCDIR "/svtools/source/java/javainteractionhandler.cxx")
&& !loplugin::isSamePathname(fn,
SRCDIR "/svx/source/accessibility/ShapeTypeHandler.cxx")
&& !loplugin::isSamePathname(fn,
SRCDIR "/svx/source/tbxctrls/tbunosearchcontrollers.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/svx/source/form/fmscriptingenv.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/svx/source/fmcomp/fmgridif.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/toolkit/source/awt/vclxspinbutton.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/toolkit/source/awt/vclxtoolkit.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/toolkit/source/awt/vclxtoolkit.cxx")
&& !loplugin::isSamePathname(fn, && !loplugin::isSamePathname(fn,
SRCDIR "/toolkit/source/controls/tree/treecontrolpeer.cxx") SRCDIR "/toolkit/source/controls/tree/treecontrolpeer.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/toolkit/source/awt/vclxcontainer.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/ucb/source/ucp/image/ucpimage.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/toolkit/source/awt/vclxdevice.cxx") && !loplugin::hasPathnamePrefix(fn, SRCDIR "/vcl/")
// not sure
&& !loplugin::isSamePathname(fn,
SRCDIR "/dbaccess/source/ui/browser/AsynchronousLink.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/framework/source/services/autorecovery.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sfx2/source/dialog/filedlghelper.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sfx2/source/appl/appdispatchprovider.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/ucb/source/ucp/tdoc/tdoc_storage.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sc/source/core/data/poolhelp.cxx")
// touching shared global data // touching shared global data
&& !loplugin::isSamePathname(fn, SRCDIR && !loplugin::isSamePathname(fn, SRCDIR
"/framework/source/fwi/classes/protocolhandlercache.cxx") "/framework/source/fwi/classes/protocolhandlercache.cxx")
// lock around access to static methods in vcl && !loplugin::isSamePathname(fn,
&& !loplugin::isSamePathname(fn, SRCDIR "/framework/source/services/taskcreatorsrv.cxx") SRCDIR "/basic/source/basmgr/basicmanagerrepository.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/svx/source/dialog/SafeModeUI.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/basic/source/classes/sb.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR && !loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/unoobj/docuno.cxx")
"/svx/source/accessibility/AccessibleFrameSelector.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/unoobj/afmtuno.cxx")
// not sure && !loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/unoobj/appluno.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sfx2/source/dialog/filedlghelper.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/vba/vbaapplication.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sfx2/source/appl/appdispatchprovider.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/access/accdoc.cxx")
// touching shared global data && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/access/acccontext.cxx")
&& !loplugin::isSamePathname(fn,
SRCDIR "/sw/source/core/bastyp/proofreadingiterator.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unoftn.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unoftn.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unolinebreak.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unolinebreak.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unoobj.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unoobj.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unorefmk.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unorefmk.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unotbl.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unotbl.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/filter/xml/xmltexti.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/uibase/uno/dlelstnr.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/access/accdoc.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/access/acccontext.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unocontentcontrol.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unocontentcontrol.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unobkm.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unobkm.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/unoobj/docuno.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unocoll.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/unoobj/afmtuno.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/core/unocore/unostyle.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/unoobj/appluno.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/sw/source/filter/xml/xmltexti.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/uibase/uno/dlelstnr.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/uibase/uno/unoatxt.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/uibase/uno/unodoc.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/uibase/uno/unomodule.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/uibase/uno/SwXFilterOptions.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/uibase/shells/translatehelper.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/source/ui/vba/vbaapplication.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/unoxml/source/dom/documentbuilder.cxx") && !loplugin::isSamePathname(fn, SRCDIR "/unoxml/source/dom/documentbuilder.cxx")
&& !loplugin::isSamePathname( && !loplugin::isSamePathname(
fn, SRCDIR "/sd/source/ui/accessibility/AccessibleDrawDocumentView.cxx") fn, SRCDIR "/sd/source/ui/accessibility/AccessibleDrawDocumentView.cxx")
@ -139,15 +218,15 @@ bool UnnecessaryLocking::TraverseCXXMethodDecl(CXXMethodDecl* cxxMethodDecl)
&& !loplugin::isSamePathname(fn, && !loplugin::isSamePathname(fn,
SRCDIR "/starmath/source/AccessibleSmElementsControl.cxx")) SRCDIR "/starmath/source/AccessibleSmElementsControl.cxx"))
{ {
report(DiagnosticsEngine::Warning, "unnecessary locking", cxxMethodDecl->getBeginLoc()) report(DiagnosticsEngine::Warning, "unnecessary locking", compoundStmt->getBeginLoc())
<< cxxMethodDecl->getSourceRange(); << compoundStmt->getSourceRange();
} }
} }
m_TouchesThis.pop_back(); m_TouchesThis.pop_back();
m_IgnoreThis.pop_back(); m_IgnoreThis.pop_back();
return rv; return true;
} }
bool UnnecessaryLocking::isSolarMutexLockGuardStmt(const Stmt* stmt) bool UnnecessaryLocking::isSolarMutexLockGuardStmt(const Stmt* stmt)
@ -169,7 +248,7 @@ bool UnnecessaryLocking::isSolarMutexLockGuardStmt(const Stmt* stmt)
return true; return true;
} }
const Stmt* UnnecessaryLocking::isOtherMutexLockGuardStmt(const Stmt* stmt) const CXXThisExpr* UnnecessaryLocking::isOtherMutexLockGuardStmt(const Stmt* stmt)
{ {
auto declStmt = dyn_cast<DeclStmt>(stmt); auto declStmt = dyn_cast<DeclStmt>(stmt);
if (!declStmt) if (!declStmt)
@ -180,16 +259,25 @@ const Stmt* UnnecessaryLocking::isOtherMutexLockGuardStmt(const Stmt* stmt)
if (!varDecl) if (!varDecl)
return nullptr; return nullptr;
auto tc = loplugin::TypeCheck(varDecl->getType()); auto tc = loplugin::TypeCheck(varDecl->getType());
if (!tc.Class("unique_lock").StdNamespace() && !tc.Class("scoped_lock").StdNamespace()) if (!tc.Class("unique_lock").StdNamespace() && !tc.Class("scoped_lock").StdNamespace()
&& !tc.Class("Guard") && !tc.Class("ClearableGuard") && !tc.Class("ResettableGuard"))
return nullptr; return nullptr;
auto cxxConstructExpr = dyn_cast<CXXConstructExpr>(varDecl->getInit()); auto cxxConstructExpr = dyn_cast<CXXConstructExpr>(varDecl->getInit());
if (!cxxConstructExpr || cxxConstructExpr->getNumArgs() < 1) if (!cxxConstructExpr || cxxConstructExpr->getNumArgs() < 1)
return nullptr; return nullptr;
auto memberExpr = dyn_cast<MemberExpr>(cxxConstructExpr->getArg(0)); auto arg0 = cxxConstructExpr->getArg(0);
if (!memberExpr) if (auto memberExpr = dyn_cast<MemberExpr>(arg0))
return nullptr; {
auto thisStmt = memberExpr->getBase(); const CXXThisExpr* thisStmt
return thisStmt; = dyn_cast<CXXThisExpr>(memberExpr->getBase()->IgnoreImplicit());
return thisStmt;
}
else if (auto memberCallExpr = dyn_cast<CXXMemberCallExpr>(arg0))
{
return dyn_cast_or_null<CXXThisExpr>(
memberCallExpr->getImplicitObjectArgument()->IgnoreImplicit());
}
return nullptr;
} }
bool UnnecessaryLocking::VisitCXXThisExpr(const CXXThisExpr* cxxThisExpr) bool UnnecessaryLocking::VisitCXXThisExpr(const CXXThisExpr* cxxThisExpr)
@ -208,6 +296,25 @@ bool UnnecessaryLocking::VisitCXXThisExpr(const CXXThisExpr* cxxThisExpr)
return true; return true;
} }
bool UnnecessaryLocking::VisitCallExpr(const CallExpr* callExpr)
{
if (ignoreLocation(callExpr))
return true;
// just in case
if (m_TouchesThis.empty())
return true;
// already found something
if (m_TouchesThis.back())
return true;
const CXXMethodDecl* callee = dyn_cast_or_null<CXXMethodDecl>(callExpr->getDirectCallee());
if (!callee)
return true;
auto dc = loplugin::DeclCheck(callee->getParent());
if (dc.Class("VCLUnoHelper") || dc.Class("Application"))
m_TouchesThis.back() = true;
return true;
}
/** off by default because each warning needs to be carefully inspected */ /** off by default because each warning needs to be carefully inspected */
loplugin::Plugin::Registration<UnnecessaryLocking> unnecessarylocking("unnecessarylocking", false); loplugin::Plugin::Registration<UnnecessaryLocking> unnecessarylocking("unnecessarylocking", false);
} }

1
connectivity/source/drivers/firebird/Connection.cxx
View File

@ -440,7 +440,6 @@ Reference< XPreparedStatement > SAL_CALL Connection::prepareCall(
OUString SAL_CALL Connection::nativeSQL( const OUString& _sSql ) OUString SAL_CALL Connection::nativeSQL( const OUString& _sSql )
{ {
MutexGuard aGuard( m_aMutex );
// We do not need to adapt the SQL for Firebird atm. // We do not need to adapt the SQL for Firebird atm.
return _sSql; return _sSql;
} }

2
connectivity/source/drivers/flat/EDatabaseMetaData.cxx
View File

@ -47,8 +47,6 @@ OFlatDatabaseMetaData::~OFlatDatabaseMetaData()
Reference< XResultSet > OFlatDatabaseMetaData::impl_getTypeInfo_throw( ) Reference< XResultSet > OFlatDatabaseMetaData::impl_getTypeInfo_throw( )
{ {
::osl::MutexGuard aGuard( m_aMutex );
rtl::Reference<::connectivity::ODatabaseMetaDataResultSet> pResult = new ::connectivity::ODatabaseMetaDataResultSet(::connectivity::ODatabaseMetaDataResultSet::eTypeInfo); rtl::Reference<::connectivity::ODatabaseMetaDataResultSet> pResult = new ::connectivity::ODatabaseMetaDataResultSet(::connectivity::ODatabaseMetaDataResultSet::eTypeInfo);
static ODatabaseMetaDataResultSet::ORows aRows = []() static ODatabaseMetaDataResultSet::ORows aRows = []()

2
connectivity/source/drivers/mysqlc/mysqlc_connection.cxx
View File

@ -257,8 +257,6 @@ Reference<XPreparedStatement> SAL_CALL OConnection::prepareCall(const OUString&
OUString SAL_CALL OConnection::nativeSQL(const OUString& /*_sSql*/) OUString SAL_CALL OConnection::nativeSQL(const OUString& /*_sSql*/)
{ {
MutexGuard aGuard(m_aMutex);
// const OUString sSqlStatement = transFormPreparedStatement( _sSql ); // const OUString sSqlStatement = transFormPreparedStatement( _sSql );
// TODO // TODO
return OUString(); return OUString();

1
extensions/source/propctrlr/formcomponenthandler.cxx
View File

@ -881,7 +881,6 @@ namespace pcr
Sequence< OUString > SAL_CALL FormComponentPropertyHandler::getActuatingProperties( ) Sequence< OUString > SAL_CALL FormComponentPropertyHandler::getActuatingProperties( )
{ {
::osl::MutexGuard aGuard( m_aMutex );
return return
{ {
PROPERTY_DATASOURCE, PROPERTY_DATASOURCE,

1
fpicker/source/office/contentenumeration.cxx
View File

@ -251,7 +251,6 @@ namespace svt
if ( pData->mbIsFolder ) if ( pData->mbIsFolder )
{ {
SolarMutexGuard aGuard;
::svtools::VolumeInfo aVolInfo( pData->mbIsVolume, pData->mbIsRemote, ::svtools::VolumeInfo aVolInfo( pData->mbIsVolume, pData->mbIsRemote,
pData->mbIsRemoveable, pData->mbIsFloppy, pData->mbIsRemoveable, pData->mbIsFloppy,
pData->mbIsCompactDisc ); pData->mbIsCompactDisc );

1
sd/source/ui/unoidl/randomnode.cxx
View File

@ -224,7 +224,6 @@ void SAL_CALL RandomAnimationNode::initialize( const Sequence< Any >& aArguments
// XAnimationNode // XAnimationNode
sal_Int16 SAL_CALL RandomAnimationNode::getType() sal_Int16 SAL_CALL RandomAnimationNode::getType()
{ {
std::unique_lock aGuard( maMutex );
return css::animations::AnimationNodeType::PAR; return css::animations::AnimationNodeType::PAR;
} }

2
sfx2/source/appl/shutdownicon.cxx
View File

@ -235,8 +235,6 @@ void ShutdownIcon::FromTemplate()
OUString ShutdownIcon::GetUrlDescription( std::u16string_view aUrl ) OUString ShutdownIcon::GetUrlDescription( std::u16string_view aUrl )
{ {
::SolarMutexGuard aGuard;
return SvFileInformationManager::GetDescription( INetURLObject( aUrl ) ); return SvFileInformationManager::GetDescription( INetURLObject( aUrl ) );
} }

4
slideshow/source/engine/slideshowimpl.cxx
View File

@ -1268,8 +1268,6 @@ void SlideShowImpl::rewindEffectToPreviousSlide()
sal_Bool SlideShowImpl::startShapeActivity( sal_Bool SlideShowImpl::startShapeActivity(
uno::Reference<drawing::XShape> const& /*xShape*/ ) uno::Reference<drawing::XShape> const& /*xShape*/ )
{ {
osl::MutexGuard const guard( m_aMutex );
// precondition: must only be called from the main thread! // precondition: must only be called from the main thread!
DBG_TESTSOLARMUTEX(); DBG_TESTSOLARMUTEX();
@ -1281,8 +1279,6 @@ sal_Bool SlideShowImpl::startShapeActivity(
sal_Bool SlideShowImpl::stopShapeActivity( sal_Bool SlideShowImpl::stopShapeActivity(
uno::Reference<drawing::XShape> const& /*xShape*/ ) uno::Reference<drawing::XShape> const& /*xShape*/ )
{ {
osl::MutexGuard const guard( m_aMutex );
// precondition: must only be called from the main thread! // precondition: must only be called from the main thread!
DBG_TESTSOLARMUTEX(); DBG_TESTSOLARMUTEX();

4
slideshow/source/engine/slideview.cxx
View File

@ -934,8 +934,6 @@ cppcanvas::CustomSpriteSharedPtr SlideView::createSprite(
void SlideView::setPriority( const basegfx::B1DRange& /*rRange*/ ) void SlideView::setPriority( const basegfx::B1DRange& /*rRange*/ )
{ {
osl::MutexGuard aGuard( m_aMutex );
OSL_FAIL( "SlideView::setPriority() is a NOOP for slide view - " OSL_FAIL( "SlideView::setPriority() is a NOOP for slide view - "
"content will always be shown in the background" ); "content will always be shown in the background" );
} }
@ -976,8 +974,6 @@ void SlideView::setClip( const basegfx::B2DPolyPolygon& rClip )
bool SlideView::resize( const ::basegfx::B2DRange& /*rArea*/ ) bool SlideView::resize( const ::basegfx::B2DRange& /*rArea*/ )
{ {
osl::MutexGuard aGuard( m_aMutex );
OSL_FAIL( "SlideView::resize(): ignored for the View, can't change size " OSL_FAIL( "SlideView::resize(): ignored for the View, can't change size "
"effectively, anyway" ); "effectively, anyway" );

1
svl/source/numbers/zforlist.cxx
View File

@ -3605,7 +3605,6 @@ sal_uInt32 SvNumberFormatter::GetFormatIndex( NfIndexTableOffset nTabOff,
NfIndexTableOffset SvNumberFormatter::GetIndexTableOffset( sal_uInt32 nFormat ) const NfIndexTableOffset SvNumberFormatter::GetIndexTableOffset( sal_uInt32 nFormat ) const
{ {
::osl::MutexGuard aGuard( GetInstanceMutex() );
sal_uInt32 nOffset = nFormat % SV_COUNTRY_LANGUAGE_OFFSET; // relative index sal_uInt32 nOffset = nFormat % SV_COUNTRY_LANGUAGE_OFFSET; // relative index
if ( nOffset > SV_MAX_COUNT_STANDARD_FORMATS ) if ( nOffset > SV_MAX_COUNT_STANDARD_FORMATS )
{ {

1
svx/source/accessibility/svxpixelctlaccessiblecontext.cxx
View File

@ -56,7 +56,6 @@ uno::Reference< XAccessibleContext > SvxPixelCtlAccessible::getAccessibleContext
sal_Int64 SvxPixelCtlAccessible::getAccessibleChildCount( ) sal_Int64 SvxPixelCtlAccessible::getAccessibleChildCount( )
{ {
::osl::MutexGuard aGuard( m_aMutex );
return SvxPixelCtl::GetSquares(); return SvxPixelCtl::GetSquares();
} }
uno::Reference< XAccessible > SvxPixelCtlAccessible::getAccessibleChild( sal_Int64 i ) uno::Reference< XAccessible > SvxPixelCtlAccessible::getAccessibleChild( sal_Int64 i )

2
svx/source/accessibility/svxrectctaccessiblecontext.cxx
View File

@ -129,8 +129,6 @@ Reference< XAccessible > SAL_CALL SvxRectCtlAccessibleContext::getAccessibleAtPo
// XAccessibleContext // XAccessibleContext
sal_Int64 SAL_CALL SvxRectCtlAccessibleContext::getAccessibleChildCount() sal_Int64 SAL_CALL SvxRectCtlAccessibleContext::getAccessibleChildCount()
{ {
::osl::MutexGuard aGuard( m_aMutex );
return SvxRectCtl::NO_CHILDREN; return SvxRectCtl::NO_CHILDREN;
} }

1
sw/source/core/unocore/unocoll.cxx
View File

@ -475,7 +475,6 @@ const SvEventDescription* sw_GetSupportedMacroItems()
OUString SwXServiceProvider::GetProviderName(SwServiceType nObjectType) OUString SwXServiceProvider::GetProviderName(SwServiceType nObjectType)
{ {
SolarMutexGuard aGuard;
OUString sRet; OUString sRet;
const sal_uInt16 nEntries = SAL_N_ELEMENTS(aProvNamesId); const sal_uInt16 nEntries = SAL_N_ELEMENTS(aProvNamesId);
if(static_cast<sal_uInt16>(nObjectType) < nEntries) if(static_cast<sal_uInt16>(nObjectType) < nEntries)

11
toolkit/source/controls/unocontrolmodel.cxx
View File

@ -511,8 +511,6 @@ css::uno::Any UnoControlModel::getPropertyDefault( const OUString& rPropertyName
// css::io::XPersistObjec // css::io::XPersistObjec
OUString UnoControlModel::getServiceName( ) OUString UnoControlModel::getServiceName( )
{ {
::osl::Guard< ::osl::Mutex > aGuard( GetMutex() );
OSL_FAIL( "ServiceName of UnoControlModel ?!" ); OSL_FAIL( "ServiceName of UnoControlModel ?!" );
return OUString(); return OUString();
} }
@ -1218,17 +1216,12 @@ void UnoControlModel::getFastPropertyValue( css::uno::Any& rValue, sal_Int32 nPr
// css::beans::XPropertySet // css::beans::XPropertySet
void UnoControlModel::setPropertyValue( const OUString& rPropertyName, const css::uno::Any& rValue ) void UnoControlModel::setPropertyValue( const OUString& rPropertyName, const css::uno::Any& rValue )
{ {
sal_Int32 nPropId = 0; sal_Int32 nPropId = static_cast<sal_Int32>(GetPropertyId( rPropertyName ));
{ DBG_ASSERT( nPropId, "Invalid ID in UnoControlModel::setPropertyValue" );
::osl::Guard< ::osl::Mutex > aGuard( GetMutex() );
nPropId = static_cast<sal_Int32>(GetPropertyId( rPropertyName ));
DBG_ASSERT( nPropId, "Invalid ID in UnoControlModel::setPropertyValue" );
}
if( !nPropId ) if( !nPropId )
throw css::beans::UnknownPropertyException(rPropertyName); throw css::beans::UnknownPropertyException(rPropertyName);
setFastPropertyValue( nPropId, rValue ); setFastPropertyValue( nPropId, rValue );
} }
// css::beans::XFastPropertySet // css::beans::XFastPropertySet

2
ucb/source/sorter/sortresult.cxx
View File

@ -732,8 +732,6 @@ void SAL_CALL SortedResultSet::setPropertyValue(
const OUString& PropertyName, const OUString& PropertyName,
const Any& ) const Any& )
{ {
std::unique_lock aGuard( maMutex );
if ( PropertyName == "RowCount" || PropertyName == "IsRowCountFinal" ) if ( PropertyName == "RowCount" || PropertyName == "IsRowCountFinal" )
throw IllegalArgumentException(); throw IllegalArgumentException();
else else