Don't shadow locals, parameters or fields

Summary:
This accounts for the places that were triggering warnings 4456, 4457, and 4458, which are all related to shadowing names, be they locals, parameters, or even types.
This doesn't deal with 4459, which is specifically for shadowing global variables, because folly/gen defines globals by the name of `count`, `min`, `max` and a few other similar names.

Reviewed By: meyering

Differential Revision: D4296781

fbshipit-source-id: a2e625095e2c65a53a9226b000aaf0ca95a3a393

folly/Synchronized.h

@@ -1312,6 +1312,11 @@ void swap(Synchronized<T, M>& lhs, Synchronized<T, M>& rhs) {
 #define SYNCHRONIZED(...)                                             \
   FOLLY_PUSH_WARNING                                                  \
   FOLLY_GCC_DISABLE_WARNING(shadow)                                   \
+  FOLLY_MSVC_DISABLE_WARNING(4189) /* initialized but unreferenced */ \
+  FOLLY_MSVC_DISABLE_WARNING(4456) /* declaration hides local */      \
+  FOLLY_MSVC_DISABLE_WARNING(4457) /* declaration hides parameter */  \
+  FOLLY_MSVC_DISABLE_WARNING(4458) /* declaration hides member */     \
+  FOLLY_MSVC_DISABLE_WARNING(4459) /* declaration hides global */     \
   FOLLY_GCC_DISABLE_NEW_SHADOW_WARNINGS                               \
   if (bool SYNCHRONIZED_state = false) {                              \
   } else                                                              \

folly/experimental/test/FutureDAGTest.cpp

@@ -255,8 +255,8 @@ TEST_F(FutureDAGTest, DestroyBeforeComplete) {
   auto barrier = std::make_shared<boost::barrier>(2);
   Future<Unit> f;
   {
-    auto dag = FutureDAG::create();
-    auto h1 = dag->add([barrier] {
+    auto localDag = FutureDAG::create();
+    auto h1 = localDag->add([barrier] {
       auto p = std::make_shared<Promise<Unit>>();
       std::thread t([p, barrier] {
         barrier->wait();
@@ -265,9 +265,9 @@ TEST_F(FutureDAGTest, DestroyBeforeComplete) {
       t.detach();
       return p->getFuture();
     });
-    auto h2 = dag->add(makeFutureFunc);
-    dag->dependency(h1, h2);
-    f = dag->go();
+    auto h2 = localDag->add(makeFutureFunc);
+    localDag->dependency(h1, h2);
+    f = localDag->go();
   }
   barrier->wait();
   ASSERT_NO_THROW(f.get());

folly/experimental/test/StringKeyedTest.cpp

@@ -244,8 +244,8 @@ TEST(StringKeyedSetTest, sanity) {
 
   EXPECT_EQ(set.size(), 1);
 
-  for (auto it : set) {
-    EXPECT_EQ(it, "lo");
+  for (auto entry : set) {
+    EXPECT_EQ(entry, "lo");
   }
 }
 
@@ -324,8 +324,8 @@ TEST(StringKeyedUnorderedSetTest, sanity) {
 
   EXPECT_EQ(set.size(), 1);
 
-  for (auto it : set) {
-    EXPECT_EQ(it, "lo");
+  for (auto entry : set) {
+    EXPECT_EQ(entry, "lo");
   }
 }
 
@@ -379,8 +379,8 @@ TEST(StringKeyedUnorderedSetTest, constructors) {
   EXPECT_EQ(set2.size(), 2);
 
   set2.erase("lo");
-  for (auto it : set2) {
-    EXPECT_EQ(it, "hello");
+  for (auto entry : set2) {
+    EXPECT_EQ(entry, "hello");
   }
 
   set2.clear();
@@ -451,8 +451,8 @@ TEST(StringKeyedMapTest, sanity) {
 
   EXPECT_EQ(map.size(), 1);
 
-  for (auto& it : map) {
-    EXPECT_EQ(it.first, "lo");
+  for (auto& entry : map) {
+    EXPECT_EQ(entry.first, "lo");
   }
 }
 
@@ -466,8 +466,8 @@ TEST(StringKeyedMapTest, constructors) {
   EXPECT_EQ(map2.size(), 2);
 
   map2.erase("lo");
-  for (auto& it : map2) {
-    EXPECT_EQ(it.first, "hello");
+  for (auto& entry : map2) {
+    EXPECT_EQ(entry.first, "hello");
   }
 
   map2.clear();

folly/io/async/test/AsyncSSLSocketTest.h

@@ -74,13 +74,13 @@ public:
   }
 
   void writeErr(
-    size_t bytesWritten,
+    size_t nBytesWritten,
     const AsyncSocketException& ex) noexcept override {
-    std::cerr << "writeError: bytesWritten " << bytesWritten
+    std::cerr << "writeError: bytesWritten " << nBytesWritten
          << ", exception " << ex.what() << std::endl;
 
     state = STATE_FAILED;
-    this->bytesWritten = bytesWritten;
+    this->bytesWritten = nBytesWritten;
     exception = ex;
     socket_->close();
     socket_->detachEventBase();
@@ -181,10 +181,10 @@ public:
       buffer = nullptr;
       length = 0;
     }
-    void allocate(size_t length) {
+    void allocate(size_t len) {
       assert(buffer == nullptr);
-      this->buffer = static_cast<char*>(malloc(length));
-      this->length = length;
+      this->buffer = static_cast<char*>(malloc(len));
+      this->length = len;
     }
     void free() {
       ::free(buffer);

folly/io/async/test/AsyncSocketTest.h

@@ -70,11 +70,11 @@ class WriteCallback : public folly::AsyncTransportWrapper::WriteCallback {
     }
   }
 
-  void writeErr(size_t bytesWritten,
+  void writeErr(size_t nBytesWritten,
                 const folly::AsyncSocketException& ex) noexcept override {
     LOG(ERROR) << ex.what();
     state = STATE_FAILED;
-    this->bytesWritten = bytesWritten;
+    this->bytesWritten = nBytesWritten;
     exception = ex;
     if (errorCallback) {
       errorCallback();
@@ -160,10 +160,10 @@ class ReadCallback : public folly::AsyncTransportWrapper::ReadCallback {
       buffer = nullptr;
       length = 0;
     }
-    void allocate(size_t length) {
+    void allocate(size_t len) {
       assert(buffer == nullptr);
-      this->buffer = static_cast<char*>(malloc(length));
-      this->length = length;
+      this->buffer = static_cast<char*>(malloc(len));
+      this->length = len;
     }
     void free() {
       ::free(buffer);

folly/test/BitsTest.cpp

@@ -49,14 +49,14 @@ void testFFS() {
 
 template <class INT>
 void testFLS() {
-  typedef typename std::make_unsigned<INT>::type UINT;
+  typedef typename std::make_unsigned<INT>::type UINT_T;
   EXPECT_EQ(0, findLastSet(static_cast<INT>(0)));
-  size_t bits = std::numeric_limits<UINT>::digits;
+  size_t bits = std::numeric_limits<UINT_T>::digits;
   for (size_t i = 0; i < bits; i++) {
-    INT v1 = static_cast<UINT>(1) << i;
+    INT v1 = static_cast<UINT_T>(1) << i;
     EXPECT_EQ(i + 1, findLastSet(v1));
 
-    INT v2 = (static_cast<UINT>(1) << i) - 1;
+    INT v2 = (static_cast<UINT_T>(1) << i) - 1;
     EXPECT_EQ(i, findLastSet(v2));
   }
 }

folly/test/ConcurrentSkipListTest.cpp

@@ -218,8 +218,8 @@ TEST(ConcurrentSkipList, SequentialAccess) {
     skipList.add(3);
     CHECK(skipList.contains(3));
     int pos = 0;
-    FOR_EACH(it, skipList) {
-      LOG(INFO) << "pos= " << pos++ << " value= " << *it;
+    for (auto entry : skipList) {
+      LOG(INFO) << "pos= " << pos++ << " value= " << entry;
     }
   }
 
@@ -468,11 +468,11 @@ void TestNonTrivialDeallocation(SkipListPtrType& list) {
 template <typename ParentAlloc>
 void NonTrivialDeallocationWithParanoid() {
   using Alloc = ParanoidArenaAlloc<ParentAlloc>;
-  using SkipListType =
+  using ParanoidSkipListType =
       ConcurrentSkipList<NonTrivialValue, std::less<NonTrivialValue>, Alloc>;
   ParentAlloc parentAlloc;
   Alloc paranoidAlloc(&parentAlloc);
-  auto list = SkipListType::createInstance(10, paranoidAlloc);
+  auto list = ParanoidSkipListType::createInstance(10, paranoidAlloc);
   TestNonTrivialDeallocation(list);
   EXPECT_TRUE(paranoidAlloc.isEmpty());
 }
@@ -486,9 +486,9 @@ TEST(ConcurrentSkipList, NonTrivialDeallocationWithParanoidSysArena) {
 }
 
 TEST(ConcurrentSkipList, NonTrivialDeallocationWithSysArena) {
-  using SkipListType =
+  using SysArenaSkipListType =
       ConcurrentSkipList<NonTrivialValue, std::less<NonTrivialValue>, SysArena>;
-  auto list = SkipListType::createInstance(10);
+  auto list = SysArenaSkipListType::createInstance(10);
   TestNonTrivialDeallocation(list);
 }
 

folly/test/ConvTest.cpp

@@ -664,8 +664,8 @@ TEST(Conv, DoubleToInt) {
   auto i = to<int>(42.0);
   EXPECT_EQ(i, 42);
   try {
-    auto i = to<int>(42.1);
-    LOG(ERROR) << "to<int> returned " << i << " instead of throwing";
+    auto i2 = to<int>(42.1);
+    LOG(ERROR) << "to<int> returned " << i2 << " instead of throwing";
     EXPECT_TRUE(false);
   } catch (std::range_error& e) {
     //LOG(INFO) << e.what();
@@ -679,9 +679,9 @@ TEST(Conv, EnumToInt) {
   auto j = to<char>(x);
   EXPECT_EQ(j, 42);
   try {
-    auto i = to<char>(y);
+    auto i2 = to<char>(y);
     LOG(ERROR) << "to<char> returned "
-               << static_cast<unsigned int>(i)
+               << static_cast<unsigned int>(i2)
                << " instead of throwing";
     EXPECT_TRUE(false);
   } catch (std::range_error& e) {
@@ -704,9 +704,9 @@ TEST(Conv, IntToEnum) {
   auto j = to<A>(100);
   EXPECT_EQ(j, 100);
   try {
-    auto i = to<A>(5000000000L);
+    auto i2 = to<A>(5000000000L);
     LOG(ERROR) << "to<A> returned "
-               << static_cast<unsigned int>(i)
+               << static_cast<unsigned int>(i2)
                << " instead of throwing";
     EXPECT_TRUE(false);
   } catch (std::range_error& e) {

folly/test/FBVectorTest.cpp

@@ -101,11 +101,11 @@ TEST(fbvector, clause_23_3_6_2_6) {
 
 TEST(fbvector, clause_23_3_6_4_ambiguity) {
   fbvector<int> v;
-  fbvector<int>::const_iterator i = v.end();
-  v.insert(i, 10, 20);
+  fbvector<int>::const_iterator it = v.end();
+  v.insert(it, 10, 20);
   EXPECT_EQ(v.size(), 10);
-  FOR_EACH (i, v) {
-    EXPECT_EQ(*i, 20);
+  for (auto i : v) {
+    EXPECT_EQ(i, 20);
   }
 }
 

folly/test/FormatTest.cpp

@@ -126,20 +126,22 @@ TEST(Format, Simple) {
   EXPECT_EQ("0042", sformat("{0[3]:04}", defaulted(v2, 42)));
   EXPECT_EQ("0042", svformat("{3:04}", defaulted(v2, 42)));
 
-  const int p[] = {10, 20, 30};
-  const int* q = p;
-  EXPECT_EQ("0020", sformat("{0[1]:04}", p));
-  EXPECT_EQ("0020", svformat("{1:04}", p));
-  EXPECT_EQ("0020", sformat("{0[1]:04}", q));
-  EXPECT_EQ("0020", svformat("{1:04}", q));
-  EXPECT_NE("", sformat("{}", q));
-
-  EXPECT_EQ("0x", sformat("{}", p).substr(0, 2));
-  EXPECT_EQ("10", svformat("{}", p));
-  EXPECT_EQ("0x", sformat("{}", q).substr(0, 2));
-  EXPECT_EQ("10", svformat("{}", q));
-  q = nullptr;
-  EXPECT_EQ("(null)", sformat("{}", q));
+  {
+    const int p[] = { 10, 20, 30 };
+    const int* q = p;
+    EXPECT_EQ("0020", sformat("{0[1]:04}", p));
+    EXPECT_EQ("0020", svformat("{1:04}", p));
+    EXPECT_EQ("0020", sformat("{0[1]:04}", q));
+    EXPECT_EQ("0020", svformat("{1:04}", q));
+    EXPECT_NE("", sformat("{}", q));
+
+    EXPECT_EQ("0x", sformat("{}", p).substr(0, 2));
+    EXPECT_EQ("10", svformat("{}", p));
+    EXPECT_EQ("0x", sformat("{}", q).substr(0, 2));
+    EXPECT_EQ("10", svformat("{}", q));
+    q = nullptr;
+    EXPECT_EQ("(null)", sformat("{}", q));
+  }
 
   std::map<int, std::string> m { {10, "hello"}, {20, "world"} };
   EXPECT_EQ("worldXX", sformat("{[20]:X<7}", m));