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

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG113f0190dd38: [clang-tidy] Fix minor bug in 
bugprone-too-small-loop-variable (authored by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

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 ) const noexcept {
 return WidthWithoutSignBit < Other.WidthWithoutSignBit;
   }
+
+  bool operator!=(const MagnitudeBits ) 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  = *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);
 


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 ) const noexcept {
 return WidthWithoutSignBit < Other.WidthWithoutSignBit;
   }
+
+  bool operator!=(const MagnitudeBits ) 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  = *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);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 


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 ) const noexcept {
 return WidthWithoutSignBit < Other.WidthWithoutSignBit;
   }
+
+  bool operator!=(const MagnitudeBits ) 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  = *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);
 


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 ) const noexcept {
 return WidthWithoutSignBit < Other.WidthWithoutSignBit;
   }
+
+  bool operator!=(const MagnitudeBits ) 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  = *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);
 
___
cfe-commits mailing list