diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h @@ -23,7 +23,8 @@ const bool IgnoreMacros; SourceLocation LastReplacementEnd; - SourceRange LastTagDeclRange; + llvm::DenseMap LastTagDeclRanges; + std::string FirstTypedefType; std::string FirstTypedefName; diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp @@ -16,6 +16,10 @@ 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::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(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"); + const auto *MatchedTagDecl = Result.Nodes.getNodeAs(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("typedef"); + const auto *MatchedDecl = Result.Nodes.getNodeAs(TypedefName); if (MatchedDecl->getLocation().isInvalid()) return; @@ -102,13 +128,14 @@ 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/test/clang-tidy/checkers/modernize-use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp --- 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 @@ // 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; };