https://github.com/OmarAzizi updated https://github.com/llvm/llvm-project/pull/207604
>From 88fc084c01a661e276fd50ca932bb108a2045a89 Mon Sep 17 00:00:00 2001 From: OmarAzizi <[email protected]> Date: Sun, 5 Jul 2026 21:47:18 +0300 Subject: [PATCH 1/3] [clang-tidy] Support C++26 placeholder bindings --- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../modernize/ModernizeTidyModule.cpp | 3 + .../modernize/UsePlaceholderBindingCheck.cpp | 88 ++++++++++ .../modernize/UsePlaceholderBindingCheck.h | 35 ++++ clang-tools-extra/docs/ReleaseNotes.rst | 7 + .../docs/clang-tidy/checks/list.rst | 1 + .../modernize/use-placeholder-binding.rst | 47 +++++ .../modernize/use-placeholder-binding.cpp | 163 ++++++++++++++++++ 8 files changed, 345 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/use-placeholder-binding.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-placeholder-binding.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 2c5c44db587fe..dca3abd63756f 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -44,6 +44,7 @@ add_clang_library(clangTidyModernizeModule STATIC UseNoexceptCheck.cpp UseNullptrCheck.cpp UseOverrideCheck.cpp + UsePlaceholderBindingCheck.cpp UseRangesCheck.cpp UseScopedLockCheck.cpp UseStartsEndsWithCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index cc13da7535bcb..32241ea319b78 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -44,6 +44,7 @@ #include "UseNoexceptCheck.h" #include "UseNullptrCheck.h" #include "UseOverrideCheck.h" +#include "UsePlaceholderBindingCheck.h" #include "UseRangesCheck.h" #include "UseScopedLockCheck.h" #include "UseStartsEndsWithCheck.h" @@ -92,6 +93,8 @@ class ModernizeModule : public ClangTidyModule { "modernize-use-designated-initializers"); CheckFactories.registerCheck<UseIntegerSignComparisonCheck>( "modernize-use-integer-sign-comparison"); + CheckFactories.registerCheck<UsePlaceholderBindingCheck>( + "modernize-use-placeholder-binding"); CheckFactories.registerCheck<UseRangesCheck>("modernize-use-ranges"); CheckFactories.registerCheck<UseScopedLockCheck>( "modernize-use-scoped-lock"); diff --git a/clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.cpp new file mode 100644 index 0000000000000..8411b4396ff53 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.cpp @@ -0,0 +1,88 @@ +//===----------------------------------------------------------------------===// +// +// 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 "UsePlaceholderBindingCheck.h" +#include "../utils/DeclRefExprUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +namespace { +AST_POLYMORPHIC_MATCHER(isInMacro, + AST_POLYMORPHIC_SUPPORTED_TYPES(Stmt, Decl)) { + return Node.getBeginLoc().isMacroID() || Node.getEndLoc().isMacroID(); +} + +AST_MATCHER(VarDecl, hasAutomaticStorageDurationAndNoSpecifiers) { + return !Node.isStaticLocal() && !Node.isConstexpr() && !Node.hasAttrs() && + Node.getStorageClass() == SC_None && + Node.getTSCSpec() == TSCS_unspecified; +} +} // namespace + +void UsePlaceholderBindingCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + decompositionDecl(unless(isInMacro()), + hasAutomaticStorageDurationAndNoSpecifiers(), + hasAncestor(compoundStmt().bind("scope"))) + .bind("decomp"), + this); +} + +void UsePlaceholderBindingCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Decomp = Result.Nodes.getNodeAs<DecompositionDecl>("decomp"); + const auto *Scope = Result.Nodes.getNodeAs<CompoundStmt>("scope"); + ASTContext &Context = *Result.Context; + + for (const BindingDecl *Binding : Decomp->bindings()) { + if (Binding->isParameterPack() || + Binding->isPlaceholderVar(Context.getLangOpts())) + continue; + + llvm::SmallPtrSet<const DeclRefExpr *, 16> Refs = + utils::decl_ref_expr::allDeclRefExprs(*Binding, *Scope, Context); + if (Refs.size() != 1) + continue; + + const DeclRefExpr *Ref = *Refs.begin(); + + const DynTypedNodeList RefParents = Context.getParents(*Ref); + if (RefParents.size() != 1) + continue; + const auto *Cast = + dyn_cast_or_null<CStyleCastExpr>(RefParents[0].get<Expr>()); + if (!Cast || !Cast->getType()->isVoidType() || + Cast->getSubExpr()->IgnoreParens() != Ref || + Cast->getBeginLoc().isMacroID() || Cast->getEndLoc().isMacroID()) + continue; + + const DynTypedNodeList CastParents = Context.getParents(*Cast); + if (CastParents.size() != 1 || !CastParents[0].get<CompoundStmt>()) + continue; + + const SourceLocation SemiLoc = Lexer::findLocationAfterToken( + Cast->getEndLoc(), tok::semi, Context.getSourceManager(), + Context.getLangOpts(), /*SkipTrailingWhitespaceAndNewLine=*/true); + if (SemiLoc.isInvalid()) + continue; + + diag(Binding->getLocation(), + "binding %0 is only used to suppress an unused variable warning; " + "use a placeholder '_' instead") + << Binding + << FixItHint::CreateReplacement(Binding->getSourceRange(), "_") + << FixItHint::CreateRemoval( + CharSourceRange::getCharRange(Cast->getBeginLoc(), SemiLoc)); + } +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.h b/clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.h new file mode 100644 index 0000000000000..4fff939f99138 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.h @@ -0,0 +1,35 @@ +//===----------------------------------------------------------------------===// +// +// 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_USEPLACEHOLDERBINDINGCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEPLACEHOLDERBINDINGCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::modernize { + +/// Finds structured bindings where a binding is only used to suppress an +/// "unused variable" warning via a ``(void)name;`` statement, and suggests +/// replacing the binding with a C++26 placeholder (``_``) instead. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-placeholder-binding.html +class UsePlaceholderBindingCheck : public ClangTidyCheck { +public: + UsePlaceholderBindingCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus26; + } +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEPLACEHOLDERBINDINGCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 68f0fa568dbf9..018d0cdda8fba 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -265,6 +265,13 @@ New checks Finds cyclical initialization of static variables. +- New :doc:`modernize-use-placeholder-binding + <clang-tidy/checks/modernize/use-placeholder-binding>` check. + + Finds structured bindings where a binding is only used to suppress an + unused variable warning and suggests replacing it with a C++26 + placeholder (``_``). + - New :doc:`modernize-use-std-bit <clang-tidy/checks/modernize/use-std-bit>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 2a44dc78fbc89..4e1f98f29c08a 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -325,6 +325,7 @@ Clang-Tidy Checks :doc:`modernize-use-noexcept <modernize/use-noexcept>`, "Yes" :doc:`modernize-use-nullptr <modernize/use-nullptr>`, "Yes" :doc:`modernize-use-override <modernize/use-override>`, "Yes" + :doc:`modernize-use-placeholder-binding <modernize/use-placeholder-binding>`, "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" diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-placeholder-binding.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-placeholder-binding.rst new file mode 100644 index 0000000000000..0f847454e30ae --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-placeholder-binding.rst @@ -0,0 +1,47 @@ +.. title:: clang-tidy - modernize-use-placeholder-binding + +modernize-use-placeholder-binding +================================== + +Finds structured bindings where one of the bindings is only used to suppress +an "unused variable" warning via a ``(void)name;`` statement, and suggests +replacing that binding with a C++26 placeholder (``_``) instead, removing the +now unnecessary suppression statement. + +.. code-block:: c++ + + const auto [x, y] = compute(); + (void)y; + use(x); + + // becomes + + const auto [x, _] = compute(); + use(x); + +This also applies to structured bindings declared in the init-statement of an +``if``, ``switch``, or ``for`` statement: + +.. code-block:: c++ + + if (const auto [it, inserted] = set.insert(value); inserted) { + (void)it; + // ... + } + + // becomes + + if (const auto [_, inserted] = set.insert(value); inserted) { + // ... + } + +Limitations +----------- + +The check only recognizes the ``(void)name;`` suppression idiom as a +standalone statement directly inside a compound statement. It does not +diagnose bindings that are simply unused with no suppression statement, nor +bindings suppressed some other way. It also does not diagnose a suppression +cast that is the sole substatement of a ``case`` or ``default`` label (for +example, inside a ``switch``), since removing it safely would require +special-casing the label rather than the statement. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-placeholder-binding.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-placeholder-binding.cpp new file mode 100644 index 0000000000000..943b873c63023 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-placeholder-binding.cpp @@ -0,0 +1,163 @@ +// RUN: %check_clang_tidy -std=c++26-or-later %s modernize-use-placeholder-binding %t + +struct Pair { + int first; + bool second; +}; + +struct Triple { + int a, b, c; +}; + +Pair getPair(); +Triple getTriple(); +void use(int); + +void suppressedBindingIsReplaced() { + const auto [x, y] = getPair(); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: binding 'y' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] + // CHECK-FIXES: const auto [x, _] = getPair(); + (void)y; + // CHECK-FIXES-NOT: (void)y; + use(x); +} + +void ifInitBindingIsReplaced() { + if (const auto [it, inserted] = getPair(); inserted) { + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: binding 'it' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] + // CHECK-FIXES: if (const auto [_, inserted] = getPair(); inserted) { + (void)it; + // CHECK-FIXES-NOT: (void)it; + } +} + +void bothBindingsUsed() { + const auto [x, y] = getPair(); + use(x); + if (y) + use(x); +} + +void noSuppressionCast() { + const auto [x, y] = getPair(); + use(x); +} + +void alreadyPlaceholder() { + const auto [x, _] = getPair(); + use(x); +} + +void bothBindingsSuppressed() { + auto [x, y] = getPair(); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: binding 'x' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] + // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: binding 'y' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] + // CHECK-FIXES: auto [_, _] = getPair(); + (void)x; + // CHECK-FIXES-NOT: (void)x; + (void)y; + // CHECK-FIXES-NOT: (void)y; +} + +void threeBindings() { + auto [a, b, c] = getTriple(); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: binding 'b' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] + // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: binding 'c' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] + // CHECK-FIXES: auto [a, _, _] = getTriple(); + (void)b; + (void)c; + use(a); +} + +void referenceBinding() { + Pair p{}; + auto &[x, y] = p; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: binding 'y' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] + // CHECK-FIXES: auto &[x, _] = p; + (void)y; + use(x); +} + +struct Item { + int key; + int value; +}; + +void forRangeBindingIsReplaced() { + Item items[1] = {}; + for (auto [k, v] : items) { + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: binding 'v' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] + // CHECK-FIXES: for (auto [k, _] : items) { + (void)v; + // CHECK-FIXES-NOT: (void)v; + use(k); + } +} + +void switchCaseSuppressionIsNotDiagnosed() { + switch (auto [a, b] = getPair(); a) { + case 0: + (void)b; + break; + } +} + +void lambdaCaptureIsUnaffected() { + auto [x, y] = getPair(); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: binding 'y' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] + // CHECK-FIXES: auto [x, _] = getPair(); + (void)y; + // CHECK-FIXES-NOT: (void)y; + auto l = [x] { use(x); }; + l(); +} + +#define SUPPRESS(x) (void)(x) +void macroSuppressionIsNotDiagnosed() { + auto [x, y] = getPair(); + SUPPRESS(y); + use(x); +} + +void multipleDecompositionsInSameScope() { + auto [x1, y1] = getPair(); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: binding 'y1' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] + // CHECK-FIXES: auto [x1, _] = getPair(); + (void)y1; + // CHECK-FIXES-NOT: (void)y1; + use(x1); + + auto [x2, y2] = getPair(); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: binding 'x2' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] + // CHECK-FIXES: auto [_, y2] = getPair(); + (void)x2; + // CHECK-FIXES-NOT: (void)x2; + use(y2 ? 1 : 0); +} + +Pair &&getPairRef(); + +void rvalueReferenceBinding() { + auto &&[x, y] = getPairRef(); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: binding 'y' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] + // CHECK-FIXES: auto &&[x, _] = getPairRef(); + (void)y; + // CHECK-FIXES-NOT: (void)y; + use(x); + // CHECK-FIXES: use(x); +} + +void staticBindingIsNotDiagnosed() { + static auto [x1, y1] = getPair(); + (void)y1; + static auto [x2, y2] = getPair(); + (void)y2; + use(x1); + use(x2); +} + +void threadLocalBindingIsNotDiagnosed() { + thread_local auto [x, y] = getPair(); + (void)y; + use(x); +} >From 2ce4174e35683659933950e1f270ee5f95aeadd7 Mon Sep 17 00:00:00 2001 From: OmarAzizi <[email protected]> Date: Sun, 5 Jul 2026 22:39:30 +0300 Subject: [PATCH 2/3] [clang-tidy] fix linter error in the CI --- .../clang-tidy/modernize/UsePlaceholderBindingCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.cpp index 8411b4396ff53..95282b6d84530 100644 --- a/clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.cpp @@ -48,7 +48,7 @@ void UsePlaceholderBindingCheck::check(const MatchFinder::MatchResult &Result) { Binding->isPlaceholderVar(Context.getLangOpts())) continue; - llvm::SmallPtrSet<const DeclRefExpr *, 16> Refs = + const llvm::SmallPtrSet<const DeclRefExpr *, 16> Refs = utils::decl_ref_expr::allDeclRefExprs(*Binding, *Scope, Context); if (Refs.size() != 1) continue; >From 5d38a3ac648bde83d12ed6e5e39c38d5347cfdda Mon Sep 17 00:00:00 2001 From: OmarAzizi <[email protected]> Date: Wed, 8 Jul 2026 14:26:34 +0300 Subject: [PATCH 3/3] [clang-tidy] Address review feedback on UsePlaceholderBindingCheck Drop redundant matcher conditions, move suppression-cast matching into the AST matcher, support case/default labels, fix doc nits. --- .../modernize/UsePlaceholderBindingCheck.cpp | 76 ++++++++----------- clang-tools-extra/docs/ReleaseNotes.rst | 7 +- .../modernize/use-placeholder-binding.rst | 30 ++++++-- .../modernize/use-placeholder-binding.cpp | 15 +++- 4 files changed, 73 insertions(+), 55 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.cpp index 95282b6d84530..c0379a5e34acc 100644 --- a/clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UsePlaceholderBindingCheck.cpp @@ -23,66 +23,54 @@ AST_POLYMORPHIC_MATCHER(isInMacro, } AST_MATCHER(VarDecl, hasAutomaticStorageDurationAndNoSpecifiers) { - return !Node.isStaticLocal() && !Node.isConstexpr() && !Node.hasAttrs() && - Node.getStorageClass() == SC_None && + return !Node.hasAttrs() && Node.getStorageClass() == SC_None && Node.getTSCSpec() == TSCS_unspecified; } } // namespace void UsePlaceholderBindingCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( - decompositionDecl(unless(isInMacro()), - hasAutomaticStorageDurationAndNoSpecifiers(), - hasAncestor(compoundStmt().bind("scope"))) - .bind("decomp"), + cStyleCastExpr( + unless(isInMacro()), hasType(voidType()), + hasParent(stmt(anyOf(compoundStmt(), switchCase()))), + hasSourceExpression(ignoringParens(declRefExpr( + unless(isInMacro()), + to(bindingDecl(forDecomposition(varDecl( + unless(isInMacro()), + hasAutomaticStorageDurationAndNoSpecifiers(), + hasAncestor(compoundStmt().bind("scope"))))) + .bind("binding")))))) + .bind("cast"), this); } void UsePlaceholderBindingCheck::check(const MatchFinder::MatchResult &Result) { - const auto *Decomp = Result.Nodes.getNodeAs<DecompositionDecl>("decomp"); + const auto *Cast = Result.Nodes.getNodeAs<CStyleCastExpr>("cast"); + const auto *Binding = Result.Nodes.getNodeAs<BindingDecl>("binding"); const auto *Scope = Result.Nodes.getNodeAs<CompoundStmt>("scope"); ASTContext &Context = *Result.Context; - for (const BindingDecl *Binding : Decomp->bindings()) { - if (Binding->isParameterPack() || - Binding->isPlaceholderVar(Context.getLangOpts())) - continue; + if (Binding->isParameterPack() || + Binding->isPlaceholderVar(Context.getLangOpts())) + return; - const llvm::SmallPtrSet<const DeclRefExpr *, 16> Refs = - utils::decl_ref_expr::allDeclRefExprs(*Binding, *Scope, Context); - if (Refs.size() != 1) - continue; + const llvm::SmallPtrSet<const DeclRefExpr *, 16> Refs = + utils::decl_ref_expr::allDeclRefExprs(*Binding, *Scope, Context); + if (Refs.size() != 1) + return; - const DeclRefExpr *Ref = *Refs.begin(); + const SourceLocation SemiLoc = Lexer::findLocationAfterToken( + Cast->getEndLoc(), tok::semi, Context.getSourceManager(), + Context.getLangOpts(), /*SkipTrailingWhitespaceAndNewLine=*/true); + if (SemiLoc.isInvalid()) + return; - const DynTypedNodeList RefParents = Context.getParents(*Ref); - if (RefParents.size() != 1) - continue; - const auto *Cast = - dyn_cast_or_null<CStyleCastExpr>(RefParents[0].get<Expr>()); - if (!Cast || !Cast->getType()->isVoidType() || - Cast->getSubExpr()->IgnoreParens() != Ref || - Cast->getBeginLoc().isMacroID() || Cast->getEndLoc().isMacroID()) - continue; - - const DynTypedNodeList CastParents = Context.getParents(*Cast); - if (CastParents.size() != 1 || !CastParents[0].get<CompoundStmt>()) - continue; - - const SourceLocation SemiLoc = Lexer::findLocationAfterToken( - Cast->getEndLoc(), tok::semi, Context.getSourceManager(), - Context.getLangOpts(), /*SkipTrailingWhitespaceAndNewLine=*/true); - if (SemiLoc.isInvalid()) - continue; - - diag(Binding->getLocation(), - "binding %0 is only used to suppress an unused variable warning; " - "use a placeholder '_' instead") - << Binding - << FixItHint::CreateReplacement(Binding->getSourceRange(), "_") - << FixItHint::CreateRemoval( - CharSourceRange::getCharRange(Cast->getBeginLoc(), SemiLoc)); - } + diag(Binding->getLocation(), + "binding %0 is only used to suppress an unused variable warning; " + "use a placeholder '_' instead") + << Binding << FixItHint::CreateReplacement(Binding->getSourceRange(), "_") + << FixItHint::CreateRemoval( + CharSourceRange::getCharRange(Cast->getBeginLoc(), SemiLoc)); } } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 018d0cdda8fba..c6fcbcc5cb39a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -268,9 +268,10 @@ New checks - New :doc:`modernize-use-placeholder-binding <clang-tidy/checks/modernize/use-placeholder-binding>` check. - Finds structured bindings where a binding is only used to suppress an - unused variable warning and suggests replacing it with a C++26 - placeholder (``_``). + Finds structured bindings where one of the bindings is only used to + suppress an "unused variable" warning via a ``(void)name;`` statement, + and suggests replacing that binding with a C++26 placeholder (``_``) + instead, removing the now unnecessary suppression statement. - New :doc:`modernize-use-std-bit <clang-tidy/checks/modernize/use-std-bit>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-placeholder-binding.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-placeholder-binding.rst index 0f847454e30ae..c81b8e9035965 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-placeholder-binding.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-placeholder-binding.rst @@ -1,7 +1,7 @@ .. title:: clang-tidy - modernize-use-placeholder-binding modernize-use-placeholder-binding -================================== +================================= Finds structured bindings where one of the bindings is only used to suppress an "unused variable" warning via a ``(void)name;`` statement, and suggests @@ -35,13 +35,29 @@ This also applies to structured bindings declared in the init-statement of an // ... } +The suppression statement is also recognized when it is the sole +substatement of a ``case`` or ``default`` label: + +.. code-block:: c++ + + switch (auto [a, b] = compute(); a) { + case 0: + (void)b; + break; + } + + // becomes + + switch (auto [a, _] = compute(); a) { + case 0: + break; + } + Limitations ----------- The check only recognizes the ``(void)name;`` suppression idiom as a -standalone statement directly inside a compound statement. It does not -diagnose bindings that are simply unused with no suppression statement, nor -bindings suppressed some other way. It also does not diagnose a suppression -cast that is the sole substatement of a ``case`` or ``default`` label (for -example, inside a ``switch``), since removing it safely would require -special-casing the label rather than the statement. +standalone statement directly inside a compound statement, or as the sole +substatement of a ``case`` or ``default`` label. It does not diagnose +bindings that are simply unused with no suppression statement, nor bindings +suppressed some other way. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-placeholder-binding.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-placeholder-binding.cpp index 943b873c63023..89b5e13e26b51 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-placeholder-binding.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-placeholder-binding.cpp @@ -94,14 +94,27 @@ void forRangeBindingIsReplaced() { } } -void switchCaseSuppressionIsNotDiagnosed() { +void switchCaseBindingIsReplaced() { switch (auto [a, b] = getPair(); a) { + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: binding 'b' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] + // CHECK-FIXES: switch (auto [a, _] = getPair(); a) { case 0: (void)b; + // CHECK-FIXES-NOT: (void)b; break; } } +void defaultCaseSoleSubstatementIsReplaced() { + switch (auto [a, b] = getPair(); a) { + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: binding 'b' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] + // CHECK-FIXES: switch (auto [a, _] = getPair(); a) { + default: + (void)b; + // CHECK-FIXES-NOT: (void)b; + } +} + void lambdaCaptureIsUnaffected() { auto [x, y] = getPair(); // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: binding 'y' is only used to suppress an unused variable warning; use a placeholder '_' instead [modernize-use-placeholder-binding] _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
