Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🧹 Fix GCC complaints #2400

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 11 additions & 17 deletions arbor/tree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ tree::tree(std::vector<tree::int_type> parent_index) {
parents_[0] = no_parent;

// compute offsets into children_ array
arb::util::make_partition(child_index_, child_count(parents_));
util::make_partition(child_index_, child_count(parents_));

std::vector<int_type> pos(parents_.size(), 0);
for (auto i = 1u; i < parents_.size(); ++i) {
Expand Down Expand Up @@ -199,11 +199,9 @@ tree::iarray tree::select_new_root(int_type root) {
}

// maps new indices to old indices
iarray indices (num_nodes);
// fill array with indices
for (auto i: make_span(num_nodes)) {
indices[i] = i;
}
iarray indices(num_nodes);
std::iota(indices.begin(), indices.end(), 0);

// perform sort by depth index to get the permutation
std::sort(indices.begin(), indices.end(), [&](auto i, auto j){
if (reduced_depth[i] != reduced_depth[j]) {
Expand All @@ -214,16 +212,12 @@ tree::iarray tree::select_new_root(int_type root) {
}
return depth[i] < depth[j];
});
// maps old indices to new indices
iarray indices_inv (num_nodes);
// fill array with indices
for (auto i: make_span(num_nodes)) {
indices_inv[i] = i;
}
// perform sort
std::sort(indices_inv.begin(), indices_inv.end(), [&](auto i, auto j){
return indices[i] < indices[j];
});

// inverse permutation
iarray indices_inv(num_nodes, 0);
std::iota(indices_inv.begin(), indices_inv.end(), 0);
std::sort(indices_inv.begin(), indices_inv.end(),
[&](auto i, auto j){ return indices[i] < indices[j]; });

// translate the parent vetor to new indices
for (auto i: make_span(num_nodes)) {
Expand All @@ -241,7 +235,7 @@ tree::iarray tree::select_new_root(int_type root) {

// recompute the children array
memory::copy(new_parents, parents_);
arb::util::make_partition(child_index_, child_count(parents_));
util::make_partition(child_index_, child_count(parents_));

std::vector<int_type> pos(parents_.size(), 0);
for (auto i = 1u; i < parents_.size(); ++i) {
Expand Down
4 changes: 0 additions & 4 deletions arbor/tree.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,12 @@

#include <algorithm>
#include <cassert>
#include <fstream>
#include <numeric>
#include <vector>

#include <arbor/export.hpp>
#include <arbor/common_types.hpp>

#include "memory/memory.hpp"
#include "util/rangeutil.hpp"
#include "util/span.hpp"

namespace arb {

Expand Down
1 change: 0 additions & 1 deletion arbor/util/meta.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include <array>
#include <cstddef>
#include <iterator>
#include <tuple>
#include <utility>
#include <type_traits>

Expand Down
16 changes: 5 additions & 11 deletions arbor/util/range.hpp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using std::begin; using std::end; are brought in scope such that ADL works for types that have an overload for begin(), see https://en.cppreference.com/w/cpp/iterator/begin

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I think all of our containers use a member of begin, it's the proper way.

Original file line number Diff line number Diff line change
Expand Up @@ -156,24 +156,18 @@ range<T, T> range_n(T t, size_t n) {

template <typename Seq>
auto canonical_view(Seq&& s) {
using std::begin;
using std::end;

return make_range(
make_sentinel_iterator(begin(s), end(s)),
make_sentinel_end(begin(s), end(s)));
auto b = std::begin(s);
auto e = std::end(s);
return make_range(make_sentinel_iterator(b, e), make_sentinel_end(b, e));
}

// Strictly evaluate end point in sentinel-terminated range and present as a range over
// iterators. Note: O(N) behaviour with forward iterator ranges or sentinel-terminated ranges.

template <typename Seq>
auto strict_view(Seq&& s) {
using std::begin;
using std::end;

auto b = begin(s);
auto e = end(s);
auto b = std::begin(s);
auto e = std::end(s);
return make_range(b, b==e? b: std::next(util::upto(b, e)));
}

Expand Down
1 change: 0 additions & 1 deletion arbor/util/rangeutil.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include <algorithm>
#include <iterator>
#include <ostream>
#include <numeric>
#include <cstring>
#include <type_traits>
Expand Down
13 changes: 6 additions & 7 deletions test/unit-distributed/test_distributed_for_each.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,9 @@ TEST(distributed_for_each, one_zero) {
for (int i = 0; i < rank; ++i) { data.push_back(rank); }

auto sample = [&](const util::range<int*>& range) {
const auto origin_rank = range.empty() ? 0 : range.front();

std::size_t origin_rank = range.empty() ? 0 : range.front();
EXPECT_EQ(origin_rank, range.size());
for (const auto& value: range) { EXPECT_EQ(value, origin_rank); }
for (std::size_t value: range) { EXPECT_EQ(value, origin_rank); }
++call_count;
};

Expand Down Expand Up @@ -71,14 +70,14 @@ TEST(distributed_for_each, multiple) {
auto sample = [&](const util::range<int*>& range_1,
const util::range<double*>& range_2,
const util::range<std::complex<double>*>& range_3) {
const auto origin_rank = range_1.empty() ? 0 : range_1.front();
std::size_t origin_rank = range_1.empty() ? 0 : range_1.front();

EXPECT_EQ(origin_rank + 1, range_1.size());
EXPECT_EQ(range_2.size(), 2 * range_1.size());
EXPECT_EQ(range_3.size(), 3 * range_1.size());
for (const auto& value: range_1) { EXPECT_EQ(value, origin_rank); }
for (const auto& value: range_2) { EXPECT_EQ(value, double(origin_rank)); }
for (const auto& value: range_3) { EXPECT_EQ(value, std::complex<double>(origin_rank)); }
for (std::size_t value: range_1) { EXPECT_EQ(value, origin_rank); }
for (auto value: range_2) { EXPECT_EQ(value, double(origin_rank)); }
for (auto value: range_3) { EXPECT_EQ(value, std::complex<double>(origin_rank)); }
++call_count;
};

Expand Down
8 changes: 4 additions & 4 deletions test/unit-distributed/test_network_generation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ TEST(network_generation, all) {
}

for (const auto& group: decomp.groups()) {
const auto num_dest = group.kind == cell_kind::spike_source ? 0 : 1;
for (const auto gid: group.gids) {
std::size_t num_dest = group.kind == cell_kind::spike_source ? 0 : 1;
for (std::size_t gid: group.gids) {
EXPECT_EQ(connections_by_dest[gid].size(), num_cells * num_dest);
}
}
Expand All @@ -141,7 +141,7 @@ TEST(network_generation, cable_only) {
const auto weight = 2.0;
const auto delay = 3.0;

const auto num_cells = 3 * num_ranks;
const std::size_t num_cells = 3 * num_ranks;

auto rec = network_test_recipe(num_cells, selection, weight, delay);

Expand All @@ -161,7 +161,7 @@ TEST(network_generation, cable_only) {
for (const auto gid: group.gids) {
// Only one third is a cable cell
EXPECT_EQ(connections_by_dest[gid].size(),
group.kind == cell_kind::cable ? num_cells / 3 : 0);
group.kind == cell_kind::cable ? num_cells / 3 : 0);
}
}
}
1 change: 1 addition & 0 deletions test/unit/common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <utility>
#include <vector>
#include <algorithm>
#include <ostream>

#include <gtest/gtest.h>

Expand Down
64 changes: 28 additions & 36 deletions test/unit/test_range.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "util/meta.hpp"
#include "util/range.hpp"
#include "util/rangeutil.hpp"
#include "util/sentinel.hpp"
#include "util/transform.hpp"

#include "common.hpp"
Expand Down Expand Up @@ -442,41 +441,34 @@ struct foo {
};

TEST(range, sort) {
char cstr[] = "howdy";

auto cstr_range = util::make_range(std::begin(cstr), null_terminated);

// Alas, no forward_iterator sort yet, so make a strict (non-sentinel)
// range to sort on below

// simple sort
util::sort(util::strict_view(cstr_range));
EXPECT_EQ("dhowy"s, cstr);

// reverse sort by transform c to -c
util::sort_by(util::strict_view(cstr_range), [](char c) { return -c; });
EXPECT_EQ("ywohd"s, cstr);

// stable sort: move capitals to front, numbers to back
auto rank = [](char c) {
return std::isupper(c)? 0: std::isdigit(c)? 2: 1;
};

char mixed[] = "t5hH4E3erLL2e1O";
auto mixed_range = util::make_range(std::begin(mixed), null_terminated);

util::stable_sort_by(util::strict_view(mixed_range), rank);
EXPECT_EQ("HELLOthere54321"s, mixed);


// sort with user-provided less comparison function

std::vector<foo> X = {{0, 5}, {1, 4}, {2, 3}, {3, 2}, {4, 1}, {5, 0}};

util::sort(X, [](const foo& l, const foo& r) {return l.y<r.y;});
EXPECT_EQ(X, (std::vector<foo>{{5, 0}, {4, 1}, {3, 2}, {2, 3}, {1, 4}, {0, 5}}));
util::sort(X, [](const foo& l, const foo& r) {return l.x<r.x;});
EXPECT_EQ(X, (std::vector<foo>{{0, 5}, {1, 4}, {2, 3}, {3, 2}, {4, 1}, {5, 0}}));
{
// simple sort
char cstr[] = "howdy";
util::sort(util::range_n(cstr, 5));
EXPECT_EQ("dhowy"s, cstr);
}
{
// reverse sort by transform c to -c
char cstr[] = "howdy";
util::sort_by(util::range_n(cstr, 5),
[](char c) { return -c; });
EXPECT_EQ("ywohd"s, cstr);
}
{
// stable sort: move capitals to front, numbers to back
char mixed[] = "t5hH4E3erLL2e1O";
util::stable_sort_by(util::strict_view(util::make_range(std::begin(mixed), null_terminated)),
[](char c) { return std::isupper(c)? 0: std::isdigit(c)? 2: 1; });
EXPECT_EQ("HELLOthere54321"s, mixed);
}
{
// sort with user-provided less comparison function
std::vector<foo> X = {{0, 5}, {1, 4}, {2, 3}, {3, 2}, {4, 1}, {5, 0}};
util::sort(X, [](const foo& l, const foo& r) {return l.y<r.y;});
EXPECT_EQ(X, (std::vector<foo>{{5, 0}, {4, 1}, {3, 2}, {2, 3}, {1, 4}, {0, 5}}));
util::sort(X, [](const foo& l, const foo& r) {return l.x<r.x;});
EXPECT_EQ(X, (std::vector<foo>{{0, 5}, {1, 4}, {2, 3}, {3, 2}, {4, 1}, {5, 0}}));
}
}

TEST(range, sum) {
Expand Down
4 changes: 2 additions & 2 deletions test/unit/test_simd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -359,13 +359,13 @@ TYPED_TEST_P(simd_value, comparison) {
for (unsigned i = 0; i<nrounds; ++i) {
int cmp[N];
bool test[N];
simd a, b;
simd a = 0, b = 0;

fill_random(b, rng);

for (unsigned j = 0; j<N; ++j) {
cmp[j] = sgn(rng);
a[j] = b[j]+17*cmp[j];
a[j] = b[j] + 17*cmp[j];
}

mask gt = a>b;
Expand Down