fwolff updated this revision to Diff 387377.
fwolff added a comment.
Thanks for your suggestions @whisperity! I have fixed both of them now.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113804/new/
https://reviews.llvm.org/D113804
Files:
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
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -302,3 +302,15 @@
// 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; };
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
@@ -23,7 +23,8 @@
const bool IgnoreMacros;
SourceLocation LastReplacementEnd;
- SourceRange LastTagDeclRange;
+ llvm::DenseMap<const Decl *, SourceRange> LastTagDeclRanges;
+
std::string FirstTypedefType;
std::string FirstTypedefName;
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -25,19 +25,42 @@
}
void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(typedefDecl(unless(isInstantiated())).bind("typedef"),
+ Finder->addMatcher(typedefDecl(unless(isInstantiated()),
+ hasParent(decl().bind("parent-decl")))
+ .bind("typedef"),
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("parent-decl"))),
+ // We want the parent of the ClassTemplateDecl, not the parent
+ // of the specialization.
+ classTemplateSpecializationDecl(hasAncestor(
+ classTemplateDecl(hasParent(decl().bind("parent-decl")))))))
+ .bind("tagdecl"),
+ this);
}
void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *ParentDecl = Result.Nodes.getNodeAs<Decl>("parent-decl");
+ 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");
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;
}
@@ -102,12 +125,14 @@
auto Diag = diag(ReplaceRange.getBegin(), UseUsingWarning);
// If typedef contains a full tag declaration, extract its full text.
- if (LastTagDeclRange.isValid() &&
- ReplaceRange.fullyContains(LastTagDeclRange)) {
+ auto LastTagDeclRange = LastTagDeclRanges.find(ParentDecl);
+ if (LastTagDeclRange != LastTagDeclRanges.end() &&
+ LastTagDeclRange->second.isValid() &&
+ ReplaceRange.fullyContains(LastTagDeclRange->second)) {
bool Invalid;
- Type = std::string(
- Lexer::getSourceText(CharSourceRange::getTokenRange(LastTagDeclRange),
- *Result.SourceManager, getLangOpts(), &Invalid));
+ Type = std::string(Lexer::getSourceText(
+ CharSourceRange::getTokenRange(LastTagDeclRange->second),
+ *Result.SourceManager, getLangOpts(), &Invalid));
if (Invalid)
return;
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
