[clang-tidy] Add new check modernize-use-structured-binding

[flovent]

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.

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: None (flovent)

Changes

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.

---

Patch is 33.29 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/158462.diff

11 Files Affected:

• (modified) clang-tools-extra/clang-tidy/modernize/CMakeLists.txt (+1)
• (modified) clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp (+3)
• (added) clang-tools-extra/clang-tidy/modernize/UseStructuredBindingCheck.cpp (+419)
• (added) clang-tools-extra/clang-tidy/modernize/UseStructuredBindingCheck.h (+36)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6)
• (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1)
• (added) clang-tools-extra/docs/clang-tidy/checks/modernize/use-structured-binding.rst (+58)
• (added) clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-structured-binding/fake_std_pair_tuple.h (+23)
• (added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-structured-binding-custom.cpp (+32)
• (added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-structured-binding-skip-lambda-capture-in-cxx17.cpp (+67)
• (added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-structured-binding.cpp (+216)

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<UseNoexceptCheck>("modernize-use-noexcept");
     CheckFactories.registerCheck<UseNullptrCheck>("modernize-use-nullptr");
     CheckFactories.registerCheck<UseOverrideCheck>("modernize-use-override");
+    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..d6d6ae6cb83b3
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseStructuredBindingCheck.cpp
@@ -0,0 +1,419 @@
+//===--- UseStructuredBindingCheck.cpp - clang-tidy -----------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseStructuredBindingCheck.h"
+#include "../utils/DeclRefExprUtils.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+namespace {
+constexpr const char *DefaultPairTypes = "std::pair";
+constexpr llvm::StringLiteral PairDeclName = "PairVarD";
+constexpr llvm::StringLiteral PairVarTypeName = "PairVarType";
+constexpr llvm::StringLiteral FirstVarDeclName = "FirstVarDecl";
+constexpr llvm::StringLiteral SecondVarDeclName = "SecondVarDecl";
+constexpr llvm::StringLiteral FirstDeclStmtName = "FirstDeclStmt";
+constexpr llvm::StringLiteral SecondDeclStmtName = "SecondDeclStmt";
+constexpr llvm::StringLiteral FirstTypeName = "FirstType";
+constexpr llvm::StringLiteral SecondTypeName = "SecondType";
+constexpr llvm::StringLiteral ScopeBlockName = "ScopeBlock";
+constexpr llvm::StringLiteral StdTieAssignStmtName = "StdTieAssign";
+constexpr llvm::StringLiteral StdTieExprName = "StdTieExpr";
+constexpr llvm::StringLiteral ForRangeStmtName = "ForRangeStmt";
+
+/// 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
+};
+
+/// Try to match exactly two VarDecl inside two DeclStmts, and set binding for
+/// the used DeclStmts.
+bool matchTwoVarDecl(const DeclStmt *DS1, const DeclStmt *DS2,
+                     ast_matchers::internal::Matcher<VarDecl> InnerMatcher1,
+                     ast_matchers::internal::Matcher<VarDecl> InnerMatcher2,
+                     internal::ASTMatchFinder *Finder,
+                     internal::BoundNodesTreeBuilder *Builder) {
+  SmallVector<std::pair<const VarDecl *, const DeclStmt *>, 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<VarDecl>(VD))
+        Vars.emplace_back(Var, DS);
+      else
+        return false;
+    }
+
+    return true;
+  };
+
+  if (!CollectVarsInDeclStmt(DS1) || !CollectVarsInDeclStmt(DS2))
+    return false;
+
+  if (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;
+}
+
+/// 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<VarDecl>,
+               InnerMatcher1, ast_matchers::internal::Matcher<VarDecl>,
+               InnerMatcher2) {
+  DynTypedNodeList Parents = Finder->getASTContext().getParents(Node);
+  if (Parents.size() != 1)
+    return false;
+
+  auto *C = Parents[0].get<CompoundStmt>();
+  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<DeclStmt>(*(I + 1));
+  if (!DS2)
+    return false;
+
+  const DeclStmt *DS1 = (!DS2->isSingleDecl() || ((I + 2) == C->body_rend())
+                             ? nullptr
+                             : dyn_cast<DeclStmt>(*(I + 2)));
+
+  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<VarDecl>, InnerMatcher1,
+               ast_matchers::internal::Matcher<VarDecl>, InnerMatcher2) {
+  DynTypedNodeList Parents = Finder->getASTContext().getParents(Node);
+  if (Parents.size() != 1)
+    return false;
+
+  auto *C = Parents[0].get<CompoundStmt>();
+  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<DeclStmt>(*(I + 1)),
+          ((I + 2) == C->body_end() ? nullptr : dyn_cast<DeclStmt>(*(I + 2))),
+          InnerMatcher1, InnerMatcher2, Finder, Builder)) {
+    Builder->setBinding(ScopeBlockName, clang::DynTypedNode::create(*C));
+    return true;
+  }
+
+  return false;
+}
+
+/// Matches a Stmt whose parent is a CompoundStmt, and there a two VarDecls
+/// matching the inner matcher in the beginning of CompoundStmt.
+AST_MATCHER_P2(CompoundStmt, hasFirstTwoVarDecl,
+               ast_matchers::internal::Matcher<VarDecl>, InnerMatcher1,
+               ast_matchers::internal::Matcher<VarDecl>, InnerMatcher2) {
+  const auto *I = Node.body_begin();
+  if ((I) == Node.body_end())
+    return false;
+
+  return matchTwoVarDecl(
+      dyn_cast<DeclStmt>(*(I)),
+      ((I + 1) == Node.body_end() ? nullptr : dyn_cast<DeclStmt>(*(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<Expr>, InnerMatcher) {
+  if (const auto *CtorE = dyn_cast<CXXConstructExpr>(&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) {
+  return varDecl(
+             unless(hasAnySpecifiersShouldBeIgnored()), unless(isInMarco()),
+             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 VarInitWithFirstMember = getVarInitWithMemberMatcher(
+      PairDeclName, "first", FirstTypeName, FirstVarDeclName);
+  auto VarInitWithSecondMember = getVarInitWithMemberMatcher(
+      PairDeclName, "second", SecondTypeName, SecondVarDeclName);
+
+  // 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,
+                              declRefExpr(to(
+                                  varDecl(
+                                      unless(hasAnySpecifiersShouldBeIgnored()),
+                                      unless(isInMarco()))
+                                      .bind(FirstVarDeclName)))),
+                          hasArgument(
+                              1,
+                              declRefExpr(to(
+                                  varDecl(
+                                      unless(hasAnySpecifiersShouldBeIgnored()),
+                                      unless(isInMarco()))
+                                      .bind(SecondVarDeclName)))))
+                          .bind(StdTieExprName))),
+                  hasRHS(expr(hasType(PairType))))
+                  .bind(StdTieAssignStmtName)),
+          hasPreTwoVarDecl(
+              varDecl(equalsBoundNode(std::string(FirstVarDeclName))),
+              varDecl(equalsBoundNode(std::string(SecondVarDeclName))))),
+      this);
+
+  // pair<X, Y> p = ...;
+  // X x = p.first;
+  // Y y = p.second;
+  Finder->addMatcher(
+      declStmt(
+          unless(isInMarco()),
+          hasSingleDecl(
+              varDecl(unless(hasAnySpecifiersShouldBeIgnored()),
+                      hasType(qualType(anyOf(PairType, lValueReferenceType(
+                                                           pointee(PairType))))
+                                  .bind(PairVarTypeName)),
+                      hasInitializer(expr()))
+                  .bind(PairDeclName)),
+          hasNextTwoVarDecl(VarInitWithFirstMember, VarInitWithSecondMember)),
+      this);
+
+  // for (pair<X, Y> 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<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 *DS1 = Result.Nodes.getNodeAs<DeclStmt>(FirstDeclStmtName);
+  const auto *DS2 = Result.Nodes.getNodeAs<DeclStmt>(SecondDeclStmtName);
+  const auto *ScopeBlock = Result.Nodes.getNodeAs<CompoundStmt>(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<CXXForRangeStmt>(ForRangeStmtName);
+  auto DiagAndFix = [&](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<FixItHint> Hints;
+    if (DS1)
+      Hints.emplace_back(FixItHint::CreateRemoval(DS1->getSourceRange()));
+    if (DS2)
+      Hints.emplace_back(FixItHint::CreateRemoval(DS2->getSourceRange()));
+
+    std::string ReplacementText = Prefix.str() + " [" +
+                                  FirstVar->getNameAsString() + ", " +
+                                  SecondVar->getNameAsString() + "]";
+    if (CFRS)
+      ReplacementText += " :";
+    diag(DiagLoc, "Should use structured binding to decompose pair")
+        << FixItHint::CreateReplacement(ReplaceRange, ReplacementText) << Hints;
+  };
+
+  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 Expr *InitE = PairVar->getInit();
+  if (auto Res =
+          match(expr(ignoringCopyCtorAndImplicitCast(expr().bind("init_expr"))),
+                *InitE, *Result.Context);
+      !Res.empty())
+    InitE = Res[0].getNodeAs<Expr>("init_expr");
+
+  std::optional<TransferType> PairCaptureType =
+      getTransferType(*Result.Context, PairVar->getType(), InitE->getType());
+  std::optional<TransferType> FirstVarCaptureType =
+      getTransferType(*Result.Context, FirstVar->getType(),
+                      *Result.Nodes.getNodeAs<QualType>(FirstTypeName));
+  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..63bc0a8c3da45
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseStructuredBindingCheck.h
@@ -0,0 +1,36 @@
+//===--- UseStructuredBindingCheck.h - clang-tidy ---------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_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<StringRef> 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...
[truncated]

[flovent]

It's tested in llvm codebase with -DLLVM_ENABLE_PROJECTS="bolt;clang;clang-tools-extra;compiler-rt", since plenty of llvm code it's written in pre-c++17 standard.

It provides ~431 warnings and fixits with check option modernize-use-structured-binding.PairTypes: 'std::pair; llvm::detail::DenseMapPair'

See apply modernize-structured-binding commit based on trunk ad9d551

[vbvictor]

Very interesting check, thank you for working on it!
For now, looked only at tests/docs

[EugeneZelenko]

Please use as much of 80 characters limit as possible.

[vbvictor]

Instead of concrete pair types we could benefit from duck-typing. A "pair" is class with only 2 non-ref non-ponter public values.

So we wouldn't need list of types.

[HerrCai0907]

Yes, but I am not sure whether we should directly use duck typing here. Actually structured binding can be used for all of POD type like.

struct A {
  int a;
  int b;
  int c;
};

void f(A a) { auto const &[c, d, e] = a; }

Perhaps relaxing the two-member limit would make this inspection more worthy of the name.
But i am not sure whether will it lead to abuse.

[flovent]

This check is currently designed to only handle pair-like type, because they are more common than other cases.

I'm not very sure what abuse means here. Structured binding can be used for any class without any non-public data member, we may need to check variable's class type's all fields, and it's the first thing we do when we meet VarDecl.

[HerrCai0907]

what I mean by "abuse" is I am not sure whether using structured binding is always the best approach for structures?
For the current check, could we relax the requirement for 2 members. Then try to match all type defined in PairTypes options (maybe rename is needed) with default option ::std::pair. Then the check is suitable for all kind of structured binding by defined targeted structure name.

[flovent]

I think if there is code matches this pattern like what we check for pair types:

struct A {
  int a;
  int b;
  int c;
};

void f() { 
     A a = ...;
     int x = a.a;
     int y = a.b;
     int z = a.c;
 }

is a good idea to use structured binding. Passing type name through option is better in that case, and we are going to need a matchNVarDecl matcher for this. but it just seems uncommon to me.

And for all kinds of structured binding, there are tuple-like class, the pattern should be:

   Tuple t = ...;
   auto x = std::get<0>(t);
   auto y = std::get<1>(t);
   // ....

and array:

  int x[10];
  int a = x[0];
  int b = x[1];
  // ...

[vbvictor]

I think this could be placed inside regular matchers. with Cxx20 ? notCapturedByLambda() : anything()

[flovent]

For execution efficiency i set binding for ScopeBlock in hasPreTwoVarDecl and hasNextTwoVarDecl because they need to call getParents anyway rather than using hasParent again in the matcher of Finder->addMatcher, is there a way to get it before we enter check?

Or we use hasParent first, pass it to hasPreTwoVarDecl, but there is not AST_MATCHER_P3, we might need to create one.

[HerrCai0907]

It does not match the pattern like VarDecl, VarDecl, NonVarDecl though I cannot construct a real example. but since it can match NonVarDecl, VarDecl, VarDecl. I think we should handle this pattern also.

[flovent]

What does NonVarDecl, VarDecl, VarDecl means, in one DeclStmt? I think it will return false when it hits a NonVarDecl.

Although I did find a problem, we should stop collecting from DS2 when we already got two VarDecls from DS1, this leads to FN.