From d5c1c7940fc29aadbd8bb8e8d1990add57a7b4f3 Mon Sep 17 00:00:00 2001 From: Christopher Dykes Date: Thu, 8 Dec 2016 17:37:20 -0800 Subject: [PATCH] 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 | 5 ++++ folly/experimental/test/FutureDAGTest.cpp | 10 +++---- folly/experimental/test/StringKeyedTest.cpp | 20 +++++++------- folly/io/async/test/AsyncSSLSocketTest.h | 12 ++++----- folly/io/async/test/AsyncSocketTest.h | 10 +++---- folly/test/BitsTest.cpp | 8 +++--- folly/test/ConcurrentSkipListTest.cpp | 12 ++++----- folly/test/ConvTest.cpp | 12 ++++----- folly/test/FBVectorTest.cpp | 8 +++--- folly/test/FormatTest.cpp | 30 +++++++++++---------- 10 files changed, 67 insertions(+), 60 deletions(-) diff --git a/folly/Synchronized.h b/folly/Synchronized.h index aeae0549..9b627237 100644 --- a/folly/Synchronized.h +++ b/folly/Synchronized.h @@ -1312,6 +1312,11 @@ void swap(Synchronized& lhs, Synchronized& 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 c15a8a21..1e39848e 100644 --- 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(2); Future f; { - auto dag = FutureDAG::create(); - auto h1 = dag->add([barrier] { + auto localDag = FutureDAG::create(); + auto h1 = localDag->add([barrier] { auto p = std::make_shared>(); 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 afd5a2c0..3007f29d 100644 --- 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 65c62cb0..cc09b106 100644 --- 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(malloc(length)); - this->length = length; + this->buffer = static_cast(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 3bae6a08..7a23c6e1 100644 --- 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(malloc(length)); - this->length = length; + this->buffer = static_cast(malloc(len)); + this->length = len; } void free() { ::free(buffer); diff --git a/folly/test/BitsTest.cpp b/folly/test/BitsTest.cpp index 96abd60c..662bcd51 100644 --- a/folly/test/BitsTest.cpp +++ b/folly/test/BitsTest.cpp @@ -49,14 +49,14 @@ void testFFS() { template void testFLS() { - typedef typename std::make_unsigned::type UINT; + typedef typename std::make_unsigned::type UINT_T; EXPECT_EQ(0, findLastSet(static_cast(0))); - size_t bits = std::numeric_limits::digits; + size_t bits = std::numeric_limits::digits; for (size_t i = 0; i < bits; i++) { - INT v1 = static_cast(1) << i; + INT v1 = static_cast(1) << i; EXPECT_EQ(i + 1, findLastSet(v1)); - INT v2 = (static_cast(1) << i) - 1; + INT v2 = (static_cast(1) << i) - 1; EXPECT_EQ(i, findLastSet(v2)); } } diff --git a/folly/test/ConcurrentSkipListTest.cpp b/folly/test/ConcurrentSkipListTest.cpp index cdd72ac0..bfaf7f5b 100644 --- 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 void NonTrivialDeallocationWithParanoid() { using Alloc = ParanoidArenaAlloc; - using SkipListType = + using ParanoidSkipListType = ConcurrentSkipList, 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, 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 878171a2..2109d1a6 100644 --- a/folly/test/ConvTest.cpp +++ b/folly/test/ConvTest.cpp @@ -664,8 +664,8 @@ TEST(Conv, DoubleToInt) { auto i = to(42.0); EXPECT_EQ(i, 42); try { - auto i = to(42.1); - LOG(ERROR) << "to returned " << i << " instead of throwing"; + auto i2 = to(42.1); + LOG(ERROR) << "to 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(x); EXPECT_EQ(j, 42); try { - auto i = to(y); + auto i2 = to(y); LOG(ERROR) << "to returned " - << static_cast(i) + << static_cast(i2) << " instead of throwing"; EXPECT_TRUE(false); } catch (std::range_error& e) { @@ -704,9 +704,9 @@ TEST(Conv, IntToEnum) { auto j = to(100); EXPECT_EQ(j, 100); try { - auto i = to(5000000000L); + auto i2 = to(5000000000L); LOG(ERROR) << "to returned " - << static_cast(i) + << static_cast(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 e4bfe89b..3b73ef07 100644 --- 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 v; - fbvector::const_iterator i = v.end(); - v.insert(i, 10, 20); + fbvector::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 f8e27e31..cd06751a 100644 --- 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 m { {10, "hello"}, {20, "world"} }; EXPECT_EQ("worldXX", sformat("{[20]:X<7}", m)); -- 2.34.1