diff options
4 files changed, 123 insertions, 20 deletions
diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp index 24eefdb..30fcba3 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp @@ -7,9 +7,14 @@ //===----------------------------------------------------------------------===// #include "UseUsingCheck.h" -#include "clang/AST/ASTContext.h" +#include "../utils/LexerUtils.h" #include "clang/AST/DeclGroup.h" +#include "clang/Basic/LangOptions.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Basic/TokenKinds.h" #include "clang/Lex/Lexer.h" +#include <string> using namespace clang::ast_matchers; namespace { @@ -83,6 +88,9 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { if (!ParentDecl) return; + const SourceManager &SM = *Result.SourceManager; + const LangOptions &LO = getLangOpts(); + // 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>(TagDeclName); @@ -119,14 +127,51 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { return; } - PrintingPolicy PrintPolicy(getLangOpts()); - PrintPolicy.SuppressScope = true; - PrintPolicy.ConstantArraySizeAsWritten = true; - PrintPolicy.UseVoidForZeroParams = false; - PrintPolicy.PrintInjectedClassNameWithArguments = false; + const TypeLoc TL = MatchedDecl->getTypeSourceInfo()->getTypeLoc(); + + auto [Type, QualifierStr] = [MatchedDecl, this, &TL, &SM, + &LO]() -> std::pair<std::string, std::string> { + SourceRange TypeRange = TL.getSourceRange(); + + // Function pointer case, get the left and right side of the identifier + // without the identifier. + if (TypeRange.fullyContains(MatchedDecl->getLocation())) { + const auto RangeLeftOfIdentifier = CharSourceRange::getCharRange( + TypeRange.getBegin(), MatchedDecl->getLocation()); + const auto RangeRightOfIdentifier = CharSourceRange::getCharRange( + Lexer::getLocForEndOfToken(MatchedDecl->getLocation(), 0, SM, LO), + Lexer::getLocForEndOfToken(TypeRange.getEnd(), 0, SM, LO)); + const std::string VerbatimType = + (Lexer::getSourceText(RangeLeftOfIdentifier, SM, LO) + + Lexer::getSourceText(RangeRightOfIdentifier, SM, LO)) + .str(); + return {VerbatimType, ""}; + } + + StringRef ExtraReference = ""; + if (MainTypeEndLoc.isValid() && TypeRange.fullyContains(MainTypeEndLoc)) { + // Each type introduced in a typedef can specify being a reference or + // pointer type seperately, so we need to sigure out if the new using-decl + // needs to be to a reference or pointer as well. + const SourceLocation Tok = utils::lexer::findPreviousAnyTokenKind( + MatchedDecl->getLocation(), SM, LO, tok::TokenKind::star, + tok::TokenKind::amp, tok::TokenKind::comma, + tok::TokenKind::kw_typedef); + + ExtraReference = Lexer::getSourceText( + CharSourceRange::getCharRange(Tok, Tok.getLocWithOffset(1)), SM, LO); - std::string Type = MatchedDecl->getUnderlyingType().getAsString(PrintPolicy); - std::string Name = MatchedDecl->getNameAsString(); + if (ExtraReference != "*" && ExtraReference != "&") + ExtraReference = ""; + + TypeRange.setEnd(MainTypeEndLoc); + } + return { + Lexer::getSourceText(CharSourceRange::getTokenRange(TypeRange), SM, LO) + .str(), + ExtraReference.str()}; + }(); + StringRef Name = MatchedDecl->getName(); SourceRange ReplaceRange = MatchedDecl->getSourceRange(); // typedefs with multiple comma-separated definitions produce multiple @@ -143,7 +188,8 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { // 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; + FirstTypedefName = Name.str(); + MainTypeEndLoc = TL.getEndLoc(); } else { // This is additional TypedefDecl in a comma-separated typedef declaration. // Start replacement *after* prior replacement and separate with semicolon. @@ -153,10 +199,10 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { // 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 (Type == FirstTypedefType && !QualifierStr.empty()) + Type = FirstTypedefName; } + if (!ReplaceRange.getEnd().isMacroID()) { const SourceLocation::IntTy Offset = MatchedDecl->getFunctionType() ? 0 : Name.size(); @@ -171,13 +217,12 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { LastTagDeclRange->second.isValid() && ReplaceRange.fullyContains(LastTagDeclRange->second)) { Type = std::string(Lexer::getSourceText( - CharSourceRange::getTokenRange(LastTagDeclRange->second), - *Result.SourceManager, getLangOpts())); + CharSourceRange::getTokenRange(LastTagDeclRange->second), SM, LO)); if (Type.empty()) return; } - std::string Replacement = Using + Name + " = " + Type; + std::string Replacement = (Using + Name + " = " + Type + QualifierStr).str(); Diag << FixItHint::CreateReplacement(ReplaceRange, Replacement); } } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h index 7054778..1e54bbf 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h @@ -26,6 +26,7 @@ class UseUsingCheck : public ClangTidyCheck { std::string FirstTypedefType; std::string FirstTypedefName; + SourceLocation MainTypeEndLoc; public: UseUsingCheck(StringRef Name, ClangTidyContext *Context); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 3fd7a4f..fa3a8e5 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -317,6 +317,9 @@ Changes in existing checks member function calls too and to only expand macros starting with ``PRI`` and ``__PRI`` from ``<inttypes.h>`` in the format string. +- Improved :doc:`modernize-use-using + <clang-tidy/checks/modernize/use-using>` check by not expanding macros. + - Improved :doc:`performance-avoid-endl <clang-tidy/checks/performance/avoid-endl>` check to use ``std::endl`` as placeholder when lexer cannot get source text. 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 index 925e5f9..214a66f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp @@ -80,7 +80,7 @@ typedef Test<my_class *> another; typedef int* PInt; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' -// CHECK-FIXES: using PInt = int *; +// CHECK-FIXES: using PInt = int*; typedef int bla1, bla2, bla3; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' @@ -112,7 +112,7 @@ TYPEDEF Foo Bak; typedef FOO Bam; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' // CHECK-FIXES: #define FOO Foo -// CHECK-FIXES: using Bam = Foo; +// CHECK-FIXES: using Bam = FOO; typedef struct Foo Bap; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' @@ -247,7 +247,7 @@ typedef Q<T{0 < 0}.b> Q3_t; typedef TwoArgTemplate<TwoArgTemplate<int, Q<T{0 < 0}.b> >, S<(0 < 0), Q<b[0 < 0]> > > Nested_t; // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' -// CHECK-FIXES: using Nested_t = TwoArgTemplate<TwoArgTemplate<int, Q<T{0 < 0}.b>>, S<(0 < 0), Q<b[0 < 0]>>>; +// CHECK-FIXES: using Nested_t = TwoArgTemplate<TwoArgTemplate<int, Q<T{0 < 0}.b> >, S<(0 < 0), Q<b[0 < 0]> > >; template <typename a> class TemplateKeyword { @@ -265,12 +265,12 @@ class Variadic {}; typedef Variadic<Variadic<int, bool, Q<T{0 < 0}.b> >, S<(0 < 0), Variadic<Q<b[0 < 0]> > > > Variadic_t; // CHECK-MESSAGES: :[[@LINE-1]]:1: 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: using Variadic_t = Variadic<Variadic<int, bool, Q<T{0 < 0}.b> >, S<(0 < 0), Variadic<Q<b[0 < 0]> > > > 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-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: 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; @@ -383,3 +383,57 @@ namespace ISSUE_72179 // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: use 'using' instead of 'typedef' [modernize-use-using] // CHECK-FIXES: const auto foo4 = [](int a){using d = int;}; } + + +typedef int* int_ptr, *int_ptr_ptr; +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-MESSAGES: :[[@LINE-2]]:21: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using int_ptr = int*; +// CHECK-FIXES-NEXT: using int_ptr_ptr = int_ptr*; + +#ifndef SpecialMode +#define SomeMacro(x) x +#else +#define SomeMacro(x) SpecialType +#endif + +class SomeMacro(GH33760) { }; + +typedef void(SomeMacro(GH33760)::* FunctionType)(float, int); +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using FunctionType = void(SomeMacro(GH33760)::* )(float, int); + +#define CDECL __attribute((cdecl)) + +// GH37846 & GH41685 +typedef void (CDECL *GH37846)(int); +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using GH37846 = void (CDECL *)(int); + +typedef void (__attribute((cdecl)) *GH41685)(int); +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using GH41685 = void (__attribute((cdecl)) *)(int); + +namespace GH83568 { + typedef int(*name)(int arg1, int arg2); +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using name = int(*)(int arg1, int arg2); +} + +#ifdef FOO +#define GH95716 float +#else +#define GH95716 double +#endif + +typedef GH95716 foo; +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using foo = GH95716; + +namespace GH97009 { + typedef double PointType[3]; +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef' [modernize-use-using] + typedef bool (*Function)(PointType, PointType); +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CHECK-FIXES: using Function = bool (*)(PointType, PointType); +} |