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 @@ -22,6 +22,10 @@ class UseUsingCheck : public ClangTidyCheck { const bool IgnoreMacros; + SourceLocation LastReplacementEnd; + SourceRange LastCxxDeclRange; + std::string FirstTypedefType; + std::string FirstTypedefName; public: UseUsingCheck(StringRef Name, ClangTidyContext *Context); 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 @@ -25,107 +25,88 @@ return; Finder->addMatcher(typedefDecl(unless(isInstantiated())).bind("typedef"), this); + // This matcher used to find structs defined in source code within typedefs. + // They appear in the AST just *prior* to the typedefs. + Finder->addMatcher(cxxRecordDecl(unless(isImplicit())).bind("struct"), this); } -// Checks if 'typedef' keyword can be removed - we do it only if -// it is the only declaration in a declaration chain. -static bool CheckRemoval(SourceManager &SM, SourceLocation StartLoc, - ASTContext &Context) { - assert(StartLoc.isFileID() && "StartLoc must not be in a macro"); - std::pair LocInfo = SM.getDecomposedLoc(StartLoc); - StringRef File = SM.getBufferData(LocInfo.first); - const char *TokenBegin = File.data() + LocInfo.second; - Lexer DeclLexer(SM.getLocForStartOfFile(LocInfo.first), Context.getLangOpts(), - File.begin(), TokenBegin, File.end()); - - Token Tok; - int NestingLevel = 0; // Parens, braces, and square brackets - int AngleBracketLevel = 0; - bool FoundTypedef = false; - - while (!DeclLexer.LexFromRawLexer(Tok) && !Tok.is(tok::semi)) { - switch (Tok.getKind()) { - case tok::l_brace: - if (NestingLevel == 0 && AngleBracketLevel == 0) { - // At top level, this might be the `typedef struct {...} T;` case. - // Inside parens, square brackets, or angle brackets it's not. - return false; - } - ++NestingLevel; - break; - case tok::l_paren: - case tok::l_square: - ++NestingLevel; - break; - case tok::r_brace: - case tok::r_paren: - case tok::r_square: - --NestingLevel; - break; - case tok::less: - // If not nested in paren/brace/square bracket, treat as opening angle bracket. - if (NestingLevel == 0) - ++AngleBracketLevel; - break; - case tok::greater: - // Per C++ 17 Draft N4659, Section 17.2/3 - // https://timsong-cpp.github.io/cppwp/n4659/temp.names#3: - // "When parsing a template-argument-list, the first non-nested > is - // taken as the ending delimiter rather than a greater-than operator." - // If not nested in paren/brace/square bracket, treat as closing angle bracket. - if (NestingLevel == 0) - --AngleBracketLevel; - break; - case tok::comma: - if (NestingLevel == 0 && AngleBracketLevel == 0) { - // If there is a non-nested comma we have two or more declarations in this chain. - return false; - } - break; - case tok::raw_identifier: - if (Tok.getRawIdentifier() == "typedef") { - FoundTypedef = true; - } - break; - default: - break; - } +void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { + // 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 *MatchedCxxRecordDecl = + Result.Nodes.getNodeAs("struct"); + if (MatchedCxxRecordDecl) { + LastCxxDeclRange = MatchedCxxRecordDecl->getSourceRange(); + return; } - // Sanity check against weird macro cases. - return FoundTypedef; -} - -void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { const auto *MatchedDecl = Result.Nodes.getNodeAs("typedef"); if (MatchedDecl->getLocation().isInvalid()) return; - auto &Context = *Result.Context; - auto &SM = *Result.SourceManager; - SourceLocation StartLoc = MatchedDecl->getBeginLoc(); if (StartLoc.isMacroID() && IgnoreMacros) return; - auto Diag = diag(StartLoc, "use 'using' instead of 'typedef'"); + static const char *UseUsingWarning = "use 'using' instead of 'typedef'"; - // do not fix if there is macro or array - if (MatchedDecl->getUnderlyingType()->isArrayType() || StartLoc.isMacroID()) + // Warn at StartLoc but do not fix if there is macro or array. + if (MatchedDecl->getUnderlyingType()->isArrayType() || StartLoc.isMacroID()) { + diag(StartLoc, UseUsingWarning); return; + } - if (CheckRemoval(SM, StartLoc, Context)) { - auto printPolicy = PrintingPolicy(getLangOpts()); - printPolicy.SuppressScope = true; - printPolicy.ConstantArraySizeAsWritten = true; - printPolicy.UseVoidForZeroParams = false; - - Diag << FixItHint::CreateReplacement( - MatchedDecl->getSourceRange(), - "using " + MatchedDecl->getNameAsString() + " = " + - MatchedDecl->getUnderlyingType().getAsString(printPolicy)); + auto printPolicy = PrintingPolicy(getLangOpts()); + printPolicy.SuppressScope = true; + printPolicy.ConstantArraySizeAsWritten = true; + printPolicy.UseVoidForZeroParams = false; + + std::string Type = MatchedDecl->getUnderlyingType().getAsString(printPolicy); + std::string Name = MatchedDecl->getNameAsString(); + SourceRange ReplaceRange = MatchedDecl->getSourceRange(); + + // typedefs with multiple comma-separated definitions produce multiple + // consecutive TypedefDecl nodes whose SourceRanges overlap. Each range starts + // at the "typedef" and then continues *across* previous definitions through + // the end of the current TypedefDecl definition. + std::string Using = "using "; + if (ReplaceRange.getBegin().isMacroID() || + ReplaceRange.getBegin() >= LastReplacementEnd) { + // This is the first (and possibly the only) TypedefDecl in a typedef. Save + // Type and Name in case we find subsequent TypedefDecl's in this typedef. + FirstTypedefType = Type; + FirstTypedefName = Name; + } else { + // This is additional TypedefDecl in a comma-separated typedef declaration. + // Start replacement *after* prior replacement and separate with semicolon. + ReplaceRange.setBegin(LastReplacementEnd); + Using = ";\nusing "; + + // If this additional TypedefDecl's Type starts with the first TypedefDecl's + // type, make this using statement refer back to the first type, e.g. make + // "typedef int Foo, *Foo_p;" -> "using Foo = int;\nusing Foo_p = Foo*;" + if (Type.size() > FirstTypedefType.size() && + Type.substr(0, FirstTypedefType.size()) == FirstTypedefType) + Type = FirstTypedefName + Type.substr(FirstTypedefType.size() + 1); } + if (!ReplaceRange.getEnd().isMacroID()) + LastReplacementEnd = ReplaceRange.getEnd().getLocWithOffset(Name.size()); + + auto Diag = diag(ReplaceRange.getBegin(), UseUsingWarning); + + // If typedef contains a full struct/class declaration, extract its full text. + if (LastCxxDeclRange.isValid() && ReplaceRange.fullyContains(LastCxxDeclRange)) { + bool Invalid; + Type = + Lexer::getSourceText(CharSourceRange::getTokenRange(LastCxxDeclRange), + *Result.SourceManager, getLangOpts(), &Invalid); + if (Invalid) + return; + } + + std::string Replacement = Using + Name + " = " + Type; + Diag << FixItHint::CreateReplacement(ReplaceRange, Replacement); } } // namespace modernize diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -166,6 +166,10 @@ Finds types that could be made trivially-destructible by removing out-of-line defaulted destructor declarations. +- The :doc:`modernize-use-using + ` check now converts typedefs containing + struct definitions and multiple comma-separated types. + - Improved :doc:`bugprone-posix-return ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst --- a/clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst @@ -14,6 +14,8 @@ class Class{}; typedef void (Class::* MyPtrType)() const; + typedef struct { int a; } R_t, *R_p; + After: .. code-block:: c++ @@ -23,6 +25,9 @@ class Class{}; using MyPtrType = void (Class::*)() const; + using R_t = struct { int a; }; + using R_p = R_t*; + This check requires using C++11 or higher to run. Options 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 @@ -84,7 +84,11 @@ typedef int bla1, bla2, bla3; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' -// CHECK-FIXES: typedef int bla1, bla2, bla3; +// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: use 'using' instead of 'typedef' +// CHECK-MESSAGES: :[[@LINE-3]]:23: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using bla1 = int; +// CHECK-FIXES-NEXT: using bla2 = int; +// CHECK-FIXES-NEXT: using bla3 = int; #define CODE typedef int INT @@ -136,16 +140,16 @@ typedef struct Q1 { int a; } S1; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' -// CHECK-FIXES: typedef struct Q1 { int a; } S1; +// CHECK-FIXES: using S1 = struct Q1 { int a; }; typedef struct { int b; } S2; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' -// CHECK-FIXES: typedef struct { int b; } S2; +// CHECK-FIXES: using S2 = struct { int b; }; struct Q2 { int c; } typedef S3; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' -// CHECK-FIXES: struct Q2 { int c; } typedef S3; +// CHECK-FIXES: using S3 = struct Q2 { int c; }; struct { int d; } typedef S4; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' -// CHECK-FIXES: struct { int d; } typedef S4; +// CHECK-FIXES: using S4 = struct { int d; }; namespace my_space { class my_cclass {}; @@ -196,11 +200,15 @@ typedef S<(0 > 0), int> S_t, *S_p; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' -// CHECK-FIXES: typedef S<(0 > 0), int> S_t, *S_p; +// CHECK-MESSAGES: :[[@LINE-2]]:28: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using S_t = S<(0 > 0), int>; +// CHECK-FIXES-NEXT: using S_p = S_t*; typedef S<(0 < 0), int> S2_t, *S2_p; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' -// CHECK-FIXES: typedef S<(0 < 0), int> S2_t, *S2_p; +// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using S2_t = S<(0 < 0), int>; +// CHECK-FIXES-NEXT: using S2_p = S2_t*; typedef S<(0 > 0 && (3 > 1) && (1 < 1)), int> S3_t; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' @@ -213,7 +221,9 @@ typedef Q Q_t, *Q_p; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' -// CHECK-FIXES: typedef Q Q_t, *Q_p; +// CHECK-MESSAGES: :[[@LINE-2]]:24: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using Q_t = Q; +// CHECK-FIXES-NEXT: using Q_p = Q_t*; typedef Q Q2_t; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' @@ -227,7 +237,9 @@ typedef Q Q3_t, *Q3_p; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' -// CHECK-FIXES: typedef Q Q3_t, *Q3_p; +// CHECK-MESSAGES: :[[@LINE-2]]:27: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using Q3_t = Q; +// CHECK-FIXES-NEXT: using Q3_p = Q3_t*; typedef Q Q3_t; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' @@ -246,4 +258,12 @@ typedef Variadic >, S<(0 < 0), Variadic > > > Variadic_t, *Variadic_p; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' -// CHECK-FIXES: typedef Variadic >, S<(0 < 0), Variadic > > > Variadic_t, *Variadic_p; +// CHECK-MESSAGES: :[[@LINE-2]]:103: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using Variadic_t = Variadic >, S<(0 < 0), Variadic > > >; +// CHECK-FIXES-NEXT: using Variadic_p = Variadic_t*; + +typedef struct { int a; } R_t, *R_p; +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' +// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using R_t = struct { int a; }; +// CHECK-FIXES-NEXT: using R_p = R_t*; diff --git a/clang/include/clang/Basic/SourceLocation.h b/clang/include/clang/Basic/SourceLocation.h --- a/clang/include/clang/Basic/SourceLocation.h +++ b/clang/include/clang/Basic/SourceLocation.h @@ -230,6 +230,11 @@ return B != X.B || E != X.E; } + // Returns true iff other is wholly contained within this range. + bool fullyContains(const SourceRange &other) const { + return B <= other.B && E >= other.E; + } + void print(raw_ostream &OS, const SourceManager &SM) const; std::string printToString(const SourceManager &SM) const; void dump(const SourceManager &SM) const;