Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 58 additions & 13 deletions clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@
//===----------------------------------------------------------------------===//

#include "UseUsingCheck.h"
#include "clang/AST/ASTContext.h"
#include "../utils/LexerUtils.h"
#include "clang/AST/DeclGroup.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/TokenKinds.h"
#include "clang/Lex/Lexer.h"
#include <string>

using namespace clang::ast_matchers;
namespace {
Expand Down Expand Up @@ -119,14 +122,55 @@ 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, &Result, this,
&TL]() -> 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())) {
return {(Lexer::getSourceText(
CharSourceRange::getCharRange(TL.getBeginLoc(),
MatchedDecl->getLocation()),
*Result.SourceManager, getLangOpts()) +
Lexer::getSourceText(
CharSourceRange::getCharRange(
Lexer::getLocForEndOfToken(MatchedDecl->getLocation(), 0,
*Result.SourceManager,
getLangOpts()),
Lexer::getLocForEndOfToken(TL.getEndLoc(), 0,
*Result.SourceManager,
getLangOpts())),
*Result.SourceManager, getLangOpts()))
.str(),
""};
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you somehow refactor this part of code. It is too hard to understand what is the meaning of each substring due to indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is something like this what you had in mind?


StringRef ExtraReference = "";
if (MainTypeEndLoc.isValid() && TypeRange.fullyContains(MainTypeEndLoc)) {
const SourceLocation Tok = utils::lexer::findPreviousAnyTokenKind(
MatchedDecl->getLocation(), *Result.SourceManager, getLangOpts(),
tok::TokenKind::star, tok::TokenKind::amp, tok::TokenKind::comma,
tok::TokenKind::kw_typedef);

ExtraReference = Lexer::getSourceText(
CharSourceRange::getCharRange(Tok, Tok.getLocWithOffset(1)),
*Result.SourceManager, getLangOpts());

std::string Type = MatchedDecl->getUnderlyingType().getAsString(PrintPolicy);
std::string Name = MatchedDecl->getNameAsString();
if (ExtraReference != "*" && ExtraReference != "&")
ExtraReference = "";

if (MainTypeEndLoc.isValid())
TypeRange.setEnd(MainTypeEndLoc);
}
return {Lexer::getSourceText(CharSourceRange::getTokenRange(TypeRange),
*Result.SourceManager, getLangOpts())
.str(),
ExtraReference.str()};
}();
StringRef Name = MatchedDecl->getName();
SourceRange ReplaceRange = MatchedDecl->getSourceRange();

// typedefs with multiple comma-separated definitions produce multiple
Expand All @@ -143,7 +187,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.
Expand All @@ -153,10 +198,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();
Expand All @@ -177,7 +222,7 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
return;
}

std::string Replacement = Using + Name + " = " + Type;
std::string Replacement = (Using + Name + " = " + Type + QualifierStr).str();
Diag << FixItHint::CreateReplacement(ReplaceRange, Replacement);
}
} // namespace clang::tidy::modernize
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class UseUsingCheck : public ClangTidyCheck {

std::string FirstTypedefType;
std::string FirstTypedefName;
SourceLocation MainTypeEndLoc;

public:
UseUsingCheck(StringRef Name, ClangTidyContext *Context);
Expand Down
3 changes: 3 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,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.
Expand Down
36 changes: 31 additions & 5 deletions clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
Expand Down Expand Up @@ -383,3 +383,29 @@ 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))

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);

Loading