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 <congcongcai0...@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 <congcongcai0...@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 <congcongcai0...@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 <congcongcai0...@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 = +} + _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits