Author: Congcong Cai Date: 2024-01-20T20:05:22+08:00 New Revision: fd3346dba825f6b9c2873bdeafe34da8f8b4f3e1
URL: https://github.com/llvm/llvm-project/commit/fd3346dba825f6b9c2873bdeafe34da8f8b4f3e1 DIFF: https://github.com/llvm/llvm-project/commit/fd3346dba825f6b9c2873bdeafe34da8f8b4f3e1.diff LOG: [clang-tidy] fix modernize-use-auto incorrect fix hints for pointer (#77943) Added: clang-tools-extra/test/clang-tidy/checkers/modernize/use-auto-for-pointer.cpp Modified: clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp index 7af30e688b6a71..aec67808846b12 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,25 @@ void UseAutoCheck::replaceIterators(const DeclStmt *D, ASTContext *Context) { << FixItHint::CreateReplacement(Range, "auto"); } +static void ignoreTypeLocClasses( + TypeLoc &Loc, + std::initializer_list<TypeLoc::TypeLocClass> const &LocClasses) { + while (llvm::is_contained(LocClasses, Loc.getTypeLocClass())) + Loc = Loc.getNextTypeLoc(); +} + +static bool isMutliLevelPointerToTypeLocClasses( + TypeLoc Loc, + std::initializer_list<TypeLoc::TypeLocClass> const &LocClasses) { + ignoreTypeLocClasses(Loc, {TypeLoc::Paren, TypeLoc::Qualified}); + TypeLoc::TypeLocClass TLC = Loc.getTypeLocClass(); + if (TLC != TypeLoc::Pointer && TLC != TypeLoc::MemberPointer) + return false; + ignoreTypeLocClasses(Loc, {TypeLoc::Paren, TypeLoc::Qualified, + TypeLoc::Pointer, TypeLoc::MemberPointer}); + return llvm::is_contained(LocClasses, Loc.getTypeLocClass()); +} + void UseAutoCheck::replaceExpr( const DeclStmt *D, ASTContext *Context, llvm::function_ref<QualType(const Expr *)> GetType, StringRef Message) { @@ -342,6 +363,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()) { @@ -383,17 +408,11 @@ 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(); - 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(); - } + TypeLoc Loc = TSI->getTypeLoc(); + 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 +424,19 @@ void UseAutoCheck::replaceExpr( auto Diag = diag(Range.getBegin(), Message); + bool ShouldReplenishVariableName = isMutliLevelPointerToTypeLocClasses( + 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'. - // 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 c8e93231fd11b8..bdbdee31f39d1c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -434,6 +434,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. 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 00000000000000..8a3e0bab26c12a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-auto-for-pointer.cpp @@ -0,0 +1,29 @@ +// 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 = +} + +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
