[PATCH] D145958: [clang-tidy] Fix minor bug in bugprone-too-small-loop-variable

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Correct issue when incorrectly matched bitfield loop
variable would still be considered valid and equal to
base type, because check didnt compare size of bitfield.

Fixes issue introduced in: D142587 <https://reviews.llvm.org/D142587>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145958

Files:
  clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  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
@@ -373,3 +373,20 @@
   }
 }
 
+void goodForLoopWithBitfieldOnUpperBoundOnly() {
+  struct S {
+    int x : 4;
+  } s;
+
+  for (int i = 10; i > s.x; --i) {
+  }
+}
+
+void goodForLoopWithIntegersOnUpperBoundOnly() {
+  struct S {
+    short x;
+  } s;
+
+  for (int i = 10; i > s.x; --i) {
+  }
+}
Index: clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -34,6 +34,11 @@
   bool operator<(const MagnitudeBits &Other) const noexcept {
     return WidthWithoutSignBit < Other.WidthWithoutSignBit;
   }
+
+  bool operator!=(const MagnitudeBits &Other) const noexcept {
+    return WidthWithoutSignBit != Other.WidthWithoutSignBit ||
+           BitFieldWidth != Other.BitFieldWidth;
+  }
 };
 
 } // namespace
@@ -184,13 +189,19 @@
   if (LoopVar->getType() != LoopIncrement->getType())
     return;
 
-  const QualType LoopVarType = LoopVar->getType();
-  const QualType UpperBoundType = UpperBound->getType();
-
   ASTContext &Context = *Result.Context;
 
+  const QualType LoopVarType = LoopVar->getType();
   const MagnitudeBits LoopVarMagnitudeBits =
       calcMagnitudeBits(Context, LoopVarType, LoopVar);
+
+  const MagnitudeBits LoopIncrementMagnitudeBits =
+      calcMagnitudeBits(Context, LoopIncrement->getType(), LoopIncrement);
+  // We matched the loop variable incorrectly.
+  if (LoopIncrementMagnitudeBits != LoopVarMagnitudeBits)
+    return;
+
+  const QualType UpperBoundType = UpperBound->getType();
   const MagnitudeBits UpperBoundMagnitudeBits =
       calcUpperBoundMagnitudeBits(Context, UpperBound, UpperBoundType);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D145958.504730.patch
Type: text/x-patch
Size: 2101 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230313/8008a037/attachment-0001.bin>