[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/PiotrZSL closed https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/Sh0g0-1758 edited https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
Sh0g0-1758 wrote: Please merge if no further changes are required. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -117,6 +117,10 @@ Changes in existing checks options `HeaderFileExtensions` and `ImplementationFileExtensions` by the global options of the same name. +- Improved :doc:`bugprone-too-small-loop-variable + ` check by correctly + implementing the check for const loop variable initialized with a function declaration. Sh0g0-1758 wrote: Right, I see. I certainly need some experience with writing good docs. Done. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/Sh0g0-1758 updated
https://github.com/llvm/llvm-project/pull/81183
>From 88dac6713284ee4f0b7ce73c944f78085412645f Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:14 +0530
Subject: [PATCH 01/12] Fix : bugprone-too-small-loop-variable
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 12 ++--
clang-tools-extra/docs/ReleaseNotes.rst | 3 +++
.../checks/bugprone/too-small-loop-variable.rst | 2 ++
.../checkers/bugprone/too-small-loop-variable.cpp| 12
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8ba8b893e03a6..8f6547f4ea9e6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -81,12 +81,12 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
- StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
- .bind(LoopUpperBoundName);
+ StatementMatcher LoopBoundMatcher =
+ expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
+unless(integerLiteral()),
+
unless(allOf(hasType(isConstQualified()),
declRefExpr(to(varDecl(anyOf(hasInitializer(ignoringParenImpCasts(integerLiteral())),
isConstexpr(), isConstinit())),
+unless(hasType(enumType())
+ .bind(LoopUpperBoundName);
// We use the loop increment expression only to make sure we found the right
// loop variable.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index e50914aed5f07..17b1349d20db8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -169,6 +169,9 @@ Miscellaneous
option is specified. Now ``clang-apply-replacements`` applies formatting
only with
the option.
+- Fixed incorrect implementation of ``too-small-loop-variable`` check when a
const loop
+ variable is initialized with a function declaration.
+
Improvements to include-fixer
-
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
index 0f45cc2fe1146..9ee23be45cfea 100644
---
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
+++
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
@@ -44,3 +44,5 @@ a larger user input.
for (unsigned i = 0; i < size; ++i) {} // no warning with
MagnitudeBitsUpperLimit = 31 on a system where unsigned is 32-bit
for (int i = 0; i < size; ++i) {} // warning with MagnitudeBitsUpperLimit
= 31 on a system where int is 32-bit
}
+
+``-Wtautological-constant-out-of-range-compare`` compiler warning should also
be used.
\ No newline at end of file
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
index 3229deb93bada..113150b168650 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -93,6 +93,18 @@ void voidBadForLoopWithMacroBound() {
}
}
+unsigned int getVal() {
+return 300;
+}
+
+// The iteration's upper bound has a function declaration.
+void voidBadForLoop8() {
+ const unsigned int l = getVal();
+ for (unsigned char i = 0; i < l; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower
type 'unsigned char' than iteration's upper bound 'const unsigned int'
[bugprone-too-small-loop-variable]
+ }
+}
+
/// Correct loops: we should not warn here.
>From 41acf22b481ba6800c1d07e96d1d3ba8052b20a7 Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:39 +0530
Subject: [PATCH 02/12] Ran git clang Formatter
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8f6547f4ea9e6..a73d46f01d9b2 100644
---
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -117,6 +117,10 @@ Changes in existing checks options `HeaderFileExtensions` and `ImplementationFileExtensions` by the global options of the same name. +- Improved :doc:`bugprone-too-small-loop-variable + ` check by correctly + implementing the check for const loop variable initialized with a function declaration. PiotrZSL wrote: change this second 'check' to 'support' to avoid duplicates, and its not const loop variable but an const loop boundary, and you can remove that "initialized with a function declaration", because this will work also for global variables. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/PiotrZSL approved this pull request. LGTM. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -28,6 +28,8 @@ In a real use case size means a container's size which depends on the user input This algorithm works for a small amount of objects, but will lead to freeze for a larger user input. +It's recommended to enable the compiler warning -Wtautological-constant-out-of-range-compare as well, since check does not inspect compile-time constant loop boundaries to avoid overlaps with the warning. Sh0g0-1758 wrote: done. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/Sh0g0-1758 updated
https://github.com/llvm/llvm-project/pull/81183
>From 88dac6713284ee4f0b7ce73c944f78085412645f Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:14 +0530
Subject: [PATCH 01/11] Fix : bugprone-too-small-loop-variable
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 12 ++--
clang-tools-extra/docs/ReleaseNotes.rst | 3 +++
.../checks/bugprone/too-small-loop-variable.rst | 2 ++
.../checkers/bugprone/too-small-loop-variable.cpp| 12
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8ba8b893e03a6..8f6547f4ea9e6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -81,12 +81,12 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
- StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
- .bind(LoopUpperBoundName);
+ StatementMatcher LoopBoundMatcher =
+ expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
+unless(integerLiteral()),
+
unless(allOf(hasType(isConstQualified()),
declRefExpr(to(varDecl(anyOf(hasInitializer(ignoringParenImpCasts(integerLiteral())),
isConstexpr(), isConstinit())),
+unless(hasType(enumType())
+ .bind(LoopUpperBoundName);
// We use the loop increment expression only to make sure we found the right
// loop variable.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index e50914aed5f07..17b1349d20db8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -169,6 +169,9 @@ Miscellaneous
option is specified. Now ``clang-apply-replacements`` applies formatting
only with
the option.
+- Fixed incorrect implementation of ``too-small-loop-variable`` check when a
const loop
+ variable is initialized with a function declaration.
+
Improvements to include-fixer
-
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
index 0f45cc2fe1146..9ee23be45cfea 100644
---
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
+++
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
@@ -44,3 +44,5 @@ a larger user input.
for (unsigned i = 0; i < size; ++i) {} // no warning with
MagnitudeBitsUpperLimit = 31 on a system where unsigned is 32-bit
for (int i = 0; i < size; ++i) {} // warning with MagnitudeBitsUpperLimit
= 31 on a system where int is 32-bit
}
+
+``-Wtautological-constant-out-of-range-compare`` compiler warning should also
be used.
\ No newline at end of file
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
index 3229deb93bada..113150b168650 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -93,6 +93,18 @@ void voidBadForLoopWithMacroBound() {
}
}
+unsigned int getVal() {
+return 300;
+}
+
+// The iteration's upper bound has a function declaration.
+void voidBadForLoop8() {
+ const unsigned int l = getVal();
+ for (unsigned char i = 0; i < l; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower
type 'unsigned char' than iteration's upper bound 'const unsigned int'
[bugprone-too-small-loop-variable]
+ }
+}
+
/// Correct loops: we should not warn here.
>From 41acf22b481ba6800c1d07e96d1d3ba8052b20a7 Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:39 +0530
Subject: [PATCH 02/11] Ran git clang Formatter
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8f6547f4ea9e6..a73d46f01d9b2 100644
---
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -28,6 +28,8 @@ In a real use case size means a container's size which depends on the user input This algorithm works for a small amount of objects, but will lead to freeze for a larger user input. +It's recommended to enable the compiler warning -Wtautological-constant-out-of-range-compare as well, since check does not inspect compile-time constant loop boundaries to avoid overlaps with the warning. PiotrZSL wrote: wrap this on 80 column, and put warning name into single \` https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
Sh0g0-1758 wrote: I believe all the issues are addressed now. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -156,6 +156,10 @@ Changes in existing checks `AllowStringArrays` option, enabling the exclusion of array types with deduced length initialized from string literals. +- Improved :doc:`bugprone-too-small-loop-variable Sh0g0-1758 wrote: Ah, that's what you meant. Done. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/Sh0g0-1758 updated
https://github.com/llvm/llvm-project/pull/81183
>From 88dac6713284ee4f0b7ce73c944f78085412645f Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:14 +0530
Subject: [PATCH 01/10] Fix : bugprone-too-small-loop-variable
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 12 ++--
clang-tools-extra/docs/ReleaseNotes.rst | 3 +++
.../checks/bugprone/too-small-loop-variable.rst | 2 ++
.../checkers/bugprone/too-small-loop-variable.cpp| 12
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8ba8b893e03a6f..8f6547f4ea9e65 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -81,12 +81,12 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
- StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
- .bind(LoopUpperBoundName);
+ StatementMatcher LoopBoundMatcher =
+ expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
+unless(integerLiteral()),
+
unless(allOf(hasType(isConstQualified()),
declRefExpr(to(varDecl(anyOf(hasInitializer(ignoringParenImpCasts(integerLiteral())),
isConstexpr(), isConstinit())),
+unless(hasType(enumType())
+ .bind(LoopUpperBoundName);
// We use the loop increment expression only to make sure we found the right
// loop variable.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index e50914aed5f07a..17b1349d20db8d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -169,6 +169,9 @@ Miscellaneous
option is specified. Now ``clang-apply-replacements`` applies formatting
only with
the option.
+- Fixed incorrect implementation of ``too-small-loop-variable`` check when a
const loop
+ variable is initialized with a function declaration.
+
Improvements to include-fixer
-
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
index 0f45cc2fe11463..9ee23be45cfea0 100644
---
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
+++
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
@@ -44,3 +44,5 @@ a larger user input.
for (unsigned i = 0; i < size; ++i) {} // no warning with
MagnitudeBitsUpperLimit = 31 on a system where unsigned is 32-bit
for (int i = 0; i < size; ++i) {} // warning with MagnitudeBitsUpperLimit
= 31 on a system where int is 32-bit
}
+
+``-Wtautological-constant-out-of-range-compare`` compiler warning should also
be used.
\ No newline at end of file
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
index 3229deb93bada8..113150b168650b 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -93,6 +93,18 @@ void voidBadForLoopWithMacroBound() {
}
}
+unsigned int getVal() {
+return 300;
+}
+
+// The iteration's upper bound has a function declaration.
+void voidBadForLoop8() {
+ const unsigned int l = getVal();
+ for (unsigned char i = 0; i < l; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower
type 'unsigned char' than iteration's upper bound 'const unsigned int'
[bugprone-too-small-loop-variable]
+ }
+}
+
/// Correct loops: we should not warn here.
>From 41acf22b481ba6800c1d07e96d1d3ba8052b20a7 Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:39 +0530
Subject: [PATCH 02/10] Ran git clang Formatter
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8f6547f4ea9e65..a73d46f01d9b2d 100644
---
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -82,10 +82,14 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
+ expr(ignoringParenImpCasts(allOf(
+ hasType(isInteger()), unless(integerLiteral()),
+ unless(allOf(
+ hasType(isConstQualified()),
+ declRefExpr(to(varDecl(anyOf(
+ hasInitializer(ignoringParenImpCasts(integerLiteral())),
+ isConstexpr(), isConstinit())),
+ unless(hasType(enumType())
Sh0g0-1758 wrote:
sure thing.
https://github.com/llvm/llvm-project/pull/81183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -156,6 +156,10 @@ Changes in existing checks `AllowStringArrays` option, enabling the exclusion of array types with deduced length initialized from string literals. +- Improved :doc:`bugprone-too-small-loop-variable PiotrZSL wrote: In short, move this release notes entry to line 119, so release notes entry's could be put in an order. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -82,10 +82,14 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
+ expr(ignoringParenImpCasts(allOf(
+ hasType(isInteger()), unless(integerLiteral()),
+ unless(allOf(
+ hasType(isConstQualified()),
+ declRefExpr(to(varDecl(anyOf(
+ hasInitializer(ignoringParenImpCasts(integerLiteral())),
+ isConstexpr(), isConstinit())),
+ unless(hasType(enumType())
PiotrZSL wrote:
If tests do not pass, then leave it, it can be changed under separate issue
later.
https://github.com/llvm/llvm-project/pull/81183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
Sh0g0-1758 wrote: The changes in LibASTMatcher that you suggested resulted in test failures so I did not incorporate them. As far as doc change is considered, please elaborate on it. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/Sh0g0-1758 deleted https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -82,10 +82,14 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
+ expr(ignoringParenImpCasts(allOf(
+ hasType(isInteger()), unless(integerLiteral()),
+ unless(allOf(
+ hasType(isConstQualified()),
+ declRefExpr(to(varDecl(anyOf(
+ hasInitializer(ignoringParenImpCasts(integerLiteral())),
+ isConstexpr(), isConstinit())),
+ unless(hasType(enumType())
Sh0g0-1758 wrote:
I don't think the tests will pass with this change.
https://github.com/llvm/llvm-project/pull/81183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/Sh0g0-1758 updated
https://github.com/llvm/llvm-project/pull/81183
>From 88dac6713284ee4f0b7ce73c944f78085412645f Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:14 +0530
Subject: [PATCH 1/9] Fix : bugprone-too-small-loop-variable
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 12 ++--
clang-tools-extra/docs/ReleaseNotes.rst | 3 +++
.../checks/bugprone/too-small-loop-variable.rst | 2 ++
.../checkers/bugprone/too-small-loop-variable.cpp| 12
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8ba8b893e03a6f..8f6547f4ea9e65 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -81,12 +81,12 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
- StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
- .bind(LoopUpperBoundName);
+ StatementMatcher LoopBoundMatcher =
+ expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
+unless(integerLiteral()),
+
unless(allOf(hasType(isConstQualified()),
declRefExpr(to(varDecl(anyOf(hasInitializer(ignoringParenImpCasts(integerLiteral())),
isConstexpr(), isConstinit())),
+unless(hasType(enumType())
+ .bind(LoopUpperBoundName);
// We use the loop increment expression only to make sure we found the right
// loop variable.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index e50914aed5f07a..17b1349d20db8d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -169,6 +169,9 @@ Miscellaneous
option is specified. Now ``clang-apply-replacements`` applies formatting
only with
the option.
+- Fixed incorrect implementation of ``too-small-loop-variable`` check when a
const loop
+ variable is initialized with a function declaration.
+
Improvements to include-fixer
-
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
index 0f45cc2fe11463..9ee23be45cfea0 100644
---
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
+++
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
@@ -44,3 +44,5 @@ a larger user input.
for (unsigned i = 0; i < size; ++i) {} // no warning with
MagnitudeBitsUpperLimit = 31 on a system where unsigned is 32-bit
for (int i = 0; i < size; ++i) {} // warning with MagnitudeBitsUpperLimit
= 31 on a system where int is 32-bit
}
+
+``-Wtautological-constant-out-of-range-compare`` compiler warning should also
be used.
\ No newline at end of file
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
index 3229deb93bada8..113150b168650b 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -93,6 +93,18 @@ void voidBadForLoopWithMacroBound() {
}
}
+unsigned int getVal() {
+return 300;
+}
+
+// The iteration's upper bound has a function declaration.
+void voidBadForLoop8() {
+ const unsigned int l = getVal();
+ for (unsigned char i = 0; i < l; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower
type 'unsigned char' than iteration's upper bound 'const unsigned int'
[bugprone-too-small-loop-variable]
+ }
+}
+
/// Correct loops: we should not warn here.
>From 41acf22b481ba6800c1d07e96d1d3ba8052b20a7 Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:39 +0530
Subject: [PATCH 2/9] Ran git clang Formatter
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8f6547f4ea9e65..a73d46f01d9b2d 100644
---
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -82,10 +82,14 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
+ expr(ignoringParenImpCasts(allOf(
+ hasType(isInteger()), unless(integerLiteral()),
+ unless(allOf(
+ hasType(isConstQualified()),
+ declRefExpr(to(varDecl(anyOf(
+ hasInitializer(ignoringParenImpCasts(integerLiteral())),
+ isConstexpr(), isConstinit())),
+ unless(hasType(enumType())
Sh0g0-1758 wrote:
I see your point. Made the changes.
https://github.com/llvm/llvm-project/pull/81183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -156,6 +156,10 @@ Changes in existing checks `AllowStringArrays` option, enabling the exclusion of array types with deduced length initialized from string literals. +- Improved :doc:`bugprone-too-small-loop-variable Sh0g0-1758 wrote: I don't think I understand what you mean here. The way I have written is the standard way of writing docs in this file I think. If not please give me the changes that you want to be implemented in this documentation, I will update it. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/Sh0g0-1758 updated
https://github.com/llvm/llvm-project/pull/81183
>From 88dac6713284ee4f0b7ce73c944f78085412645f Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:14 +0530
Subject: [PATCH 1/8] Fix : bugprone-too-small-loop-variable
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 12 ++--
clang-tools-extra/docs/ReleaseNotes.rst | 3 +++
.../checks/bugprone/too-small-loop-variable.rst | 2 ++
.../checkers/bugprone/too-small-loop-variable.cpp| 12
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8ba8b893e03a6..8f6547f4ea9e6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -81,12 +81,12 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
- StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
- .bind(LoopUpperBoundName);
+ StatementMatcher LoopBoundMatcher =
+ expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
+unless(integerLiteral()),
+
unless(allOf(hasType(isConstQualified()),
declRefExpr(to(varDecl(anyOf(hasInitializer(ignoringParenImpCasts(integerLiteral())),
isConstexpr(), isConstinit())),
+unless(hasType(enumType())
+ .bind(LoopUpperBoundName);
// We use the loop increment expression only to make sure we found the right
// loop variable.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index e50914aed5f07..17b1349d20db8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -169,6 +169,9 @@ Miscellaneous
option is specified. Now ``clang-apply-replacements`` applies formatting
only with
the option.
+- Fixed incorrect implementation of ``too-small-loop-variable`` check when a
const loop
+ variable is initialized with a function declaration.
+
Improvements to include-fixer
-
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
index 0f45cc2fe1146..9ee23be45cfea 100644
---
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
+++
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
@@ -44,3 +44,5 @@ a larger user input.
for (unsigned i = 0; i < size; ++i) {} // no warning with
MagnitudeBitsUpperLimit = 31 on a system where unsigned is 32-bit
for (int i = 0; i < size; ++i) {} // warning with MagnitudeBitsUpperLimit
= 31 on a system where int is 32-bit
}
+
+``-Wtautological-constant-out-of-range-compare`` compiler warning should also
be used.
\ No newline at end of file
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
index 3229deb93bada..113150b168650 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -93,6 +93,18 @@ void voidBadForLoopWithMacroBound() {
}
}
+unsigned int getVal() {
+return 300;
+}
+
+// The iteration's upper bound has a function declaration.
+void voidBadForLoop8() {
+ const unsigned int l = getVal();
+ for (unsigned char i = 0; i < l; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower
type 'unsigned char' than iteration's upper bound 'const unsigned int'
[bugprone-too-small-loop-variable]
+ }
+}
+
/// Correct loops: we should not warn here.
>From 41acf22b481ba6800c1d07e96d1d3ba8052b20a7 Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:39 +0530
Subject: [PATCH 2/8] Ran git clang Formatter
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8f6547f4ea9e6..a73d46f01d9b2 100644
---
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -156,6 +156,10 @@ Changes in existing checks `AllowStringArrays` option, enabling the exclusion of array types with deduced length initialized from string literals. +- Improved :doc:`bugprone-too-small-loop-variable PiotrZSL wrote: put this in alphabetical order by a check name. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -82,10 +82,14 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
+ expr(ignoringParenImpCasts(allOf(
+ hasType(isInteger()), unless(integerLiteral()),
+ unless(allOf(
+ hasType(isConstQualified()),
+ declRefExpr(to(varDecl(anyOf(
+ hasInitializer(ignoringParenImpCasts(integerLiteral())),
+ isConstexpr(), isConstinit())),
+ unless(hasType(enumType())
PiotrZSL wrote:
i were thinking, instead of:
```
unless(hasType(enumType()))
```
simply write:
```
unless(declRefExpr(to(enumConstantDecl(,
unless(allOf(hasType(isConstQualified()),
declRefExpr(to(varDecl(anyOf(hasInitializer(ignoringParenImpCasts(declRefExpr(to(enumConstantDecl(),
isConstexpr(), isConstinit())
```
Simply to exclude only those that are enum constant and those that use const
variable that is initialized with enum constant, you could merge this with
exist changes, and reduce duplications.
https://github.com/llvm/llvm-project/pull/81183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
Sh0g0-1758 wrote: done. Please review and suggest further changes. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -82,10 +82,14 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
+ expr(ignoringParenImpCasts(allOf(
+ hasType(isInteger()), unless(integerLiteral()),
+ unless(allOf(
+ hasType(isConstQualified()),
+ declRefExpr(to(varDecl(anyOf(
+ hasInitializer(ignoringParenImpCasts(integerLiteral())),
+ isConstexpr(), isConstinit())),
+ unless(hasType(enumType())
Sh0g0-1758 wrote:
Should I change it? if so please provide me with some more details.
https://github.com/llvm/llvm-project/pull/81183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -44,3 +44,5 @@ a larger user input.
for (unsigned i = 0; i < size; ++i) {} // no warning with
MagnitudeBitsUpperLimit = 31 on a system where unsigned is 32-bit
for (int i = 0; i < size; ++i) {} // warning with MagnitudeBitsUpperLimit
= 31 on a system where int is 32-bit
}
+
+``-Wtautological-constant-out-of-range-compare`` compiler warning should also
be used.
Sh0g0-1758 wrote:
done.
https://github.com/llvm/llvm-project/pull/81183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -169,6 +169,9 @@ Miscellaneous option is specified. Now ``clang-apply-replacements`` applies formatting only with the option. +- Fixed incorrect implementation of ``too-small-loop-variable`` check when a const loop + variable is initialized with a function declaration. + Sh0g0-1758 wrote: done. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/Sh0g0-1758 updated
https://github.com/llvm/llvm-project/pull/81183
>From 88dac6713284ee4f0b7ce73c944f78085412645f Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:14 +0530
Subject: [PATCH 1/4] Fix : bugprone-too-small-loop-variable
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 12 ++--
clang-tools-extra/docs/ReleaseNotes.rst | 3 +++
.../checks/bugprone/too-small-loop-variable.rst | 2 ++
.../checkers/bugprone/too-small-loop-variable.cpp| 12
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8ba8b893e03a6f..8f6547f4ea9e65 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -81,12 +81,12 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
- StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
- .bind(LoopUpperBoundName);
+ StatementMatcher LoopBoundMatcher =
+ expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
+unless(integerLiteral()),
+
unless(allOf(hasType(isConstQualified()),
declRefExpr(to(varDecl(anyOf(hasInitializer(ignoringParenImpCasts(integerLiteral())),
isConstexpr(), isConstinit())),
+unless(hasType(enumType())
+ .bind(LoopUpperBoundName);
// We use the loop increment expression only to make sure we found the right
// loop variable.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index e50914aed5f07a..17b1349d20db8d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -169,6 +169,9 @@ Miscellaneous
option is specified. Now ``clang-apply-replacements`` applies formatting
only with
the option.
+- Fixed incorrect implementation of ``too-small-loop-variable`` check when a
const loop
+ variable is initialized with a function declaration.
+
Improvements to include-fixer
-
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
index 0f45cc2fe11463..9ee23be45cfea0 100644
---
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
+++
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
@@ -44,3 +44,5 @@ a larger user input.
for (unsigned i = 0; i < size; ++i) {} // no warning with
MagnitudeBitsUpperLimit = 31 on a system where unsigned is 32-bit
for (int i = 0; i < size; ++i) {} // warning with MagnitudeBitsUpperLimit
= 31 on a system where int is 32-bit
}
+
+``-Wtautological-constant-out-of-range-compare`` compiler warning should also
be used.
\ No newline at end of file
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
index 3229deb93bada8..113150b168650b 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -93,6 +93,18 @@ void voidBadForLoopWithMacroBound() {
}
}
+unsigned int getVal() {
+return 300;
+}
+
+// The iteration's upper bound has a function declaration.
+void voidBadForLoop8() {
+ const unsigned int l = getVal();
+ for (unsigned char i = 0; i < l; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower
type 'unsigned char' than iteration's upper bound 'const unsigned int'
[bugprone-too-small-loop-variable]
+ }
+}
+
/// Correct loops: we should not warn here.
>From 41acf22b481ba6800c1d07e96d1d3ba8052b20a7 Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:39 +0530
Subject: [PATCH 2/4] Ran git clang Formatter
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8f6547f4ea9e65..a73d46f01d9b2d 100644
---
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/Sh0g0-1758 updated
https://github.com/llvm/llvm-project/pull/81183
>From 88dac6713284ee4f0b7ce73c944f78085412645f Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:14 +0530
Subject: [PATCH 1/3] Fix : bugprone-too-small-loop-variable
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 12 ++--
clang-tools-extra/docs/ReleaseNotes.rst | 3 +++
.../checks/bugprone/too-small-loop-variable.rst | 2 ++
.../checkers/bugprone/too-small-loop-variable.cpp| 12
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8ba8b893e03a6f..8f6547f4ea9e65 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -81,12 +81,12 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
- StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
- .bind(LoopUpperBoundName);
+ StatementMatcher LoopBoundMatcher =
+ expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
+unless(integerLiteral()),
+
unless(allOf(hasType(isConstQualified()),
declRefExpr(to(varDecl(anyOf(hasInitializer(ignoringParenImpCasts(integerLiteral())),
isConstexpr(), isConstinit())),
+unless(hasType(enumType())
+ .bind(LoopUpperBoundName);
// We use the loop increment expression only to make sure we found the right
// loop variable.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index e50914aed5f07a..17b1349d20db8d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -169,6 +169,9 @@ Miscellaneous
option is specified. Now ``clang-apply-replacements`` applies formatting
only with
the option.
+- Fixed incorrect implementation of ``too-small-loop-variable`` check when a
const loop
+ variable is initialized with a function declaration.
+
Improvements to include-fixer
-
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
index 0f45cc2fe11463..9ee23be45cfea0 100644
---
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
+++
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
@@ -44,3 +44,5 @@ a larger user input.
for (unsigned i = 0; i < size; ++i) {} // no warning with
MagnitudeBitsUpperLimit = 31 on a system where unsigned is 32-bit
for (int i = 0; i < size; ++i) {} // warning with MagnitudeBitsUpperLimit
= 31 on a system where int is 32-bit
}
+
+``-Wtautological-constant-out-of-range-compare`` compiler warning should also
be used.
\ No newline at end of file
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
index 3229deb93bada8..113150b168650b 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -93,6 +93,18 @@ void voidBadForLoopWithMacroBound() {
}
}
+unsigned int getVal() {
+return 300;
+}
+
+// The iteration's upper bound has a function declaration.
+void voidBadForLoop8() {
+ const unsigned int l = getVal();
+ for (unsigned char i = 0; i < l; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower
type 'unsigned char' than iteration's upper bound 'const unsigned int'
[bugprone-too-small-loop-variable]
+ }
+}
+
/// Correct loops: we should not warn here.
>From 41acf22b481ba6800c1d07e96d1d3ba8052b20a7 Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:39 +0530
Subject: [PATCH 2/3] Ran git clang Formatter
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8f6547f4ea9e65..a73d46f01d9b2d 100644
---
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
PiotrZSL wrote: Note: unless you want this change to be merged later with `114918019+sh0g0-1...@users.noreply.github.com` as author, change your email privacy settings. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -82,10 +82,14 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
+ expr(ignoringParenImpCasts(allOf(
+ hasType(isInteger()), unless(integerLiteral()),
+ unless(allOf(
+ hasType(isConstQualified()),
+ declRefExpr(to(varDecl(anyOf(
+ hasInitializer(ignoringParenImpCasts(integerLiteral())),
+ isConstexpr(), isConstinit())),
+ unless(hasType(enumType())
PiotrZSL wrote:
Off topic, i'm also not sure about this enum type, as it looks like tests are
testing only this scenario: `declRefExpr(to(enumConstantDecl()))`.
https://github.com/llvm/llvm-project/pull/81183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -44,3 +44,5 @@ a larger user input.
for (unsigned i = 0; i < size; ++i) {} // no warning with
MagnitudeBitsUpperLimit = 31 on a system where unsigned is 32-bit
for (int i = 0; i < size; ++i) {} // warning with MagnitudeBitsUpperLimit
= 31 on a system where int is 32-bit
}
+
+``-Wtautological-constant-out-of-range-compare`` compiler warning should also
be used.
PiotrZSL wrote:
Not here, this is description to an MagnitudeBitsUpperLimit option, put this
before it. And add info like:
"It's recommended to enable the compiler warning
`-Wtautological-constant-out-of-range-compare` as well, since check does not
inspect compile-time constant loop boundaries to avoid overlaps with the
warning."
https://github.com/llvm/llvm-project/pull/81183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
@@ -169,6 +169,9 @@ Miscellaneous option is specified. Now ``clang-apply-replacements`` applies formatting only with the option. +- Fixed incorrect implementation of ``too-small-loop-variable`` check when a const loop + variable is initialized with a function declaration. + PiotrZSL wrote: Wrong place, should be under "Changes in existing checks", follow exist style https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/PiotrZSL requested changes to this pull request. Fixes in documentation mainly needed, and could be. https://github.com/llvm/llvm-project/pull/81183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
llvmbot wrote:
@llvm/pr-subscribers-clang-tidy
Author: Shourya Goel (Sh0g0-1758)
Changes
Fixes: #79580
**PR SUMMARY**: Changed LibASTMatcher to give an appropriate warning when a
const loop bound is initialized with a function declaration.
---
Full diff: https://github.com/llvm/llvm-project/pull/81183.diff
4 Files Affected:
- (modified)
clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp (+8-4)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3)
- (modified)
clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
(+2)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
(+12)
``diff
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8ba8b893e03a6..a73d46f01d9b2 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -82,10 +82,14 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
+ expr(ignoringParenImpCasts(allOf(
+ hasType(isInteger()), unless(integerLiteral()),
+ unless(allOf(
+ hasType(isConstQualified()),
+ declRefExpr(to(varDecl(anyOf(
+ hasInitializer(ignoringParenImpCasts(integerLiteral())),
+ isConstexpr(), isConstinit())),
+ unless(hasType(enumType())
.bind(LoopUpperBoundName);
// We use the loop increment expression only to make sure we found the right
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index e50914aed5f07..17b1349d20db8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -169,6 +169,9 @@ Miscellaneous
option is specified. Now ``clang-apply-replacements`` applies formatting
only with
the option.
+- Fixed incorrect implementation of ``too-small-loop-variable`` check when a
const loop
+ variable is initialized with a function declaration.
+
Improvements to include-fixer
-
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
index 0f45cc2fe1146..9ee23be45cfea 100644
---
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
+++
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
@@ -44,3 +44,5 @@ a larger user input.
for (unsigned i = 0; i < size; ++i) {} // no warning with
MagnitudeBitsUpperLimit = 31 on a system where unsigned is 32-bit
for (int i = 0; i < size; ++i) {} // warning with MagnitudeBitsUpperLimit
= 31 on a system where int is 32-bit
}
+
+``-Wtautological-constant-out-of-range-compare`` compiler warning should also
be used.
\ No newline at end of file
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
index 3229deb93bada..113150b168650 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -93,6 +93,18 @@ void voidBadForLoopWithMacroBound() {
}
}
+unsigned int getVal() {
+return 300;
+}
+
+// The iteration's upper bound has a function declaration.
+void voidBadForLoop8() {
+ const unsigned int l = getVal();
+ for (unsigned char i = 0; i < l; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower
type 'unsigned char' than iteration's upper bound 'const unsigned int'
[bugprone-too-small-loop-variable]
+ }
+}
+
/// Correct loops: we should not warn here.
``
https://github.com/llvm/llvm-project/pull/81183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] FIX : bugprone-too-small-loop-variable - false-negative when const variable is used as loop bound (PR #81183)
https://github.com/Sh0g0-1758 created
https://github.com/llvm/llvm-project/pull/81183
Fixes: #79580
**PR SUMMARY**: Changed LibASTMatcher to give an appropriate warning when a
const loop bound is initialized with a function declaration.
>From 88dac6713284ee4f0b7ce73c944f78085412645f Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:14 +0530
Subject: [PATCH 1/2] Fix : bugprone-too-small-loop-variable
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 12 ++--
clang-tools-extra/docs/ReleaseNotes.rst | 3 +++
.../checks/bugprone/too-small-loop-variable.rst | 2 ++
.../checkers/bugprone/too-small-loop-variable.cpp| 12
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 8ba8b893e03a6f..8f6547f4ea9e65 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -81,12 +81,12 @@ void
TooSmallLoopVariableCheck::registerMatchers(MatchFinder *Finder) {
// We are interested in only those cases when the loop bound is a variable
// value (not const, enum, etc.).
- StatementMatcher LoopBoundMatcher =
- expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
- unless(integerLiteral()),
- unless(hasType(isConstQualified())),
- unless(hasType(enumType())
- .bind(LoopUpperBoundName);
+ StatementMatcher LoopBoundMatcher =
+ expr(ignoringParenImpCasts(allOf(hasType(isInteger()),
+unless(integerLiteral()),
+
unless(allOf(hasType(isConstQualified()),
declRefExpr(to(varDecl(anyOf(hasInitializer(ignoringParenImpCasts(integerLiteral())),
isConstexpr(), isConstinit())),
+unless(hasType(enumType())
+ .bind(LoopUpperBoundName);
// We use the loop increment expression only to make sure we found the right
// loop variable.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index e50914aed5f07a..17b1349d20db8d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -169,6 +169,9 @@ Miscellaneous
option is specified. Now ``clang-apply-replacements`` applies formatting
only with
the option.
+- Fixed incorrect implementation of ``too-small-loop-variable`` check when a
const loop
+ variable is initialized with a function declaration.
+
Improvements to include-fixer
-
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
index 0f45cc2fe11463..9ee23be45cfea0 100644
---
a/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
+++
b/clang-tools-extra/docs/clang-tidy/checks/bugprone/too-small-loop-variable.rst
@@ -44,3 +44,5 @@ a larger user input.
for (unsigned i = 0; i < size; ++i) {} // no warning with
MagnitudeBitsUpperLimit = 31 on a system where unsigned is 32-bit
for (int i = 0; i < size; ++i) {} // warning with MagnitudeBitsUpperLimit
= 31 on a system where int is 32-bit
}
+
+``-Wtautological-constant-out-of-range-compare`` compiler warning should also
be used.
\ No newline at end of file
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
index 3229deb93bada8..113150b168650b 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -93,6 +93,18 @@ void voidBadForLoopWithMacroBound() {
}
}
+unsigned int getVal() {
+return 300;
+}
+
+// The iteration's upper bound has a function declaration.
+void voidBadForLoop8() {
+ const unsigned int l = getVal();
+ for (unsigned char i = 0; i < l; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower
type 'unsigned char' than iteration's upper bound 'const unsigned int'
[bugprone-too-small-loop-variable]
+ }
+}
+
/// Correct loops: we should not warn here.
>From 41acf22b481ba6800c1d07e96d1d3ba8052b20a7 Mon Sep 17 00:00:00 2001
From: Sh0g0-1758
Date: Fri, 9 Feb 2024 01:21:39 +0530
Subject: [PATCH 2/2] Ran git clang Formatter
---
.../bugprone/TooSmallLoopVariableCheck.cpp | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git
