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

diff --git a/folly/Synchronized.h b/folly/Synchronized.h
index aeae0549c96643d24e97b96e5797777894ea3f96..9b627237c909b4da61122a65b0ee8c8dd7c1af39 100644 (file)
--- a/folly/Synchronized.h
+++ b/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                                                              \

diff --git a/folly/experimental/test/FutureDAGTest.cpp b/folly/experimental/test/FutureDAGTest.cpp
index c15a8a21e2b502bb82cdd25f26612be2b3cd72f9..1e39848e6cc689c47066500ffa1ef26b2ea1c735 100644 (file)
--- a/folly/experimental/test/FutureDAGTest.cpp
+++ b/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());

diff --git a/folly/experimental/test/StringKeyedTest.cpp b/folly/experimental/test/StringKeyedTest.cpp
index afd5a2c0c2462bd4aa4ca13e9e9b9f57fd6af2f4..3007f29d8c291d0f78e98f192e8bf3fa92a6ae09 100644 (file)
--- a/folly/experimental/test/StringKeyedTest.cpp
+++ b/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();

diff --git a/folly/io/async/test/AsyncSSLSocketTest.h b/folly/io/async/test/AsyncSSLSocketTest.h
index 65c62cb0ad270ca9b6250766081225d17a72d21c..cc09b1064366dce9cdfb704add72638371e92c6a 100644 (file)
--- a/folly/io/async/test/AsyncSSLSocketTest.h
+++ b/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);

diff --git a/folly/io/async/test/AsyncSocketTest.h b/folly/io/async/test/AsyncSocketTest.h
index 3bae6a087d0870642c546f6523ccc780a7634b2b..7a23c6e1ecb79aa436563056f452d2682d84f5dc 100644 (file)
--- a/folly/io/async/test/AsyncSocketTest.h
+++ b/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);

diff --git a/folly/test/BitsTest.cpp b/folly/test/BitsTest.cpp
index 96abd60c8e0ffe221610165a4ba74ffb35eb19a7..662bcd51c1ff0c17c84b548c1ec9ac6ad9263ec7 100644 (file)
--- a/folly/test/BitsTest.cpp
+++ b/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));
   }
 }

diff --git a/folly/test/ConcurrentSkipListTest.cpp b/folly/test/ConcurrentSkipListTest.cpp
index cdd72ac02d9e18e09d8ace1dcd0eaf8584686442..bfaf7f5b4d4bda23620298b8e707b4a17f03cb1b 100644 (file)
--- a/folly/test/ConcurrentSkipListTest.cpp
+++ b/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);
 }
 

diff --git a/folly/test/ConvTest.cpp b/folly/test/ConvTest.cpp
index 878171a28e8ac0e1384c0307d5dd3965e35be418..2109d1a6a2c532308ef26cde3ecb5d934000cece 100644 (file)
--- a/folly/test/ConvTest.cpp
+++ b/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) {

diff --git a/folly/test/FBVectorTest.cpp b/folly/test/FBVectorTest.cpp
index e4bfe89b6b5bfc6ce7618f979f330886c0ffcae4..3b73ef071e969c36110de591ce9608d3a3f5dad4 100644 (file)
--- a/folly/test/FBVectorTest.cpp
+++ b/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);
   }
 }
 

diff --git a/folly/test/FormatTest.cpp b/folly/test/FormatTest.cpp
index f8e27e317ed8ae6a6a772414417a2c601e1626bb..cd06751a40c1ee45558d9205b8c5b9e849edbcb1 100644 (file)
--- a/folly/test/FormatTest.cpp
+++ b/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));