Skip to content

Commit

Permalink
[caffe2] Fix shadowed variable warnings in clang (pytorch#80902)
Browse files Browse the repository at this point in the history 
Summary:
Pull Request resolved: pytorch#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<T, FindKey, ArgumentHash, Hasher, ArgumentEqual, Equal, ArgumentAlloc, EntryAlloc>' [-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<std::pair<c10::IValue, c10::IValue>, c10::IValue, c10::detail::DictKeyHash, ska_ordered::detailv3::KeyOrValueHasher<c10::IValue, std::pair<c10::IValue, c10::IValue>, c10::detail::DictKeyHash>, c10::detail::DictKeyEqualTo, ska_ordered::detailv3::KeyOrValueEquality<c10::IValue, std::pair<c10::IValue, c10::IValue>, c10::detail::DictKeyEqualTo>, std::allocator<std::pair<c10::IValue, c10::IValue> >, std::allocator<ska_ordered::detailv3::sherwood_v3_entry<std::pair<c10::IValue, c10::IValue> > > >::~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<T, FindKey, ArgumentHash, Hasher, ArgumentEqual, Equal, ArgumentAlloc, EntryAlloc>' [-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<T, FindKey, ArgumentHash, Hasher, ArgumentEqual, Equal, ArgumentAlloc, EntryAlloc>' [-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<std::pair<c10::IValue, c10::IValue>, c10::IValue, c10::detail::DictKeyHash, ska_ordered::detailv3::KeyOrValueHasher<c10::IValue, std::pair<c10::IValue, c10::IValue>, c10::detail::DictKeyHash>, c10::detail::DictKeyEqualTo, ska_ordered::detailv3::KeyOrValueEquality<c10::IValue, std::pair<c10::IValue, c10::IValue>, c10::detail::DictKeyEqualTo>, std::allocator<std::pair<c10::IValue, c10::IValue> >, std::allocator<ska_ordered::detailv3::sherwood_v3_entry<std::pair<c10::IValue, c10::IValue> > > >::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<std::pair<c10::IValue, c10::IValue>, c10::IValue, c10::detail::DictKeyHash, ska_ordered::detailv3::KeyOrValueHasher<c10::IValue, std::pair<c10::IValue, c10::IValue>, c10::detail::DictKeyHash>, c10::detail::DictKeyEqualTo, ska_ordered::detailv3::KeyOrValueEquality<c10::IValue, std::pair<c10::IValue, c10::IValue>, c10::detail::DictKeyEqualTo>, std::allocator<std::pair<c10::IValue, c10::IValue> >, std::allocator<ska_ordered::detailv3::sherwood_v3_entry<std::pair<c10::IValue, c10::IValue> > > >::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<std::pair<c10::IValue, c10::IValue>, c10::IValue, c10::detail::DictKeyHash, ska_ordered::detailv3::KeyOrValueHasher<c10::IValue, std::pair<c10::IValue, c10::IValue>, c10::detail::DictKeyHash>, c10::detail::DictKeyEqualTo, ska_ordered::detailv3::KeyOrValueEquality<c10::IValue, std::pair<c10::IValue, c10::IValue>, c10::detail::DictKeyEqualTo>, std::allocator<std::pair<c10::IValue, c10::IValue> >, std::allocator<ska_ordered::detailv3::sherwood_v3_entry<std::pair<c10::IValue, c10::IValue> > > >::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<c10::detail::DictImpl, c10::detail::intrusive_target_default_null_type<c10::detail::DictImpl> >::make<const ska_ordered::order_preserving_flat_hash_map<c10::IValue, c10::IValue, c10::detail::DictKeyHash, c10::detail::DictKeyEqualTo, std::allocator<std::pair<c10::IValue, c10::IValue> > > &, const c10::detail::DictImpl::DictElementTypes &>' requested here
  return intrusive_ptr<TTarget, NullType>::make(std::forward<Args>(args)...);
                                           ^
xplat/caffe2/aten/src\ATen/core/Dict_inl.h(62,10): note: in instantiation of function template specialization 'c10::make_intrusive<c10::detail::DictImpl, c10::detail::intrusive_target_default_null_type<c10::detail::DictImpl>, const ska_ordered::order_preserving_flat_hash_map<c10::IValue, c10::IValue, c10::detail::DictKeyHash, c10::detail::DictKeyEqualTo, std::allocator<std::pair<c10::IValue, c10::IValue> > > &, const c10::detail::DictImpl::DictElementTypes &>' requested here
  return make_intrusive<DictImpl>(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: pytorch#80902
Approved by: https://github.com/malfet
  • Loading branch information
@pytorchmergebot
Kevin Stich authored and pytorchmergebot committed Jul 7, 2022
1 parent 7d0975f commit 7591444
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 36 deletions.
27 changes: 9 additions & 18 deletions c10/util/flat_hash_map.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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<uint64_t>(std::ceil(
num_elements / std::min(0.5, static_cast<double>(_max_load_factor))));
num_elements_ / std::min(0.5, static_cast<double>(_max_load_factor))));
}
void rehash_for_other_container(const sherwood_v3_table& other) {
rehash(
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2106,8 +2101,4 @@ struct power_of_two_std_hash : std::hash<T> {

} // end namespace ska

#ifndef _MSC_VER
#pragma GCC diagnostic pop
#endif

C10_CLANG_DIAGNOSTIC_POP()
27 changes: 9 additions & 18 deletions c10/util/order_preserving_flat_hash_map.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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<uint64_t>(std::ceil(
num_elements / std::min(0.5, static_cast<double>(_max_load_factor))));
num_elements_ / std::min(0.5, static_cast<double>(_max_load_factor))));
}
void rehash_for_other_container(const sherwood_v3_table& other) {
rehash(
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2234,8 +2229,4 @@ struct power_of_two_std_hash : std::hash<T> {

} // namespace ska_ordered

#ifndef _MSC_VER
#pragma GCC diagnostic pop
#endif

C10_CLANG_DIAGNOSTIC_POP()

0 comments on commit 7591444

Please sign in to comment.