[llvm] fb4f605 - [RGT] Recode more unreachable assertions and tautologies

[Paul Robinson via llvm-commits]

Author: Paul Robinson
Date: 2021-03-19T09:17:22-07:00
New Revision: fb4f6057a637cabc687f7457d20a29da2c890ec0

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

LOG: [RGT] Recode more unreachable assertions and tautologies

Count iterations of zero-trip loops and assert the count is zero,
rather than asserting inside the loop.
Unreachable functions should use llvm_unreachable.
Remove tautological 'if' statements, even when they're following a
pattern of checks.

Found by the Rotten Green Tests project.

Added: 
    

Modified: 
    llvm/unittests/ADT/BitVectorTest.cpp
    llvm/unittests/ADT/ImmutableListTest.cpp
    llvm/unittests/ADT/StringRefTest.cpp
    llvm/unittests/IR/BasicBlockTest.cpp
    llvm/unittests/Linker/LinkModulesTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/unittests/ADT/BitVectorTest.cpp b/llvm/unittests/ADT/BitVectorTest.cpp
index 0f15a478e452..995f04e7efbb 100644
--- a/llvm/unittests/ADT/BitVectorTest.cpp
+++ b/llvm/unittests/ADT/BitVectorTest.cpp
@@ -1142,10 +1142,12 @@ TYPED_TEST(BitVectorTest, Iterators) {
 
   TypeParam Empty;
   EXPECT_EQ(Empty.set_bits_begin(), Empty.set_bits_end());
+  int BitCount = 0;
   for (unsigned Bit : Empty.set_bits()) {
     (void)Bit;
-    EXPECT_TRUE(false);
+    BitCount++;
   }
+  ASSERT_EQ(BitCount, 0);
 
   TypeParam ToFill(100, false);
   ToFill.set(0);

diff  --git a/llvm/unittests/ADT/ImmutableListTest.cpp b/llvm/unittests/ADT/ImmutableListTest.cpp
index ab3b8b472b90..28624c0d551d 100644
--- a/llvm/unittests/ADT/ImmutableListTest.cpp
+++ b/llvm/unittests/ADT/ImmutableListTest.cpp
@@ -245,7 +245,6 @@ TEST_F(ImmutableListTest, LongListOrderingTest) {
   int i = 0;
   for (ImmutableList<Wrapper<long>>::iterator I = L.begin(), E = L.end();
        I != E; ++I) {
-    ASSERT_EQ(i, *I);
     i++;
   }
   ASSERT_EQ(0, i);

diff  --git a/llvm/unittests/ADT/StringRefTest.cpp b/llvm/unittests/ADT/StringRefTest.cpp
index 50e38c50f621..e3f943bdbf41 100644
--- a/llvm/unittests/ADT/StringRefTest.cpp
+++ b/llvm/unittests/ADT/StringRefTest.cpp
@@ -646,12 +646,8 @@ TEST(StringRefTest, getAsInteger) {
       ASSERT_TRUE(U32Success);
     }
     bool U64Success = StringRef(Unsigned[i].Str).getAsInteger(0, U64);
-    if (static_cast<uint64_t>(Unsigned[i].Expected) == Unsigned[i].Expected) {
-      ASSERT_FALSE(U64Success);
-      EXPECT_EQ(U64, Unsigned[i].Expected);
-    } else {
-      ASSERT_TRUE(U64Success);
-    }
+    ASSERT_FALSE(U64Success);
+    EXPECT_EQ(U64, Unsigned[i].Expected);
   }
 
   int8_t S8;
@@ -682,12 +678,8 @@ TEST(StringRefTest, getAsInteger) {
       ASSERT_TRUE(S32Success);
     }
     bool S64Success = StringRef(Signed[i].Str).getAsInteger(0, S64);
-    if (static_cast<int64_t>(Signed[i].Expected) == Signed[i].Expected) {
-      ASSERT_FALSE(S64Success);
-      EXPECT_EQ(S64, Signed[i].Expected);
-    } else {
-      ASSERT_TRUE(S64Success);
-    }
+    ASSERT_FALSE(S64Success);
+    EXPECT_EQ(S64, Signed[i].Expected);
   }
 }
 
@@ -828,14 +820,9 @@ TEST(StringRefTest, consumeIntegerUnsigned) {
 
     Str = ConsumeUnsigned[i].Str;
     bool U64Success = Str.consumeInteger(0, U64);
-    if (static_cast<uint64_t>(ConsumeUnsigned[i].Expected) ==
-        ConsumeUnsigned[i].Expected) {
-      ASSERT_FALSE(U64Success);
-      EXPECT_EQ(U64, ConsumeUnsigned[i].Expected);
-      EXPECT_EQ(Str, ConsumeUnsigned[i].Leftover);
-    } else {
-      ASSERT_TRUE(U64Success);
-    }
+    ASSERT_FALSE(U64Success);
+    EXPECT_EQ(U64, ConsumeUnsigned[i].Expected);
+    EXPECT_EQ(Str, ConsumeUnsigned[i].Leftover);
   }
 }
 
@@ -881,14 +868,9 @@ TEST(StringRefTest, consumeIntegerSigned) {
 
     Str = ConsumeSigned[i].Str;
     bool S64Success = Str.consumeInteger(0, S64);
-    if (static_cast<int64_t>(ConsumeSigned[i].Expected) ==
-        ConsumeSigned[i].Expected) {
-      ASSERT_FALSE(S64Success);
-      EXPECT_EQ(S64, ConsumeSigned[i].Expected);
-      EXPECT_EQ(Str, ConsumeSigned[i].Leftover);
-    } else {
-      ASSERT_TRUE(S64Success);
-    }
+    ASSERT_FALSE(S64Success);
+    EXPECT_EQ(S64, ConsumeSigned[i].Expected);
+    EXPECT_EQ(Str, ConsumeSigned[i].Leftover);
   }
 }
 

diff  --git a/llvm/unittests/IR/BasicBlockTest.cpp b/llvm/unittests/IR/BasicBlockTest.cpp
index fa923c90c729..408275732058 100644
--- a/llvm/unittests/IR/BasicBlockTest.cpp
+++ b/llvm/unittests/IR/BasicBlockTest.cpp
@@ -37,10 +37,12 @@ TEST(BasicBlockTest, PhiRange) {
   BranchInst::Create(BB.get(), BB2.get());
 
   // Make sure this doesn't crash if there are no phis.
+  int PhiCount = 0;
   for (auto &PN : BB->phis()) {
     (void)PN;
-    EXPECT_TRUE(false) << "empty block should have no phis";
+    PhiCount++;
   }
+  ASSERT_EQ(PhiCount, 0) << "empty block should have no phis";
 
   // Make it a cycle.
   auto *BI = BranchInst::Create(BB.get(), BB.get());

diff  --git a/llvm/unittests/Linker/LinkModulesTest.cpp b/llvm/unittests/Linker/LinkModulesTest.cpp
index 05523c56cc2a..793c744a2df5 100644
--- a/llvm/unittests/Linker/LinkModulesTest.cpp
+++ b/llvm/unittests/Linker/LinkModulesTest.cpp
@@ -72,7 +72,7 @@ class LinkModuleTest : public testing::Test {
 };
 
 static void expectNoDiags(const DiagnosticInfo &DI, void *C) {
-  EXPECT_TRUE(false);
+  llvm_unreachable("expectNoDiags called!");
 }
 
 TEST_F(LinkModuleTest, BlockAddress) {