Author: flovent Date: 2026-01-31T22:13:40+08:00 New Revision: 8c575312fc49378a3ee7fb070819d3f7790bdb0a
URL: https://github.com/llvm/llvm-project/commit/8c575312fc49378a3ee7fb070819d3f7790bdb0a DIFF: https://github.com/llvm/llvm-project/commit/8c575312fc49378a3ee7fb070819d3f7790bdb0a.diff LOG: [clang-tidy] Add new check `modernize-use-structured-binding` (#158462) This check detect code which can be transfered to c++17 structured binding, and provides fix-it. Limitations: 1. Ignore variables with attributes or qualifiers except `const` and `&` since it's not very common: ``` static auto pair = getPair(); // no warning from this check static int b = pair.first; static int c = pair.second; ``` 2. Some possibly convertable case: (1) ``` const auto& results = mapping.try_emplace("hello!"); const iterator& it = results.first; bool succeed = results.second; // No change for succeed ``` In theory this can be transfered to structured binding: ``` const auto& [it, succeed] = mapping.try_emplace("hello!"); ``` But it's needed to check whether `succeed` is changed after definition. (2) ``` const auto results = mapping.try_emplace("hello!"); if (results.second) { handle_inserted(results.first); } // no name deduced ``` That's not checked too. It's coming from #138735, but leaves some unhanlded cases mentioned above. --------- Co-authored-by: Baranov Victor <[email protected]> Co-authored-by: EugeneZelenko <[email protected]> Co-authored-by: Congcong Cai <[email protected]> Co-authored-by: Victor Chernyakin <[email protected]> Added: clang-tools-extra/clang-tidy/modernize/UseStructuredBindingCheck.cpp clang-tools-extra/clang-tidy/modernize/UseStructuredBindingCheck.h clang-tools-extra/docs/clang-tidy/checks/modernize/use-structured-binding.rst clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-structured-binding/fake_std_pair_tuple.h clang-tools-extra/test/clang-tidy/checkers/modernize/use-structured-binding.cpp Modified: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 858cf921f9d34..cc4cc7a02b594 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -51,6 +51,7 @@ add_clang_library(clangTidyModernizeModule STATIC UseStdNumbersCheck.cpp UseStdPrintCheck.cpp UseStringViewCheck.cpp + UseStructuredBindingCheck.cpp UseTrailingReturnTypeCheck.cpp UseTransparentFunctorsCheck.cpp UseUncaughtExceptionsCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index eb73478b44023..fcb860d8c5298 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -51,6 +51,7 @@ #include "UseStdNumbersCheck.h" #include "UseStdPrintCheck.h" #include "UseStringViewCheck.h" +#include "UseStructuredBindingCheck.h" #include "UseTrailingReturnTypeCheck.h" #include "UseTransparentFunctorsCheck.h" #include "UseUncaughtExceptionsCheck.h" @@ -133,6 +134,8 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<UseOverrideCheck>("modernize-use-override"); CheckFactories.registerCheck<UseStringViewCheck>( "modernize-use-string-view"); + CheckFactories.registerCheck<UseStructuredBindingCheck>( + "modernize-use-structured-binding"); CheckFactories.registerCheck<UseTrailingReturnTypeCheck>( "modernize-use-trailing-return-type"); CheckFactories.registerCheck<UseTransparentFunctorsCheck>( diff --git a/clang-tools-extra/clang-tidy/modernize/UseStructuredBindingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseStructuredBindingCheck.cpp new file mode 100644 index 0000000000000..b56fb0579f1e3 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseStructuredBindingCheck.cpp @@ -0,0 +1,405 @@ +//===----------------------------------------------------------------------===// +// +// 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 "UseStructuredBindingCheck.h" +#include "../utils/DeclRefExprUtils.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +static constexpr StringRef PairDeclName = "PairVarD"; +static constexpr StringRef PairVarTypeName = "PairVarType"; +static constexpr StringRef FirstVarDeclName = "FirstVarDecl"; +static constexpr StringRef SecondVarDeclName = "SecondVarDecl"; +static constexpr StringRef BeginDeclStmtName = "BeginDeclStmt"; +static constexpr StringRef EndDeclStmtName = "EndDeclStmt"; +static constexpr StringRef FirstTypeName = "FirstType"; +static constexpr StringRef SecondTypeName = "SecondType"; +static constexpr StringRef ScopeBlockName = "ScopeBlock"; +static constexpr StringRef StdTieAssignStmtName = "StdTieAssign"; +static constexpr StringRef StdTieExprName = "StdTieExpr"; +static constexpr StringRef ForRangeStmtName = "ForRangeStmt"; +static constexpr StringRef InitExprName = "init_expr"; + +/// Matches a sequence of VarDecls matching the inner matchers, starting from +/// the \p Iter to \p EndIter and set bindings for the first DeclStmt and the +/// last DeclStmt if matched. +/// +/// \p Backwards indicates whether to match the VarDecls in reverse order. +template <typename Iterator> +static bool matchNVarDeclStartingWith( + Iterator Iter, const Iterator &EndIter, + ArrayRef<ast_matchers::internal::Matcher<VarDecl>> InnerMatchers, + ast_matchers::internal::ASTMatchFinder *Finder, + ast_matchers::internal::BoundNodesTreeBuilder *Builder, + bool Backwards = false) { + const DeclStmt *BeginDS = nullptr; + const DeclStmt *EndDS = nullptr; + const size_t N = InnerMatchers.size(); + size_t Count = 0; + + auto Matches = [&](const Decl *VD) { + // We don't want redundant decls in DeclStmt. + if (Count == N) + return false; + + if (const auto *Var = dyn_cast<VarDecl>(VD); + Var && InnerMatchers[Backwards ? N - Count - 1 : Count].matches( + *Var, Finder, Builder)) { + ++Count; + return true; + } + + return false; + }; + + for (; Iter != EndIter; ++Iter) { + EndDS = dyn_cast<DeclStmt>(*Iter); + if (!EndDS) + break; + + if (!BeginDS) + BeginDS = EndDS; + + for (const auto *VD : + llvm::reverse_conditionally(EndDS->decls(), Backwards)) { + if (!Matches(VD)) + return false; + } + + // All the matchers is satisfied in those DeclStmts. + if (Count == N) { + if (Backwards) + std::swap(BeginDS, EndDS); + Builder->setBinding(BeginDeclStmtName, DynTypedNode::create(*BeginDS)); + Builder->setBinding(EndDeclStmtName, DynTypedNode::create(*EndDS)); + return true; + } + } + + return false; +} + +namespace { +/// What qualifiers and specifiers are used to create structured binding +/// declaration, it only supports the following four cases now. +enum TransferType : uint8_t { + TT_ByVal, + TT_ByConstVal, + TT_ByRef, + TT_ByConstRef +}; + +/// Matches a Stmt whose parent is a CompoundStmt, and which is directly +/// following two VarDecls matching the inner matcher. +AST_MATCHER_P(Stmt, hasPreTwoVarDecl, + llvm::SmallVector<ast_matchers::internal::Matcher<VarDecl>>, + InnerMatchers) { + const DynTypedNodeList Parents = Finder->getASTContext().getParents(Node); + if (Parents.size() != 1) + return false; + + const auto *C = Parents[0].get<CompoundStmt>(); + if (!C) + return false; + + const auto It = llvm::find(llvm::reverse(C->body()), &Node); + assert(It != C->body_rend() && "C is parent of Node"); + return matchNVarDeclStartingWith(It + 1, C->body_rend(), InnerMatchers, + Finder, Builder, true); +} + +/// Matches a Stmt whose parent is a CompoundStmt, and which is directly +/// followed by two VarDecls matching the inner matcher. +AST_MATCHER_P(Stmt, hasNextTwoVarDecl, + llvm::SmallVector<ast_matchers::internal::Matcher<VarDecl>>, + InnerMatchers) { + const DynTypedNodeList Parents = Finder->getASTContext().getParents(Node); + if (Parents.size() != 1) + return false; + + const auto *C = Parents[0].get<CompoundStmt>(); + if (!C) + return false; + + const auto *It = llvm::find(C->body(), &Node); + assert(It != C->body_end() && "C is parent of Node"); + return matchNVarDeclStartingWith(It + 1, C->body_end(), InnerMatchers, Finder, + Builder); +} + +/// Matches a CompoundStmt which has two VarDecls matching the inner matcher in +/// the beginning. +AST_MATCHER_P(CompoundStmt, hasFirstTwoVarDecl, + llvm::SmallVector<ast_matchers::internal::Matcher<VarDecl>>, + InnerMatchers) { + return matchNVarDeclStartingWith(Node.body_begin(), Node.body_end(), + InnerMatchers, Finder, Builder); +} + +/// It's not very common to have specifiers for variables used to decompose a +/// pair, so we ignore these cases. +AST_MATCHER(VarDecl, hasAnySpecifiersShouldBeIgnored) { + return Node.isStaticLocal() || Node.isConstexpr() || Node.hasAttrs() || + Node.isInlineSpecified() || Node.getStorageClass() != SC_None || + Node.getTSCSpec() != TSCS_unspecified; +} + +// Ignore nodes inside macros. +AST_POLYMORPHIC_MATCHER(isInMacro, + AST_POLYMORPHIC_SUPPORTED_TYPES(Stmt, Decl)) { + return Node.getBeginLoc().isMacroID() || Node.getEndLoc().isMacroID(); +} + +AST_MATCHER_P(Expr, ignoringCopyCtorAndImplicitCast, + ast_matchers::internal::Matcher<Expr>, InnerMatcher) { + if (const auto *CtorE = dyn_cast<CXXConstructExpr>(&Node)) { + if (const CXXConstructorDecl *CtorD = CtorE->getConstructor(); + CtorD->isCopyConstructor() && CtorE->getNumArgs() == 1) { + return InnerMatcher.matches(*CtorE->getArg(0)->IgnoreImpCasts(), Finder, + Builder); + } + } + + return InnerMatcher.matches(*Node.IgnoreImpCasts(), Finder, Builder); +} + +AST_MATCHER(CXXRecordDecl, isPairType) { + return llvm::all_of(Node.fields(), [](const FieldDecl *FD) { + return FD->getAccess() == AS_public && + (FD->getName() == "first" || FD->getName() == "second"); + }); +} + +AST_MATCHER(VarDecl, isDirectInitialization) { + return Node.getInitStyle() != VarDecl::InitializationStyle::CInit; +} + +} // namespace + +static auto getVarInitWithMemberMatcher( + StringRef PairName, StringRef MemberName, StringRef TypeName, + StringRef BindingName, + const ast_matchers::internal::Matcher<VarDecl> &ExtraMatcher) { + return varDecl(ExtraMatcher, + hasInitializer(ignoringCopyCtorAndImplicitCast(memberExpr( + hasObjectExpression(ignoringImpCasts(declRefExpr( + to(equalsBoundNode(std::string(PairName)))))), + member(fieldDecl(hasName(MemberName), + hasType(qualType().bind(TypeName)))))))) + .bind(BindingName); +} + +static auto typeOrLValueReferenceTo( + const ast_matchers::internal::Matcher<QualType> &TypeMatcher) { + return qualType( + anyOf(TypeMatcher, lValueReferenceType(pointee(TypeMatcher)))); +} + +void UseStructuredBindingCheck::registerMatchers(MatchFinder *Finder) { + auto PairType = qualType(unless(isVolatileQualified()), + hasUnqualifiedDesugaredType(recordType( + hasDeclaration(cxxRecordDecl(isPairType()))))); + + auto UnlessShouldBeIgnored = + unless(anyOf(hasAnySpecifiersShouldBeIgnored(), isInMacro())); + + auto VarInitWithFirstMember = + getVarInitWithMemberMatcher(PairDeclName, "first", FirstTypeName, + FirstVarDeclName, UnlessShouldBeIgnored); + auto VarInitWithSecondMember = + getVarInitWithMemberMatcher(PairDeclName, "second", SecondTypeName, + SecondVarDeclName, UnlessShouldBeIgnored); + + auto RefToBindName = [&UnlessShouldBeIgnored](const StringRef &Name) { + return declRefExpr(to(varDecl(UnlessShouldBeIgnored).bind(Name))); + }; + + auto HasAnyLambdaCaptureThisVar = + [](const ast_matchers::internal::Matcher<VarDecl> &VDMatcher) { + return compoundStmt(hasDescendant( + lambdaExpr(hasAnyCapture(capturesVar(varDecl(VDMatcher)))))); + }; + + // Captured structured bindings are a C++20 extension + auto UnlessFirstVarOrSecondVarIsCapturedByLambda = + getLangOpts().CPlusPlus20 + ? compoundStmt() + : compoundStmt(unless(HasAnyLambdaCaptureThisVar( + anyOf(equalsBoundNode(std::string(FirstVarDeclName)), + equalsBoundNode(std::string(SecondVarDeclName)))))); + + // X x; + // Y y; + // std::tie(x, y) = ...; + Finder->addMatcher( + exprWithCleanups( + unless(isInMacro()), + has(cxxOperatorCallExpr( + hasOverloadedOperatorName("="), + hasLHS(ignoringImplicit( + callExpr(callee(functionDecl(isInStdNamespace(), + hasName("tie"))), + hasArgument(0, RefToBindName(FirstVarDeclName)), + hasArgument(1, RefToBindName(SecondVarDeclName))) + .bind(StdTieExprName))), + hasRHS(expr(hasType(PairType)))) + .bind(StdTieAssignStmtName)), + hasPreTwoVarDecl( + llvm::SmallVector<ast_matchers::internal::Matcher<VarDecl>>{ + varDecl(equalsBoundNode(std::string(FirstVarDeclName))), + varDecl(equalsBoundNode(std::string(SecondVarDeclName)))}), + hasParent(compoundStmt(UnlessFirstVarOrSecondVarIsCapturedByLambda) + .bind(ScopeBlockName))), + this); + + // pair<X, Y> p = ...; + // X x = p.first; + // Y y = p.second; + Finder->addMatcher( + declStmt( + unless(isInMacro()), + hasSingleDecl(varDecl(UnlessShouldBeIgnored, + unless(isDirectInitialization()), + hasType(typeOrLValueReferenceTo(PairType).bind( + PairVarTypeName)), + hasInitializer(ignoringCopyCtorAndImplicitCast( + expr().bind(InitExprName)))) + .bind(PairDeclName)), + hasNextTwoVarDecl( + llvm::SmallVector<ast_matchers::internal::Matcher<VarDecl>>{ + VarInitWithFirstMember, VarInitWithSecondMember}), + hasParent(compoundStmt(UnlessFirstVarOrSecondVarIsCapturedByLambda) + .bind(ScopeBlockName))), + this); + + // for (pair<X, Y> p : map) { + // X x = p.first; + // Y y = p.second; + // } + Finder->addMatcher( + cxxForRangeStmt( + unless(isInMacro()), + hasLoopVariable( + varDecl(hasType(typeOrLValueReferenceTo(PairType).bind( + PairVarTypeName)), + hasInitializer(ignoringCopyCtorAndImplicitCast( + expr().bind(InitExprName)))) + .bind(PairDeclName)), + hasBody( + compoundStmt( + hasFirstTwoVarDecl(llvm::SmallVector< + ast_matchers::internal::Matcher<VarDecl>>{ + VarInitWithFirstMember, VarInitWithSecondMember}), + UnlessFirstVarOrSecondVarIsCapturedByLambda) + .bind(ScopeBlockName))) + .bind(ForRangeStmtName), + this); +} + +static std::optional<TransferType> getTransferType(const ASTContext &Ctx, + QualType ResultType, + QualType OriginType) { + ResultType = ResultType.getCanonicalType(); + OriginType = OriginType.getCanonicalType(); + + if (ResultType == Ctx.getLValueReferenceType(OriginType.withConst())) + return TT_ByConstRef; + + if (ResultType == Ctx.getLValueReferenceType(OriginType)) + return TT_ByRef; + + if (ResultType == OriginType.withConst()) + return TT_ByConstVal; + + if (ResultType == OriginType) + return TT_ByVal; + + return std::nullopt; +} + +void UseStructuredBindingCheck::check(const MatchFinder::MatchResult &Result) { + const auto *FirstVar = Result.Nodes.getNodeAs<VarDecl>(FirstVarDeclName); + const auto *SecondVar = Result.Nodes.getNodeAs<VarDecl>(SecondVarDeclName); + + const auto *BeginDS = Result.Nodes.getNodeAs<DeclStmt>(BeginDeclStmtName); + const auto *EndDS = Result.Nodes.getNodeAs<DeclStmt>(EndDeclStmtName); + const auto *ScopeBlock = Result.Nodes.getNodeAs<CompoundStmt>(ScopeBlockName); + + const auto *CFRS = Result.Nodes.getNodeAs<CXXForRangeStmt>(ForRangeStmtName); + auto DiagAndFix = [&BeginDS, &EndDS, &FirstVar, &SecondVar, &CFRS, + this](SourceLocation DiagLoc, SourceRange ReplaceRange, + TransferType TT = TT_ByVal) { + const auto Prefix = [&TT]() -> StringRef { + switch (TT) { + case TT_ByVal: + return "auto"; + case TT_ByConstVal: + return "const auto"; + case TT_ByRef: + return "auto&"; + case TT_ByConstRef: + return "const auto&"; + } + }(); + + const std::string ReplacementText = + (Twine(Prefix) + " [" + FirstVar->getNameAsString() + ", " + + SecondVar->getNameAsString() + "]" + (CFRS ? " :" : "")) + .str(); + diag(DiagLoc, "use a structured binding to decompose a pair") + << FixItHint::CreateReplacement(ReplaceRange, ReplacementText) + << FixItHint::CreateRemoval( + SourceRange{BeginDS->getBeginLoc(), EndDS->getEndLoc()}); + }; + + if (const auto *COCE = + Result.Nodes.getNodeAs<CXXOperatorCallExpr>(StdTieAssignStmtName)) { + DiagAndFix(COCE->getBeginLoc(), + Result.Nodes.getNodeAs<Expr>(StdTieExprName)->getSourceRange()); + return; + } + + // Check whether PairVar, FirstVar and SecondVar have the same transfer type, + // so they can be combined to structured binding. + const auto *PairVar = Result.Nodes.getNodeAs<VarDecl>(PairDeclName); + + const std::optional<TransferType> PairCaptureType = + getTransferType(*Result.Context, PairVar->getType(), + Result.Nodes.getNodeAs<Expr>(InitExprName)->getType()); + const std::optional<TransferType> FirstVarCaptureType = + getTransferType(*Result.Context, FirstVar->getType(), + *Result.Nodes.getNodeAs<QualType>(FirstTypeName)); + const std::optional<TransferType> SecondVarCaptureType = + getTransferType(*Result.Context, SecondVar->getType(), + *Result.Nodes.getNodeAs<QualType>(SecondTypeName)); + if (!PairCaptureType || !FirstVarCaptureType || !SecondVarCaptureType || + *PairCaptureType != *FirstVarCaptureType || + *FirstVarCaptureType != *SecondVarCaptureType) + return; + + // Check PairVar is not used except for assignment members to firstVar and + // SecondVar. + if (auto AllRef = utils::decl_ref_expr::allDeclRefExprs(*PairVar, *ScopeBlock, + *Result.Context); + AllRef.size() != 2) + return; + + DiagAndFix(PairVar->getBeginLoc(), + CFRS ? PairVar->getSourceRange() + : SourceRange(PairVar->getBeginLoc(), + Lexer::getLocForEndOfToken( + PairVar->getLocation(), 0, + Result.Context->getSourceManager(), + Result.Context->getLangOpts())), + *PairCaptureType); +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseStructuredBindingCheck.h b/clang-tools-extra/clang-tidy/modernize/UseStructuredBindingCheck.h new file mode 100644 index 0000000000000..0b11ce38c44db --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseStructuredBindingCheck.h @@ -0,0 +1,34 @@ +//===----------------------------------------------------------------------===// +// +// 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_USESTRUCTUREDBINDINGCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTRUCTUREDBINDINGCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::modernize { + +/// Finds places where structured bindings could be used to decompose pairs and +/// suggests replacing them. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-structured-binding.html +class UseStructuredBindingCheck : public ClangTidyCheck { +public: + UseStructuredBindingCheck(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.CPlusPlus17; + } +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESTRUCTUREDBINDINGCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 04c970402d4e1..976223e5c8149 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -115,6 +115,12 @@ New checks Looks for functions returning ``std::[w|u8|u16|u32]string`` and suggests to change it to ``std::[...]string_view`` for performance reasons if possible. + +- New :doc:`modernize-use-structured-binding + <clang-tidy/checks/modernize/use-structured-binding>` check. + + Finds places where structured bindings could be used to decompose pairs and + suggests replacing them. - New :doc:`performance-string-view-conversions <clang-tidy/checks/performance/string-view-conversions>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index d9a6e4fd6593c..2c9d5face0bee 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -334,6 +334,7 @@ Clang-Tidy Checks :doc:`modernize-use-std-numbers <modernize/use-std-numbers>`, "Yes" :doc:`modernize-use-std-print <modernize/use-std-print>`, "Yes" :doc:`modernize-use-string-view <modernize/use-string-view>`, "Yes" + :doc:`modernize-use-structured-binding <modernize/use-structured-binding>`, "Yes" :doc:`modernize-use-trailing-return-type <modernize/use-trailing-return-type>`, "Yes" :doc:`modernize-use-transparent-functors <modernize/use-transparent-functors>`, "Yes" :doc:`modernize-use-uncaught-exceptions <modernize/use-uncaught-exceptions>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-structured-binding.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-structured-binding.rst new file mode 100644 index 0000000000000..9e8ce6b312bc0 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-structured-binding.rst @@ -0,0 +1,83 @@ +.. title:: clang-tidy - modernize-use-structured-binding + +modernize-use-structured-binding +================================ + +Finds places where structured bindings could be used to decompose pairs and +suggests replacing them. + +This check finds three code patterns and recommends using structured bindings +for clearer, more idiomatic C++17 code. + +1. Decompose a pair variable by assigning its members to separate variables +right after its definition: + +.. code-block:: c++ + + auto p = getPair<int, int>(); + int x = p.first; + int y = p.second; + + into: + + auto [x, y] = getPair<int, int>(); + +2. Use ``std::tie`` to decompose a pair into two predefined variables: + +.. code-block:: c++ + + int a; + int b; + std::tie(a, b) = getPair<int, int>(); + + into: + + auto [a, b] = getPair<int, int>(); + +3. Manually decompose a pair by assigning to its members to local variables +in a range-based for loop: + +.. code-block:: c++ + + for (auto p : vecOfPairs) { + int x = p.first; + int y = p.second; + // ... + } + + into: + + for (auto [x, y] : vecOfPairs) { + // use x and y + } + +Limitations +----------- + +The check currently ignores variables defined with attributes or qualifiers +except ``const`` and ``&`` since it's not very common: + +.. code-block:: c++ + + static auto pair = getPair(); + static int b = pair.first; + static int c = pair.second; + +The check doesn't handle some situations which could possibly be transferred +to structured bindings, for example: + +.. code-block:: c++ + + const auto& results = mapping.try_emplace("hello!"); + const iterator& it = results.first; + bool succeed = results.second; + // succeed is not changed in the following code + +and: + +.. code-block:: c++ + + const auto results = mapping.try_emplace("hello!"); + if (results.second) { + handle_inserted(results.first); + } diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-structured-binding/fake_std_pair_tuple.h b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-structured-binding/fake_std_pair_tuple.h new file mode 100644 index 0000000000000..2f8ce2b157e3f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-structured-binding/fake_std_pair_tuple.h @@ -0,0 +1,62 @@ +namespace std { + struct _Swallow_assign + { + template<class _Tp> + const _Swallow_assign& + operator=(const _Tp&) const + { return *this; } + }; + + constexpr _Swallow_assign ignore{}; + + template<typename T1, typename T2> + struct pair { + T1 first; + T2 second; + + pair() = default; + pair(T1 first, T2 second) : first(first), second(second) {} + }; + + template<typename... Args> + struct tuple { + tuple(Args&...) {} + + template<typename T1, typename T2> + tuple<T1, T2> operator=(const std::pair<T1, T2>&); + }; + + template<typename... Args> + tuple<Args...> tie(Args&... args) { + return tuple<Args...>(args...); + } + + template <typename Key, typename Value> + class unordered_map { + public: + using value_type = pair<Key, Value>; + + class iterator { + public: + iterator& operator++(); + bool operator!=(const iterator &other); + const value_type &operator*() const; + value_type operator*(); + const value_type* operator->() const; + }; + + iterator begin() const; + iterator end() const; + }; +} + +template<typename T1, typename T2> +std::pair<T1, T2> getPair(); + +template<typename T1, typename T2> +constexpr std::pair<T1, T2> getConstexprPair() { + return std::pair<T1, T2>(); +} + +template<typename T1, typename T2, typename T3> +std::tuple<T1, T2, T3> getTuple(); diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-structured-binding.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-structured-binding.cpp new file mode 100644 index 0000000000000..bbf5c3d582b6a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-structured-binding.cpp @@ -0,0 +1,883 @@ +// RUN: %check_clang_tidy -check-suffix=,CPP20ORLATER -std=c++20-or-later %s modernize-use-structured-binding %t -- -- -I %S/Inputs/use-structured-binding/ +// RUN: %check_clang_tidy -std=c++17 %s modernize-use-structured-binding %t -- -- -I %S/Inputs/use-structured-binding/ +#include "fake_std_pair_tuple.h" + +template<typename T> +void MarkUsed(T x); + +struct TestClass { + int a; + int b; + TestClass() : a(0), b(0) {} + TestClass& operator++(); + TestClass(int x, int y) : a(x), b(y) {} +}; + +void DecomposeByAssignWarnCases() { + { + auto P = getPair<int, int>(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = getPair<int, int>(); + // CHECK-NEXT: // REMOVE + int x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + { + auto P = getPair<int, int>(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = getPair<int, int>(); + int x = P.first, y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + { + auto P = getPair<int, int>(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = getPair<int, int>(); + int x = P.first, y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + int z; + } + + { + auto P = getPair<int, int>(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = getPair<int, int>(); + // CHECK-NEXT: // REMOVE + int x = P.first; + auto y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + { + const auto P = getPair<int, int>(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: const auto [x, y] = getPair<int, int>(); + // CHECK-NEXT: // REMOVE + const int x = P.first; + const auto y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + { + std::pair<int, int> otherP; + auto& P = otherP; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto& [x, y] = otherP; + // CHECK-NEXT: // REMOVE + int& x = P.first; + auto& y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + { + std::pair<int, int> otherP; + const auto& P = otherP; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: const auto& [x, y] = otherP; + // CHECK-NEXT: // REMOVE + const int& x = P.first; + const auto& y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + { + auto P = getPair<int, int>(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = getPair<int, int>(); + // CHECK-NEXT: // REMOVE + int x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + + auto another_p = getPair<int, int>(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [another_x, another_y] = getPair<int, int>(); + // CHECK-NEXT: // REMOVE + int another_x = another_p.first; + int another_y = another_p.second; // REMOVE + // CHECK-FIXES: // REMOVE + } +} + +void forRangeWarnCases() { + std::pair<int, int> Pairs[10]; + for (auto P : Pairs) { + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: for (auto [x, y] : Pairs) { + // CHECK-NEXT: // REMOVE + int x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + for (auto P : Pairs) { + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: for (auto [x, y] : Pairs) { + int x = P.first, y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + for (auto P : Pairs) { + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: for (auto [x, y] : Pairs) { + int x = P.first, y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + int z; + } + + for (const auto P : Pairs) { + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: for (const auto [x, y] : Pairs) { + // CHECK-NEXT: // REMOVE + const int x = P.first; + const int y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + for (auto& P : Pairs) { + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: for (auto& [x, y] : Pairs) { + // CHECK-NEXT: // REMOVE + int& x = P.first; + int& y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + for (const auto& P : Pairs) { + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: for (const auto& [x, y] : Pairs) { + // CHECK-NEXT: // REMOVE + const int& x = P.first; + const int& y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + std::pair<TestClass, TestClass> ClassPairs[10]; + for (auto P : ClassPairs) { + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: for (auto [c1, c2] : ClassPairs) { + // CHECK-NEXT: // REMOVE + TestClass c1 = P.first; + TestClass c2 = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + for (const auto P : ClassPairs) { + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: for (const auto [c1, c2] : ClassPairs) { + // CHECK-NEXT: // REMOVE + const TestClass c1 = P.first; + const TestClass c2 = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } +} + +void forRangeNotWarnCases() { + std::pair<int, int> Pairs[10]; + for (auto P : Pairs) { + int x = P.first; + MarkUsed(x); + int y = P.second; + } + + for (auto P : Pairs) { + MarkUsed(P); + int x = P.first; + int y = P.second; + } + + for (auto P : Pairs) { + int x = P.first; + int y = P.second; + MarkUsed(P); + } + + std::pair<TestClass, TestClass> ClassPairs[10]; + for (auto P : ClassPairs) { + TestClass c1 = P.first; + ++ c1 ; + TestClass c2 = P.second; + } + + int c; + for (auto P : ClassPairs) { + TestClass c1 = P.first; + c ++ ; + TestClass c2 = P.second; + } +} + +void stdTieWarnCases() { + // CHECK-NEXT: // REMOVE + int a = 0; + int b = 0; // REMOVE + // CHECK-FIXES: // REMOVE + std::tie(a, b) = getPair<int, int>(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [a, b] = getPair<int, int>(); + + int x = 0, y = 0; // REMOVE + // CHECK-FIXES: // REMOVE + std::tie(x, y) = getPair<int, int>(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = getPair<int, int>(); + + // CHECK-NEXT: // REMOVE + int* pa = nullptr; + int* pb = nullptr; // REMOVE + // CHECK-FIXES: // REMOVE + std::tie(pa, pb) = getPair<int*, int*>(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [pa, pb] = getPair<int*, int*>(); + + // CHECK-NEXT: // REMOVE + TestClass c1 (1, 2); + TestClass c2 = TestClass {3, 4}; // REMOVE + // CHECK-FIXES: // REMOVE + std::tie(c1, c2) = getPair<TestClass, TestClass>(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [c1, c2] = getPair<TestClass, TestClass>(); +} + +void stdTieNotWarnCases() { + int a = 0; + int b = 0; + a = 4; + std::tie(a, b) = getPair<int, int>(); // no warning + + int c = 0, d = 0; + int e = 0; + std::tie(a, b) = getPair<int, int>(); // no warning + + int* pa = nullptr; + int* pb = nullptr; + MarkUsed(pa); + std::tie(pa, pb) = getPair<int*, int*>(); // no warning + + TestClass c1 (1, 2); + TestClass c2 = TestClass {3, 4}; + MarkUsed(c2); + std::tie(c1, c2) = getPair<TestClass, TestClass>(); +} + +void NotWarnForVarHasSpecifiers() { + { + auto P = getPair<int, int>(); + const int x = P.first; + int y = P.second; + } + + { + auto P = getPair<int, int>(); + volatile int x = P.first; + int y = P.second; + } + + { + auto P = getPair<int, int>(); + int x = P.first; + [[maybe_unused]] int y = P.second; + } + + { + static auto P = getPair<int, int>(); + int x = P.first; + int y = P.second; + } + + { + thread_local auto P = getPair<int, int>(); + int x = P.first; + int y = P.second; + } + + { + extern int a; + int b = 0; + std::tie(a, b) = getPair<int, int>(); + } +} + +void NotWarnForMultiUsedPairVar() { + { + auto P = getPair<int, int>(); + int x = P.first; + int y = P.second; + MarkUsed(P); + } + + { + auto P = getPair<int, int>(); + int x = P.first; + MarkUsed(P); + int y = P.second; + } + + { + auto P = getPair<int, int>(); + MarkUsed(P); + int x = P.first; + int y = P.second; + } + + { + std::pair<int, int> Pairs[10]; + for (auto P : Pairs) { + int x = P.first; + int y = P.second; + + MarkUsed(P); + } + } +} + +#define DECOMPOSE(P) \ + int x = P.first; \ + int y = P.second; \ + +void NotWarnForMacro1() { + auto P = getPair<int, int>(); + DECOMPOSE(P); +} + +#define GETPAIR auto P = getPair<int, int>() + +void NotWarnForMacro2() { + GETPAIR; + int x = P.first; + int y = P.second; +} + +void captureByVal() { + auto P = getPair<int, int>(); + // CHECK-MESSAGES-CPP20ORLATER: :[[@LINE-1]]:3: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-CPP20ORLATER: auto [x, y] = getPair<int, int>(); + // CHECK-NEXT-CPP20ORLATER: // REMOVE + int x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + + auto lambda = [x]() { + int y = x; + }; +} + +void captureByRef() { + auto P = getPair<int, int>(); + // CHECK-MESSAGES-CPP20ORLATER: :[[@LINE-1]]:3: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-CPP20ORLATER: auto [x, y] = getPair<int, int>(); + // CHECK-NEXT-CPP20ORLATER: // REMOVE + int x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + + auto lambda = [&x]() { + x = 1; + }; +} + +void captureByAllRef() { + auto P = getPair<int, int>(); + // CHECK-MESSAGES-CPP20ORLATER: :[[@LINE-1]]:3: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-CPP20ORLATER: auto [x, y] = getPair<int, int>(); + // CHECK-NEXT-CPP20ORLATER: // REMOVE + int x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + + auto lambda = [&]() { + x = 1; + }; +} + +void deepLambda() { + auto P = getPair<int, int>(); + // CHECK-MESSAGES-CPP20ORLATER: :[[@LINE-1]]:3: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-CPP20ORLATER: auto [x, y] = getPair<int, int>(); + // CHECK-NEXT-CPP20ORLATER: // REMOVE + int x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + + { + auto lambda = [x]() { + int y = x; + }; + } +} + +void forRangeNotWarn() { + std::pair<int, int> Pairs[10]; + for (auto P : Pairs) { + // CHECK-MESSAGES-CPP20ORLATER: :[[@LINE-1]]:8: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-CPP20ORLATER: for (auto [x, y] : Pairs) { + // CHECK-NEXT-CPP20ORLATER: // REMOVE + int x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + + auto lambda = [&]() { + x = 1; + }; + } +} + +void stdTieNotWarn() { + // CHECK-NEXT-CPP20ORLATER: // REMOVE + int x = 0; + int y = 0; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + std::tie(x, y) = getPair<int, int>(); + // CHECK-MESSAGES-CPP20ORLATER: :[[@LINE-1]]:3: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-CPP20ORLATER: auto [x, y] = getPair<int, int>(); + + auto lambda = [&x]() { + x = 1; + }; +} + +struct otherPair { + int first; + int second; +}; + +void OtherPairTest() { + { + auto P = otherPair(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = otherPair(); + // CHECK-NEXT: // REMOVE + int x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + { + const auto P = otherPair(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: const auto [x, y] = otherPair(); + // CHECK-NEXT: // REMOVE + const int x = P.first; + const auto y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + { + otherPair otherP; + auto& P = otherP; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto& [x, y] = otherP; + // CHECK-NEXT: // REMOVE + int& x = P.first; + auto& y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + { + std::pair<int, int> otherP; + const auto& P = otherP; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: const auto& [x, y] = otherP; + // CHECK-NEXT: // REMOVE + const int& x = P.first; + const auto& y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } +} + +void OtherPairNotWarnCases() { + { + auto P = otherPair(); + const int x = P.first; + int y = P.second; + } + + { + auto P = otherPair(); + volatile int x = P.first; + int y = P.second; + } + + { + auto P = otherPair(); + int x = P.first; + [[maybe_unused]] int y = P.second; + } + + { + static auto P = getPair<int, int>(); + int x = P.first; + int y = P.second; + } +} + +struct otherNonPair1 { + int first; + int second; + +private: + int third; +}; + +struct otherNonPair2 { + int first; + int second; + int third; +}; + +void OtherNonPairTest() { + { + auto P = otherNonPair1(); + int x = P.first; + int y = P.second; + } + + { + auto P = otherNonPair2(); + int x = P.first; + int y = P.second; + } +} + +template<typename PairType> +PairType getCertainPair(); + +struct ConstFieldPair { + const int first; + int second; +}; + +void ConstFieldPairTests() { + { + const ConstFieldPair P = getCertainPair<ConstFieldPair>(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: const auto [x, y] = getCertainPair<ConstFieldPair>(); + // CHECK-NEXT: // REMOVE + const int x = P.first; + const int y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + { + const ConstFieldPair& P = getCertainPair<ConstFieldPair>(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: const auto& [x, y] = getCertainPair<ConstFieldPair>(); + // CHECK-NEXT: // REMOVE + const int& x = P.first; + const int& y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + { + ConstFieldPair P = getCertainPair<ConstFieldPair>(); // no warning + int x = P.first; + int y = P.second; + } +} + +struct PointerFieldPair { + int* first; + int second; +}; + +void PointerFieldPairTests() { + { + PointerFieldPair P = getCertainPair<PointerFieldPair>(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = getCertainPair<PointerFieldPair>(); + // CHECK-NEXT: // REMOVE + int* x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + { + PointerFieldPair P = getCertainPair<PointerFieldPair>(); // no warning + const int* x = P.first; + int y = P.second; + } +} + +struct ConstRefFieldPair { + const int& first; + int second; + ConstRefFieldPair(int& f, int s) : first(f), second(s) {} +}; + +void ConstRefFieldPairTests() { + { + ConstRefFieldPair P = getCertainPair<ConstRefFieldPair>(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = getCertainPair<ConstRefFieldPair>(); + // CHECK-NEXT: // REMOVE + const int& x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + { + ConstRefFieldPair P = getCertainPair<ConstRefFieldPair>();; // no warning + int x = P.first; + int y = P.second; + } +} + +struct StaticFieldPair { + static int first; + int second; +}; + +void StaticFieldPairTests() { + { + StaticFieldPair P; // Should not warn + int x = P.first; + int y = P.second; + } + + { + StaticFieldPair P; // Should not warn + static int x = P.first; + int y = P.second; + } +} + +void IgnoreDirectInit() { + { + std::pair<int, int> P{1, 1}; + int x = P.first; + int y = P.second; + } + + { + std::pair<int, int> P(1, 1); + int x = P.first; + int y = P.second; + } + + { + std::pair<int, int> P; + int x = P.first; + int y = P.second; + } +} + +void StdMapTestCases() { + for (auto p : std::unordered_map<int, int>()) { + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: for (auto [x, y] : std::unordered_map<int, int>()) { + // CHECK-NEXT: // REMOVE + int x = p.first; + int y = p.second; // REMOVE + // CHECK-FIXES: // REMOVE + } +} + +void SpecialCases1() { + { + auto P = getPair<int, int>(); + int y = P.second; // second first + int x = P.first; + } + + { + auto P = getPair<int, int>(); + int x = P.first; + // P.second never assigned + } + + { + auto P = getPair<int, int>(); + int x = P.first + 1; + int y = P.second; + } + { + auto P = getPair<int, int>(); + float x = P.first; // int -> float + float y = P.second; + } + + { + auto P = getPair<int, int>(); + int x = P.first; + int dummy = 42; + int y = P.second; + } + + { + for (auto P : std::unordered_map<int, int>()) { + int x = P.first; + int dummy = 42; + int y = P.second; + } + } + + { + std::pair<int, int> somePair; + std::pair<int, int>* P = &somePair; + int x = P->first; + int y = P->second; + } + + { + std::pair<int, int> somePair; + std::pair<int, int>& P = somePair; + int x = P.first; + int y = P.second; + } + + { + std::pair<int, int> somePair; + const std::pair<int, int>& P = somePair; + int x = P.first; + int y = P.second; + } +} + +// Partial and full templates +// Currently not supported because we can't get type from CXXDependentScopeMemberExpr, needs some extra work to do that. +template<typename T> +void templateFuncTest() { + auto P = getPair<T, int>(); + T x = P.first; + int y = P.second; +} + +template<typename T1, typename T2> +void templateFuncTest() { + auto P = getPair<T1, T2>(); + T1 x = P.first; + T2 y = P.second; +} + +void SpecialCases2() { + // nested pairs + { + auto P = getPair<std::pair<int, int>, int>(); + int val = P.second; + std::pair<int, int> inner = P.first; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [a_in, b_in] = P.first; + // CHECK-NEXT: // REMOVE + int a_in = inner.first; + int b_in = inner.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + // shadow + { + int x = 10; + { + auto P = getPair<int, int>(); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = getPair<int, int>(); + // CHECK-NEXT: // REMOVE + int x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + } + + // only use first + { + auto P = getPair<int, int>(); + int x = P.first; + int y = P.first; + } + + // std::ignore in std::tie + { + int a = 0; + std::tie(a, std::ignore) = getPair<int, int>(); + } + + // tuple pair mixup + { + int a = 0, b = 0, c = 0; + std::tie(a, b, c) = getTuple<int, int, int>(); + std::tie(a, a) = getPair<int, int>(); + } + + // volatile specifier + { + volatile std::pair<int, int> P = getPair<int, int>(); + int x = P.first; + int y = P.second; + } + + // decltype usage (there are invisible DeclRefExpr in decltype) + { + auto P = getPair<int, int>(); + decltype(P.first) x = P.first; + decltype(P.second) y = P.second; + } + + // constexpr pair + { + constexpr auto P = getConstexprPair<int, int>(); + constexpr int x = P.first; + constexpr int y = P.second; + } + + // initializer list + { + struct AggregatePair { + int first; + int second; + }; + auto P = AggregatePair{.first = 1, .second = 2}; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = AggregatePair{.first = 1, .second = 2}; + // CHECK-NEXT: // REMOVE + int x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + // reference to original pair + { + std::pair<int, int> original = getPair<int, int>(); + const auto& P = original; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: const auto& [x, y] = original; + // CHECK-NEXT: // REMOVE + const int& x = P.first; + const int& y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + // typedef pair + { + using IntPair = std::pair<int, int>; + IntPair P = getPair<int, int>(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = getPair<int, int>(); + // CHECK-NEXT: // REMOVE + int x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } + + // assign to existing vars + { + int x, y; + auto P = getPair<int, int>(); + x = P.first; + y = P.second; + } + + // conditional operator + { + bool cond; + std::pair<int, int> p1{1, 2}, p2{3, 4}; + auto P = cond ? p1 : p2; + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = cond ? p1 : p2; + // CHECK-NEXT: // REMOVE + int x = P.first; + int y = P.second; // REMOVE + // CHECK-FIXES: // REMOVE + } +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
