poelmanc updated this revision to Diff 235039.
poelmanc added a subscriber: sammccall.
poelmanc added a comment.
Update patch to rebase on latest: Changed `SourceLocation::contains` to
`SourceLocation::fullyContains`, and removed new `SourceLocation` comparison
operators since coincidentally @sammccall added them just days ago.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70270/new/
https://reviews.llvm.org/D70270
Files:
clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst
clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
clang/include/clang/Basic/SourceLocation.h
Index: clang/include/clang/Basic/SourceLocation.h
===================================================================
--- clang/include/clang/Basic/SourceLocation.h
+++ 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;
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,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<b[0 < 0]> Q_t, *Q_p;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef Q<b[0 < 0]> Q_t, *Q_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:24: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q_t = Q<b[0 < 0]>;
+// CHECK-FIXES-NEXT: using Q_p = Q_t*;
typedef Q<b[0 < 0]> Q2_t;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -227,7 +237,9 @@
typedef Q<T{0 < 0}.b> Q3_t, *Q3_p;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef Q<T{0 < 0}.b> Q3_t, *Q3_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:27: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q3_t = Q<T{0 < 0}.b>;
+// CHECK-FIXES-NEXT: using Q3_p = Q3_t*;
typedef Q<T{0 < 0}.b> Q3_t;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -246,4 +258,12 @@
typedef Variadic<Variadic<int, bool, Q<T{0 < 0}.b> >, S<(0 < 0), Variadic<Q<b[0 < 0]> > > > Variadic_t, *Variadic_p;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef Variadic<Variadic<int, bool, Q<T{0 < 0}.b> >, S<(0 < 0), Variadic<Q<b[0 < 0]> > > > Variadic_t, *Variadic_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:103: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Variadic_t = Variadic<Variadic<int, bool, Q<T{0 < 0}.b> >, S<(0 < 0), Variadic<Q<b[0 < 0]> > > >;
+// 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*;
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,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
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -160,6 +160,10 @@
Finds types that could be made trivially-destructible by removing out-of-line
defaulted destructor declarations.
+- The :doc:`modernize-use-using
+ <clang-tidy/checks/modernize-use-using>` check now converts typedefs containing
+ struct definitions and multiple comma-separated types.
+
- Improved :doc:`bugprone-posix-return
<clang-tidy/checks/bugprone-posix-return>` check.
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,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<FileID, unsigned> 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<CXXRecordDecl>("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<TypedefDecl>("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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
