[clang-tools-extra] 95d7738 - [clang-tidy] Fix behavior of `modernize-use-using` with nested structs/unions
Fabian Wolff via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 21 06:21:27 PDT 2022
Author: Fabian Wolff
Date: 2022-04-21T15:18:31+02:00
New Revision: 95d77383f2ba8d3136856b52520d3f73f9bc89e7
URL: https://github.com/llvm/llvm-project/commit/95d77383f2ba8d3136856b52520d3f73f9bc89e7
DIFF: https://github.com/llvm/llvm-project/commit/95d77383f2ba8d3136856b52520d3f73f9bc89e7.diff
LOG: [clang-tidy] Fix behavior of `modernize-use-using` with nested structs/unions
Fixes https://github.com/llvm/llvm-project/issues/50334.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D113804
Added:
Modified:
clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index a12a056f13ea1..fa3cfa5e9cfca 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -16,6 +16,10 @@ namespace clang {
namespace tidy {
namespace modernize {
+static constexpr llvm::StringLiteral ParentDeclName = "parent-decl";
+static constexpr llvm::StringLiteral TagDeclName = "tag-decl";
+static constexpr llvm::StringLiteral TypedefName = "typedef";
+
UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
@@ -25,23 +29,45 @@ void UseUsingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
}
void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(typedefDecl(unless(isInstantiated())).bind("typedef"),
+ Finder->addMatcher(typedefDecl(unless(isInstantiated()),
+ hasParent(decl().bind(ParentDeclName)))
+ .bind(TypedefName),
this);
- // This matcher used to find tag declarations in source code within typedefs.
- // They appear in the AST just *prior* to the typedefs.
- Finder->addMatcher(tagDecl(unless(isImplicit())).bind("tagdecl"), this);
+
+ // This matcher is used to find tag declarations in source code within
+ // typedefs. They appear in the AST just *prior* to the typedefs.
+ Finder->addMatcher(
+ tagDecl(
+ anyOf(allOf(unless(anyOf(isImplicit(),
+ classTemplateSpecializationDecl())),
+ hasParent(decl().bind(ParentDeclName))),
+ // We want the parent of the ClassTemplateDecl, not the parent
+ // of the specialization.
+ classTemplateSpecializationDecl(hasAncestor(classTemplateDecl(
+ hasParent(decl().bind(ParentDeclName)))))))
+ .bind(TagDeclName),
+ this);
}
void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *ParentDecl = Result.Nodes.getNodeAs<Decl>(ParentDeclName);
+ if (!ParentDecl)
+ return;
+
// Match CXXRecordDecl only to store the range of the last non-implicit full
// declaration, to later check whether it's within the typdef itself.
- const auto *MatchedTagDecl = Result.Nodes.getNodeAs<TagDecl>("tagdecl");
+ const auto *MatchedTagDecl = Result.Nodes.getNodeAs<TagDecl>(TagDeclName);
if (MatchedTagDecl) {
- LastTagDeclRange = MatchedTagDecl->getSourceRange();
+ // It is not sufficient to just track the last TagDecl that we've seen,
+ // because if one struct or union is nested inside another, the last TagDecl
+ // before the typedef will be the nested one (PR#50990). Therefore, we also
+ // keep track of the parent declaration, so that we can look up the last
+ // TagDecl that is a sibling of the typedef in the AST.
+ LastTagDeclRanges[ParentDecl] = MatchedTagDecl->getSourceRange();
return;
}
- const auto *MatchedDecl = Result.Nodes.getNodeAs<TypedefDecl>("typedef");
+ const auto *MatchedDecl = Result.Nodes.getNodeAs<TypedefDecl>(TypedefName);
if (MatchedDecl->getLocation().isInvalid())
return;
@@ -102,13 +128,14 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
auto Diag = diag(ReplaceRange.getBegin(), UseUsingWarning);
// If typedef contains a full tag declaration, extract its full text.
- if (LastTagDeclRange.isValid() &&
- ReplaceRange.fullyContains(LastTagDeclRange)) {
- bool Invalid;
- Type = std::string(
- Lexer::getSourceText(CharSourceRange::getTokenRange(LastTagDeclRange),
- *Result.SourceManager, getLangOpts(), &Invalid));
- if (Invalid)
+ auto LastTagDeclRange = LastTagDeclRanges.find(ParentDecl);
+ if (LastTagDeclRange != LastTagDeclRanges.end() &&
+ LastTagDeclRange->second.isValid() &&
+ ReplaceRange.fullyContains(LastTagDeclRange->second)) {
+ Type = std::string(Lexer::getSourceText(
+ CharSourceRange::getTokenRange(LastTagDeclRange->second),
+ *Result.SourceManager, getLangOpts()));
+ if (Type.empty())
return;
}
diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
index f116ff2445b9e..745cbb86df117 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
@@ -23,7 +23,8 @@ class UseUsingCheck : public ClangTidyCheck {
const bool IgnoreMacros;
SourceLocation LastReplacementEnd;
- SourceRange LastTagDeclRange;
+ llvm::DenseMap<const Decl *, SourceRange> LastTagDeclRanges;
+
std::string FirstTypedefType;
std::string FirstTypedefName;
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 b74e67a17f862..ec4ea61426582 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
@@ -302,3 +302,15 @@ struct InjectedClassNameWithUnnamedArgument {
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
// CHECK-FIXES: using b = InjectedClassNameWithUnnamedArgument;
};
+
+typedef struct { int a; union { int b; }; } PR50990;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using PR50990 = struct { int a; union { int b; }; };
+
+typedef struct { struct { int a; struct { struct { int b; } c; int d; } e; } f; int g; } PR50990_nested;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using PR50990_nested = struct { struct { int a; struct { struct { int b; } c; int d; } e; } f; int g; };
+
+typedef struct { struct { int a; } b; union { int c; float d; struct { int e; }; }; struct { double f; } g; } PR50990_siblings;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using PR50990_siblings = struct { struct { int a; } b; union { int c; float d; struct { int e; }; }; struct { double f; } g; };
More information about the cfe-commits
mailing list