[clang-tools-extra] [clang-tid]fix modernize-use-auto incorrect fix hints for pointer (PR #77943)
Congcong Cai via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 15 14:45:13 PST 2024
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/77943
>From 537d283288f555c2bb7cff90aee89fe9b18f08b8 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Sat, 13 Jan 2024 00:31:33 +0800
Subject: [PATCH 1/4] [clang-tid]fix modernize-use-auto incorrect fix hints for
pointer
avoid create incorrect fix hints for pointer to array type and pointer to function type
Fixes: #77891
---
.../clang-tidy/modernize/UseAutoCheck.cpp | 52 ++++++++++++++-----
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++
2 files changed, 42 insertions(+), 14 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
index 7af30e688b6a716..af41c4b50717968 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
@@ -8,10 +8,12 @@
#include "UseAutoCheck.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/TypeLoc.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Tooling/FixIt.h"
+#include "llvm/ADT/STLExtras.h"
using namespace clang;
using namespace clang::ast_matchers;
@@ -333,6 +335,26 @@ void UseAutoCheck::replaceIterators(const DeclStmt *D, ASTContext *Context) {
<< FixItHint::CreateReplacement(Range, "auto");
}
+namespace {
+
+void ignoreTypeLocClasses(
+ TypeLoc &Loc, llvm::SmallVector<TypeLoc::TypeLocClass> const &LocClasses) {
+ while (llvm::is_contained(LocClasses, Loc.getTypeLocClass()))
+ Loc = Loc.getNextTypeLoc();
+}
+
+bool isMutliLevelPointerToTypeLocClasses(
+ TypeLoc Loc, llvm::SmallVector<TypeLoc::TypeLocClass> const &LocClasses) {
+ ignoreTypeLocClasses(Loc, {TypeLoc::Paren, TypeLoc::Qualified});
+ if (Loc.getTypeLocClass() != TypeLoc::Pointer)
+ return false;
+ ignoreTypeLocClasses(Loc,
+ {TypeLoc::Paren, TypeLoc::Qualified, TypeLoc::Pointer});
+ return llvm::is_contained(LocClasses, Loc.getTypeLocClass());
+}
+
+} // namespace
+
void UseAutoCheck::replaceExpr(
const DeclStmt *D, ASTContext *Context,
llvm::function_ref<QualType(const Expr *)> GetType, StringRef Message) {
@@ -384,16 +406,10 @@ void UseAutoCheck::replaceExpr(
// information is not reliable where CV qualifiers are concerned so we can't
// do anything about this case for now.
TypeLoc Loc = FirstDecl->getTypeSourceInfo()->getTypeLoc();
- if (!RemoveStars) {
- while (Loc.getTypeLocClass() == TypeLoc::Pointer ||
- Loc.getTypeLocClass() == TypeLoc::Qualified)
- Loc = Loc.getNextTypeLoc();
- }
- while (Loc.getTypeLocClass() == TypeLoc::LValueReference ||
- Loc.getTypeLocClass() == TypeLoc::RValueReference ||
- Loc.getTypeLocClass() == TypeLoc::Qualified) {
- Loc = Loc.getNextTypeLoc();
- }
+ if (!RemoveStars)
+ ignoreTypeLocClasses(Loc, {TypeLoc::Pointer, TypeLoc::Qualified});
+ ignoreTypeLocClasses(Loc, {TypeLoc::LValueReference, TypeLoc::RValueReference,
+ TypeLoc::Qualified});
SourceRange Range(Loc.getSourceRange());
if (MinTypeNameLength != 0 &&
@@ -405,12 +421,20 @@ void UseAutoCheck::replaceExpr(
auto Diag = diag(Range.getBegin(), Message);
+ bool ShouldReplenishVariableName = isMutliLevelPointerToTypeLocClasses(
+ FirstDecl->getTypeSourceInfo()->getTypeLoc(),
+ {TypeLoc::FunctionProto, TypeLoc::ConstantArray});
+
// Space after 'auto' to handle cases where the '*' in the pointer type is
// next to the identifier. This avoids changing 'int *p' into 'autop'.
- // FIXME: This doesn't work for function pointers because the variable name
- // is inside the type.
- Diag << FixItHint::CreateReplacement(Range, RemoveStars ? "auto " : "auto")
- << StarRemovals;
+ llvm::StringRef Auto = ShouldReplenishVariableName
+ ? (RemoveStars ? "auto " : "auto *")
+ : (RemoveStars ? "auto " : "auto");
+ std::string ReplenishedVariableName =
+ ShouldReplenishVariableName ? FirstDecl->getQualifiedNameAsString() : "";
+ std::string Replacement =
+ (Auto + llvm::StringRef{ReplenishedVariableName}).str();
+ Diag << FixItHint::CreateReplacement(Range, Replacement) << StarRemovals;
}
void UseAutoCheck::check(const MatchFinder::MatchResult &Result) {
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index b4d87e0ed2a67ae..9bf34177ebff2cf 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -405,6 +405,10 @@ Changes in existing checks
false-positives when constructing the container with ``count`` copies of
elements with value ``value``.
+- Improved :doc:`modernize-use-auto
+ <clang-tidy/checks/modernize/use-auto>` to avoid create incorrect fix hints
+ for pointer to array type and pointer to function type.
+
- Improved :doc:`modernize-use-emplace
<clang-tidy/checks/modernize/use-emplace>` to not replace aggregates that
``emplace`` cannot construct with aggregate initialization.
>From 62f91d90602a6a91d2c3e338022ccfa3d6ce02a2 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Sat, 13 Jan 2024 07:36:53 +0800
Subject: [PATCH 2/4] add test
---
.../modernize/use-auto-for-pointer.cpp | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-auto-for-pointer.cpp
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-auto-for-pointer.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-auto-for-pointer.cpp
new file mode 100644
index 000000000000000..298fda9124b88f7
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-auto-for-pointer.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy -check-suffix=REMOVE %s modernize-use-auto %t -- \
+// RUN: -config="{CheckOptions: {modernize-use-auto.RemoveStars: 'true', modernize-use-auto.MinTypeNameLength: '0'}}"
+// RUN: %check_clang_tidy %s modernize-use-auto %t -- \
+// RUN: -config="{CheckOptions: {modernize-use-auto.RemoveStars: 'false', modernize-use-auto.MinTypeNameLength: '0'}}"
+
+void pointerToFunction() {
+ void (*(*(f1)))() = static_cast<void (**)()>(nullptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing
+ // CHECK-FIXES-REMOVE: auto f1 =
+ // CHECK-FIXES: auto *f1 =
+}
+
+void pointerToArray() {
+ int(*a1)[2] = new int[10][2];
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing
+ // CHECK-FIXES-REMOVE: auto a1 =
+ // CHECK-FIXES: auto *a1 =
+}
>From 606c1f6925003a394ad7c88f11dbfadd88a83043 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Sat, 13 Jan 2024 07:46:09 +0800
Subject: [PATCH 3/4] fix nit
---
.../clang-tidy/modernize/UseAutoCheck.cpp | 23 ++++++++++---------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
index af41c4b50717968..e5420ff64f2724c 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
@@ -335,16 +335,16 @@ void UseAutoCheck::replaceIterators(const DeclStmt *D, ASTContext *Context) {
<< FixItHint::CreateReplacement(Range, "auto");
}
-namespace {
-
-void ignoreTypeLocClasses(
- TypeLoc &Loc, llvm::SmallVector<TypeLoc::TypeLocClass> const &LocClasses) {
+static void ignoreTypeLocClasses(
+ TypeLoc &Loc,
+ std::initializer_list<TypeLoc::TypeLocClass> const &LocClasses) {
while (llvm::is_contained(LocClasses, Loc.getTypeLocClass()))
Loc = Loc.getNextTypeLoc();
}
-bool isMutliLevelPointerToTypeLocClasses(
- TypeLoc Loc, llvm::SmallVector<TypeLoc::TypeLocClass> const &LocClasses) {
+static bool isMutliLevelPointerToTypeLocClasses(
+ TypeLoc Loc,
+ std::initializer_list<TypeLoc::TypeLocClass> const &LocClasses) {
ignoreTypeLocClasses(Loc, {TypeLoc::Paren, TypeLoc::Qualified});
if (Loc.getTypeLocClass() != TypeLoc::Pointer)
return false;
@@ -353,8 +353,6 @@ bool isMutliLevelPointerToTypeLocClasses(
return llvm::is_contained(LocClasses, Loc.getTypeLocClass());
}
-} // namespace
-
void UseAutoCheck::replaceExpr(
const DeclStmt *D, ASTContext *Context,
llvm::function_ref<QualType(const Expr *)> GetType, StringRef Message) {
@@ -364,6 +362,10 @@ void UseAutoCheck::replaceExpr(
return;
const QualType FirstDeclType = FirstDecl->getType().getCanonicalType();
+ TypeSourceInfo *TSI = FirstDecl->getTypeSourceInfo();
+
+ if (TSI == nullptr)
+ return;
std::vector<FixItHint> StarRemovals;
for (const auto *Dec : D->decls()) {
@@ -405,7 +407,7 @@ void UseAutoCheck::replaceExpr(
// is the same as the initializer, just more CV-qualified. However, TypeLoc
// information is not reliable where CV qualifiers are concerned so we can't
// do anything about this case for now.
- TypeLoc Loc = FirstDecl->getTypeSourceInfo()->getTypeLoc();
+ TypeLoc Loc = TSI->getTypeLoc();
if (!RemoveStars)
ignoreTypeLocClasses(Loc, {TypeLoc::Pointer, TypeLoc::Qualified});
ignoreTypeLocClasses(Loc, {TypeLoc::LValueReference, TypeLoc::RValueReference,
@@ -422,8 +424,7 @@ void UseAutoCheck::replaceExpr(
auto Diag = diag(Range.getBegin(), Message);
bool ShouldReplenishVariableName = isMutliLevelPointerToTypeLocClasses(
- FirstDecl->getTypeSourceInfo()->getTypeLoc(),
- {TypeLoc::FunctionProto, TypeLoc::ConstantArray});
+ TSI->getTypeLoc(), {TypeLoc::FunctionProto, TypeLoc::ConstantArray});
// Space after 'auto' to handle cases where the '*' in the pointer type is
// next to the identifier. This avoids changing 'int *p' into 'autop'.
>From 60889e82ab922a6142e556c46c09124b476fe38b Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Tue, 16 Jan 2024 06:44:59 +0800
Subject: [PATCH 4/4] consider member pointer
---
.../clang-tidy/modernize/UseAutoCheck.cpp | 7 ++++---
.../checkers/modernize/use-auto-for-pointer.cpp | 11 +++++++++++
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
index e5420ff64f2724c..aec67808846b12f 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
@@ -346,10 +346,11 @@ static bool isMutliLevelPointerToTypeLocClasses(
TypeLoc Loc,
std::initializer_list<TypeLoc::TypeLocClass> const &LocClasses) {
ignoreTypeLocClasses(Loc, {TypeLoc::Paren, TypeLoc::Qualified});
- if (Loc.getTypeLocClass() != TypeLoc::Pointer)
+ TypeLoc::TypeLocClass TLC = Loc.getTypeLocClass();
+ if (TLC != TypeLoc::Pointer && TLC != TypeLoc::MemberPointer)
return false;
- ignoreTypeLocClasses(Loc,
- {TypeLoc::Paren, TypeLoc::Qualified, TypeLoc::Pointer});
+ ignoreTypeLocClasses(Loc, {TypeLoc::Paren, TypeLoc::Qualified,
+ TypeLoc::Pointer, TypeLoc::MemberPointer});
return llvm::is_contained(LocClasses, Loc.getTypeLocClass());
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-auto-for-pointer.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-auto-for-pointer.cpp
index 298fda9124b88f7..8a3e0bab26c12a3 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-auto-for-pointer.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-auto-for-pointer.cpp
@@ -16,3 +16,14 @@ void pointerToArray() {
// CHECK-FIXES-REMOVE: auto a1 =
// CHECK-FIXES: auto *a1 =
}
+
+void memberFunctionPointer() {
+ class A {
+ void f();
+ };
+ void(A::* a1)() = static_cast<void(A::*)()>(nullptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing
+ // CHECK-FIXES-REMOVE: auto a1 =
+ // CHECK-FIXES: auto *a1 =
+}
+
More information about the cfe-commits
mailing list