[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

FĂ©lix-Antoine Constantin via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 10 10:46:07 PDT 2024


https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/82947

>From f1d957413bcc81e9ced06ad40570859769f4c1c1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 <felix-antoine.constantin at polymtl.ca>
Date: Sun, 25 Feb 2024 20:20:59 -0500
Subject: [PATCH 1/6] [clang-tidy] Improved modernize-use-using by fixing a
 false-negative

The check needs a parent decl to match but if the typedef is in a function,
the parent is a declStmt which is not a decl by itself.
Improved the matcher to match on either a decl or a declstmt and extract
the decl from the stmt in the latter case.

fixes #72179
---
 .../clang-tidy/modernize/UseUsingCheck.cpp     | 16 +++++++++++++---
 clang-tools-extra/docs/ReleaseNotes.rst        |  3 +++
 .../checkers/modernize/use-using.cpp           | 18 ++++++++++++++++++
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index bb05f206c717ce..50a07fc02e31b4 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = "extern-c-decl";
 static constexpr llvm::StringLiteral ParentDeclName = "parent-decl";
 static constexpr llvm::StringLiteral TagDeclName = "tag-decl";
 static constexpr llvm::StringLiteral TypedefName = "typedef";
+static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt";
 
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
@@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
           unless(isInstantiated()),
           optionally(hasAncestor(
               linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))),
-          hasParent(decl().bind(ParentDeclName)))
+          anyOf(hasParent(decl().bind(ParentDeclName)),
+                hasParent(declStmt().bind(DeclStmtName))))
           .bind(TypedefName),
       this);
 
@@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
       tagDecl(
           anyOf(allOf(unless(anyOf(isImplicit(),
                                    classTemplateSpecializationDecl())),
-                      hasParent(decl().bind(ParentDeclName))),
+                      anyOf(hasParent(decl().bind(ParentDeclName)),
+                            hasParent(declStmt().bind(DeclStmtName)))),
                 // We want the parent of the ClassTemplateDecl, not the parent
                 // of the specialization.
                 classTemplateSpecializationDecl(hasAncestor(classTemplateDecl(
-                    hasParent(decl().bind(ParentDeclName)))))))
+                    anyOf(hasParent(decl().bind(ParentDeclName)),
+                          hasParent(declStmt().bind(DeclStmtName))))))))
           .bind(TagDeclName),
       this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *ParentDecl = Result.Nodes.getNodeAs<Decl>(ParentDeclName);
+
+  if (!ParentDecl) {
+    const auto *ParentDeclStmt = Result.Nodes.getNodeAs<DeclStmt>(DeclStmtName);
+    ParentDecl = ParentDeclStmt->getSingleDecl();
+  }
+
   if (!ParentDecl)
     return;
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 44680f79de6f54..479b9b86444184 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -224,6 +224,9 @@ Changes in existing checks
   analyzed, se the check now handles the common patterns
   `const auto e = (*vector_ptr)[i]` and `const auto e = vector_ptr->at(i);`.
 
+- Improved :doc:`modernize-use-using <clang-tidy/checks/modernize/use-using>`
+  check by fixing false-negative in functions.
+
 - Improved :doc:`readability-implicit-bool-conversion
   <clang-tidy/checks/readability/implicit-bool-conversion>` check to provide
   valid fix suggestions for ``static_cast`` without a preceding space and
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 462bc984fd3ad5..230c7c94cda1cf 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -342,3 +342,21 @@ typedef int InExternCPP;
 // CHECK-FIXES: using InExternCPP = int;
 
 }
+
+namespace ISSUE_72179
+{  
+  void foo()
+  {
+    typedef int a;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using]
+    // CHECK-FIXES: using a = int;
+
+  }
+
+  void foo2()
+  {
+    typedef struct { int a; union { int b; }; } c;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using]
+    // CHECK-FIXES: using c = struct { int a; union { int b; }; };
+  }
+}

>From e7dcbbcf92e896354e9a52da259251a7d3a74369 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 <felix-antoine.constantin at polymtl.ca>
Date: Sun, 25 Feb 2024 22:14:22 -0500
Subject: [PATCH 2/6] fixup! [clang-tidy] Improved modernize-use-using by
 fixing a false-negative

