accessibility/source/extended/accessibleiconchoicectrlentry.cxx | 4
accessibility/source/extended/accessiblelistboxentry.cxx | 2
accessibility/source/standard/vclxaccessibletextfield.cxx | 2
compilerplugins/clang/test/unnecessarylocking.cxx | 53 ++
compilerplugins/clang/unnecessarylocking.cxx | 195
+++++++---
connectivity/source/drivers/firebird/Connection.cxx | 1
connectivity/source/drivers/flat/EDatabaseMetaData.cxx | 2
connectivity/source/drivers/mysqlc/mysqlc_connection.cxx | 2
extensions/source/propctrlr/formcomponenthandler.cxx | 1
fpicker/source/office/contentenumeration.cxx | 1
sd/source/ui/unoidl/randomnode.cxx | 1
sfx2/source/appl/shutdownicon.cxx | 2
slideshow/source/engine/slideshowimpl.cxx | 4
slideshow/source/engine/slideview.cxx | 4
svl/source/numbers/zforlist.cxx | 1
svx/source/accessibility/svxpixelctlaccessiblecontext.cxx | 1
svx/source/accessibility/svxrectctaccessiblecontext.cxx | 2
sw/source/core/unocore/unocoll.cxx | 1
toolkit/source/controls/unocontrolmodel.cxx | 11
ucb/source/sorter/sortresult.cxx | 2
20 files changed, 201 insertions(+), 91 deletions(-)
New commits:
commit 50b68c341f2543c4d841fce4d4b3e080f4491e1d
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Thu Mar 9 14:28:47 2023 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Fri Mar 10 11:34:00 2023 +0000
improve loplugin:unnecessarylocking
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.gran...@collabora.co.uk>
diff --git a/accessibility/source/extended/accessibleiconchoicectrlentry.cxx
b/accessibility/source/extended/accessibleiconchoicectrlentry.cxx
index f8933e49a5ff..82e8ba091f22 100644
--- a/accessibility/source/extended/accessibleiconchoicectrlentry.cxx
+++ b/accessibility/source/extended/accessibleiconchoicectrlentry.cxx
@@ -626,8 +626,6 @@ namespace accessibility
sal_Int32 SAL_CALL
AccessibleIconChoiceCtrlEntry::getAccessibleActionCount( )
{
- ::osl::MutexGuard aGuard( m_aMutex );
-
// three actions supported
return ACCESSIBLE_ACTION_COUNT;
}
@@ -665,8 +663,6 @@ namespace accessibility
Reference< XAccessibleKeyBinding >
AccessibleIconChoiceCtrlEntry::getAccessibleActionKeyBinding( sal_Int32 nIndex )
{
- ::osl::MutexGuard aGuard( m_aMutex );
-
Reference< XAccessibleKeyBinding > xRet;
checkActionIndex_Impl( nIndex );
// ... which key?
diff --git a/accessibility/source/extended/accessiblelistboxentry.cxx
b/accessibility/source/extended/accessiblelistboxentry.cxx
index e22c8ce2ba5d..826d9f54cb1e 100644
--- a/accessibility/source/extended/accessiblelistboxentry.cxx
+++ b/accessibility/source/extended/accessiblelistboxentry.cxx
@@ -808,8 +808,6 @@ namespace accessibility
Reference< XAccessibleKeyBinding >
AccessibleListBoxEntry::getAccessibleActionKeyBinding( sal_Int32 nIndex )
{
- ::osl::MutexGuard aGuard( m_aMutex );
-
Reference< XAccessibleKeyBinding > xRet;
checkActionIndex_Impl( nIndex );
// ... which key?
diff --git a/accessibility/source/standard/vclxaccessibletextfield.cxx
b/accessibility/source/standard/vclxaccessibletextfield.cxx
index e5121affd7ce..aaaa86593506 100644
--- a/accessibility/source/standard/vclxaccessibletextfield.cxx
+++ b/accessibility/source/standard/vclxaccessibletextfield.cxx
@@ -74,8 +74,6 @@ Reference<XAccessible> SAL_CALL
VCLXAccessibleTextField::getAccessibleChild (sal
sal_Int16 SAL_CALL VCLXAccessibleTextField::getAccessibleRole()
{
- ::osl::Guard< ::osl::Mutex > aGuard( GetMutex() );
-
return AccessibleRole::TEXT;
}
diff --git a/compilerplugins/clang/test/unnecessarylocking.cxx
b/compilerplugins/clang/test/unnecessarylocking.cxx
index 6dda5d333191..a2319623140a 100644
--- a/compilerplugins/clang/test/unnecessarylocking.cxx
+++ b/compilerplugins/clang/test/unnecessarylocking.cxx
@@ -8,14 +8,17 @@
*/
#include <mutex>
+#include <osl/mutex.hxx>
static std::mutex gSolarMutex;
-class SolarMutexGuard : public std::unique_lock<std::mutex>
+class SolarMutexGuard
{
+ std::unique_lock<std::mutex> lock;
+
public:
SolarMutexGuard()
- : std::unique_lock<std::mutex>(gSolarMutex)
+ : lock(gSolarMutex)
{
}
};
@@ -25,8 +28,8 @@ namespace test1
struct Foo
{
int m_foo;
- // expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
int bar1()
+ // expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
{
SolarMutexGuard guard;
return 1;
@@ -42,19 +45,22 @@ struct Foo
namespace test2
{
+int free_function() { return 1; }
+
struct Foo
{
std::mutex m_aMutex;
+ osl::Mutex m_aOslMutex;
int m_foo;
- // expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
int bar1()
+ // expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
{
std::unique_lock guard(m_aMutex);
return 1;
}
- // expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
int bar2()
+ // expected-error@+1 {{unnecessary locking [loplugin:unnecessarylocking]}}
{
std::scoped_lock guard(m_aMutex);
return 1;
@@ -65,7 +71,44 @@ struct Foo
std::scoped_lock guard(m_aMutex);
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: */
diff --git a/compilerplugins/clang/unnecessarylocking.cxx
b/compilerplugins/clang/unnecessarylocking.cxx
index c00758b810dc..40b15518571d 100644
--- a/compilerplugins/clang/unnecessarylocking.cxx
+++ b/compilerplugins/clang/unnecessarylocking.cxx
@@ -54,34 +54,28 @@ public:
TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
}
- bool TraverseCXXMethodDecl(CXXMethodDecl*);
+ bool VisitCompoundStmt(const CompoundStmt*);
bool VisitCXXThisExpr(const CXXThisExpr*);
+ bool VisitCallExpr(const CallExpr*);
private:
bool isSolarMutexLockGuardStmt(const Stmt*);
- const Stmt* isOtherMutexLockGuardStmt(const Stmt*);
+ const CXXThisExpr* isOtherMutexLockGuardStmt(const Stmt*);
std::vector<bool> m_TouchesThis;
// 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;
-
- if (!cxxMethodDecl->isInstance())
- return true;
- if (!cxxMethodDecl->isThisDeclarationADefinition())
- return true;
-
- auto compoundStmt =
dyn_cast_or_null<CompoundStmt>(cxxMethodDecl->getBody());
- if (!compoundStmt || compoundStmt->size() < 1)
+ if (compoundStmt->size() < 1)
return true;
const Stmt* firstStmt = *compoundStmt->body_begin();
bool solarMutex = isSolarMutexLockGuardStmt(firstStmt);
- const Stmt* ignoreThisStmt = nullptr;
+ const CXXThisExpr* ignoreThisStmt = nullptr;
if (!solarMutex)
ignoreThisStmt = isOtherMutexLockGuardStmt(firstStmt);
if (!solarMutex && ignoreThisStmt == nullptr)
@@ -90,48 +84,133 @@ bool
UnnecessaryLocking::TraverseCXXMethodDecl(CXXMethodDecl* cxxMethodDecl)
m_TouchesThis.push_back(false);
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())
{
StringRef fn = getFilenameOfLocation(
-
compiler.getSourceManager().getSpellingLoc(cxxMethodDecl->getBeginLoc()));
+
compiler.getSourceManager().getSpellingLoc(compoundStmt->getBeginLoc()));
if (
// template magic
!loplugin::isSamePathname(fn, SRCDIR
"/include/comphelper/unique_disposing_ptr.hxx")
&& !loplugin::isSamePathname(fn, SRCDIR "/sw/inc/unobaseclass.hxx")
- // toolkit needs to lock around access to static methods in vcl
- && !loplugin::isSamePathname(fn, SRCDIR
"/toolkit/source/awt/vclxtoolkit.cxx")
+
+ // 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/controls/tree/treecontrolpeer.cxx")
- && !loplugin::isSamePathname(fn, SRCDIR
"/toolkit/source/awt/vclxcontainer.cxx")
- && !loplugin::isSamePathname(fn, SRCDIR
"/toolkit/source/awt/vclxdevice.cxx")
- // touching shared global data
+ SRCDIR
"/toolkit/source/helper/listenermultiplexer.cxx")
+ && !loplugin::isSamePathname(fn, SRCDIR
"/include/toolkit/helper/macros.hxx")
&& !loplugin::isSamePathname(fn, SRCDIR
-
"/framework/source/fwi/classes/protocolhandlercache.cxx")
- // lock around access to static methods in vcl
- && !loplugin::isSamePathname(fn, SRCDIR
"/framework/source/services/taskcreatorsrv.cxx")
- && !loplugin::isSamePathname(fn, SRCDIR
"/svx/source/dialog/SafeModeUI.cxx")
+
"/chart2/source/controller/main/CommandDispatch.cxx")
+ && !loplugin::isSamePathname(fn, SRCDIR
"/chart2/source/controller/main/ChartView.cxx")
&& !loplugin::isSamePathname(fn, SRCDIR
-
"/svx/source/accessibility/AccessibleFrameSelector.cxx")
+
"/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/controls/tree/treecontrolpeer.cxx")
+ && !loplugin::isSamePathname(fn, SRCDIR
"/ucb/source/ucp/image/ucpimage.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
+ && !loplugin::isSamePathname(fn, SRCDIR
+
"/framework/source/fwi/classes/protocolhandlercache.cxx")
+ && !loplugin::isSamePathname(fn,
+ SRCDIR
"/basic/source/basmgr/basicmanagerrepository.cxx")
+ && !loplugin::isSamePathname(fn, SRCDIR
"/basic/source/classes/sb.cxx")
+ && !loplugin::isSamePathname(fn, SRCDIR
"/sc/source/ui/unoobj/docuno.cxx")
+ && !loplugin::isSamePathname(fn, SRCDIR
"/sc/source/ui/unoobj/afmtuno.cxx")
+ && !loplugin::isSamePathname(fn, SRCDIR
"/sc/source/ui/unoobj/appluno.cxx")
+ && !loplugin::isSamePathname(fn, SRCDIR
"/sc/source/ui/vba/vbaapplication.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/bastyp/proofreadingiterator.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/unoobj.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/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/unobkm.cxx")
- && !loplugin::isSamePathname(fn, SRCDIR
"/sc/source/ui/unoobj/docuno.cxx")
- && !loplugin::isSamePathname(fn, SRCDIR
"/sc/source/ui/unoobj/afmtuno.cxx")
- && !loplugin::isSamePathname(fn, SRCDIR
"/sc/source/ui/unoobj/appluno.cxx")
+ && !loplugin::isSamePathname(fn, SRCDIR
"/sw/source/core/unocore/unocoll.cxx")
+ && !loplugin::isSamePathname(fn, SRCDIR
"/sw/source/core/unocore/unostyle.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
"/sd/source/ui/accessibility/AccessibleDrawDocumentView.cxx")
@@ -139,15 +218,15 @@ bool
UnnecessaryLocking::TraverseCXXMethodDecl(CXXMethodDecl* cxxMethodDecl)
&& !loplugin::isSamePathname(fn,
SRCDIR
"/starmath/source/AccessibleSmElementsControl.cxx"))
{
- report(DiagnosticsEngine::Warning, "unnecessary locking",
cxxMethodDecl->getBeginLoc())
- << cxxMethodDecl->getSourceRange();
+ report(DiagnosticsEngine::Warning, "unnecessary locking",
compoundStmt->getBeginLoc())
+ << compoundStmt->getSourceRange();
}
}
m_TouchesThis.pop_back();
m_IgnoreThis.pop_back();
- return rv;
+ return true;
}
bool UnnecessaryLocking::isSolarMutexLockGuardStmt(const Stmt* stmt)
@@ -169,7 +248,7 @@ bool UnnecessaryLocking::isSolarMutexLockGuardStmt(const
Stmt* stmt)
return true;
}
-const Stmt* UnnecessaryLocking::isOtherMutexLockGuardStmt(const Stmt* stmt)
+const CXXThisExpr* UnnecessaryLocking::isOtherMutexLockGuardStmt(const Stmt*
stmt)
{
auto declStmt = dyn_cast<DeclStmt>(stmt);
if (!declStmt)
@@ -180,16 +259,25 @@ const Stmt*
UnnecessaryLocking::isOtherMutexLockGuardStmt(const Stmt* stmt)
if (!varDecl)
return nullptr;
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;
auto cxxConstructExpr = dyn_cast<CXXConstructExpr>(varDecl->getInit());
if (!cxxConstructExpr || cxxConstructExpr->getNumArgs() < 1)
return nullptr;
- auto memberExpr = dyn_cast<MemberExpr>(cxxConstructExpr->getArg(0));
- if (!memberExpr)
- return nullptr;
- auto thisStmt = memberExpr->getBase();
- return thisStmt;
+ auto arg0 = cxxConstructExpr->getArg(0);
+ if (auto memberExpr = dyn_cast<MemberExpr>(arg0))
+ {
+ const CXXThisExpr* 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)
@@ -208,6 +296,25 @@ bool UnnecessaryLocking::VisitCXXThisExpr(const
CXXThisExpr* cxxThisExpr)
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 */
loplugin::Plugin::Registration<UnnecessaryLocking>
unnecessarylocking("unnecessarylocking", false);
}
diff --git a/connectivity/source/drivers/firebird/Connection.cxx
b/connectivity/source/drivers/firebird/Connection.cxx
index e673d3f9fbff..179b8e19ceb4 100644
--- a/connectivity/source/drivers/firebird/Connection.cxx
+++ b/connectivity/source/drivers/firebird/Connection.cxx
@@ -440,7 +440,6 @@ Reference< XPreparedStatement > SAL_CALL
Connection::prepareCall(
OUString SAL_CALL Connection::nativeSQL( const OUString& _sSql )
{
- MutexGuard aGuard( m_aMutex );
// We do not need to adapt the SQL for Firebird atm.
return _sSql;
}
diff --git a/connectivity/source/drivers/flat/EDatabaseMetaData.cxx
b/connectivity/source/drivers/flat/EDatabaseMetaData.cxx
index b711645b591e..729b55ef9e2d 100644
--- a/connectivity/source/drivers/flat/EDatabaseMetaData.cxx
+++ b/connectivity/source/drivers/flat/EDatabaseMetaData.cxx
@@ -47,8 +47,6 @@ OFlatDatabaseMetaData::~OFlatDatabaseMetaData()
Reference< XResultSet > OFlatDatabaseMetaData::impl_getTypeInfo_throw( )
{
- ::osl::MutexGuard aGuard( m_aMutex );
-
rtl::Reference<::connectivity::ODatabaseMetaDataResultSet> pResult = new
::connectivity::ODatabaseMetaDataResultSet(::connectivity::ODatabaseMetaDataResultSet::eTypeInfo);
static ODatabaseMetaDataResultSet::ORows aRows = []()
diff --git a/connectivity/source/drivers/mysqlc/mysqlc_connection.cxx
b/connectivity/source/drivers/mysqlc/mysqlc_connection.cxx
index d56993f89230..8e0915f5fd05 100644
--- a/connectivity/source/drivers/mysqlc/mysqlc_connection.cxx
+++ b/connectivity/source/drivers/mysqlc/mysqlc_connection.cxx
@@ -257,8 +257,6 @@ Reference<XPreparedStatement> SAL_CALL
OConnection::prepareCall(const OUString&
OUString SAL_CALL OConnection::nativeSQL(const OUString& /*_sSql*/)
{
- MutexGuard aGuard(m_aMutex);
-
// const OUString sSqlStatement = transFormPreparedStatement( _sSql );
// TODO
return OUString();
diff --git a/extensions/source/propctrlr/formcomponenthandler.cxx
b/extensions/source/propctrlr/formcomponenthandler.cxx
index dd4f1302fc31..81f90518d2c5 100644
--- a/extensions/source/propctrlr/formcomponenthandler.cxx
+++ b/extensions/source/propctrlr/formcomponenthandler.cxx
@@ -881,7 +881,6 @@ namespace pcr
Sequence< OUString > SAL_CALL
FormComponentPropertyHandler::getActuatingProperties( )
{
- ::osl::MutexGuard aGuard( m_aMutex );
return
{
PROPERTY_DATASOURCE,
diff --git a/fpicker/source/office/contentenumeration.cxx
b/fpicker/source/office/contentenumeration.cxx
index 5eb12feeef2e..fe2f59310a47 100644
--- a/fpicker/source/office/contentenumeration.cxx
+++ b/fpicker/source/office/contentenumeration.cxx
@@ -251,7 +251,6 @@ namespace svt
if ( pData->mbIsFolder )
{
- SolarMutexGuard aGuard;
::svtools::VolumeInfo aVolInfo(
pData->mbIsVolume, pData->mbIsRemote,
pData->mbIsRemoveable, pData->mbIsFloppy,
pData->mbIsCompactDisc );
diff --git a/sd/source/ui/unoidl/randomnode.cxx
b/sd/source/ui/unoidl/randomnode.cxx
index 08c0e9fda594..74f4eb0a30e4 100644
--- a/sd/source/ui/unoidl/randomnode.cxx
+++ b/sd/source/ui/unoidl/randomnode.cxx
@@ -224,7 +224,6 @@ void SAL_CALL RandomAnimationNode::initialize( const
Sequence< Any >& aArguments
// XAnimationNode
sal_Int16 SAL_CALL RandomAnimationNode::getType()
{
- std::unique_lock aGuard( maMutex );
return css::animations::AnimationNodeType::PAR;
}
diff --git a/sfx2/source/appl/shutdownicon.cxx
b/sfx2/source/appl/shutdownicon.cxx
index 0dd218004154..9490c52d23be 100644
--- a/sfx2/source/appl/shutdownicon.cxx
+++ b/sfx2/source/appl/shutdownicon.cxx
@@ -235,8 +235,6 @@ void ShutdownIcon::FromTemplate()
OUString ShutdownIcon::GetUrlDescription( std::u16string_view aUrl )
{
- ::SolarMutexGuard aGuard;
-
return SvFileInformationManager::GetDescription( INetURLObject( aUrl ) );
}
diff --git a/slideshow/source/engine/slideshowimpl.cxx
b/slideshow/source/engine/slideshowimpl.cxx
index 923ea26d0bc0..9109bb6bb2e0 100644
--- a/slideshow/source/engine/slideshowimpl.cxx
+++ b/slideshow/source/engine/slideshowimpl.cxx
@@ -1268,8 +1268,6 @@ void SlideShowImpl::rewindEffectToPreviousSlide()
sal_Bool SlideShowImpl::startShapeActivity(
uno::Reference<drawing::XShape> const& /*xShape*/ )
{
- osl::MutexGuard const guard( m_aMutex );
-
// precondition: must only be called from the main thread!
DBG_TESTSOLARMUTEX();
@@ -1281,8 +1279,6 @@ sal_Bool SlideShowImpl::startShapeActivity(
sal_Bool SlideShowImpl::stopShapeActivity(
uno::Reference<drawing::XShape> const& /*xShape*/ )
{
- osl::MutexGuard const guard( m_aMutex );
-
// precondition: must only be called from the main thread!
DBG_TESTSOLARMUTEX();
diff --git a/slideshow/source/engine/slideview.cxx
b/slideshow/source/engine/slideview.cxx
index 26c4e0f971bb..8b527630bca3 100644
--- a/slideshow/source/engine/slideview.cxx
+++ b/slideshow/source/engine/slideview.cxx
@@ -934,8 +934,6 @@ cppcanvas::CustomSpriteSharedPtr SlideView::createSprite(
void SlideView::setPriority( const basegfx::B1DRange& /*rRange*/ )
{
- osl::MutexGuard aGuard( m_aMutex );
-
OSL_FAIL( "SlideView::setPriority() is a NOOP for slide view - "
"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*/ )
{
- osl::MutexGuard aGuard( m_aMutex );
-
OSL_FAIL( "SlideView::resize(): ignored for the View, can't change size "
"effectively, anyway" );
diff --git a/svl/source/numbers/zforlist.cxx b/svl/source/numbers/zforlist.cxx
index 51e9b3e6cc33..53914a0266cd 100644
--- a/svl/source/numbers/zforlist.cxx
+++ b/svl/source/numbers/zforlist.cxx
@@ -3605,7 +3605,6 @@ sal_uInt32 SvNumberFormatter::GetFormatIndex(
NfIndexTableOffset nTabOff,
NfIndexTableOffset SvNumberFormatter::GetIndexTableOffset( sal_uInt32 nFormat
) const
{
- ::osl::MutexGuard aGuard( GetInstanceMutex() );
sal_uInt32 nOffset = nFormat % SV_COUNTRY_LANGUAGE_OFFSET; //
relative index
if ( nOffset > SV_MAX_COUNT_STANDARD_FORMATS )
{
diff --git a/svx/source/accessibility/svxpixelctlaccessiblecontext.cxx
b/svx/source/accessibility/svxpixelctlaccessiblecontext.cxx
index f1badee58bf6..af34fd21de72 100644
--- a/svx/source/accessibility/svxpixelctlaccessiblecontext.cxx
+++ b/svx/source/accessibility/svxpixelctlaccessiblecontext.cxx
@@ -56,7 +56,6 @@ uno::Reference< XAccessibleContext >
SvxPixelCtlAccessible::getAccessibleContext
sal_Int64 SvxPixelCtlAccessible::getAccessibleChildCount( )
{
- ::osl::MutexGuard aGuard( m_aMutex );
return SvxPixelCtl::GetSquares();
}
uno::Reference< XAccessible > SvxPixelCtlAccessible::getAccessibleChild(
sal_Int64 i )
diff --git a/svx/source/accessibility/svxrectctaccessiblecontext.cxx
b/svx/source/accessibility/svxrectctaccessiblecontext.cxx
index 9366c4f6d44c..6736cf5a7e5e 100644
--- a/svx/source/accessibility/svxrectctaccessiblecontext.cxx
+++ b/svx/source/accessibility/svxrectctaccessiblecontext.cxx
@@ -129,8 +129,6 @@ Reference< XAccessible > SAL_CALL
SvxRectCtlAccessibleContext::getAccessibleAtPo
// XAccessibleContext
sal_Int64 SAL_CALL SvxRectCtlAccessibleContext::getAccessibleChildCount()
{
- ::osl::MutexGuard aGuard( m_aMutex );
-
return SvxRectCtl::NO_CHILDREN;
}
diff --git a/sw/source/core/unocore/unocoll.cxx
b/sw/source/core/unocore/unocoll.cxx
index d4e1028534e7..a71a650794a9 100644
--- a/sw/source/core/unocore/unocoll.cxx
+++ b/sw/source/core/unocore/unocoll.cxx
@@ -475,7 +475,6 @@ const SvEventDescription* sw_GetSupportedMacroItems()
OUString SwXServiceProvider::GetProviderName(SwServiceType nObjectType)
{
- SolarMutexGuard aGuard;
OUString sRet;
const sal_uInt16 nEntries = SAL_N_ELEMENTS(aProvNamesId);
if(static_cast<sal_uInt16>(nObjectType) < nEntries)
diff --git a/toolkit/source/controls/unocontrolmodel.cxx
b/toolkit/source/controls/unocontrolmodel.cxx
index 90c61bfcb2a9..5b68774b678e 100644
--- a/toolkit/source/controls/unocontrolmodel.cxx
+++ b/toolkit/source/controls/unocontrolmodel.cxx
@@ -511,8 +511,6 @@ css::uno::Any UnoControlModel::getPropertyDefault( const
OUString& rPropertyName
// css::io::XPersistObjec
OUString UnoControlModel::getServiceName( )
{
- ::osl::Guard< ::osl::Mutex > aGuard( GetMutex() );
-
OSL_FAIL( "ServiceName of UnoControlModel ?!" );
return OUString();
}
@@ -1218,17 +1216,12 @@ void UnoControlModel::getFastPropertyValue(
css::uno::Any& rValue, sal_Int32 nPr
// css::beans::XPropertySet
void UnoControlModel::setPropertyValue( const OUString& rPropertyName, const
css::uno::Any& rValue )
{
- sal_Int32 nPropId = 0;
- {
- ::osl::Guard< ::osl::Mutex > aGuard( GetMutex() );
- nPropId = static_cast<sal_Int32>(GetPropertyId( rPropertyName ));
- DBG_ASSERT( nPropId, "Invalid ID in UnoControlModel::setPropertyValue"
);
- }
+ sal_Int32 nPropId = static_cast<sal_Int32>(GetPropertyId( rPropertyName ));
+ DBG_ASSERT( nPropId, "Invalid ID in UnoControlModel::setPropertyValue" );
if( !nPropId )
throw css::beans::UnknownPropertyException(rPropertyName);
setFastPropertyValue( nPropId, rValue );
-
}
// css::beans::XFastPropertySet
diff --git a/ucb/source/sorter/sortresult.cxx b/ucb/source/sorter/sortresult.cxx
index a074b7f85210..38b3c7583ed0 100644
--- a/ucb/source/sorter/sortresult.cxx
+++ b/ucb/source/sorter/sortresult.cxx
@@ -732,8 +732,6 @@ void SAL_CALL SortedResultSet::setPropertyValue(
const OUString& PropertyName,
const Any& )
{
- std::unique_lock aGuard( maMutex );
-
if ( PropertyName == "RowCount" || PropertyName == "IsRowCountFinal" )
throw IllegalArgumentException();
else
