From 05294a4e68851e16737e88ded1ffac775ab265c6 Mon Sep 17 00:00:00 2001
From: Paul Robinson <paul.robinson@sony.com>
Date: Fri, 19 Mar 2021 07:43:36 -0700
Subject: [PATCH] [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.
---
 unittests/ADT/BitVectorTest.cpp      |  4 ++-
 unittests/ADT/ImmutableListTest.cpp  |  1 -
 unittests/ADT/StringRefTest.cpp      | 38 ++++++++--------------------
 unittests/IR/BasicBlockTest.cpp      |  4 ++-
 unittests/Linker/LinkModulesTest.cpp |  2 +-
 5 files changed, 17 insertions(+), 32 deletions(-)

diff --git a/unittests/ADT/BitVectorTest.cpp b/unittests/ADT/BitVectorTest.cpp
index 0f15a478e45..995f04e7efb 100644
--- a/unittests/ADT/BitVectorTest.cpp
+++ b/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/unittests/ADT/ImmutableListTest.cpp b/unittests/ADT/ImmutableListTest.cpp
index ab3b8b472b9..28624c0d551 100644
--- a/unittests/ADT/ImmutableListTest.cpp
+++ b/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/unittests/ADT/StringRefTest.cpp b/unittests/ADT/StringRefTest.cpp
index 50e38c50f62..e3f943bdbf4 100644
--- a/unittests/ADT/StringRefTest.cpp
+++ b/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/unittests/IR/BasicBlockTest.cpp b/unittests/IR/BasicBlockTest.cpp
index fa923c90c72..40827573205 100644
--- a/unittests/IR/BasicBlockTest.cpp
+++ b/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/unittests/Linker/LinkModulesTest.cpp b/unittests/Linker/LinkModulesTest.cpp
index 05523c56cc2..793c744a2df 100644
--- a/unittests/Linker/LinkModulesTest.cpp
+++ b/unittests/Linker/LinkModulesTest.cpp
@@ -72,7 +72,7 @@ protected:
 };
 
 static void expectNoDiags(const DiagnosticInfo &DI, void *C) {
-  EXPECT_TRUE(false);
+  llvm_unreachable("expectNoDiags called!");
 }
 
 TEST_F(LinkModuleTest, BlockAddress) {