[PATCH] D142587: [clang-tidy] Improved too-small-loop-variable with bit-field support

2023-03-13 Thread Piotr Zegar via Phabricator via cfe-commits 
PiotrZSL added a comment.

Thank you for information, I will look into this
Indeed test for this scenario is missing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142587/new/

https://reviews.llvm.org/D142587

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142587: [clang-tidy] Improved too-small-loop-variable with bit-field support

2023-03-13 Thread Henric Karlsson via Phabricator via cfe-commits 
Henric added a comment.

Hi
With this patch, the following code gives me a warning that I don't think is 
intentional.

  typedef struct S {
int x:4;
  } S;
  
  void foo(S s) {
for (int i=10; i > s.x; --i) ;
  }
  
  loop_var.c:6:22: warning: loop variable has narrower type 'int:4' than 
iteration's upper bound 'int' [bugprone-too-small-loop-variable]
  for (int i=10; i > s.x; --i) ;
 ^

Looks like the logic i reversed here, so the loop variable gets the bit field 
type.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142587/new/

https://reviews.llvm.org/D142587

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142587: [clang-tidy] Improved too-small-loop-variable with bit-field support

2023-02-26 Thread Piotr Zegar via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc7dac52203a0: [clang-tidy] Improved too-small-loop-variable 
with bit-field support (authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142587/new/

https://reviews.llvm.org/D142587

Files:
  clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -46,6 +46,16 @@
   }
 }
 
+void voidBadForLoop7() {
+struct Int  {
+int value;
+} i;
+
+  for (i.value = 0; i.value < size(); ++i.value) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
 void voidForLoopUnsignedBound() {
   unsigned size = 3147483647;
   for (int i = 0; i < size; ++i) {
@@ -253,3 +263,113 @@
   for (short i = 0; i < size; ++i) { // no warning
   }
 }
+
+// Should detect proper size of upper bound bitfield
+void voidForLoopWithBitfieldOnUpperBound() {
+  struct StructWithBitField {
+  unsigned bitfield : 5;
+  } value = {};
+
+  for(unsigned char i = 0U; i < value.bitfield; ++i) { // no warning
+  }
+}
+
+// Should detect proper size of loop variable bitfield
+void voidForLoopWithBitfieldOnLoopVar() {
+  struct StructWithBitField {
+  unsigned bitfield : 9;
+  } value = {};
+
+  unsigned char upperLimit = 100U;
+
+  for(value.bitfield = 0U; value.bitfield < upperLimit; ++value.bitfield) {
+  }
+}
+
+// Should detect proper size of loop variable and upper bound
+void voidForLoopWithBitfieldOnLoopVarAndUpperBound() {
+  struct StructWithBitField {
+  unsigned var : 5, limit : 4;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  }
+}
+
+// Should detect proper size of loop variable and upper bound on integers
+void voidForLoopWithBitfieldOnLoopVarAndUpperBoundOnInt() {
+  struct StructWithBitField {
+  unsigned var : 5;
+  int limit : 6;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  }
+}
+
+void badForLoopWithBitfieldOnUpperBound() {
+  struct StructWithBitField {
+  unsigned bitfield : 9;
+  } value = {};
+
+  for(unsigned char i = 0U; i < value.bitfield; ++i) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower type 'unsigned char' than iteration's upper bound 'unsigned int:9' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVar() {
+  struct StructWithBitField {
+  unsigned bitfield : 7;
+  } value = {};
+
+  unsigned char upperLimit = 100U;
+
+  for(value.bitfield = 0U; value.bitfield < upperLimit; ++value.bitfield) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: loop variable has narrower type 'unsigned int:7' than iteration's upper bound 'unsigned char' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBound() {
+  struct StructWithBitField {
+  unsigned var : 5, limit : 6;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'unsigned int:6' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarOnIntAndUpperBound() {
+  struct StructWithBitField {
+  int var : 5;
+  unsigned limit : 5;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'int:5' than iteration's upper bound 'unsigned int:5' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBoundOnInt() {
+  struct StructWithBitField {
+  unsigned var : 5;
+  int limit : 7;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'int:7' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBoundOnPtr() {
+  struct StructWithBitField {
+  unsigned var : 5, limit : 6;
+  } value = {};
+
+  StructWithBitField* ptr = 
+
+  for(ptr->var = 0U; ptr->var < ptr->limit; ++ptr->var) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'unsigned int:6' [bugprone-too-small-loop-variable]
+  }
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst

[PATCH] D142587: [clang-tidy] Improved too-small-loop-variable with bit-field support

2023-02-26 Thread Carlos Galvez via Phabricator via cfe-commits 
carlosgalvezp accepted this revision.
carlosgalvezp added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142587/new/

https://reviews.llvm.org/D142587

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142587: [clang-tidy] Improved too-small-loop-variable with bit-field support

2023-02-26 Thread Piotr Zegar via Phabricator via cfe-commits 
PiotrZSL updated this revision to Diff 500523.
PiotrZSL marked 2 inline comments as done.
PiotrZSL added a comment.

Review comments fix


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142587/new/

https://reviews.llvm.org/D142587

Files:
  clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -46,6 +46,16 @@
   }
 }
 
+void voidBadForLoop7() {
+struct Int  {
+int value;
+} i;
+
+  for (i.value = 0; i.value < size(); ++i.value) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
 void voidForLoopUnsignedBound() {
   unsigned size = 3147483647;
   for (int i = 0; i < size; ++i) {
@@ -253,3 +263,113 @@
   for (short i = 0; i < size; ++i) { // no warning
   }
 }
+
+// Should detect proper size of upper bound bitfield
+void voidForLoopWithBitfieldOnUpperBound() {
+  struct StructWithBitField {
+  unsigned bitfield : 5;
+  } value = {};
+
+  for(unsigned char i = 0U; i < value.bitfield; ++i) { // no warning
+  }
+}
+
+// Should detect proper size of loop variable bitfield
+void voidForLoopWithBitfieldOnLoopVar() {
+  struct StructWithBitField {
+  unsigned bitfield : 9;
+  } value = {};
+
+  unsigned char upperLimit = 100U;
+
+  for(value.bitfield = 0U; value.bitfield < upperLimit; ++value.bitfield) {
+  }
+}
+
+// Should detect proper size of loop variable and upper bound
+void voidForLoopWithBitfieldOnLoopVarAndUpperBound() {
+  struct StructWithBitField {
+  unsigned var : 5, limit : 4;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  }
+}
+
+// Should detect proper size of loop variable and upper bound on integers
+void voidForLoopWithBitfieldOnLoopVarAndUpperBoundOnInt() {
+  struct StructWithBitField {
+  unsigned var : 5;
+  int limit : 6;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  }
+}
+
+void badForLoopWithBitfieldOnUpperBound() {
+  struct StructWithBitField {
+  unsigned bitfield : 9;
+  } value = {};
+
+  for(unsigned char i = 0U; i < value.bitfield; ++i) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower type 'unsigned char' than iteration's upper bound 'unsigned int:9' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVar() {
+  struct StructWithBitField {
+  unsigned bitfield : 7;
+  } value = {};
+
+  unsigned char upperLimit = 100U;
+
+  for(value.bitfield = 0U; value.bitfield < upperLimit; ++value.bitfield) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: loop variable has narrower type 'unsigned int:7' than iteration's upper bound 'unsigned char' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBound() {
+  struct StructWithBitField {
+  unsigned var : 5, limit : 6;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'unsigned int:6' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarOnIntAndUpperBound() {
+  struct StructWithBitField {
+  int var : 5;
+  unsigned limit : 5;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'int:5' than iteration's upper bound 'unsigned int:5' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBoundOnInt() {
+  struct StructWithBitField {
+  unsigned var : 5;
+  int limit : 7;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'int:7' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBoundOnPtr() {
+  struct StructWithBitField {
+  unsigned var : 5, limit : 6;
+  } value = {};
+
+  StructWithBitField* ptr = 
+
+  for(ptr->var = 0U; ptr->var < ptr->limit; ++ptr->var) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'unsigned int:6' [bugprone-too-small-loop-variable]
+  }
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
---

[PATCH] D142587: [clang-tidy] Improved too-small-loop-variable with bit-field support

2023-02-25 Thread Carlos Galvez via Phabricator via cfe-commits 
carlosgalvezp added a comment.

In D142587#4152943 , @Eugene.Zelenko 
wrote:

> @carlosgalvezp: Sorry, there are too much Clang specifics in this patch, so I 
> could not be reviewer.

No prob, thanks for letting us know :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142587/new/

https://reviews.llvm.org/D142587

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142587: [clang-tidy] Improved too-small-loop-variable with bit-field support

2023-02-25 Thread Eugene Zelenko via Phabricator via cfe-commits 
Eugene.Zelenko added a comment.

@carlosgalvezp: Sorry, there are too much Clang specifics in this patch, so I 
could not be reviewer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142587/new/

https://reviews.llvm.org/D142587

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142587: [clang-tidy] Improved too-small-loop-variable with bit-field support

2023-02-25 Thread Carlos Galvez via Phabricator via cfe-commits 
carlosgalvezp added a comment.

Thanks for the fix! I have some suggestions for improved readability.




Comment at: 
clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:97
 
 /// Returns the magnitude bits of an integer type.
+static std::pair

Having an `std::pair` is a bit hard to read, as it's not clear what each 
element of the pair represents. Could you replace it with something like this, 
for improved readability and maintainability? Then you can also skip the 
`utility` header.

```
struct MagnitudeBits
{
  unsigned Width;
  bool IsBitField;
};
```





Comment at: 
clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:186
+  auto Msg =
+  diag(LoopVar->getBeginLoc(), "loop variable has narrower type '%0%1%2' "
+   "than iteration's upper bound '%3%4%5'");

This is a bit confusing, could you keep it with `%0` and `%1` (which clearly 
represent "loop variable" and "upper bound"), and simply create a `std::string` 
with the appropriate content? Something like:

```
std::string type = LoopVarType.getAsString();
if (LoopVarMagnitudeBits.second) {
  type += ":" + std::to_string(LoopVarMagntiudeBits.second);
}
Msg << type;

```





Comment at: 
clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:197-202
+  if (UpperBoundMagnitudeBits.second) {
+Msg << ":" << UpperBoundMagnitudeBits.second;
+  } else {
+Msg << ""
+<< "";
+  }

Please create a helper function to remove duplication.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142587/new/

https://reviews.llvm.org/D142587

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142587: [clang-tidy] Improved too-small-loop-variable with bit-field support

2023-01-25 Thread Piotr Zegar via Phabricator via cfe-commits 
ClockMan created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
ClockMan published this revision for review.
ClockMan added a comment.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Ready for review.


Implemented support for bit-field members as a loop variable
or upper limit. Supporting also non bit-field integer members.

Fixes issues: https://github.com/llvm/llvm-project/issues/58614


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142587

Files:
  clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp
@@ -46,6 +46,16 @@
   }
 }
 
+void voidBadForLoop7() {
+struct Int  {
+int value;
+} i;
+
+  for (i.value = 0; i.value < size(); ++i.value) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
 void voidForLoopUnsignedBound() {
   unsigned size = 3147483647;
   for (int i = 0; i < size; ++i) {
@@ -253,3 +263,113 @@
   for (short i = 0; i < size; ++i) { // no warning
   }
 }
+
+// Should detect proper size of upper bound bitfield
+void voidForLoopWithBitfieldOnUpperBound() {
+  struct StructWithBitField {
+  unsigned bitfield : 5;
+  } value = {};
+
+  for(unsigned char i = 0U; i < value.bitfield; ++i) { // no warning
+  }
+}
+
+// Should detect proper size of loop variable bitfield
+void voidForLoopWithBitfieldOnLoopVar() {
+  struct StructWithBitField {
+  unsigned bitfield : 9;
+  } value = {};
+
+  unsigned char upperLimit = 100U;
+
+  for(value.bitfield = 0U; value.bitfield < upperLimit; ++value.bitfield) {
+  }
+}
+
+// Should detect proper size of loop variable and upper bound
+void voidForLoopWithBitfieldOnLoopVarAndUpperBound() {
+  struct StructWithBitField {
+  unsigned var : 5, limit : 4;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  }
+}
+
+// Should detect proper size of loop variable and upper bound on integers
+void voidForLoopWithBitfieldOnLoopVarAndUpperBoundOnInt() {
+  struct StructWithBitField {
+  unsigned var : 5;
+  int limit : 6;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  }
+}
+
+void badForLoopWithBitfieldOnUpperBound() {
+  struct StructWithBitField {
+  unsigned bitfield : 9;
+  } value = {};
+
+  for(unsigned char i = 0U; i < value.bitfield; ++i) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: loop variable has narrower type 'unsigned char' than iteration's upper bound 'unsigned int:9' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVar() {
+  struct StructWithBitField {
+  unsigned bitfield : 7;
+  } value = {};
+
+  unsigned char upperLimit = 100U;
+
+  for(value.bitfield = 0U; value.bitfield < upperLimit; ++value.bitfield) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: loop variable has narrower type 'unsigned int:7' than iteration's upper bound 'unsigned char' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBound() {
+  struct StructWithBitField {
+  unsigned var : 5, limit : 6;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'unsigned int:6' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarOnIntAndUpperBound() {
+  struct StructWithBitField {
+  int var : 5;
+  unsigned limit : 5;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'int:5' than iteration's upper bound 'unsigned int:5' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBoundOnInt() {
+  struct StructWithBitField {
+  unsigned var : 5;
+  int limit : 7;
+  } value = {};
+
+  for(value.var = 0U; value.var < value.limit; ++value.var) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: loop variable has narrower type 'unsigned int:5' than iteration's upper bound 'int:7' [bugprone-too-small-loop-variable]
+  }
+}
+
+void badForLoopWithBitfieldOnLoopVarAndUpperBoundOnPtr() {
+  struct StructWithBitField {
+  unsigned var : 5, limit : 6;
+  } value = {};
+
+  StructWithBitField* ptr = 
+
+  for(ptr->var = 0U; ptr->var < ptr->limit; ++ptr->var) {
+  //