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 @@ -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); 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,87 +25,89 @@ 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().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 ParenLevel = 0; - bool FoundTypedef = false; - - while (!DeclLexer.LexFromRawLexer(Tok) && !Tok.is(tok::semi)) { - switch (Tok.getKind()) { - case tok::l_brace: - case tok::r_brace: - // This might be the `typedef struct {...} T;` case. - return false; - case tok::l_paren: - ParenLevel++; - break; - case tok::r_paren: - ParenLevel--; - break; - case tok::comma: - if (ParenLevel == 0) { - // If there is comma and we are not between open parenthesis then it is - // 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) { + if (!MatchedCxxRecordDecl->isImplicit()) + 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()) { + auto Diag = 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 = "; using "; + + // 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; using 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.contains(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 Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -132,6 +132,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. Index: clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst +++ 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,8 @@ 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 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 @@ -84,7 +84,9 @@ 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; using bla2 = int; using bla3 = int; #define CODE typedef int INT @@ -136,16 +138,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 {}; @@ -183,3 +185,77 @@ void f() override { super::f(); } }; } + +template +class TwoArgTemplate { + typedef TwoArgTemplate self; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef' + // CHECK-FIXES: using self = TwoArgTemplate; +}; + +template +struct S {}; + +typedef S<(0 > 0), int> S_t, *S_p; +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' +// CHECK-MESSAGES: :[[@LINE-2]]:28: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using S_t = S<(0 > 0), int>; 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-MESSAGES: :[[@LINE-2]]:29: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using S2_t = S<(0 < 0), int>; 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' +// CHECK-FIXES: using S3_t = S<(0 > 0 && (3 > 1) && (1 < 1)), int>; + +template +struct Q {}; + +constexpr bool b[1] = {true}; + +typedef Q Q_t, *Q_p; +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' +// CHECK-MESSAGES: :[[@LINE-2]]:24: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using Q_t = Q; using Q_p = Q_t*; + +typedef Q Q2_t; +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using Q2_t = Q; + +struct T { + constexpr T(bool) {} + + static constexpr bool b = true; +}; + +typedef Q Q3_t, *Q3_p; +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' +// CHECK-MESSAGES: :[[@LINE-2]]:27: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using Q3_t = Q; using Q3_p = Q3_t*; + +typedef Q Q3_t; +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using Q3_t = Q; + +typedef TwoArgTemplate >, S<(0 < 0), Q > > Nested_t; +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using Nested_t = TwoArgTemplate >, S<(0 < 0), Q > >; + +template +class Variadic {}; + +typedef Variadic >, S<(0 < 0), Variadic > > > Variadic_t; +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using Variadic_t = Variadic >, S<(0 < 0), Variadic > > > + +typedef Variadic >, S<(0 < 0), Variadic > > > Variadic_t, *Variadic_p; +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' +// CHECK-MESSAGES: :[[@LINE-2]]:103: warning: use 'using' instead of 'typedef' +// CHECK-FIXES: using Variadic_t = Variadic >, S<(0 < 0), Variadic > > >; 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; }; using R_p = R_t*; Index: clang/include/clang/Basic/SourceLocation.h =================================================================== --- clang/include/clang/Basic/SourceLocation.h +++ clang/include/clang/Basic/SourceLocation.h @@ -192,6 +192,18 @@ return LHS.getRawEncoding() < RHS.getRawEncoding(); } +inline bool operator>(const SourceLocation &LHS, const SourceLocation &RHS) { + return LHS.getRawEncoding() > RHS.getRawEncoding(); +} + +inline bool operator<=(const SourceLocation &LHS, const SourceLocation &RHS) { + return !(LHS > RHS); +} + +inline bool operator>=(const SourceLocation &LHS, const SourceLocation &RHS) { + return !(LHS < RHS); +} + /// A trivial tuple used to represent a source range. class SourceRange { SourceLocation B; @@ -219,6 +231,11 @@ return B != X.B || E != X.E; } + // Returns true iff other is wholly contained within this range. + bool contains(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;