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 unordered and ordered containers ranges for empty containers #703

Merged
merged 4 commits into from
Jan 21, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
22 changes: 16 additions & 6 deletions include/oneapi/tbb/detail/_concurrent_skip_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class skip_list_node {
using pointer = typename value_allocator_traits::pointer;
using const_pointer = typename value_allocator_traits::const_pointer;

//In perfect world these constructor and destructor would have been private,
//In perfect world these constructor and destructor would have been private,
//however this seems technically impractical due to use of allocator_traits.

//Should not be called directly, instead use create method
Expand Down Expand Up @@ -671,24 +671,34 @@ class concurrent_skip_list {
using reference = typename iterator::reference;

bool empty() const {
return my_begin.my_node_ptr->next(0) == my_end.my_node_ptr;
return my_begin.my_node_ptr ? (my_begin.my_node_ptr->next(0) == my_end.my_node_ptr)
: true;
}

bool is_divisible() const {
return my_level != 0 ? my_begin.my_node_ptr->next(my_level - 1) != my_end.my_node_ptr : false;
return my_begin.my_node_ptr && my_level != 0
? my_begin.my_node_ptr->next(my_level - 1) != my_end.my_node_ptr
: false;
}

size_type size() const { return std::distance(my_begin, my_end); }

const_range_type( const_range_type& r, split)
: my_end(r.my_end) {
my_begin = iterator(r.my_begin.my_node_ptr->next(r.my_level - 1));
my_level = my_begin.my_node_ptr->height();
if (r.empty()) {
__TBB_ASSERT(my_end.my_node_ptr == nullptr, nullptr);
my_begin = my_end;
my_level = 0;
} else {
my_begin = iterator(r.my_begin.my_node_ptr->next(r.my_level - 1));
my_level = my_begin.my_node_ptr->height();
}
r.my_end = my_begin;
}

const_range_type( const concurrent_skip_list& l)
: my_end(l.end()), my_begin(l.begin()), my_level(my_begin.my_node_ptr->height() ) {}
: my_end(l.end()), my_begin(l.begin()),
my_level(my_begin.my_node_ptr ? my_begin.my_node_ptr->height() : 0) {}

iterator begin() const { return my_begin; }
iterator end() const { return my_end; }
Expand Down
4 changes: 2 additions & 2 deletions include/oneapi/tbb/detail/_concurrent_unordered_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ class concurrent_unordered_base {
using difference_type = typename concurrent_unordered_base::difference_type;
using iterator = typename concurrent_unordered_base::const_iterator;

bool empty() const { return my_begin_node == my_end_node; }
bool empty() const { return my_instance.first_value_node(my_begin_node) == my_end_node; }

bool is_divisible() const {
return my_midpoint_node != my_end_node;
Expand Down Expand Up @@ -733,7 +733,7 @@ class concurrent_unordered_base {
}
private:
void set_midpoint() const {
if (my_begin_node == my_end_node) {
if (empty()) {
my_midpoint_node = my_end_node;
} else {
sokey_type invalid_key = ~sokey_type(0);
Expand Down
29 changes: 29 additions & 0 deletions test/common/concurrent_associative_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,16 @@ void test_comparison_operators() {
check_unequal(cont, cont6);
}

template <typename Range, typename Container>
void test_empty_container_range(Container&& cont) {
REQUIRE(cont.empty());
Range r = cont.range();
REQUIRE_MESSAGE(r.empty(), "Empty container range should be empty");
REQUIRE_MESSAGE(!r.is_divisible(), "Empty container range should not be divisible");
REQUIRE_MESSAGE(r.begin() == r.end(), "Incorrect iterators on empty range");
REQUIRE_MESSAGE(r.begin() == cont.begin(), "Incorrect iterators on empty range");
}

template<typename T, typename CheckElementState>
void test_basic_common()
{
Expand All @@ -446,6 +456,25 @@ void test_basic_common()
REQUIRE_MESSAGE(ccont.begin() == ccont.end(), "Concurrent container iterators are invalid after construction");
REQUIRE_MESSAGE(cont.cbegin() == cont.cend(), "Concurrent container iterators are invalid after construction");

// Test range for empty container
using range_type = typename T::range_type;
using const_range_type = typename T::const_range_type;
test_empty_container_range<range_type>(cont);
test_empty_container_range<const_range_type>(cont);
test_empty_container_range<const_range_type>(ccont);

T empty_cont;
const T& empty_ccont = empty_cont;

for (int i = 0; i < 1000; ++i) {
empty_cont.insert(Value<T>::make(i));
}
empty_cont.clear();

test_empty_container_range<range_type>(empty_cont);
test_empty_container_range<const_range_type>(empty_cont);
test_empty_container_range<const_range_type>(empty_ccont);

//std::pair<iterator, bool> insert(const value_type& obj);
std::pair<typename T::iterator, bool> ins = cont.insert(Value<T>::make(1));
REQUIRE_MESSAGE((ins.second == true && Value<T>::get(*(ins.first)) == 1), "Element 1 has not been inserted properly");
Expand Down