Author: Julian Schmidt Date: 2024-12-21T13:06:30+01:00 New Revision: f77152d9a44b49ee38b15085034bbfe22b4b3992
URL: https://github.com/llvm/llvm-project/commit/f77152d9a44b49ee38b15085034bbfe22b4b3992 DIFF: https://github.com/llvm/llvm-project/commit/f77152d9a44b49ee38b15085034bbfe22b4b3992.diff LOG: [clang-tidy] use specified type verbatim in modernize-use-using fix (#113837) Previously, the implementation used the printed type, which contains expanded macro arguments, deletes comments, and removes function argument names from the alias declaration. Instead, this check can be more surgical and use the actual written type verbatim. Fixes #33760 Fixes #37846 Fixes #41685 Fixes #83568 Fixes #95716 Fixes #97009 Added: Modified: 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/test/clang-tidy/checkers/modernize/use-using.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp index 24eefdb082eb32..30fcba367db67e 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 7054778d84a0c6..1e54bbf23c984f 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 3fd7a4f9da18ad..fa3a8e577a33ad 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 925e5f9c1ca54e..214a66f3dcc889 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); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
