Thread Safety Analysis: implement review suggestions from Aaron Ballman.

llvm-svn: 209847

clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h

@@ -52,6 +52,7 @@
 #include "ThreadSafetyUtil.h"
 
 #include <stdint.h>
+#include <algorithm>
 #include <cassert>
 #include <cstddef>
 #include <utility>
@@ -598,6 +599,8 @@ public:
 };
 
 
+template <class T> class LiteralT;
+
 // Base class for literal values.
 class Literal : public SExpr {
 public:
@@ -614,6 +617,13 @@ public:
 
   ValueType valueType() const { return ValType; }
 
+  template<class T> const LiteralT<T>& as() const {
+    return *static_cast<const LiteralT<T>*>(this);
+  }
+  template<class T> LiteralT<T>& as() {
+    return *static_cast<LiteralT<T>*>(this);
+  }
+
   template <class V> typename V::R_SExpr traverse(V &Vs, typename V::R_Ctx Ctx);
 
   template <class C> typename C::CType compare(Literal* E, C& Cmp) {
@@ -621,7 +631,7 @@ public:
     return Cmp.comparePointers(Cexpr, E->Cexpr);
   }
 
-protected:
+private:
   const ValueType ValType;
   const clang::Expr *Cexpr;
 };
@@ -642,6 +652,7 @@ private:
 };
 
 
+
 template <class V>
 typename V::R_SExpr Literal::traverse(V &Vs, typename V::R_Ctx Ctx) {
   if (Cexpr)
@@ -651,29 +662,29 @@ typename V::R_SExpr Literal::traverse(V &Vs, typename V::R_Ctx Ctx) {
   case ValueType::BT_Void:
     break;
   case ValueType::BT_Bool:
-    return Vs.reduceLiteralT(*static_cast<LiteralT<bool>*>(this));
+    return Vs.reduceLiteralT(as<bool>());
   case ValueType::BT_Int: {
     switch (ValType.Size) {
     case ValueType::ST_8:
       if (ValType.Signed)
-        return Vs.reduceLiteralT(*static_cast<LiteralT<int8_t>*>(this));
+        return Vs.reduceLiteralT(as<int8_t>());
       else
-        return Vs.reduceLiteralT(*static_cast<LiteralT<uint8_t>*>(this));
+        return Vs.reduceLiteralT(as<uint8_t>());
     case ValueType::ST_16:
       if (ValType.Signed)
-        return Vs.reduceLiteralT(*static_cast<LiteralT<int16_t>*>(this));
+        return Vs.reduceLiteralT(as<int16_t>());
       else
-        return Vs.reduceLiteralT(*static_cast<LiteralT<uint16_t>*>(this));
+        return Vs.reduceLiteralT(as<uint16_t>());
     case ValueType::ST_32:
       if (ValType.Signed)
-        return Vs.reduceLiteralT(*static_cast<LiteralT<int32_t>*>(this));
+        return Vs.reduceLiteralT(as<int32_t>());
       else
-        return Vs.reduceLiteralT(*static_cast<LiteralT<uint32_t>*>(this));
+        return Vs.reduceLiteralT(as<uint32_t>());
     case ValueType::ST_64:
       if (ValType.Signed)
-        return Vs.reduceLiteralT(*static_cast<LiteralT<int64_t>*>(this));
+        return Vs.reduceLiteralT(as<int64_t>());
       else
-        return Vs.reduceLiteralT(*static_cast<LiteralT<uint64_t>*>(this));
+        return Vs.reduceLiteralT(as<uint64_t>());
     default:
       break;
     }
@@ -681,17 +692,17 @@ typename V::R_SExpr Literal::traverse(V &Vs, typename V::R_Ctx Ctx) {
   case ValueType::BT_Float: {
     switch (ValType.Size) {
     case ValueType::ST_32:
-      return Vs.reduceLiteralT(*static_cast<LiteralT<float>*>(this));
+      return Vs.reduceLiteralT(as<float>());
     case ValueType::ST_64:
-      return Vs.reduceLiteralT(*static_cast<LiteralT<double>*>(this));
+      return Vs.reduceLiteralT(as<double>());
     default:
       break;
     }
   }
   case ValueType::BT_String:
-    return Vs.reduceLiteralT(*static_cast<LiteralT<StringRef>*>(this));
+    return Vs.reduceLiteralT(as<StringRef>());
   case ValueType::BT_Pointer:
-    return Vs.reduceLiteralT(*static_cast<LiteralT<void*>*>(this));
+    return Vs.reduceLiteralT(as<void*>());
   case ValueType::BT_ValueRef:
     break;
   }
@@ -1458,13 +1469,9 @@ public:
   void reservePredecessors(unsigned NumPreds);
 
   // Return the index of BB, or Predecessors.size if BB is not a predecessor.
-  unsigned findPredecessorIndex(BasicBlock *BB) {
-    unsigned I = 0;
-    for (BasicBlock *B : Predecessors) {
-      if (B == BB) return I;
-      ++I;
-    }
-    return Predecessors.size();
+  unsigned findPredecessorIndex(const BasicBlock *BB) const {
+    auto I = std::find(Predecessors.cbegin(), Predecessors.cend(), BB);
+    return std::distance(Predecessors.cbegin(), I);
   }
 
   // Set id numbers for variables.

clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h

@@ -843,7 +843,7 @@ protected:
     if (E->parent())
       SS << " BB_" << E->parent()->blockID();
     newline(SS);
-    for (auto A : E->arguments()) {
+    for (auto *A : E->arguments()) {
       SS << "let ";
       self()->printVariable(A, SS, true);
       SS << " = ";
@@ -851,7 +851,7 @@ protected:
       SS << ";";
       newline(SS);
     }
-    for (auto I : E->instructions()) {
+    for (auto *I : E->instructions()) {
       if (I->definition()->opcode() != COP_Store) {
         SS << "let ";
         self()->printVariable(I, SS, true);

clang/include/clang/Analysis/Analyses/ThreadSafetyUtil.h

@@ -123,9 +123,9 @@ public:
   // Reserve space for at least N more items.
   void reserveCheck(size_t N, MemRegionRef A) {
     if (Capacity == 0)
-      reserve(InitialCapacity, A);
+      reserve(u_max(InitialCapacity, N), A);
     else if (Size + N < Capacity)
-      reserve(Capacity*2, A);
+      reserve(u_max(Size + N, Capacity * 2), A);
   }
 
   typedef T *iterator;
@@ -172,7 +172,11 @@ public:
   }
 
 private:
-  static const unsigned InitialCapacity = 4;
+  // std::max is annoying here, because it requires a reference,
+  // thus forcing InitialCapacity to be initialized outside the .h file.
+  size_t u_max(size_t i, size_t j) { return (i < j) ? j : i; }
+
+  static const size_t InitialCapacity = 4;
 
   SimpleArray(const SimpleArray<T> &A) LLVM_DELETED_FUNCTION;
 

clang/lib/Analysis/ThreadSafetyCommon.cpp

@@ -725,7 +725,7 @@ void SExprBuilder::exitCFGBlockBody(const CFGBlock *B) {
   if (N == 1) {
     til::BasicBlock *BB = *It ? lookupBlock(*It) : nullptr;
     // TODO: set index
-    unsigned Idx = BB->findPredecessorIndex(CurrentBB);
+    unsigned Idx = BB ? BB->findPredecessorIndex(CurrentBB) : 0;
     til::SExpr *Tm = new (Arena) til::Goto(BB, Idx);
     CurrentBB->setTerminator(Tm);
   }