https://github.com/vbvictor updated https://github.com/llvm/llvm-project/pull/126434
>From bc1b4ada7615d407c2df009f414d62da3857ee86 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Mon, 3 Mar 2025 09:25:03 +0300 Subject: [PATCH 1/9] [clang-tidy] add scoped-lock-check --- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../modernize/ModernizeTidyModule.cpp | 3 + .../modernize/UseScopedLockCheck.cpp | 319 +++++++++++++++ .../clang-tidy/modernize/UseScopedLockCheck.h | 54 +++ clang-tools-extra/docs/ReleaseNotes.rst | 6 + .../docs/clang-tidy/checks/list.rst | 1 + .../checks/modernize/use-scoped-lock.rst | 102 +++++ .../clang-tidy/checkers/Inputs/Headers/mutex | 33 ++ ...e-lock-warn-on-using-and-typedef-false.cpp | 31 ++ ...scoped-lock-warn-on-single-locks-false.cpp | 96 +++++ .../checkers/modernize/use-scoped-lock.cpp | 372 ++++++++++++++++++ 11 files changed, 1018 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/use-scoped-lock.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/mutex create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-scope-lock-warn-on-using-and-typedef-false.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock-warn-on-single-locks-false.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index bab1167fb15ff..619a27b2f9bb6 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -42,6 +42,7 @@ add_clang_library(clangTidyModernizeModule STATIC UseNullptrCheck.cpp UseOverrideCheck.cpp UseRangesCheck.cpp + UseScopedLockCheck.cpp UseStartsEndsWithCheck.cpp UseStdFormatCheck.cpp UseStdNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index fc46c72982fdc..b2d4ddd667502 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -43,6 +43,7 @@ #include "UseNullptrCheck.h" #include "UseOverrideCheck.h" #include "UseRangesCheck.h" +#include "UseScopedLockCheck.h" #include "UseStartsEndsWithCheck.h" #include "UseStdFormatCheck.h" #include "UseStdNumbersCheck.h" @@ -80,6 +81,8 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<UseIntegerSignComparisonCheck>( "modernize-use-integer-sign-comparison"); CheckFactories.registerCheck<UseRangesCheck>("modernize-use-ranges"); + CheckFactories.registerCheck<UseScopedLockCheck>( + "modernize-use-scoped-lock"); CheckFactories.registerCheck<UseStartsEndsWithCheck>( "modernize-use-starts-ends-with"); CheckFactories.registerCheck<UseStdFormatCheck>("modernize-use-std-format"); diff --git a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp new file mode 100644 index 0000000000000..5dbb3a2f671e6 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp @@ -0,0 +1,319 @@ +//===--- UseScopedLockCheck.cpp - clang-tidy ------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "UseScopedLockCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/Stmt.h" +#include "clang/AST/Type.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Lex/Lexer.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/Twine.h" +#include <iterator> + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +namespace { + +bool isLockGuard(const QualType &Type) { + if (const auto *Record = Type->getAs<RecordType>()) { + if (const RecordDecl *Decl = Record->getDecl()) { + return Decl->getName() == "lock_guard" && Decl->isInStdNamespace(); + } + } + + if (const auto *TemplateSpecType = + Type->getAs<TemplateSpecializationType>()) { + if (const TemplateDecl *Decl = + TemplateSpecType->getTemplateName().getAsTemplateDecl()) { + return Decl->getName() == "lock_guard" && Decl->isInStdNamespace(); + } + } + + return false; +} + +llvm::SmallVector<const VarDecl *> getLockGuardsFromDecl(const DeclStmt *DS) { + llvm::SmallVector<const VarDecl *> LockGuards; + + for (const Decl *Decl : DS->decls()) { + if (const auto *VD = dyn_cast<VarDecl>(Decl)) { + const QualType Type = VD->getType().getCanonicalType(); + if (isLockGuard(Type)) { + LockGuards.push_back(VD); + } + } + } + + return LockGuards; +} + +// Scans through the statements in a block and groups consecutive +// 'std::lock_guard' variable declarations together. +llvm::SmallVector<llvm::SmallVector<const VarDecl *>> +findLocksInCompoundStmt(const CompoundStmt *Block, + const ast_matchers::MatchFinder::MatchResult &Result) { + // store groups of consecutive 'std::lock_guard' declarations + llvm::SmallVector<llvm::SmallVector<const VarDecl *>> LockGuardGroups; + llvm::SmallVector<const VarDecl *> CurrentLockGuardGroup; + + auto AddAndClearCurrentGroup = [&]() { + if (!CurrentLockGuardGroup.empty()) { + LockGuardGroups.push_back(CurrentLockGuardGroup); + CurrentLockGuardGroup.clear(); + } + }; + + for (const Stmt *Stmt : Block->body()) { + if (const auto *DS = dyn_cast<DeclStmt>(Stmt)) { + llvm::SmallVector<const VarDecl *> LockGuards = getLockGuardsFromDecl(DS); + + if (!LockGuards.empty()) { + CurrentLockGuardGroup.insert( + CurrentLockGuardGroup.end(), + std::make_move_iterator(LockGuards.begin()), + std::make_move_iterator(LockGuards.end())); + continue; + } + } + AddAndClearCurrentGroup(); + } + + AddAndClearCurrentGroup(); + + return LockGuardGroups; +} + +TemplateSpecializationTypeLoc +getTemplateLockGuardTypeLoc(const TypeSourceInfo *SourceInfo) { + const TypeLoc Loc = SourceInfo->getTypeLoc(); + + const auto ElaboratedLoc = Loc.getAs<ElaboratedTypeLoc>(); + if (!ElaboratedLoc) + return {}; + + return ElaboratedLoc.getNamedTypeLoc().getAs<TemplateSpecializationTypeLoc>(); +} + +// Find the exact source range of the 'lock_guard<...>' token +SourceRange getLockGuardTemplateRange(const TypeSourceInfo *SourceInfo) { + const TemplateSpecializationTypeLoc TemplateLoc = + getTemplateLockGuardTypeLoc(SourceInfo); + if (!TemplateLoc) + return {}; + + return SourceRange(TemplateLoc.getTemplateNameLoc(), + TemplateLoc.getRAngleLoc()); +} + +// Find the exact source range of the 'lock_guard' token +SourceRange getLockGuardRange(const TypeSourceInfo *SourceInfo) { + const TemplateSpecializationTypeLoc TemplateLoc = + getTemplateLockGuardTypeLoc(SourceInfo); + if (!TemplateLoc) + return {}; + + return SourceRange(TemplateLoc.getTemplateNameLoc(), + TemplateLoc.getLAngleLoc().getLocWithOffset(-1)); +} + +const StringRef UseScopedLockMessage = + "use 'std::scoped_lock' instead of 'std::lock_guard'"; + +} // namespace + +UseScopedLockCheck::UseScopedLockCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + WarnOnSingleLocks(Options.get("WarnOnSingleLocks", true)), + WarnOnUsingAndTypedef(Options.get("WarnOnUsingAndTypedef", true)) {} + +void UseScopedLockCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "WarnOnSingleLocks", WarnOnSingleLocks); + Options.store(Opts, "WarnOnUsingAndTypedef", WarnOnUsingAndTypedef); +} + +void UseScopedLockCheck::registerMatchers(MatchFinder *Finder) { + auto LockGuardClassDecl = + namedDecl(anyOf(classTemplateDecl(), classTemplateSpecializationDecl()), + hasName("lock_guard"), isInStdNamespace()); + auto LockGuardType = qualType(hasDeclaration(LockGuardClassDecl)); + auto LockVarDecl = varDecl(hasType(LockGuardType)); + + if (WarnOnSingleLocks) { + // Match CompoundStmt with only one 'std::lock_guard' + Finder->addMatcher( + compoundStmt(unless(isExpansionInSystemHeader()), + has(declStmt(has(LockVarDecl.bind("lock-decl-single")))), + unless(hasDescendant(declStmt(has(varDecl( + hasType(LockGuardType), + unless(equalsBoundNode("lock-decl-single")))))))), + this); + } + + // Match CompoundStmt with multiple 'std::lock_guard' + Finder->addMatcher( + compoundStmt(unless(isExpansionInSystemHeader()), + has(declStmt(has(LockVarDecl.bind("lock-decl-multiple")))), + hasDescendant(declStmt(has(varDecl( + hasType(LockGuardType), + unless(equalsBoundNode("lock-decl-multiple"))))))) + .bind("block-multiple"), + this); + + if (WarnOnUsingAndTypedef) { + // Match 'typedef std::lock_guard<std::mutex> Lock' + Finder->addMatcher(typedefDecl(unless(isExpansionInSystemHeader()), + hasUnderlyingType(LockGuardType)) + .bind("lock-guard-typedef"), + this); + + // Match 'using Lock = std::lock_guard<std::mutex>' + Finder->addMatcher( + typeAliasDecl( + unless(isExpansionInSystemHeader()), + hasType(elaboratedType(namesType(templateSpecializationType( + hasDeclaration(LockGuardClassDecl)))))) + .bind("lock-guard-using-alias"), + this); + + // Match 'using std::lock_guard' + Finder->addMatcher( + usingDecl(unless(isExpansionInSystemHeader()), + hasAnyUsingShadowDecl(hasTargetDecl(LockGuardClassDecl))) + .bind("lock-guard-using-decl"), + this); + } +} + +void UseScopedLockCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *Decl = Result.Nodes.getNodeAs<VarDecl>("lock-decl-single")) { + emitDiag(Decl, Result); + return; + } + + if (const auto *Compound = + Result.Nodes.getNodeAs<CompoundStmt>("block-multiple")) { + emitDiag(findLocksInCompoundStmt(Compound, Result), Result); + return; + } + + if (const auto *Typedef = + Result.Nodes.getNodeAs<TypedefDecl>("lock-guard-typedef")) { + emitDiag(Typedef->getTypeSourceInfo(), Result); + return; + } + + if (const auto *UsingAlias = + Result.Nodes.getNodeAs<TypeAliasDecl>("lock-guard-using-alias")) { + emitDiag(UsingAlias->getTypeSourceInfo(), Result); + return; + } + + if (const auto *Using = + Result.Nodes.getNodeAs<UsingDecl>("lock-guard-using-decl")) { + emitDiag(Using, Result); + } +} + +void UseScopedLockCheck::emitDiag(const VarDecl *LockGuard, + const MatchFinder::MatchResult &Result) { + auto Diag = diag(LockGuard->getBeginLoc(), UseScopedLockMessage); + + const SourceRange LockGuardTypeRange = + getLockGuardTemplateRange(LockGuard->getTypeSourceInfo()); + + if (LockGuardTypeRange.isInvalid()) { + return; + } + + // Create Fix-its only if we can find the constructor call to properly handle + // 'std::lock_guard l(m, std::adopt_lock)' case. + const auto *CtorCall = dyn_cast<CXXConstructExpr>(LockGuard->getInit()); + if (!CtorCall) { + return; + } + + if (CtorCall->getNumArgs() == 1) { + Diag << FixItHint::CreateReplacement(LockGuardTypeRange, "scoped_lock"); + return; + } + + if (CtorCall->getNumArgs() == 2) { + const Expr *const *CtorArgs = CtorCall->getArgs(); + + const Expr *MutexArg = CtorArgs[0]; + const Expr *AdoptLockArg = CtorArgs[1]; + + const StringRef MutexSourceText = Lexer::getSourceText( + CharSourceRange::getTokenRange(MutexArg->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + const StringRef AdoptLockSourceText = Lexer::getSourceText( + CharSourceRange::getTokenRange(AdoptLockArg->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + + Diag << FixItHint::CreateReplacement(LockGuardTypeRange, "scoped_lock") + << FixItHint::CreateReplacement( + SourceRange(MutexArg->getBeginLoc(), AdoptLockArg->getEndLoc()), + (llvm::Twine(AdoptLockSourceText) + ", " + MutexSourceText) + .str()); + return; + } + + llvm_unreachable("Invalid argument number of std::lock_guard constructor"); +} + +void UseScopedLockCheck::emitDiag( + const llvm::SmallVector<llvm::SmallVector<const VarDecl *>> &LockGroups, + const ast_matchers::MatchFinder::MatchResult &Result) { + for (const llvm::SmallVector<const VarDecl *> &Group : LockGroups) { + if (Group.size() == 1 && WarnOnSingleLocks) { + emitDiag(Group[0], Result); + } else { + diag(Group[0]->getBeginLoc(), + "use single 'std::scoped_lock' instead of multiple " + "'std::lock_guard'"); + + for (const VarDecl *Lock : llvm::drop_begin(Group)) { + diag(Lock->getLocation(), "additional 'std::lock_guard' declared here", + DiagnosticIDs::Note); + } + } + } +} + +void UseScopedLockCheck::emitDiag( + const TypeSourceInfo *LockGuardSourceInfo, + const ast_matchers::MatchFinder::MatchResult &Result) { + const TypeLoc TL = LockGuardSourceInfo->getTypeLoc(); + + if (const auto ElaboratedTL = TL.getAs<ElaboratedTypeLoc>()) { + auto Diag = diag(ElaboratedTL.getBeginLoc(), UseScopedLockMessage); + + const SourceRange LockGuardRange = getLockGuardRange(LockGuardSourceInfo); + if (LockGuardRange.isInvalid()) { + return; + } + + Diag << FixItHint::CreateReplacement(LockGuardRange, "scoped_lock"); + } +} + +void UseScopedLockCheck::emitDiag( + const UsingDecl *UsingDecl, + const ast_matchers::MatchFinder::MatchResult &Result) { + diag(UsingDecl->getLocation(), UseScopedLockMessage) + << FixItHint::CreateReplacement(UsingDecl->getLocation(), "scoped_lock"); +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.h b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.h new file mode 100644 index 0000000000000..790d9205c6121 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.h @@ -0,0 +1,54 @@ +//===--- UseScopedLockCheck.h - clang-tidy ----------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESCOPEDLOCKCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESCOPEDLOCKCHECK_H + +#include "../ClangTidyCheck.h" +#include "clang/AST/ASTTypeTraits.h" +#include "clang/AST/Stmt.h" +#include <optional> + +namespace clang::tidy::modernize { + +/// Finds uses of ``std::lock_guard`` and suggests replacing them with C++17's +/// alternative ``std::scoped_lock``. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-scoped-lock.html +class UseScopedLockCheck : public ClangTidyCheck { +public: + UseScopedLockCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus17; + } + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + +private: + void emitDiag(const VarDecl *LockGuard, + const ast_matchers::MatchFinder::MatchResult &Result); + void emitDiag( + const llvm::SmallVector<llvm::SmallVector<const VarDecl *>> &LockGroups, + const ast_matchers::MatchFinder::MatchResult &Result); + void emitDiag(const TypeSourceInfo *LockGuardSourceInfo, + const ast_matchers::MatchFinder::MatchResult &Result); + void emitDiag(const UsingDecl *UsingDecl, + const ast_matchers::MatchFinder::MatchResult &Result); + + const bool WarnOnSingleLocks; + const bool WarnOnUsingAndTypedef; +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESCOPEDLOCKCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index fa68b6fabd549..ad0bc624a21a4 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -100,6 +100,12 @@ New checks Finds unintended character output from ``unsigned char`` and ``signed char`` to an ``ostream``. +- New :doc:`modernize-use-scoped-lock + <clang-tidy/checks/modernize/use-scoped-lock>` check. + + Finds uses of ``std::lock_guard`` and suggests replacing them with C++17's + alternative ``std::scoped_lock``. + - New :doc:`readability-ambiguous-smartptr-reset-call <clang-tidy/checks/readability/ambiguous-smartptr-reset-call>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index c73bc8bff3539..d9515a1b4b4cd 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -309,6 +309,7 @@ Clang-Tidy Checks :doc:`modernize-use-nullptr <modernize/use-nullptr>`, "Yes" :doc:`modernize-use-override <modernize/use-override>`, "Yes" :doc:`modernize-use-ranges <modernize/use-ranges>`, "Yes" + :doc:`modernize-use-scoped-lock <modernize/use-scoped-lock>`, "Yes" :doc:`modernize-use-starts-ends-with <modernize/use-starts-ends-with>`, "Yes" :doc:`modernize-use-std-format <modernize/use-std-format>`, "Yes" :doc:`modernize-use-std-numbers <modernize/use-std-numbers>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-scoped-lock.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-scoped-lock.rst new file mode 100644 index 0000000000000..84c896efd6258 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-scoped-lock.rst @@ -0,0 +1,102 @@ +.. title:: clang-tidy - modernize-use-scoped-lock + +modernize-use-scoped-lock +========================= + +Finds uses of ``std::lock_guard`` and suggests replacing them with C++17's +alternative ``std::scoped_lock``. The check will automatically transform only +single declarations of ``std::lock_guard`` and emit warnings for multiple +declarations of ``std::lock_guard`` that can be replaced with a single +declaration of ``std::scoped_lock``. + +Examples +-------- + +Single ``std::lock_guard`` declaration: + +.. code-block:: c++ + + std::mutex M; + std::lock_guard<std::mutex> L(M); + + +Transforms to: + +.. code-block:: c++ + + std::mutex M; + std::scoped_lock L(M); + +Single ``std::lock_guard`` declaration with ``std::adopt_lock``: + +.. code-block:: c++ + + std::mutex M; + std::lock(M); + std::lock_guard<std::mutex> L(M, std::adopt_lock); + + +Transforms to: + +.. code-block:: c++ + + std::mutex M; + std::lock(M); + std::scoped_lock L(std::adopt_lock, M); + +Multiple ``std::lock_guard`` declarations only emit warnings: + +.. code-block:: c++ + + std::mutex M1, M2; + std::lock(M1, M2); + std::lock_guard Lock1(M, std::adopt_lock); // warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + std::lock_guard Lock2(M, std::adopt_lock); // note: additional 'std::lock_guard' declared here + + +Limitations +----------- + +The check will not emit warnings if ``std::lock_guard`` is used implicitly via +``using``, ``typedef`` or as ``template`` parameter: + +.. code-block:: c++ + + template <template <typename> typename Lock> + void TemplatedLock() { + std::mutex M; + Lock<std::mutex> L(M); // no warning + } + + void UsingLock() { + using Lock = std::lock_guard<std::mutex>; + std::mutex M; + Lock L(M); // no warning + } + + +Options +------- + +.. option:: WarnOnSingleLocks + + When `true`, the check will warn on single ``std::lock_guard`` declarations. + Set this option to `false` if you want to get warnings only on multiple + ``std::lock_guard`` declarations that can be replaced with a single + ``std::scoped_lock``. Default is `true`. + +.. option:: WarnOnUsingAndTypedef + + When `true`, the check will emit warnings if ``std::lock_guard`` is used + in ``using`` or ``typedef`` context. Default is `true`. + + .. code-block:: c++ + + template <typename T> + using Lock = std::lock_guard<T>; // warning: use 'std::scoped_lock' instead of 'std::lock_guard' + + using LockMutex = std::lock_guard<std::mutex>; // warning: use 'std::scoped_lock' instead of 'std::lock_guard' + + typedef std::lock_guard<std::mutex> LockDef; // warning: use 'std::scoped_lock' instead of 'std::lock_guard' + + using std::lock_guard; // warning: use 'std::scoped_lock' instead of 'std::lock_guard' \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/mutex b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/mutex new file mode 100644 index 0000000000000..6045342b39d7a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/mutex @@ -0,0 +1,33 @@ +#ifndef _MUTEX_ +#define _MUTEX_ + +namespace std { + +struct mutex { + void lock() {} + void unlock() {} +}; + +template<class Lockable1, class Lockable2, class... LockableN > +void lock(Lockable1& lock1, Lockable2& lock2, LockableN&... lockn ); + +struct adopt_lock_t { }; +std::adopt_lock_t adopt_lock {}; + +template <typename Mutex> +struct lock_guard { + lock_guard(Mutex &m) { } + lock_guard(Mutex &m, std::adopt_lock_t t) {} + lock_guard(const lock_guard&) = delete; +}; + +template <typename... MutexTypes> +struct scoped_lock { + scoped_lock(MutexTypes&... m) {} + scoped_lock(std::adopt_lock_t t, MutexTypes&... m) {} + scoped_lock(const scoped_lock&) = delete; +}; + +} // namespace std + +#endif // _MUTEX_ \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scope-lock-warn-on-using-and-typedef-false.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scope-lock-warn-on-using-and-typedef-false.cpp new file mode 100644 index 0000000000000..a33758fa9c702 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scope-lock-warn-on-using-and-typedef-false.cpp @@ -0,0 +1,31 @@ +// RUN: %check_clang_tidy -std=c++17-or-later %s modernize-use-scoped-lock %t -- \ +// RUN: -config="{CheckOptions: {modernize-use-scoped-lock.WarnOnUsingAndTypedef: false}}" \ +// RUN: -- -isystem %clang_tidy_headers -fno-delayed-template-parsing + +#include <mutex> + +template <typename T> +using Lock = std::lock_guard<T>; + +using LockM = std::lock_guard<std::mutex>; + +typedef std::lock_guard<std::mutex> LockDef; + +void PositiveUsingDecl() { + using std::lock_guard; + + using LockMFun = std::lock_guard<std::mutex>; + + typedef std::lock_guard<std::mutex> LockDefFun; +} + +template <typename T> +void PositiveUsingDeclTemplate() { + using std::lock_guard; + + using LockFunT = std::lock_guard<T>; + + using LockMFunT = std::lock_guard<std::mutex>; + + typedef std::lock_guard<std::mutex> LockDefFunT; +} \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock-warn-on-single-locks-false.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock-warn-on-single-locks-false.cpp new file mode 100644 index 0000000000000..7a59dda18f941 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock-warn-on-single-locks-false.cpp @@ -0,0 +1,96 @@ +// RUN: %check_clang_tidy -std=c++17-or-later %s modernize-use-scoped-lock %t -- \ +// RUN: -config="{CheckOptions: {modernize-use-scoped-lock.WarnOnSingleLocks: false}}" \ +// RUN: -- -isystem %clang_tidy_headers -fno-delayed-template-parsing + +#include <mutex> + +void Positive() { + std::mutex m; + + { + std::lock_guard<std::mutex> l1(m); + std::lock_guard<std::mutex> l2(m); + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:33: note: additional 'std::lock_guard' declared here + } + + { + std::lock_guard<std::mutex> l1(m), l2(m), l3(m); + std::lock_guard<std::mutex> l4(m); + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-3]]:40: note: additional 'std::lock_guard' declared here + // CHECK-MESSAGES: :[[@LINE-4]]:47: note: additional 'std::lock_guard' declared here + // CHECK-MESSAGES: :[[@LINE-4]]:33: note: additional 'std::lock_guard' declared here + } + + { + std::lock(m, m); + std::lock_guard<std::mutex> l1(m, std::adopt_lock); + std::lock_guard<std::mutex> l2(m, std::adopt_lock); + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:33: note: additional 'std::lock_guard' declared here + } +} + +void Negative() { + std::mutex m; + { + std::lock_guard<std::mutex> l(m); + } + + { + std::lock_guard<std::mutex> l(m, std::adopt_lock); + } +} + +void PositiveInsideArg(std::mutex &m1, std::mutex &m2, std::mutex &m3) { + std::lock_guard<std::mutex> l1(m1); + std::lock_guard<std::mutex> l2(m2); + // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:31: note: additional 'std::lock_guard' declared here +} + + +void NegativeInsideArg(std::mutex &m1, std::mutex &m2, std::mutex &m3) { + std::lock_guard<std::mutex> l3(m3); +} + +template <typename T> +void PositiveTemplated() { + std::mutex m1, m2; + + std::lock_guard<std::mutex> l1(m1); + std::lock_guard<std::mutex> l2(m2); + // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:31: note: additional 'std::lock_guard' declared here +} + +template <typename T> +void NegativeTemplated() { + std::mutex m1, m2, m3; + std::lock_guard<std::mutex> l(m1); +} + +template <typename Mutex> +void PositiveTemplatedMutex() { + Mutex m1, m2; + + std::lock_guard<Mutex> l1(m1); + std::lock_guard<Mutex> l2(m2); + // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:26: note: additional 'std::lock_guard' declared here +} + +template <typename Mutex> +void NegativeTemplatedMutex() { + Mutex m1; + std::lock_guard<Mutex> l(m1); +} + +struct NegativeClass { + void Negative() { + std::lock_guard<std::mutex> l(m1); + } + + std::mutex m1; +}; diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp new file mode 100644 index 0000000000000..1957a4b7daa1f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp @@ -0,0 +1,372 @@ +// RUN: %check_clang_tidy -std=c++17-or-later %s modernize-use-scoped-lock %t -- -- -isystem %clang_tidy_headers -fno-delayed-template-parsing + +#include <mutex> + +void Positive() { + std::mutex m; + { + std::lock_guard<std::mutex> l(m); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l(m); + } + + { + std::lock_guard<std::mutex> l(m, std::adopt_lock); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l(std::adopt_lock, m); + } + + { + std::lock_guard<std::mutex> l1(m); + std::lock_guard<std::mutex> l2(m); + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:33: note: additional 'std::lock_guard' declared here + } + + { + std::lock_guard<std::mutex> l1(m), l2(m), l3(m); + std::lock_guard<std::mutex> l4(m); + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-3]]:40: note: additional 'std::lock_guard' declared here + // CHECK-MESSAGES: :[[@LINE-4]]:47: note: additional 'std::lock_guard' declared here + // CHECK-MESSAGES: :[[@LINE-4]]:33: note: additional 'std::lock_guard' declared here + } + + { + std::lock(m, m); + std::lock_guard<std::mutex> l1(m, std::adopt_lock); + std::lock_guard<std::mutex> l2(m, std::adopt_lock); + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:33: note: additional 'std::lock_guard' declared here + int a = 0; + std::lock_guard<std::mutex> l3(m); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l3(m); + int b = 0; + std::lock_guard<std::mutex> l4(m, std::adopt_lock); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l4(std::adopt_lock, m); + } +} + + +std::mutex p_m1; +void PositiveShortFunction() { + std::lock_guard<std::mutex> l(p_m1); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l(p_m1); +} + + +void PositiveNested() { + std::mutex m1; + if (true) { + std::lock_guard<std::mutex> l(m1); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l(m1); + { + std::lock_guard<std::mutex> l2(m1); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l2(m1); + { + std::lock_guard<std::mutex> l3(m1); + std::lock_guard<std::mutex> l4(m1); + // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:37: note: additional 'std::lock_guard' declared here + } + { + std::lock_guard<std::mutex> l2(m1); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l2(m1); + } + } + } + std::lock_guard<std::mutex> l(m1); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l(m1); +} + + +void PositiveInsideArg(std::mutex &m1, std::mutex &m2, std::mutex &m3) { + std::lock_guard<std::mutex> l1(m1); + std::lock_guard<std::mutex> l2(m2); + // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:31: note: additional 'std::lock_guard' declared here + int a = 0; + std::lock_guard<std::mutex> l3(m3); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l3(m3); +} + + +void PositiveInsideConditional() { + std::mutex m1; + if (true) { + std::lock_guard<std::mutex> l1(m1); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l1(m1); + } else { + std::lock_guard<std::mutex> l1(m1); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l1(m1); + } + + while (true) { + std::lock_guard<std::mutex> l1(m1); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l1(m1); + } + + for (int i = 0; i < 10; ++i) { + std::lock_guard<std::mutex> l1(m1); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l1(m1); + } +} + + +template <typename T> +void PositiveTemplated() { + std::mutex m1, m2, m3; + { + std::lock_guard<std::mutex> l(m1); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l(m1); + } + + { + std::lock_guard<std::mutex> l1(m1); + std::lock_guard<std::mutex> l2(m2); + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:33: note: additional 'std::lock_guard' declared here + } + + { + std::lock(m1, m2); + std::lock_guard<std::mutex> l1(m1, std::adopt_lock); + std::lock_guard<std::mutex> l2(m2, std::adopt_lock); + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:33: note: additional 'std::lock_guard' declared here + int a = 0; + std::lock_guard<std::mutex> l3(m3); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l3(m3); + } +} + + +template <typename Mutex> +void PositiveTemplatedMutex() { + Mutex m1, m2, m3; + { + std::lock_guard<Mutex> l(m1); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + } + + { + std::lock_guard<Mutex> l1(m1); + std::lock_guard<Mutex> l2(m2); + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:28: note: additional 'std::lock_guard' declared here + } + + { + std::lock(m1, m2); + std::lock_guard<Mutex> l1(m1, std::adopt_lock); + std::lock_guard<Mutex> l2(m2, std::adopt_lock); + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:28: note: additional 'std::lock_guard' declared here + int a = 0; + std::lock_guard<Mutex> l3(m3); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + } +} + + +template <template <typename> typename Lock> +void NegativeTemplate() { + std::mutex m1, m2; + { + Lock<std::mutex> l(m1); + } + + { + Lock<std::mutex> l1(m1); + Lock<std::mutex> l2(m2); + } +} + +void instantiate() { + NegativeTemplate<std::lock_guard>(); +} + + +struct PositiveClass { + void Positive() { + { + std::lock_guard<std::mutex> l(m1); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l(m1); + } + + { + std::lock_guard<std::mutex> l1(m1); + std::lock_guard<std::mutex> l2(m2); + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:35: note: additional 'std::lock_guard' declared here + } + + { + std::lock(m1, m2); + std::lock_guard<std::mutex> l1(m1, std::adopt_lock); + std::lock_guard<std::mutex> l2(m2, std::adopt_lock); + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:35: note: additional 'std::lock_guard' declared here + int a = 0; + std::lock_guard<std::mutex> l3(m3); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l3(m3); + } + } + + std::mutex m1; + std::mutex m2; + std::mutex m3; +}; + + +template <typename T> +struct PositiveTemplatedClass { + void Positive() { + { + std::lock_guard<std::mutex> l(m1); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l(m1); + } + + { + std::lock(m1, m2); + std::lock_guard<std::mutex> l1(m1, std::adopt_lock); + std::lock_guard<std::mutex> l2(m2, std::adopt_lock); + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:35: note: additional 'std::lock_guard' declared here + int a = 0; + std::lock_guard<std::mutex> l3(m3); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l3(m3); + } + } + + template <typename... Ts> + void TemplatedPositive() { + { + std::lock_guard<std::mutex> l(m1); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l(m1); + } + + { + std::lock(m1, m2); + std::lock_guard<std::mutex> l1(m1, std::adopt_lock); + std::lock_guard<std::mutex> l2(m2, std::adopt_lock); + // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:35: note: additional 'std::lock_guard' declared here + int a = 0; + std::lock_guard<std::mutex> l3(m3); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l3(m3); + } + } + + std::mutex m1; + std::mutex m2; + std::mutex m3; +}; + + +template <typename T> +using Lock = std::lock_guard<T>; +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use 'std::scoped_lock' instead of 'std::lock_guard' +// CHECK-FIXES: using Lock = std::scoped_lock<T>; + +using LockM = std::lock_guard<std::mutex>; +// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use 'std::scoped_lock' instead of 'std::lock_guard' +// CHECK-FIXES: using LockM = std::scoped_lock<std::mutex>; + +typedef std::lock_guard<std::mutex> LockDef; +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use 'std::scoped_lock' instead of 'std::lock_guard' +// CHECK-FIXES: typedef std::scoped_lock<std::mutex> LockDef; + + +void PositiveUsingDecl() { + using std::lock_guard; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: using std::scoped_lock; + + using LockMFun = std::lock_guard<std::mutex>; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: using LockMFun = std::scoped_lock<std::mutex>; + + typedef std::lock_guard<std::mutex> LockDefFun; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: typedef std::scoped_lock<std::mutex> LockDefFun; +} + +template <typename T> +void PositiveUsingDeclTemplate() { + using std::lock_guard; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: using std::scoped_lock; + + std::mutex m; + lock_guard<std::mutex> l(m); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: scoped_lock l(m); + + using LockFunT = std::lock_guard<T>; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: using LockFunT = std::scoped_lock<T>; + + using LockMFunT = std::lock_guard<std::mutex>; + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: using LockMFunT = std::scoped_lock<std::mutex>; + + typedef std::lock_guard<std::mutex> LockDefFunT; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: typedef std::scoped_lock<std::mutex> LockDefFunT; +} + +void NegativeUsingTypedefs() { + std::mutex m; + + { + Lock<std::mutex> l(m); + } + + { + LockM l(m); + } + + { + LockDef l(m); + } +} + +// Non-STD lock_guard. +template <typename Mutex> +struct lock_guard { + lock_guard(Mutex &m) { } + lock_guard(const lock_guard& ) = delete; +}; + +void NegativeNonStdLockGuard() { + std::mutex m; + { + lock_guard<std::mutex> l(m); + } + + { + lock_guard<std::mutex> l1(m); + lock_guard<std::mutex> l2(m); + } +} \ No newline at end of file >From fc9c1b5498624eccb223196d4b2abf322ba384c9 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Mon, 3 Mar 2025 09:28:38 +0300 Subject: [PATCH 2/9] add newlines to test files --- clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/mutex | 2 +- .../use-scope-lock-warn-on-using-and-typedef-false.cpp | 2 +- .../test/clang-tidy/checkers/modernize/use-scoped-lock.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/mutex b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/mutex index 6045342b39d7a..a0030a2b9da0a 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/mutex +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/mutex @@ -30,4 +30,4 @@ struct scoped_lock { } // namespace std -#endif // _MUTEX_ \ No newline at end of file +#endif // _MUTEX_ diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scope-lock-warn-on-using-and-typedef-false.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scope-lock-warn-on-using-and-typedef-false.cpp index a33758fa9c702..77df845756530 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scope-lock-warn-on-using-and-typedef-false.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scope-lock-warn-on-using-and-typedef-false.cpp @@ -28,4 +28,4 @@ void PositiveUsingDeclTemplate() { using LockMFunT = std::lock_guard<std::mutex>; typedef std::lock_guard<std::mutex> LockDefFunT; -} \ No newline at end of file +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp index 1957a4b7daa1f..68201612a973b 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp @@ -369,4 +369,4 @@ void NegativeNonStdLockGuard() { lock_guard<std::mutex> l1(m); lock_guard<std::mutex> l2(m); } -} \ No newline at end of file +} >From f34c9aef97f73f7309f3372956a01c921a50a366 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Mon, 10 Mar 2025 00:12:09 +0300 Subject: [PATCH 3/9] Add UnqualifiedDesugaredType to match more cases --- .../modernize/UseScopedLockCheck.cpp | 45 ++++++++++-------- .../checks/modernize/use-scoped-lock.rst | 8 ++-- .../checkers/modernize/use-scoped-lock.cpp | 47 ++++++++++++++++++- 3 files changed, 74 insertions(+), 26 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp index 5dbb3a2f671e6..73c4cc4906fba 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp @@ -48,7 +48,8 @@ llvm::SmallVector<const VarDecl *> getLockGuardsFromDecl(const DeclStmt *DS) { for (const Decl *Decl : DS->decls()) { if (const auto *VD = dyn_cast<VarDecl>(Decl)) { - const QualType Type = VD->getType().getCanonicalType(); + const QualType Type = + VD->getType().getUnqualifiedType().getCanonicalType(); if (isLockGuard(Type)) { LockGuards.push_back(VD); } @@ -105,19 +106,16 @@ getTemplateLockGuardTypeLoc(const TypeSourceInfo *SourceInfo) { return ElaboratedLoc.getNamedTypeLoc().getAs<TemplateSpecializationTypeLoc>(); } -// Find the exact source range of the 'lock_guard<...>' token -SourceRange getLockGuardTemplateRange(const TypeSourceInfo *SourceInfo) { - const TemplateSpecializationTypeLoc TemplateLoc = - getTemplateLockGuardTypeLoc(SourceInfo); - if (!TemplateLoc) - return {}; +// Find the exact source range of the 'lock_guard' token +SourceRange getLockGuardRange(const TypeSourceInfo *SourceInfo) { + const TypeLoc LockGuardTypeLoc = SourceInfo->getTypeLoc(); - return SourceRange(TemplateLoc.getTemplateNameLoc(), - TemplateLoc.getRAngleLoc()); + return SourceRange(LockGuardTypeLoc.getBeginLoc(), + LockGuardTypeLoc.getEndLoc()); } -// Find the exact source range of the 'lock_guard' token -SourceRange getLockGuardRange(const TypeSourceInfo *SourceInfo) { +// Find the exact source range of the 'lock_guard' name token +SourceRange getLockGuardNameRange(const TypeSourceInfo *SourceInfo) { const TemplateSpecializationTypeLoc TemplateLoc = getTemplateLockGuardTypeLoc(SourceInfo); if (!TemplateLoc) @@ -144,11 +142,16 @@ void UseScopedLockCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { } void UseScopedLockCheck::registerMatchers(MatchFinder *Finder) { - auto LockGuardClassDecl = - namedDecl(anyOf(classTemplateDecl(), classTemplateSpecializationDecl()), - hasName("lock_guard"), isInStdNamespace()); - auto LockGuardType = qualType(hasDeclaration(LockGuardClassDecl)); - auto LockVarDecl = varDecl(hasType(LockGuardType)); + const auto LockGuardClassDecl = + namedDecl(hasName("lock_guard"), isInStdNamespace()); + + const auto LockGuardType = qualType(anyOf( + hasUnqualifiedDesugaredType( + recordType(hasDeclaration(LockGuardClassDecl))), + elaboratedType(namesType(hasUnqualifiedDesugaredType( + templateSpecializationType(hasDeclaration(LockGuardClassDecl))))))); + + const auto LockVarDecl = varDecl(hasType(LockGuardType)); if (WarnOnSingleLocks) { // Match CompoundStmt with only one 'std::lock_guard' @@ -231,7 +234,7 @@ void UseScopedLockCheck::emitDiag(const VarDecl *LockGuard, auto Diag = diag(LockGuard->getBeginLoc(), UseScopedLockMessage); const SourceRange LockGuardTypeRange = - getLockGuardTemplateRange(LockGuard->getTypeSourceInfo()); + getLockGuardRange(LockGuard->getTypeSourceInfo()); if (LockGuardTypeRange.isInvalid()) { return; @@ -245,7 +248,8 @@ void UseScopedLockCheck::emitDiag(const VarDecl *LockGuard, } if (CtorCall->getNumArgs() == 1) { - Diag << FixItHint::CreateReplacement(LockGuardTypeRange, "scoped_lock"); + Diag << FixItHint::CreateReplacement(LockGuardTypeRange, + "std::scoped_lock"); return; } @@ -262,7 +266,7 @@ void UseScopedLockCheck::emitDiag(const VarDecl *LockGuard, CharSourceRange::getTokenRange(AdoptLockArg->getSourceRange()), *Result.SourceManager, Result.Context->getLangOpts()); - Diag << FixItHint::CreateReplacement(LockGuardTypeRange, "scoped_lock") + Diag << FixItHint::CreateReplacement(LockGuardTypeRange, "std::scoped_lock") << FixItHint::CreateReplacement( SourceRange(MutexArg->getBeginLoc(), AdoptLockArg->getEndLoc()), (llvm::Twine(AdoptLockSourceText) + ", " + MutexSourceText) @@ -300,7 +304,8 @@ void UseScopedLockCheck::emitDiag( if (const auto ElaboratedTL = TL.getAs<ElaboratedTypeLoc>()) { auto Diag = diag(ElaboratedTL.getBeginLoc(), UseScopedLockMessage); - const SourceRange LockGuardRange = getLockGuardRange(LockGuardSourceInfo); + const SourceRange LockGuardRange = + getLockGuardNameRange(LockGuardSourceInfo); if (LockGuardRange.isInvalid()) { return; } diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-scoped-lock.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-scoped-lock.rst index 84c896efd6258..02af2481ef251 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-scoped-lock.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-scoped-lock.rst @@ -58,7 +58,7 @@ Limitations ----------- The check will not emit warnings if ``std::lock_guard`` is used implicitly via -``using``, ``typedef`` or as ``template`` parameter: +``template`` parameter: .. code-block:: c++ @@ -68,10 +68,8 @@ The check will not emit warnings if ``std::lock_guard`` is used implicitly via Lock<std::mutex> L(M); // no warning } - void UsingLock() { - using Lock = std::lock_guard<std::mutex>; - std::mutex M; - Lock L(M); // no warning + void instantiate() { + TemplatedLock<std::lock_guard>(); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp index 68201612a973b..32b18b3ddcc7e 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp @@ -336,19 +336,64 @@ void PositiveUsingDeclTemplate() { // CHECK-FIXES: typedef std::scoped_lock<std::mutex> LockDefFunT; } -void NegativeUsingTypedefs() { +void PositiveInUsingTypedefs() { std::mutex m; { Lock<std::mutex> l(m); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l(m); } { LockM l(m); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l(m); } { LockDef l(m); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l(m); + } + + { + std::lock(m, m); + Lock<std::mutex> l1(m, std::adopt_lock); + LockM l2(m, std::adopt_lock); + LockDef l3(m), l4(m); + // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-3]]:11: note: additional 'std::lock_guard' declared here + // CHECK-MESSAGES: :[[@LINE-3]]:13: note: additional 'std::lock_guard' declared here + // CHECK-MESSAGES: :[[@LINE-4]]:20: note: additional 'std::lock_guard' declared here + int a = 0; + LockDef l5(m); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l5(m); + } +} + +template <typename Mutex> +void PositiveInUsingTypedefsTemplated() { + Mutex m; + + { + Lock<Mutex> l(m); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + } + + { + std::lock(m, m); + Lock<Mutex> l1(m, std::adopt_lock); + LockM l2(m, std::adopt_lock); + LockDef l3(m), l4(m); + // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-3]]:11: note: additional 'std::lock_guard' declared here + // CHECK-MESSAGES: :[[@LINE-3]]:13: note: additional 'std::lock_guard' declared here + // CHECK-MESSAGES: :[[@LINE-4]]:20: note: additional 'std::lock_guard' declared here + int a = 0; + LockDef l5(m); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' } } >From be8cfb197295c81559bfd8e0dbace447e1f3678c Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Mon, 10 Mar 2025 00:33:33 +0300 Subject: [PATCH 4/9] add more tests to WarnOnSingleLock opt --- .../clang-tidy/modernize/UseScopedLockCheck.cpp | 6 ++++-- .../use-scoped-lock-warn-on-single-locks-false.cpp | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp index 73c4cc4906fba..27ac70d10d074 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp @@ -281,8 +281,10 @@ void UseScopedLockCheck::emitDiag( const llvm::SmallVector<llvm::SmallVector<const VarDecl *>> &LockGroups, const ast_matchers::MatchFinder::MatchResult &Result) { for (const llvm::SmallVector<const VarDecl *> &Group : LockGroups) { - if (Group.size() == 1 && WarnOnSingleLocks) { - emitDiag(Group[0], Result); + if (Group.size() == 1) { + if (WarnOnSingleLocks) { + emitDiag(Group[0], Result); + } } else { diag(Group[0]->getBeginLoc(), "use single 'std::scoped_lock' instead of multiple " diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock-warn-on-single-locks-false.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock-warn-on-single-locks-false.cpp index 7a59dda18f941..7b30cdd75c801 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock-warn-on-single-locks-false.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock-warn-on-single-locks-false.cpp @@ -41,6 +41,12 @@ void Negative() { { std::lock_guard<std::mutex> l(m, std::adopt_lock); } + + { + std::lock_guard<std::mutex> l3(m); + int a = 0; + std::lock_guard<std::mutex> l4(m, std::adopt_lock); + } } void PositiveInsideArg(std::mutex &m1, std::mutex &m2, std::mutex &m3) { >From a7fec18b8dc95cdcd19a44ce68d063e20588efd8 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Thu, 13 Mar 2025 21:11:55 +0300 Subject: [PATCH 5/9] add lambda to tests --- .../checkers/modernize/use-scoped-lock.cpp | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp index 32b18b3ddcc7e..a6b34560f4c1d 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-scoped-lock.cpp @@ -124,6 +124,54 @@ void PositiveInsideConditional() { } } +void PositiveLambda() { + std::mutex m; + std::lock_guard<std::mutex> l1(m); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l1(m); + auto lambda1 = [&]() { + std::lock_guard<std::mutex> l1(m); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l1(m); + }; + + std::lock_guard<std::mutex> l3(m); + std::lock_guard<std::mutex> l4(m); + // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:31: note: additional 'std::lock_guard' declared here + auto lamda2 = [&]() { + std::lock_guard<std::mutex> l3(m); + std::lock_guard<std::mutex> l4(m); + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:33: note: additional 'std::lock_guard' declared here + }; + + auto lamda3 = [&]() { + std::lock(m, m); + std::lock_guard<std::mutex> l1(m, std::adopt_lock); + std::lock_guard<std::mutex> l2(m, std::adopt_lock); + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + // CHECK-MESSAGES: :[[@LINE-2]]:33: note: additional 'std::lock_guard' declared here + int a = 0; + std::lock_guard<std::mutex> l3(m); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l3(m); + int b = 0; + std::lock_guard<std::mutex> l4(m, std::adopt_lock); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l4(std::adopt_lock, m); + }; + + auto lamda4 = [&]() { + std::lock_guard<std::mutex> l1(m); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l1(m); + int a = 0; + std::lock_guard<std::mutex> l2(m); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'std::scoped_lock' instead of 'std::lock_guard' + // CHECK-FIXES: std::scoped_lock l2(m); + }; +} template <typename T> void PositiveTemplated() { >From c69993e568af26c015273bd9a6be62c2f5b825d2 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Sat, 15 Mar 2025 12:11:35 +0300 Subject: [PATCH 6/9] made more readable matchers --- .../modernize/UseScopedLockCheck.cpp | 58 ++++++++++++++----- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp index 27ac70d10d074..16b0ea1c63f3b 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp @@ -125,6 +125,42 @@ SourceRange getLockGuardNameRange(const TypeSourceInfo *SourceInfo) { TemplateLoc.getLAngleLoc().getLocWithOffset(-1)); } +AST_MATCHER_P(CompoundStmt, hasMultiple, ast_matchers::internal::Matcher<Stmt>, + InnerMatcher) { + size_t Count = 0; + + for (const Stmt *Stmt : Node.body()) { + if (InnerMatcher.matches(*Stmt, Finder, Builder)) + Count++; + } + + return Count > 1; +} + +AST_MATCHER_P(CompoundStmt, hasSingle, ast_matchers::internal::Matcher<Stmt>, + InnerMatcher) { + size_t Count = 0; + ast_matchers::internal::BoundNodesTreeBuilder Result; + + for (const Stmt *Stmt : Node.body()) { + ast_matchers::internal::BoundNodesTreeBuilder TB(*Builder); + if (InnerMatcher.matches(*Stmt, Finder, &TB)) { + Count++; + if (Count == 1) + Result.addMatch(TB); + } + } + + if (Count > 1) { + Builder->removeBindings( + [](const ast_matchers::internal::BoundNodesMap &) { return true; }); + return false; + } + + *Builder = std::move(Result); + return true; +} + const StringRef UseScopedLockMessage = "use 'std::scoped_lock' instead of 'std::lock_guard'"; @@ -154,25 +190,17 @@ void UseScopedLockCheck::registerMatchers(MatchFinder *Finder) { const auto LockVarDecl = varDecl(hasType(LockGuardType)); if (WarnOnSingleLocks) { - // Match CompoundStmt with only one 'std::lock_guard' Finder->addMatcher( - compoundStmt(unless(isExpansionInSystemHeader()), - has(declStmt(has(LockVarDecl.bind("lock-decl-single")))), - unless(hasDescendant(declStmt(has(varDecl( - hasType(LockGuardType), - unless(equalsBoundNode("lock-decl-single")))))))), + compoundStmt( + unless(isExpansionInSystemHeader()), + hasSingle(declStmt(has(LockVarDecl.bind("lock-decl-single"))))), this); } - // Match CompoundStmt with multiple 'std::lock_guard' - Finder->addMatcher( - compoundStmt(unless(isExpansionInSystemHeader()), - has(declStmt(has(LockVarDecl.bind("lock-decl-multiple")))), - hasDescendant(declStmt(has(varDecl( - hasType(LockGuardType), - unless(equalsBoundNode("lock-decl-multiple"))))))) - .bind("block-multiple"), - this); + Finder->addMatcher(compoundStmt(unless(isExpansionInSystemHeader()), + hasMultiple(declStmt(has(LockVarDecl)))) + .bind("block-multiple"), + this); if (WarnOnUsingAndTypedef) { // Match 'typedef std::lock_guard<std::mutex> Lock' >From 2f3d5d33fed37731f87e4ddd22153ab359f61c4a Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Mon, 17 Mar 2025 10:48:48 +0300 Subject: [PATCH 7/9] clean unnecessary braces --- .../modernize/UseScopedLockCheck.cpp | 34 ++++++------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp index 16b0ea1c63f3b..c5d3b25554fdf 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp @@ -26,19 +26,14 @@ namespace clang::tidy::modernize { namespace { bool isLockGuard(const QualType &Type) { - if (const auto *Record = Type->getAs<RecordType>()) { - if (const RecordDecl *Decl = Record->getDecl()) { + if (const auto *Record = Type->getAs<RecordType>()) + if (const RecordDecl *Decl = Record->getDecl()) return Decl->getName() == "lock_guard" && Decl->isInStdNamespace(); - } - } - if (const auto *TemplateSpecType = - Type->getAs<TemplateSpecializationType>()) { + if (const auto *TemplateSpecType = Type->getAs<TemplateSpecializationType>()) if (const TemplateDecl *Decl = - TemplateSpecType->getTemplateName().getAsTemplateDecl()) { + TemplateSpecType->getTemplateName().getAsTemplateDecl()) return Decl->getName() == "lock_guard" && Decl->isInStdNamespace(); - } - } return false; } @@ -50,9 +45,8 @@ llvm::SmallVector<const VarDecl *> getLockGuardsFromDecl(const DeclStmt *DS) { if (const auto *VD = dyn_cast<VarDecl>(Decl)) { const QualType Type = VD->getType().getUnqualifiedType().getCanonicalType(); - if (isLockGuard(Type)) { + if (isLockGuard(Type)) LockGuards.push_back(VD); - } } } @@ -129,10 +123,9 @@ AST_MATCHER_P(CompoundStmt, hasMultiple, ast_matchers::internal::Matcher<Stmt>, InnerMatcher) { size_t Count = 0; - for (const Stmt *Stmt : Node.body()) { + for (const Stmt *Stmt : Node.body()) if (InnerMatcher.matches(*Stmt, Finder, Builder)) Count++; - } return Count > 1; } @@ -264,16 +257,14 @@ void UseScopedLockCheck::emitDiag(const VarDecl *LockGuard, const SourceRange LockGuardTypeRange = getLockGuardRange(LockGuard->getTypeSourceInfo()); - if (LockGuardTypeRange.isInvalid()) { + if (LockGuardTypeRange.isInvalid()) return; - } // Create Fix-its only if we can find the constructor call to properly handle // 'std::lock_guard l(m, std::adopt_lock)' case. const auto *CtorCall = dyn_cast<CXXConstructExpr>(LockGuard->getInit()); - if (!CtorCall) { + if (!CtorCall) return; - } if (CtorCall->getNumArgs() == 1) { Diag << FixItHint::CreateReplacement(LockGuardTypeRange, @@ -310,18 +301,16 @@ void UseScopedLockCheck::emitDiag( const ast_matchers::MatchFinder::MatchResult &Result) { for (const llvm::SmallVector<const VarDecl *> &Group : LockGroups) { if (Group.size() == 1) { - if (WarnOnSingleLocks) { + if (WarnOnSingleLocks) emitDiag(Group[0], Result); - } } else { diag(Group[0]->getBeginLoc(), "use single 'std::scoped_lock' instead of multiple " "'std::lock_guard'"); - for (const VarDecl *Lock : llvm::drop_begin(Group)) { + for (const VarDecl *Lock : llvm::drop_begin(Group)) diag(Lock->getLocation(), "additional 'std::lock_guard' declared here", DiagnosticIDs::Note); - } } } } @@ -336,9 +325,8 @@ void UseScopedLockCheck::emitDiag( const SourceRange LockGuardRange = getLockGuardNameRange(LockGuardSourceInfo); - if (LockGuardRange.isInvalid()) { + if (LockGuardRange.isInvalid()) return; - } Diag << FixItHint::CreateReplacement(LockGuardRange, "scoped_lock"); } >From d55d427e4731fd2e6fb8b3b770c7685d15545cb6 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Mon, 17 Mar 2025 11:01:27 +0300 Subject: [PATCH 8/9] make diag function more readable --- .../modernize/UseScopedLockCheck.cpp | 22 +++++++++---------- .../clang-tidy/modernize/UseScopedLockCheck.h | 14 ++++++------ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp index c5d3b25554fdf..144b4b09bbde6 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp @@ -222,36 +222,36 @@ void UseScopedLockCheck::registerMatchers(MatchFinder *Finder) { void UseScopedLockCheck::check(const MatchFinder::MatchResult &Result) { if (const auto *Decl = Result.Nodes.getNodeAs<VarDecl>("lock-decl-single")) { - emitDiag(Decl, Result); + diagOnSingleLock(Decl, Result); return; } if (const auto *Compound = Result.Nodes.getNodeAs<CompoundStmt>("block-multiple")) { - emitDiag(findLocksInCompoundStmt(Compound, Result), Result); + diagOnMultipleLocks(findLocksInCompoundStmt(Compound, Result), Result); return; } if (const auto *Typedef = Result.Nodes.getNodeAs<TypedefDecl>("lock-guard-typedef")) { - emitDiag(Typedef->getTypeSourceInfo(), Result); + diagOnSourceInfo(Typedef->getTypeSourceInfo(), Result); return; } if (const auto *UsingAlias = Result.Nodes.getNodeAs<TypeAliasDecl>("lock-guard-using-alias")) { - emitDiag(UsingAlias->getTypeSourceInfo(), Result); + diagOnSourceInfo(UsingAlias->getTypeSourceInfo(), Result); return; } if (const auto *Using = Result.Nodes.getNodeAs<UsingDecl>("lock-guard-using-decl")) { - emitDiag(Using, Result); + diagOnUsingDecl(Using, Result); } } -void UseScopedLockCheck::emitDiag(const VarDecl *LockGuard, - const MatchFinder::MatchResult &Result) { +void UseScopedLockCheck::diagOnSingleLock( + const VarDecl *LockGuard, const MatchFinder::MatchResult &Result) { auto Diag = diag(LockGuard->getBeginLoc(), UseScopedLockMessage); const SourceRange LockGuardTypeRange = @@ -296,13 +296,13 @@ void UseScopedLockCheck::emitDiag(const VarDecl *LockGuard, llvm_unreachable("Invalid argument number of std::lock_guard constructor"); } -void UseScopedLockCheck::emitDiag( +void UseScopedLockCheck::diagOnMultipleLocks( const llvm::SmallVector<llvm::SmallVector<const VarDecl *>> &LockGroups, const ast_matchers::MatchFinder::MatchResult &Result) { for (const llvm::SmallVector<const VarDecl *> &Group : LockGroups) { if (Group.size() == 1) { if (WarnOnSingleLocks) - emitDiag(Group[0], Result); + diagOnSingleLock(Group[0], Result); } else { diag(Group[0]->getBeginLoc(), "use single 'std::scoped_lock' instead of multiple " @@ -315,7 +315,7 @@ void UseScopedLockCheck::emitDiag( } } -void UseScopedLockCheck::emitDiag( +void UseScopedLockCheck::diagOnSourceInfo( const TypeSourceInfo *LockGuardSourceInfo, const ast_matchers::MatchFinder::MatchResult &Result) { const TypeLoc TL = LockGuardSourceInfo->getTypeLoc(); @@ -332,7 +332,7 @@ void UseScopedLockCheck::emitDiag( } } -void UseScopedLockCheck::emitDiag( +void UseScopedLockCheck::diagOnUsingDecl( const UsingDecl *UsingDecl, const ast_matchers::MatchFinder::MatchResult &Result) { diag(UsingDecl->getLocation(), UseScopedLockMessage) diff --git a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.h b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.h index 790d9205c6121..a5697805c15ca 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.h @@ -35,15 +35,15 @@ class UseScopedLockCheck : public ClangTidyCheck { } private: - void emitDiag(const VarDecl *LockGuard, - const ast_matchers::MatchFinder::MatchResult &Result); - void emitDiag( + void diagOnSingleLock(const VarDecl *LockGuard, + const ast_matchers::MatchFinder::MatchResult &Result); + void diagOnMultipleLocks( const llvm::SmallVector<llvm::SmallVector<const VarDecl *>> &LockGroups, const ast_matchers::MatchFinder::MatchResult &Result); - void emitDiag(const TypeSourceInfo *LockGuardSourceInfo, - const ast_matchers::MatchFinder::MatchResult &Result); - void emitDiag(const UsingDecl *UsingDecl, - const ast_matchers::MatchFinder::MatchResult &Result); + void diagOnSourceInfo(const TypeSourceInfo *LockGuardSourceInfo, + const ast_matchers::MatchFinder::MatchResult &Result); + void diagOnUsingDecl(const UsingDecl *UsingDecl, + const ast_matchers::MatchFinder::MatchResult &Result); const bool WarnOnSingleLocks; const bool WarnOnUsingAndTypedef; >From 53a0781cf6e2d663b7770ed486ae6ca0ecb6c652 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Wed, 19 Mar 2025 19:54:33 +0300 Subject: [PATCH 9/9] fix comments --- .../modernize/UseScopedLockCheck.cpp | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp index 144b4b09bbde6..0e59338a6520e 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp @@ -17,34 +17,36 @@ #include "clang/Lex/Lexer.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/Twine.h" -#include <iterator> using namespace clang::ast_matchers; namespace clang::tidy::modernize { -namespace { - -bool isLockGuard(const QualType &Type) { +static bool isLockGuard(const QualType &Type) { if (const auto *Record = Type->getAs<RecordType>()) - if (const RecordDecl *Decl = Record->getDecl()) + if (const RecordDecl *Decl = Record->getDecl()) { + assert(Decl->getDeclName().isIdentifier() && "Decl is not identifier"); return Decl->getName() == "lock_guard" && Decl->isInStdNamespace(); + } if (const auto *TemplateSpecType = Type->getAs<TemplateSpecializationType>()) if (const TemplateDecl *Decl = - TemplateSpecType->getTemplateName().getAsTemplateDecl()) + TemplateSpecType->getTemplateName().getAsTemplateDecl()) { + assert(Decl->getDeclName().isIdentifier() && "Decl is not identifier"); return Decl->getName() == "lock_guard" && Decl->isInStdNamespace(); + } return false; } -llvm::SmallVector<const VarDecl *> getLockGuardsFromDecl(const DeclStmt *DS) { +static llvm::SmallVector<const VarDecl *> +getLockGuardsFromDecl(const DeclStmt *DS) { llvm::SmallVector<const VarDecl *> LockGuards; for (const Decl *Decl : DS->decls()) { if (const auto *VD = dyn_cast<VarDecl>(Decl)) { const QualType Type = - VD->getType().getUnqualifiedType().getCanonicalType(); + VD->getType().getCanonicalType().getUnqualifiedType(); if (isLockGuard(Type)) LockGuards.push_back(VD); } @@ -55,7 +57,7 @@ llvm::SmallVector<const VarDecl *> getLockGuardsFromDecl(const DeclStmt *DS) { // Scans through the statements in a block and groups consecutive // 'std::lock_guard' variable declarations together. -llvm::SmallVector<llvm::SmallVector<const VarDecl *>> +static llvm::SmallVector<llvm::SmallVector<const VarDecl *>> findLocksInCompoundStmt(const CompoundStmt *Block, const ast_matchers::MatchFinder::MatchResult &Result) { // store groups of consecutive 'std::lock_guard' declarations @@ -74,10 +76,7 @@ findLocksInCompoundStmt(const CompoundStmt *Block, llvm::SmallVector<const VarDecl *> LockGuards = getLockGuardsFromDecl(DS); if (!LockGuards.empty()) { - CurrentLockGuardGroup.insert( - CurrentLockGuardGroup.end(), - std::make_move_iterator(LockGuards.begin()), - std::make_move_iterator(LockGuards.end())); + CurrentLockGuardGroup.append(LockGuards); continue; } } @@ -89,7 +88,7 @@ findLocksInCompoundStmt(const CompoundStmt *Block, return LockGuardGroups; } -TemplateSpecializationTypeLoc +static TemplateSpecializationTypeLoc getTemplateLockGuardTypeLoc(const TypeSourceInfo *SourceInfo) { const TypeLoc Loc = SourceInfo->getTypeLoc(); @@ -101,7 +100,7 @@ getTemplateLockGuardTypeLoc(const TypeSourceInfo *SourceInfo) { } // Find the exact source range of the 'lock_guard' token -SourceRange getLockGuardRange(const TypeSourceInfo *SourceInfo) { +static SourceRange getLockGuardRange(const TypeSourceInfo *SourceInfo) { const TypeLoc LockGuardTypeLoc = SourceInfo->getTypeLoc(); return SourceRange(LockGuardTypeLoc.getBeginLoc(), @@ -109,7 +108,7 @@ SourceRange getLockGuardRange(const TypeSourceInfo *SourceInfo) { } // Find the exact source range of the 'lock_guard' name token -SourceRange getLockGuardNameRange(const TypeSourceInfo *SourceInfo) { +static SourceRange getLockGuardNameRange(const TypeSourceInfo *SourceInfo) { const TemplateSpecializationTypeLoc TemplateLoc = getTemplateLockGuardTypeLoc(SourceInfo); if (!TemplateLoc) @@ -119,6 +118,8 @@ SourceRange getLockGuardNameRange(const TypeSourceInfo *SourceInfo) { TemplateLoc.getLAngleLoc().getLocWithOffset(-1)); } +namespace { + AST_MATCHER_P(CompoundStmt, hasMultiple, ast_matchers::internal::Matcher<Stmt>, InnerMatcher) { size_t Count = 0; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits