Skip to content

Commit

Permalink
Fix compiler warning in btree - issue #156
Browse files Browse the repository at this point in the history
  • Loading branch information
@greg7mdp
greg7mdp committed Jul 22, 2022
1 parent 85b6d20 commit 8650bcc
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 26 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ if (PHMAP_BUILD_TESTS)

## --------------- btree -----------------------------------------------
phmap_cc_test(NAME btree SRCS "tests/btree_test.cc"
CLOPTS "-w" DEPS gmock_main)
DEPS gmock_main)


endif()
Expand Down
15 changes: 11 additions & 4 deletions parallel_hashmap/btree.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1210,6 +1210,10 @@ namespace priv {
reference value(size_type i) { return params_type::element(slot(i)); }
const_reference value(size_type i) const { return params_type::element(slot(i)); }

#if defined(__GNUC__) || defined(__clang__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Warray-bounds"
#endif
// Getters/setter for the child at position i in the node.
btree_node *child(size_type i) const { return GetField<3>()[i]; }
btree_node *&mutable_child(size_type i) { return GetField<3>()[i]; }
Expand All @@ -1221,6 +1225,9 @@ namespace priv {
mutable_child(i) = c;
c->set_position((field_type)i);
}
#if defined(__GNUC__) || defined(__clang__)
#pragma GCC diagnostic pop
#endif
void init_child(int i, btree_node *c) {
set_child(i, c);
c->set_parent(this);
Expand Down Expand Up @@ -2085,8 +2092,8 @@ namespace priv {
void internal_clear(node_type *node);

// Verifies the tree structure of node.
int internal_verify(const node_type *node,
const key_type *lo, const key_type *hi) const;
size_type internal_verify(const node_type *node,
const key_type *lo, const key_type *hi) const;

node_stats internal_stats(const node_type *node) const {
// The root can be a static empty node.
Expand Down Expand Up @@ -3234,7 +3241,7 @@ namespace priv {
}

template <typename P>
int btree<P>::internal_verify(
typename btree<P>::size_type btree<P>::internal_verify(
const node_type *node, const key_type *lo, const key_type *hi) const {
assert(node->count() > 0);
assert(node->count() <= node->max_count());
Expand All @@ -3247,7 +3254,7 @@ namespace priv {
for (int i = 1; i < node->count(); ++i) {
assert(!compare_keys(node->key(i), node->key(i - 1)));
}
int count = node->count();
size_type count = node->count();
if (!node->leaf()) {
for (int i = 0; i <= node->count(); ++i) {
assert(node->child(i) != nullptr);
Expand Down
2 changes: 1 addition & 1 deletion parallel_hashmap/phmap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3397,7 +3397,7 @@ class parallel_hash_set
//
// Do not use erase APIs taking iterators when accessing the map concurrently
// --------------------------------------------------------------------
void _erase(iterator it, bool do_lock = true) {
void _erase(iterator it) {
Inner* inner = it.inner_;
assert(inner != nullptr);
auto& set = inner->set_;
Expand Down
32 changes: 16 additions & 16 deletions tests/btree_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ namespace phmap {
size_t BaseCountedInstance::num_comparisons_ = 0;

} // namespace test_internal
} // namespace phmap\
} // namespace phmap


static const size_t test_values = 10000;
Expand Down Expand Up @@ -425,7 +425,7 @@ namespace {
const T &const_b = *b;

// Test insert.
for (int i = 0; i < values.size(); ++i) {
for (size_t i = 0; i < values.size(); ++i) {
mutable_b.insert(values[i]);
mutable_b.value_check(values[i]);
}
Expand All @@ -436,14 +436,14 @@ namespace {
// Test copy constructor.
T b_copy(const_b);
EXPECT_EQ(b_copy.size(), const_b.size());
for (int i = 0; i < values.size(); ++i) {
for (size_t i = 0; i < values.size(); ++i) {
CheckPairEquals(*b_copy.find(key_of_value(values[i])), values[i]);
}

// Test range constructor.
T b_range(const_b.begin(), const_b.end());
EXPECT_EQ(b_range.size(), const_b.size());
for (int i = 0; i < values.size(); ++i) {
for (size_t i = 0; i < values.size(); ++i) {
CheckPairEquals(*b_range.find(key_of_value(values[i])), values[i]);
}

Expand All @@ -455,7 +455,7 @@ namespace {
b_range.clear();
b_range.insert(b_copy.begin(), b_copy.end());
EXPECT_EQ(b_range.size(), b_copy.size());
for (int i = 0; i < values.size(); ++i) {
for (size_t i = 0; i < values.size(); ++i) {
CheckPairEquals(*b_range.find(key_of_value(values[i])), values[i]);
}

Expand All @@ -473,7 +473,7 @@ namespace {
b_range.swap(b_copy);
EXPECT_EQ(b_copy.size(), 0);
EXPECT_EQ(b_range.size(), const_b.size());
for (int i = 0; i < values.size(); ++i) {
for (size_t i = 0; i < values.size(); ++i) {
CheckPairEquals(*b_range.find(key_of_value(values[i])), values[i]);
}
b_range.swap(b_copy);
Expand All @@ -482,13 +482,13 @@ namespace {
swap(b_range, b_copy);
EXPECT_EQ(b_copy.size(), 0);
EXPECT_EQ(b_range.size(), const_b.size());
for (int i = 0; i < values.size(); ++i) {
for (size_t i = 0; i < values.size(); ++i) {
CheckPairEquals(*b_range.find(key_of_value(values[i])), values[i]);
}
swap(b_range, b_copy);

// Test erase via values.
for (int i = 0; i < values.size(); ++i) {
for (size_t i = 0; i < values.size(); ++i) {
mutable_b.erase(key_of_value(values[i]));
// Erasing a non-existent key should have no effect.
ASSERT_EQ(mutable_b.erase(key_of_value(values[i])), 0);
Expand All @@ -499,15 +499,15 @@ namespace {

// Test erase via iterators.
mutable_b = b_copy;
for (int i = 0; i < values.size(); ++i) {
for (size_t i = 0; i < values.size(); ++i) {
mutable_b.erase(mutable_b.find(key_of_value(values[i])));
}

const_b.verify();
EXPECT_EQ(const_b.size(), 0);

// Test insert with hint.
for (int i = 0; i < values.size(); i++) {
for (size_t i = 0; i < values.size(); i++) {
mutable_b.insert(mutable_b.upper_bound(key_of_value(values[i])), values[i]);
}

Expand All @@ -521,25 +521,25 @@ namespace {
// First half.
mutable_b = b_copy;
typename T::iterator mutable_iter_end = mutable_b.begin();
for (int i = 0; i < values.size() / 2; ++i) ++mutable_iter_end;
for (size_t i = 0; i < values.size() / 2; ++i) ++mutable_iter_end;
mutable_b.erase(mutable_b.begin(), mutable_iter_end);
EXPECT_EQ(mutable_b.size(), values.size() - values.size() / 2);
const_b.verify();

// Second half.
mutable_b = b_copy;
typename T::iterator mutable_iter_begin = mutable_b.begin();
for (int i = 0; i < values.size() / 2; ++i) ++mutable_iter_begin;
for (size_t i = 0; i < values.size() / 2; ++i) ++mutable_iter_begin;
mutable_b.erase(mutable_iter_begin, mutable_b.end());
EXPECT_EQ(mutable_b.size(), values.size() / 2);
const_b.verify();

// Second quarter.
mutable_b = b_copy;
mutable_iter_begin = mutable_b.begin();
for (int i = 0; i < values.size() / 4; ++i) ++mutable_iter_begin;
for (size_t i = 0; i < values.size() / 4; ++i) ++mutable_iter_begin;
mutable_iter_end = mutable_iter_begin;
for (int i = 0; i < values.size() / 4; ++i) ++mutable_iter_end;
for (size_t i = 0; i < values.size() / 4; ++i) ++mutable_iter_end;
mutable_b.erase(mutable_iter_begin, mutable_iter_end);
EXPECT_EQ(mutable_b.size(), values.size() - values.size() / 4);
const_b.verify();
Expand Down Expand Up @@ -1862,7 +1862,7 @@ namespace {
while (s.size() < kSize) {
s.insert(MovableOnlyInstance(s.size()));
}
for (int i = 0; i < kSize; ++i) {
for (size_t i = 0; i < kSize; ++i) {
// Extract with key
auto nh = s.extract(MovableOnlyInstance(i));
EXPECT_EQ(s.size(), kSize - 1);
Expand Down Expand Up @@ -1895,7 +1895,7 @@ namespace {
m.insert(
{CopyableMovableInstance(m.size()), MovableOnlyInstance(m.size())});
}
for (int i = 0; i < kSize; ++i) {
for (size_t i = 0; i < kSize; ++i) {
// Extract with key
auto nh = m.extract(CopyableMovableInstance(i));
EXPECT_EQ(m.size(), kSize - 1);
Expand Down
8 changes: 4 additions & 4 deletions tests/btree_test.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ namespace priv {
};

// Generate n values for our tests and benchmarks. Value range is [0, maxval].
inline std::vector<int> GenerateNumbersWithSeed(int n, int maxval, int seed) {
inline std::vector<int> GenerateNumbersWithSeed(size_t n, int maxval, int seed) {
// NOTE: Some tests rely on generated numbers not changing between test runs.
// We use std::minstd_rand0 because it is well-defined, but don't use
// std::uniform_int_distribution because platforms use different algorithms.
Expand All @@ -400,7 +400,7 @@ namespace priv {
std::vector<int> values;
phmap::flat_hash_set<int> unique_values;
if (values.size() < n) {
for (size_t i = values.size(); i < (size_t)n; i++) {
for (size_t i = values.size(); i < n; i++) {
int value;
do {
value = static_cast<int>(rng()) % (maxval + 1);
Expand All @@ -414,13 +414,13 @@ namespace priv {

// Generates n values in the range [0, maxval].
template <typename V>
std::vector<V> GenerateValuesWithSeed(int n, int maxval, int seed) {
std::vector<V> GenerateValuesWithSeed(size_t n, int maxval, int seed) {
const std::vector<int> nums = GenerateNumbersWithSeed(n, maxval, seed);
Generator<V> gen(maxval);
std::vector<V> vec;

vec.reserve(n);
for (int i = 0; i < n; i++) {
for (size_t i = 0; i < n; i++) {
vec.push_back(gen(nums[i]));
}

Expand Down

0 comments on commit 8650bcc

Please sign in to comment.