https://github.com/SimplyDanny updated https://github.com/llvm/llvm-project/pull/82166
From cd3c0d0d4b3133927a830c249a541510bacfabf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 18 Feb 2024 14:46:54 +0100 Subject: [PATCH 1/3] [clang-tidy] Keep parentheses when replacing index access in `sizeof` calls --- .../clang-tidy/modernize/LoopConvertCheck.cpp | 8 ++++++-- clang-tools-extra/docs/ReleaseNotes.rst | 5 +++++ .../checkers/modernize/loop-convert-basic.cpp | 13 ++++++++++++- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp index f0791da143ad9d..6c9a330d733407 100644 --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp @@ -711,8 +711,12 @@ void LoopConvertCheck::doConversion( if (const auto *Paren = Parents[0].get<ParenExpr>()) { // Usage.Expression will be replaced with the new index variable, // and parenthesis around a simple DeclRefExpr can always be - // removed. - Range = Paren->getSourceRange(); + // removed except in case of a `sizeof` operator call. + auto GrandParents = Context->getParents(*Paren); + if (GrandParents.size() != 1 || + !GrandParents[0].get<UnaryExprOrTypeTraitExpr>()) { + Range = Paren->getSourceRange(); + } } else if (const auto *UOP = Parents[0].get<UnaryOperator>()) { // If we are taking the address of the loop variable, then we must // not use a copy, as it would mean taking the address of the loop's diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 7ca7037e2a6a4f..58629426216ba8 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -170,6 +170,11 @@ Changes in existing checks `AllowStringArrays` option, enabling the exclusion of array types with deduced length initialized from string literals. +- Improved :doc:`modernize-loop-convert + <clang-tidy/checks/modernize/loop-convert>` check by ensuring that fix-its + don't remove parentheses used in ``sizeof`` calls when they have array index + accesses as arguments. + - Improved :doc:`modernize-use-override <clang-tidy/checks/modernize/use-override>` check to also remove any trailing whitespace when deleting the ``virtual`` keyword. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp index c29fbc9f9b23b7..02601b6320491a 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp @@ -1,4 +1,6 @@ -// RUN: %check_clang_tidy %s modernize-loop-convert %t -- -- -I %S/Inputs/loop-convert +// RUN: %check_clang_tidy %s modernize-loop-convert %t -- -- -I %S/Inputs/loop-convert -isystem %clang_tidy_headers + +#include <cstddef> #include "structures.h" @@ -88,6 +90,15 @@ void f() { // CHECK-FIXES-NEXT: printf("Fibonacci number %d has address %p\n", I, &I); // CHECK-FIXES-NEXT: Sum += I + 2; + int Matrix[N][12]; + size_t size = 0; + for (int I = 0; I < N; ++I) { + size += sizeof(Matrix[I]) + sizeof Matrix[I]; + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (auto & I : Matrix) + // CHECK-FIXES-NEXT: size += sizeof(I) + sizeof I; + Val Teas[N]; for (int I = 0; I < N; ++I) { Teas[I].g(); From dff0ca1e6f12e2d11838359cc1e4b7833c02cab2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 18 Feb 2024 17:33:24 +0100 Subject: [PATCH 2/3] Avoid auto --- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp index 6c9a330d733407..cac742cd46331e 100644 --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp @@ -706,13 +706,13 @@ void LoopConvertCheck::doConversion( ReplaceText = Usage.Kind == Usage::UK_MemberThroughArrow ? VarNameOrStructuredBinding + "." : VarNameOrStructuredBinding; - auto Parents = Context->getParents(*Usage.Expression); + const DynTypedNodeList Parents = Context->getParents(*Usage.Expression); if (Parents.size() == 1) { if (const auto *Paren = Parents[0].get<ParenExpr>()) { // Usage.Expression will be replaced with the new index variable, // and parenthesis around a simple DeclRefExpr can always be // removed except in case of a `sizeof` operator call. - auto GrandParents = Context->getParents(*Paren); + const DynTypedNodeList GrandParents = Context->getParents(*Paren); if (GrandParents.size() != 1 || !GrandParents[0].get<UnaryExprOrTypeTraitExpr>()) { Range = Paren->getSourceRange(); From 9fc694b7a6015ae749b9219c935cbf7d6833a6e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 18 Feb 2024 17:35:54 +0100 Subject: [PATCH 3/3] Add more test cases --- .../checkers/modernize/loop-convert-basic.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp index 02601b6320491a..5eb30020312d44 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp @@ -93,11 +93,15 @@ void f() { int Matrix[N][12]; size_t size = 0; for (int I = 0; I < N; ++I) { - size += sizeof(Matrix[I]) + sizeof Matrix[I]; + size += sizeof(Matrix[I]); + size += sizeof Matrix[I]; + size += sizeof((Matrix[I])); } - // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead + // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead // CHECK-FIXES: for (auto & I : Matrix) - // CHECK-FIXES-NEXT: size += sizeof(I) + sizeof I; + // CHECK-FIXES-NEXT: size += sizeof(I); + // CHECK-FIXES-NEXT: size += sizeof I; + // CHECK-FIXES-NEXT: size += sizeof(I); Val Teas[N]; for (int I = 0; I < N; ++I) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits