[clang-tools-extra] 113f019 - [clang-tidy] Fix minor bug in bugprone-too-small-loop-variable

[Piotr Zegar via cfe-commits]

Author: Piotr Zegar
Date: 2023-03-18T10:44:13Z
New Revision: 113f0190dd386d93b42d781b975aa2cca5c88914

URL: https://github.com/llvm/llvm-project/commit/113f0190dd386d93b42d781b975aa2cca5c88914
DIFF: https://github.com/llvm/llvm-project/commit/113f0190dd386d93b42d781b975aa2cca5c88914.diff

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

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

Reviewed By: carlosgalvezp

Differential Revision: https://reviews.llvm.org/D145958

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
    clang-tools-extra/test/clang-tidy/checkers/bugprone/too-small-loop-variable.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
index 133fcabd78392..8ba8b893e03a6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
@@ -34,6 +34,11 @@ struct MagnitudeBits {
   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 @@ void TooSmallLoopVariableCheck::check(const MatchFinder::MatchResult &Result) {
   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);
 

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 1505dbfef519e..7821c4144ccfe 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
@@ -373,3 +373,20 @@ void badForLoopWithBitfieldOnLoopVarAndUpperBoundOnPtr() {
   }
 }
 
+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) {
+  }
+}