Fixed crash in tests
---
 clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index 50a07fc02e31b4..fa52e0c28e2747 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -69,7 +69,8 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
 
   if (!ParentDecl) {
     const auto *ParentDeclStmt = Result.Nodes.getNodeAs<DeclStmt>(DeclStmtName);
-    ParentDecl = ParentDeclStmt->getSingleDecl();
+    if (ParentDeclStmt)
+      ParentDecl = ParentDeclStmt->getSingleDecl();
   }
 
   if (!ParentDecl)

>From 65f7f8720c2978e38892191eaf7653c48d1b78d6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 <felix-antoine.constantin at polymtl.ca>
Date: Sat, 9 Mar 2024 21:07:06 -0500
Subject: [PATCH 3/6] fixup! [clang-tidy] Improved modernize-use-using by
 fixing a false-negative

Fixed crash
---
 .../clang-tidy/modernize/UseUsingCheck.cpp            | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index fa52e0c28e2747..24eefdb082eb32 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "UseUsingCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclGroup.h"
 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
@@ -69,8 +70,14 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
 
   if (!ParentDecl) {
     const auto *ParentDeclStmt = Result.Nodes.getNodeAs<DeclStmt>(DeclStmtName);
-    if (ParentDeclStmt)
-      ParentDecl = ParentDeclStmt->getSingleDecl();
+    if (ParentDeclStmt) {
+      if (ParentDeclStmt->isSingleDecl())
+        ParentDecl = ParentDeclStmt->getSingleDecl();
+      else
+        ParentDecl =
+            ParentDeclStmt->getDeclGroup().getDeclGroup()
+                [ParentDeclStmt->getDeclGroup().getDeclGroup().size() - 1];
+    }
   }
 
   if (!ParentDecl)

>From 20681a196f5c154349adb99269656450d32f2c98 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 <felix-antoine.constantin at polymtl.ca>
Date: Sun, 10 Mar 2024 09:24:52 -0400
Subject: [PATCH 4/6] fixup! [clang-tidy] Improved modernize-use-using by
 fixing a false-negative

Added a few test cases
---
 .../checkers/modernize/use-using.cpp          | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 230c7c94cda1cf..6f6a602fd84a53 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -359,4 +359,27 @@ namespace ISSUE_72179
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using]
     // CHECK-FIXES: using c = struct { int a; union { int b; }; };
   }
+
+  template <typename T>
+  void foo3()
+  {
+    typedef T b;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 'typedef' [modernize-use-using]
+    // CHECK-FIXES: using b = T;
+  }
+
+  template <typename T>
+  class MyClass
+  {
+    void foo()
+    {
+      typedef MyClass c;
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use 'using' instead of 'typedef' [modernize-use-using]
+      // CHECK-FIXES: using c = MyClass;
+    }
+  };
+
+  const auto foo4 = [](int a){typedef int d;};
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: use 'using' instead of 'typedef' [modernize-use-using]
+  // CHECK-FIXES: const auto foo4 = [](int a){using d = int;};
 }

>From 617925efb43c1b18eb4b3b01a34d609b445b3327 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 <felix-antoine.constantin at polymtl.ca>
Date: Sun, 10 Mar 2024 09:27:00 -0400
Subject: [PATCH 5/6] fixup! [clang-tidy] Improved modernize-use-using by
 fixing a false-negative

Improved release notes message
---
 clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 479b9b86444184..7e4c75b5c3a294 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -225,7 +225,7 @@ Changes in existing checks
   `const auto e = (*vector_ptr)[i]` and `const auto e = vector_ptr->at(i);`.
 
 - Improved :doc:`modernize-use-using <clang-tidy/checks/modernize/use-using>`
-  check by fixing false-negative in functions.
+  check by adding support for detection of typedefs declared on function level.
 
 - Improved :doc:`readability-implicit-bool-conversion
   <clang-tidy/checks/readability/implicit-bool-conversion>` check to provide

>From 15693757a628ca594dcf82439004a462e9ecef4e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 <felix-antoine.constantin at polymtl.ca>
Date: Sun, 10 Mar 2024 13:45:55 -0400
Subject: [PATCH 6/6] fixup! [clang-tidy] Improved modernize-use-using by
 fixing a false-negative

Attemp at fixing windows test!
---
 .../test/clang-tidy/checkers/modernize/use-using.cpp            | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 6f6a602fd84a53..925e5f9c1ca54e 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s modernize-use-using %t -- -- -I %S/Inputs/use-using/
+// RUN: %check_clang_tidy %s modernize-use-using %t -- -- -fno-delayed-template-parsing -I %S/Inputs/use-using/
 
 typedef int Type;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]



More information about the cfe-commits mailing list