[clang-tools-extra] [clang-tidy] do not expand macros in modernize-use-using fix (PR #113837)
Julian Schmidt via cfe-commits
cfe-commits at lists.llvm.org
Sun Oct 27 13:54:13 PDT 2024
https://github.com/5chmidti created https://github.com/llvm/llvm-project/pull/113837
Previously, the implementation used the printed type, which contains the
macro arguments expanded (also deleting comments). Instead, this
check can be more surgical and keep the actual written type as is,
keeping macros unexpanded.
Fixes #33760, #37846
>From 8170641109c09e55b1530081e103a1e919eb8bfc Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Sun, 27 Oct 2024 21:49:34 +0100
Subject: [PATCH] [clang-tidy] do not expand macros in modernize-use-using fix
Previously, the implementation used the printed type, which contains the
macro arguments expanded (also deleting comments). Instead, this
check can be more surgical and keep the actual written type as is,
keeping macros unexpanded.
Fixes #33760, #37846
---
.../clang-tidy/modernize/UseUsingCheck.cpp | 71 +++++++++++++++----
.../clang-tidy/modernize/UseUsingCheck.h | 1 +
clang-tools-extra/docs/ReleaseNotes.rst | 3 +
.../checkers/modernize/use-using.cpp | 36 ++++++++--
4 files changed, 93 insertions(+), 18 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index 24eefdb082eb32..74764d95d90520 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -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 {
@@ -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(),
+ ""};
+ }
+
+ 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
@@ -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.
@@ -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();
@@ -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
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 876689c40fcdb2..fe98aa9b9fd4b8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -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.
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..7833078f171bf8 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,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);
+
More information about the cfe-commits
mailing list