diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 619a27b2f9bb6..094f0a72b1570 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -47,6 +47,7 @@ add_clang_library(clangTidyModernizeModule STATIC UseStdFormatCheck.cpp UseStdNumbersCheck.cpp UseStdPrintCheck.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 fdf38bc4b6308..a79908500e904 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -48,6 +48,7 @@ #include "UseStdFormatCheck.h" #include "UseStdNumbersCheck.h" #include "UseStdPrintCheck.h" +#include "UseStructuredBindingCheck.h" #include "UseTrailingReturnTypeCheck.h" #include "UseTransparentFunctorsCheck.h" #include "UseUncaughtExceptionsCheck.h" @@ -121,6 +122,8 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck("modernize-use-noexcept"); CheckFactories.registerCheck("modernize-use-nullptr"); CheckFactories.registerCheck("modernize-use-override"); + CheckFactories.registerCheck( + "modernize-use-structured-binding"); CheckFactories.registerCheck( "modernize-use-trailing-return-type"); CheckFactories.registerCheck( 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..c471436c0c88a --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseStructuredBindingCheck.cpp @@ -0,0 +1,419 @@ +//===----------------------------------------------------------------------===// +// +// 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 "../utils/OptionsUtils.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +static constexpr const char *DefaultPairTypes = "std::pair"; +static constexpr llvm::StringLiteral PairDeclName = "PairVarD"; +static constexpr llvm::StringLiteral PairVarTypeName = "PairVarType"; +static constexpr llvm::StringLiteral FirstVarDeclName = "FirstVarDecl"; +static constexpr llvm::StringLiteral SecondVarDeclName = "SecondVarDecl"; +static constexpr llvm::StringLiteral FirstDeclStmtName = "FirstDeclStmt"; +static constexpr llvm::StringLiteral SecondDeclStmtName = "SecondDeclStmt"; +static constexpr llvm::StringLiteral FirstTypeName = "FirstType"; +static constexpr llvm::StringLiteral SecondTypeName = "SecondType"; +static constexpr llvm::StringLiteral ScopeBlockName = "ScopeBlock"; +static constexpr llvm::StringLiteral StdTieAssignStmtName = "StdTieAssign"; +static constexpr llvm::StringLiteral StdTieExprName = "StdTieExpr"; +static constexpr llvm::StringLiteral ForRangeStmtName = "ForRangeStmt"; + +/// Try to match exactly two VarDecl inside two DeclStmts, and set binding for +/// the used DeclStmts. +static bool +matchTwoVarDecl(const DeclStmt *DS1, const DeclStmt *DS2, + ast_matchers::internal::Matcher InnerMatcher1, + ast_matchers::internal::Matcher InnerMatcher2, + internal::ASTMatchFinder *Finder, + internal::BoundNodesTreeBuilder *Builder) { + SmallVector, 2> Vars; + auto CollectVarsInDeclStmt = [&Vars](const DeclStmt *DS) -> bool { + if (!DS) + return true; + + for (const auto *VD : DS->decls()) { + if (Vars.size() == 2) + return false; + + if (const auto *Var = dyn_cast(VD)) + Vars.emplace_back(Var, DS); + else + return false; + } + + return true; + }; + + // If we already get two VarDecls, then don't need to collect from DS2. + if (!CollectVarsInDeclStmt(DS1) || + (Vars.size() != 2 && !CollectVarsInDeclStmt(DS2)) || Vars.size() != 2) + return false; + + if (InnerMatcher1.matches(*Vars[0].first, Finder, Builder) && + InnerMatcher2.matches(*Vars[1].first, Finder, Builder)) { + Builder->setBinding(FirstDeclStmtName, + clang::DynTypedNode::create(*Vars[0].second)); + if (Vars[0].second != Vars[1].second) + Builder->setBinding(SecondDeclStmtName, + clang::DynTypedNode::create(*Vars[1].second)); + 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, at the same time set +/// binding for the CompoundStmt. +AST_MATCHER_P2(Stmt, hasPreTwoVarDecl, ast_matchers::internal::Matcher, + InnerMatcher1, ast_matchers::internal::Matcher, + InnerMatcher2) { + DynTypedNodeList Parents = Finder->getASTContext().getParents(Node); + if (Parents.size() != 1) + return false; + + auto *C = Parents[0].get(); + if (!C) + return false; + + const auto I = + llvm::find(llvm::make_range(C->body_rbegin(), C->body_rend()), &Node); + assert(I != C->body_rend() && "C is parent of Node"); + if ((I + 1) == C->body_rend()) + return false; + + const auto *DS2 = dyn_cast(*(I + 1)); + if (!DS2) + return false; + + const DeclStmt *DS1 = (!DS2->isSingleDecl() || ((I + 2) == C->body_rend()) + ? nullptr + : dyn_cast(*(I + 2))); + if (DS1 && !DS1->isSingleDecl()) + return false; + + if (matchTwoVarDecl(DS1, DS2, InnerMatcher1, InnerMatcher2, Finder, + Builder)) { + Builder->setBinding(ScopeBlockName, clang::DynTypedNode::create(*C)); + return true; + } + + return false; +} + +/// Matches a Stmt whose parent is a CompoundStmt, and which is directly +/// followed by two VarDecls matching the inner matcher, at the same time set +/// binding for the CompoundStmt. +AST_MATCHER_P2(Stmt, hasNextTwoVarDecl, + ast_matchers::internal::Matcher, InnerMatcher1, + ast_matchers::internal::Matcher, InnerMatcher2) { + const DynTypedNodeList Parents = Finder->getASTContext().getParents(Node); + if (Parents.size() != 1) + return false; + + auto *C = Parents[0].get(); + if (!C) + return false; + + const auto *I = llvm::find(C->body(), &Node); + assert(I != C->body_end() && "C is parent of Node"); + if ((I + 1) == C->body_end()) + return false; + + if (matchTwoVarDecl( + dyn_cast(*(I + 1)), + ((I + 2) == C->body_end() ? nullptr : dyn_cast(*(I + 2))), + InnerMatcher1, InnerMatcher2, Finder, Builder)) { + Builder->setBinding(ScopeBlockName, clang::DynTypedNode::create(*C)); + return true; + } + + return false; +} + +/// Matches a CompoundStmt which has two VarDecls matching the inner matcher in +/// the beginning. +AST_MATCHER_P2(CompoundStmt, hasFirstTwoVarDecl, + ast_matchers::internal::Matcher, InnerMatcher1, + ast_matchers::internal::Matcher, InnerMatcher2) { + const auto *I = Node.body_begin(); + if ((I) == Node.body_end()) + return false; + + return matchTwoVarDecl( + dyn_cast(*(I)), + ((I + 1) == Node.body_end() ? nullptr : dyn_cast(*(I + 1))), + InnerMatcher1, InnerMatcher2, 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(); +} + +// Ignore nodes inside macros. +AST_POLYMORPHIC_MATCHER(isInMarco, + AST_POLYMORPHIC_SUPPORTED_TYPES(Stmt, Decl)) { + return Node.getBeginLoc().isMacroID() || Node.getEndLoc().isMacroID(); +} + +AST_MATCHER_P(Expr, ignoringCopyCtorAndImplicitCast, + ast_matchers::internal::Matcher, InnerMatcher) { + if (const auto *CtorE = dyn_cast(&Node)) { + if (const auto *CtorD = CtorE->getConstructor(); + CtorD->isCopyConstructor() && CtorE->getNumArgs() == 1) { + return InnerMatcher.matches(*CtorE->getArg(0)->IgnoreImpCasts(), Finder, + Builder); + } + } + + return InnerMatcher.matches(*Node.IgnoreImpCasts(), Finder, Builder); +} + +} // namespace + +UseStructuredBindingCheck::UseStructuredBindingCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + PairTypes(utils::options::parseStringList( + Options.get("PairTypes", DefaultPairTypes))) { + ; +} + +static auto getVarInitWithMemberMatcher( + StringRef PairName, StringRef MemberName, StringRef TypeName, + StringRef BindingName, + ast_matchers::internal::Matcher ExtraMatcher) { + return varDecl( + ExtraMatcher, + hasInitializer( + ignoringImpCasts(ignoringCopyCtorAndImplicitCast(memberExpr( + hasObjectExpression(ignoringImpCasts(declRefExpr( + to(equalsBoundNode(std::string(PairName)))))), + member(fieldDecl(hasName(MemberName), + hasType(qualType().bind(TypeName))))))))) + .bind(BindingName); +} + +void UseStructuredBindingCheck::registerMatchers(MatchFinder *Finder) { + auto PairType = + qualType(unless(isVolatileQualified()), + hasUnqualifiedDesugaredType(recordType( + hasDeclaration(cxxRecordDecl(hasAnyName(PairTypes)))))); + + auto UnlessShouldBeIgnored = + unless(anyOf(hasAnySpecifiersShouldBeIgnored(), isInMarco())); + + auto VarInitWithFirstMember = + getVarInitWithMemberMatcher(PairDeclName, "first", FirstTypeName, + FirstVarDeclName, UnlessShouldBeIgnored); + auto VarInitWithSecondMember = + getVarInitWithMemberMatcher(PairDeclName, "second", SecondTypeName, + SecondVarDeclName, UnlessShouldBeIgnored); + + auto RefToBindName = [&UnlessShouldBeIgnored](const llvm::StringLiteral &Name) + -> ast_matchers::internal::BindableMatcher { + return declRefExpr(to(varDecl(UnlessShouldBeIgnored).bind(Name))); + }; + + // X x; + // Y y; + // std::tie(x, y) = ...; + Finder->addMatcher( + exprWithCleanups( + unless(isInMarco()), + 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( + varDecl(equalsBoundNode(std::string(FirstVarDeclName))), + varDecl(equalsBoundNode(std::string(SecondVarDeclName))))), + this); + + // pair p = ...; + // X x = p.first; + // Y y = p.second; + Finder->addMatcher( + declStmt( + unless(isInMarco()), + hasSingleDecl( + varDecl(UnlessShouldBeIgnored, + hasType(qualType(anyOf(PairType, lValueReferenceType( + pointee(PairType)))) + .bind(PairVarTypeName)), + hasInitializer(expr())) + .bind(PairDeclName)), + hasNextTwoVarDecl(VarInitWithFirstMember, VarInitWithSecondMember)), + this); + + // for (pair p : map) { + // X x = p.first; + // Y y = p.second; + // } + Finder->addMatcher( + cxxForRangeStmt( + unless(isInMarco()), + hasLoopVariable( + varDecl(hasType(qualType(anyOf(PairType, lValueReferenceType( + pointee(PairType)))) + .bind(PairVarTypeName)), + hasInitializer(expr())) + .bind(PairDeclName)), + hasBody(compoundStmt(hasFirstTwoVarDecl(VarInitWithFirstMember, + VarInitWithSecondMember)) + .bind(ScopeBlockName))) + .bind(ForRangeStmtName), + this); +} + +static std::optional 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(FirstVarDeclName); + const auto *SecondVar = Result.Nodes.getNodeAs(SecondVarDeclName); + + const auto *DS1 = Result.Nodes.getNodeAs(FirstDeclStmtName); + const auto *DS2 = Result.Nodes.getNodeAs(SecondDeclStmtName); + const auto *ScopeBlock = Result.Nodes.getNodeAs(ScopeBlockName); + + // Captured structured bindings are a C++20 extension + if (!Result.Context->getLangOpts().CPlusPlus20) { + if (auto Matchers = match( + compoundStmt( + hasDescendant(lambdaExpr(hasAnyCapture(capturesVar(varDecl( + anyOf(equalsNode(FirstVar), equalsNode(SecondVar)))))))), + *ScopeBlock, *Result.Context); + !Matchers.empty()) + return; + } + + const auto *CFRS = Result.Nodes.getNodeAs(ForRangeStmtName); + auto DiagAndFix = [&DS1, &DS2, &FirstVar, &SecondVar, &CFRS, + this](SourceLocation DiagLoc, SourceRange ReplaceRange, + TransferType TT = TT_ByVal) { + StringRef Prefix; + switch (TT) { + case TT_ByVal: + Prefix = "auto"; + break; + case TT_ByConstVal: + Prefix = "const auto"; + break; + case TT_ByRef: + Prefix = "auto&"; + break; + case TT_ByConstRef: + Prefix = "const auto&"; + break; + } + std::vector Hints; + if (DS1) + Hints.emplace_back(FixItHint::CreateRemoval(DS1->getSourceRange())); + if (DS2) + Hints.emplace_back(FixItHint::CreateRemoval(DS2->getSourceRange())); + + std::string ReplacementText = + (Twine(Prefix) + " [" + FirstVar->getNameAsString() + ", " + + SecondVar->getNameAsString() + "]" + (CFRS ? " :" : "")) + .str(); + diag(DiagLoc, "use structured binding to decompose a pair") + << FixItHint::CreateReplacement(ReplaceRange, ReplacementText) << Hints; + }; + + if (const auto *COCE = + Result.Nodes.getNodeAs(StdTieAssignStmtName)) { + DiagAndFix(COCE->getBeginLoc(), + Result.Nodes.getNodeAs(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(PairDeclName); + const Expr *InitE = PairVar->getInit(); + if (auto Res = + match(expr(ignoringCopyCtorAndImplicitCast(expr().bind("init_expr"))), + *InitE, *Result.Context); + !Res.empty()) + InitE = Res[0].getNodeAs("init_expr"); + + const std::optional PairCaptureType = + getTransferType(*Result.Context, PairVar->getType(), InitE->getType()); + const std::optional FirstVarCaptureType = + getTransferType(*Result.Context, FirstVar->getType(), + *Result.Nodes.getNodeAs(FirstTypeName)); + const std::optional SecondVarCaptureType = + getTransferType(*Result.Context, SecondVar->getType(), + *Result.Nodes.getNodeAs(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..83d262a5db3cd --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseStructuredBindingCheck.h @@ -0,0 +1,36 @@ +//===----------------------------------------------------------------------===// +// +// 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: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-structured-binding.html +class UseStructuredBindingCheck : public ClangTidyCheck { +public: + UseStructuredBindingCheck(StringRef Name, ClangTidyContext *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; + } + +private: + const std::vector PairTypes; +}; + +} // 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 34091906cbff2..8302cbf64f095 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -179,6 +179,12 @@ New checks Finds virtual function overrides with different visibility than the function in the base class. +- New :doc:`modernize-use-structured-binding + ` check. + + Finds places where structured bindings could be used to decompose pairs and + suggests replacing them. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index c490d2ece2e0a..843a031dd2dd1 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -324,6 +324,7 @@ Clang-Tidy Checks :doc:`modernize-use-std-format `, "Yes" :doc:`modernize-use-std-numbers `, "Yes" :doc:`modernize-use-std-print `, "Yes" + :doc:`modernize-use-structured-binding `, "Yes" :doc:`modernize-use-trailing-return-type `, "Yes" :doc:`modernize-use-transparent-functors `, "Yes" :doc:`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..0d02e6761cdd4 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-structured-binding.rst @@ -0,0 +1,95 @@ +.. 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 x = p.first; + int y = p.second; + + into: + + auto [x, y] = getPair(); + +2. Use ``std::tie`` to decompose a pair into two predefined variables: + +.. code-block:: c++ + + int a; + int b; + std::tie(a, b) = getPair(); + + into: + + auto [a, b] = getPair(); + +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 + } + +The check also supports custom pair-like types via the :option:`PairTypes` +option. + +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 check for some situations which could possibly transfered +to structured bnindings, 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); + } + +Options +------- + +.. option:: PairTypes + + A semicolon-separated list of type names to be treated as pair-like for + structured binding suggestions. Example: `MyPairType;OtherPairType`. + Default is `std::pair`. 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..b77341c852edb --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-structured-binding/fake_std_pair_tuple.h @@ -0,0 +1,23 @@ +namespace std { + template + struct pair { + T1 first; + T2 second; + }; + + template + struct tuple { + tuple(Args&...) {} + + template + tuple operator=(const std::pair&); + }; + + template + tuple tie(Args&... args) { + return tuple(args...); + } +} + +template +std::pair getPair(); diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-structured-binding-custom.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-structured-binding-custom.cpp new file mode 100644 index 0000000000000..bd335e1edaf6e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-structured-binding-custom.cpp @@ -0,0 +1,32 @@ +// RUN: %check_clang_tidy -std=c++17-or-later %s modernize-use-structured-binding %t \ +// RUN: -config="{CheckOptions: {modernize-use-structured-binding.PairTypes: 'custom::pair; otherPair'}}" + +namespace custom { + struct pair { + int first; + int second; + }; +} + +struct otherPair { + int first; + int second; +}; + +void OptionTest() { + { + auto P = custom::pair(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = custom::pair(); + int x = P.first; + int y = P.second; + } + + { + auto P = otherPair(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES: auto [x, y] = otherPair(); + int x = P.first; + int y = P.second; + } +} 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..be3418a2ac224 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-structured-binding.cpp @@ -0,0 +1,410 @@ +// RUN: %check_clang_tidy -check-suffix=ALL,CPP20ORLATER -std=c++20-or-later %s modernize-use-structured-binding %t -- -- -I %S/Inputs/use-structured-binding/ +// RUN: %check_clang_tidy -check-suffix=ALL -std=c++17 %s modernize-use-structured-binding %t -- -- -I %S/Inputs/use-structured-binding/ +#include "fake_std_pair_tuple.h" + +template +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(); + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:5: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: auto [x, y] = getPair(); + int x = P.first; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + int y = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + } + + { + auto P = getPair(); + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:5: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: auto [x, y] = getPair(); + int x = P.first, y = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + } + + { + auto P = getPair(); + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:5: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: auto [x, y] = getPair(); + int x = P.first, y = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + int z; + } + + { + auto P = getPair(); + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:5: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: auto [x, y] = getPair(); + int x = P.first; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + auto y = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + } + + { + const auto P = getPair(); + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:5: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: const auto [x, y] = getPair(); + const int x = P.first; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + const auto y = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + } + + { + std::pair otherP; + auto& P = otherP; + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:5: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: auto& [x, y] = otherP; + int& x = P.first; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + auto& y = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + } + + { + std::pair otherP; + const auto& P = otherP; + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:5: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: const auto& [x, y] = otherP; + const int& x = P.first; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + const auto& y = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + } +} + +void forRangeWarnCases() { + std::pair Pairs[10]; + for (auto P : Pairs) { + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:8: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: for (auto [x, y] : Pairs) { + int x = P.first; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + int y = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + } + + for (auto P : Pairs) { + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:8: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: for (auto [x, y] : Pairs) { + int x = P.first, y = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + } + + for (auto P : Pairs) { + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:8: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: for (auto [x, y] : Pairs) { + int x = P.first, y = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + int z; + } + + for (const auto P : Pairs) { + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:8: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: for (const auto [x, y] : Pairs) { + const int x = P.first; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + const int y = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + } + + for (auto& P : Pairs) { + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:8: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: for (auto& [x, y] : Pairs) { + int& x = P.first; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + int& y = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + } + + for (const auto& P : Pairs) { + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:8: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: for (const auto& [x, y] : Pairs) { + const int& x = P.first; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + const int& y = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + } + + std::pair ClassPairs[10]; + for (auto P : ClassPairs) { + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:8: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: for (auto [c1, c2] : ClassPairs) { + TestClass c1 = P.first; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + TestClass c2 = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + } + + for (const auto P : ClassPairs) { + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:8: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: for (const auto [c1, c2] : ClassPairs) { + const TestClass c1 = P.first; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + const TestClass c2 = P.second; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + } +} + +void forRangeNotWarnCases() { + std::pair 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 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() { + int a = 0; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + int b = 0; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + std::tie(a, b) = getPair(); + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:3: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: auto [a, b] = getPair(); + + int x = 0, y = 0; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + std::tie(x, y) = getPair(); + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:3: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: auto [x, y] = getPair(); + + int* pa = nullptr; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + int* pb = nullptr; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + std::tie(pa, pb) = getPair(); + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:3: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: auto [pa, pb] = getPair(); + + TestClass c1 (1, 2); // REMOVE + // CHECK-FIXES-ALL: // REMOVE + TestClass c2 = TestClass {3, 4}; // REMOVE + // CHECK-FIXES-ALL: // REMOVE + std::tie(c1, c2) = getPair(); + // CHECK-MESSAGES-ALL: :[[@LINE-1]]:3: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-ALL: auto [c1, c2] = getPair(); +} + +void stdTieNotWarnCases() { + int a = 0; + int b = 0; + a = 4; + std::tie(a, b) = getPair(); // no warning + + int c = 0, d = 0; + int e = 0; + std::tie(a, b) = getPair(); // no warning + + int* pa = nullptr; + int* pb = nullptr; + MarkUsed(pa); + std::tie(pa, pb) = getPair(); // no warning + + TestClass c1 (1, 2); + TestClass c2 = TestClass {3, 4}; + MarkUsed(c2); + std::tie(c1, c2) = getPair(); +} + +void NotWarnForVarHasSpecifiers() { + { + auto P = getPair(); + const int x = P.first; + int y = P.second; + } + + { + auto P = getPair(); + volatile int x = P.first; + int y = P.second; + } + + { + auto P = getPair(); + int x = P.first; + [[maybe_unused]] int y = P.second; + } + + { + static auto P = getPair(); + int x = P.first; + int y = P.second; + } +} + +void NotWarnForMultiUsedPairVar() { + { + auto P = getPair(); + int x = P.first; + int y = P.second; + MarkUsed(P); + } + + { + auto P = getPair(); + int x = P.first; + MarkUsed(P); + int y = P.second; + } + + { + auto P = getPair(); + MarkUsed(P); + int x = P.first; + int y = P.second; + } + + { + std::pair 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(); + DECOMPOSE(P); +} + +#define GETPAIR auto P = getPair() + +void NotWarnForMacro2() { + GETPAIR; + int x = P.first; + int y = P.second; +} + +void captureByVal() { + auto P = getPair(); + // CHECK-MESSAGES-CPP20ORLATER: :[[@LINE-1]]:3: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-CPP20ORLATER: auto [x, y] = getPair(); + int x = P.first; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + int y = P.second; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + + auto lambda = [x]() { + int y = x; + }; +} + +void captureByRef() { + auto P = getPair(); + // CHECK-MESSAGES-CPP20ORLATER: :[[@LINE-1]]:3: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-CPP20ORLATER: auto [x, y] = getPair(); + int x = P.first; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + int y = P.second; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + + auto lambda = [&x]() { + x = 1; + }; +} + +void captureByAllRef() { + auto P = getPair(); + // CHECK-MESSAGES-CPP20ORLATER: :[[@LINE-1]]:3: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-CPP20ORLATER: auto [x, y] = getPair(); + int x = P.first; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + int y = P.second; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + + auto lambda = [&]() { + x = 1; + }; +} + +void deepLambda() { + auto P = getPair(); + // CHECK-MESSAGES-CPP20ORLATER: :[[@LINE-1]]:3: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-CPP20ORLATER: auto [x, y] = getPair(); + int x = P.first; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + int y = P.second; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + + { + auto lambda = [x]() { + int y = x; + }; + } +} + +void forRangeNotWarn() { + std::pair Pairs[10]; + for (auto P : Pairs) { + // CHECK-MESSAGES-CPP20ORLATER: :[[@LINE-1]]:8: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-CPP20ORLATER: for (auto [x, y] : Pairs) { + int x = P.first; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + int y = P.second; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + + auto lambda = [&]() { + x = 1; + }; + } +} + +void stdTieNotWarn() { + int x = 0; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + int y = 0; // REMOVE + // CHECK-FIXES-CPP20ORLATER: // REMOVE + std::tie(x, y) = getPair(); + // CHECK-MESSAGES-CPP20ORLATER: :[[@LINE-1]]:3: warning: use structured binding to decompose a pair [modernize-use-structured-binding] + // CHECK-FIXES-CPP20ORLATER: auto [x, y] = getPair(); + + auto lambda = [&x]() { + x = 1; + }; +}