[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
Sat Mar 9 18:13:20 PST 2024
https://github.com/felix642 updated https://github.com/llvm/llvm-project/pull/82947
>From d1cbed0e2e83bd3544067fd25d7e811f1ab3f095 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/3] [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 6fd01ed9d471c5..2b36ae4acb9f1d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -183,6 +183,9 @@ Changes in existing checks
<clang-tidy/checks/modernize/use-override>` check to also remove any trailing
whitespace when deleting the ``virtual`` keyword.
+- 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 0b5c78508fbf49229d576171d7af6c6bb16b4367 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/3] 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 60df9a1dabff7abc108e559a546bfc41293a828d 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/3] 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)
More information about the cfe-commits
mailing list