https://github.com/SimplyDanny updated https://github.com/llvm/llvm-project/pull/82166
From 7aa267d752408fedcf14b62cd015d90de6719459 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/5] [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 47d265569f26f7fab74ebd75be752807e2d04e75 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/5] 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 60ab6db8c5c5d981fafd27e5ed4a54ce6e5a3a0d 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/5] 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) { From 47b6fc1b85973bb1a51dd656d6c8a7a9a1aee1a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 18 Feb 2024 22:34:06 +0100 Subject: [PATCH 4/5] Compare with `nullptr` --- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp index cac742cd46331e..3229e302eb4322 100644 --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp @@ -714,7 +714,7 @@ void LoopConvertCheck::doConversion( // removed except in case of a `sizeof` operator call. const DynTypedNodeList GrandParents = Context->getParents(*Paren); if (GrandParents.size() != 1 || - !GrandParents[0].get<UnaryExprOrTypeTraitExpr>()) { + GrandParents[0].get<UnaryExprOrTypeTraitExpr>() == nullptr) { Range = Paren->getSourceRange(); } } else if (const auto *UOP = Parents[0].get<UnaryOperator>()) { From afb68c4acce43f3b5651a0d02320b3b969c11977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= <danny.moe...@icloud.com> Date: Sun, 18 Feb 2024 22:34:56 +0100 Subject: [PATCH 5/5] Use built-in type to avoid include --- .../clang-tidy/checkers/modernize/loop-convert-basic.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 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 5eb30020312d44..695925a5d877c9 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,6 +1,4 @@ -// RUN: %check_clang_tidy %s modernize-loop-convert %t -- -- -I %S/Inputs/loop-convert -isystem %clang_tidy_headers - -#include <cstddef> +// RUN: %check_clang_tidy %s modernize-loop-convert %t -- -- -I %S/Inputs/loop-convert #include "structures.h" @@ -91,7 +89,7 @@ void f() { // CHECK-FIXES-NEXT: Sum += I + 2; int Matrix[N][12]; - size_t size = 0; + unsigned size = 0; for (int I = 0; I < N; ++I) { size += sizeof(Matrix[I]); size += sizeof Matrix[I]; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits