From 7591444cf13ede874cce74639b6f6312549bde9b Mon Sep 17 00:00:00 2001 From: Kevin Stich Date: Thu, 7 Jul 2022 16:07:40 +0000 Subject: [PATCH] [caffe2] Fix shadowed variable warnings in `clang` (#80902) Summary: Pull Request resolved: https://github.com/pytorch/pytorch/pull/80902 This resolves build errors when using [-Werror,-Wshadow] under MSVC. Manually rename method parameters to avoid shadowing instance members - suffix parameters w/ _ (would prefer to suffix members, but that's a bigger change). Renaming parameters instead of member variables should reduce unintended side-effects. Remove `pragma ignore("-Wshadow")` now that shadowed variables are resolved. Test Plan: Build as dependent library using clang asan for windows using MSVC. Before: Build Failure when using warnings as errors (due to `-Wshadow` remaining enabled under MSVC with above mentioned shadowing warnings) ``` xplat/caffe2\c10/util/order_preserving_flat_hash_map.h(2036,22): error: declaration shadows a field of 'ska_ordered::fibonacci_hash_policy' [-Werror,-Wshadow] void commit(int8_t shift) { ^ xplat/caffe2\c10/util/order_preserving_flat_hash_map.h(2044,10): note: previous declaration is here int8_t shift = 63; ^ xplat/caffe2\c10/util/order_preserving_flat_hash_map.h(986,16): error: declaration shadows a field of 'sherwood_v3_table' [-Werror,-Wshadow] uint64_t num_slots_minus_one, ^ xplat/caffe2\c10/util/order_preserving_flat_hash_map.h(2053,7): note: in instantiation of member function 'ska_ordered::detailv3::sherwood_v3_table, c10::IValue, c10::detail::DictKeyHash, ska_ordered::detailv3::KeyOrValueHasher, c10::detail::DictKeyHash>, c10::detail::DictKeyEqualTo, ska_ordered::detailv3::KeyOrValueEquality, c10::detail::DictKeyEqualTo>, std::allocator >, std::allocator > > >::~sherwood_v3_table' requested here class order_preserving_flat_hash_map ^ xplat/caffe2\c10/util/order_preserving_flat_hash_map.h(795,12): note: previous declaration is here uint64_t num_slots_minus_one = 0; ^ xplat/caffe2\c10/util/order_preserving_flat_hash_map.h(987,14): error: declaration shadows a field of 'sherwood_v3_table' [-Werror,-Wshadow] int8_t max_lookups) { ^ xplat/caffe2\c10/util/order_preserving_flat_hash_map.h(797,10): note: previous declaration is here int8_t max_lookups = detailv3::min_lookups - 1; ^ xplat/caffe2\c10/util/order_preserving_flat_hash_map.h(830,45): error: declaration shadows a field of 'sherwood_v3_table' [-Werror,-Wshadow] uint64_t num_buckets_for_reserve(uint64_t num_elements) const { ^ xplat/caffe2\c10/util/order_preserving_flat_hash_map.h(367,5): note: in instantiation of member function 'ska_ordered::detailv3::sherwood_v3_table, c10::IValue, c10::detail::DictKeyHash, ska_ordered::detailv3::KeyOrValueHasher, c10::detail::DictKeyHash>, c10::detail::DictKeyEqualTo, ska_ordered::detailv3::KeyOrValueEquality, c10::detail::DictKeyEqualTo>, std::allocator >, std::allocator > > >::rehash_for_other_container' requested here rehash_for_other_container(other); ^ xplat/caffe2\c10/util/order_preserving_flat_hash_map.h(358,9): note: in instantiation of member function 'ska_ordered::detailv3::sherwood_v3_table, c10::IValue, c10::detail::DictKeyHash, ska_ordered::detailv3::KeyOrValueHasher, c10::detail::DictKeyHash>, c10::detail::DictKeyEqualTo, ska_ordered::detailv3::KeyOrValueEquality, c10::detail::DictKeyEqualTo>, std::allocator >, std::allocator > > >::sherwood_v3_table' requested here : sherwood_v3_table( ^ xplat/caffe2\c10/util/order_preserving_flat_hash_map.h(2053,7): note: in instantiation of member function 'ska_ordered::detailv3::sherwood_v3_table, c10::IValue, c10::detail::DictKeyHash, ska_ordered::detailv3::KeyOrValueHasher, c10::detail::DictKeyHash>, c10::detail::DictKeyEqualTo, ska_ordered::detailv3::KeyOrValueEquality, c10::detail::DictKeyEqualTo>, std::allocator >, std::allocator > > >::sherwood_v3_table' requested here class order_preserving_flat_hash_map ^ xplat/caffe2\c10/util/intrusive_ptr.h(574,44): note: in instantiation of function template specialization 'c10::intrusive_ptr >::make > > &, const c10::detail::DictImpl::DictElementTypes &>' requested here return intrusive_ptr::make(std::forward(args)...); ^ xplat/caffe2/aten/src\ATen/core/Dict_inl.h(62,10): note: in instantiation of function template specialization 'c10::make_intrusive, const ska_ordered::order_preserving_flat_hash_map > > &, const c10::detail::DictImpl::DictElementTypes &>' requested here return make_intrusive(dict, elementTypes); ^ xplat/caffe2\c10/util/order_preserving_flat_hash_map.h(799,12): note: previous declaration is here uint64_t num_elements = 0; ^ 5 errors generated. ``` After: `BUILD SUCCEEDED` Run dependent tests on applications with these changes - confirm all tests pass. Reviewed By: malfet Differential Revision: D30517712 Pull Request resolved: https://github.com/pytorch/pytorch/pull/80902 Approved by: https://github.com/malfet --- c10/util/flat_hash_map.h | 27 ++++++++--------------- c10/util/order_preserving_flat_hash_map.h | 27 ++++++++--------------- 2 files changed, 18 insertions(+), 36 deletions(-) diff --git a/c10/util/flat_hash_map.h b/c10/util/flat_hash_map.h index 19cca32e2793be..0e5afed2a6ef57 100644 --- a/c10/util/flat_hash_map.h +++ b/c10/util/flat_hash_map.h @@ -31,11 +31,6 @@ C10_CLANG_DIAGNOSTIC_PUSH() C10_CLANG_DIAGNOSTIC_IGNORE("-Wimplicit-int-float-conversion") #endif -#ifndef _MSC_VER -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wshadow" -#endif - #ifdef _MSC_VER #define SKA_NOINLINE(...) __declspec(noinline) __VA_ARGS__ #else @@ -645,8 +640,8 @@ class sherwood_v3_table : private EntryAlloc, private Hasher, private Equal { deallocate_data(new_buckets, num_buckets, old_max_lookups); } - void reserve(uint64_t num_elements) { - uint64_t required_buckets = num_buckets_for_reserve(num_elements); + void reserve(uint64_t num_elements_) { + uint64_t required_buckets = num_buckets_for_reserve(num_elements_); if (required_buckets > bucket_count()) rehash(required_buckets); } @@ -789,9 +784,9 @@ class sherwood_v3_table : private EntryAlloc, private Hasher, private Equal { return std::max(detailv3::min_lookups, desired); } - uint64_t num_buckets_for_reserve(uint64_t num_elements) const { + uint64_t num_buckets_for_reserve(uint64_t num_elements_) const { return static_cast(std::ceil( - num_elements / std::min(0.5, static_cast(_max_load_factor)))); + num_elements_ / std::min(0.5, static_cast(_max_load_factor)))); } void rehash_for_other_container(const sherwood_v3_table& other) { rehash( @@ -859,10 +854,10 @@ class sherwood_v3_table : private EntryAlloc, private Hasher, private Equal { void deallocate_data( EntryPointer begin, - uint64_t num_slots_minus_one, - int8_t max_lookups) { + uint64_t num_slots_minus_one_, + int8_t max_lookups_) { AllocatorTraits::deallocate( - *this, begin, num_slots_minus_one + max_lookups + 1); + *this, begin, num_slots_minus_one_ + max_lookups_ + 1); } void reset_to_empty_state() { @@ -1909,8 +1904,8 @@ struct fibonacci_hash_policy { size = std::max(uint64_t(2), detailv3::next_power_of_two(size)); return 64 - detailv3::log2(size); } - void commit(int8_t shift) { - this->shift = shift; + void commit(int8_t shift_) { + shift = shift_; } void reset() { shift = 63; @@ -2106,8 +2101,4 @@ struct power_of_two_std_hash : std::hash { } // end namespace ska -#ifndef _MSC_VER -#pragma GCC diagnostic pop -#endif - C10_CLANG_DIAGNOSTIC_POP() diff --git a/c10/util/order_preserving_flat_hash_map.h b/c10/util/order_preserving_flat_hash_map.h index 74021f0b55e811..58994fc57abdf2 100644 --- a/c10/util/order_preserving_flat_hash_map.h +++ b/c10/util/order_preserving_flat_hash_map.h @@ -33,11 +33,6 @@ C10_CLANG_DIAGNOSTIC_PUSH() C10_CLANG_DIAGNOSTIC_IGNORE("-Wimplicit-int-float-conversion") #endif -#ifndef _MSC_VER -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wshadow" -#endif - #ifdef _MSC_VER #define SKA_NOINLINE(...) __declspec(noinline) __VA_ARGS__ #else @@ -643,8 +638,8 @@ class sherwood_v3_table : private EntryAlloc, private Hasher, private Equal { deallocate_data(new_buckets, num_buckets, old_max_lookups); } - void reserve(uint64_t num_elements) { - uint64_t required_buckets = num_buckets_for_reserve(num_elements); + void reserve(uint64_t num_elements_) { + uint64_t required_buckets = num_buckets_for_reserve(num_elements_); if (required_buckets > bucket_count()) rehash(required_buckets); } @@ -827,9 +822,9 @@ class sherwood_v3_table : private EntryAlloc, private Hasher, private Equal { return std::max(detailv3::min_lookups, desired); } - uint64_t num_buckets_for_reserve(uint64_t num_elements) const { + uint64_t num_buckets_for_reserve(uint64_t num_elements_) const { return static_cast(std::ceil( - num_elements / std::min(0.5, static_cast(_max_load_factor)))); + num_elements_ / std::min(0.5, static_cast(_max_load_factor)))); } void rehash_for_other_container(const sherwood_v3_table& other) { rehash( @@ -983,10 +978,10 @@ class sherwood_v3_table : private EntryAlloc, private Hasher, private Equal { void deallocate_data( EntryPointer begin, - uint64_t num_slots_minus_one, - int8_t max_lookups) { + uint64_t num_slots_minus_one_, + int8_t max_lookups_) { AllocatorTraits::deallocate( - *this, begin, num_slots_minus_one + max_lookups + 1); + *this, begin, num_slots_minus_one_ + max_lookups_ + 1); } void reset_to_empty_state() { @@ -2033,8 +2028,8 @@ struct fibonacci_hash_policy { size = std::max(uint64_t(2), detailv3::next_power_of_two(size)); return 64 - detailv3::log2(size); } - void commit(int8_t shift) { - this->shift = shift; + void commit(int8_t shift_) { + shift = shift_; } void reset() { shift = 63; @@ -2234,8 +2229,4 @@ struct power_of_two_std_hash : std::hash { } // namespace ska_ordered -#ifndef _MSC_VER -#pragma GCC diagnostic pop -#endif - C10_CLANG_DIAGNOSTIC_POP()