Skip to content

Commit

Permalink
add comments, reorganize test sections
Browse files Browse the repository at this point in the history
  • Loading branch information
@m-fila
m-fila committed May 22, 2024
1 parent dd79f1c commit 5c187de
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 67 deletions.
4 changes: 2 additions & 2 deletions doc/collections_as_container.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

Comparison of the PODIO `Collection`s with a C++ named requirement [*Container*](https://en.cppreference.com/w/cpp/named_req/Container).

The PODIO `Collection`s are move-only classes with emphasis on the distinction between mutable and immutable access to the elements.
The PODIO `Collection`s interface was designed to remind the standard *Container* interface, in particular `std::vector`. The perfect compliance with the *Container* is not achieved as the `Collection`s are concerned with additional semantics such as mutable/immutable element access, associations and relations, and IO which are not part of *Container*.

### Container Types

Expand Down Expand Up @@ -140,7 +140,7 @@ The C++ specifies a set of named requirements for iterators. Starting with C++20

| Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment |
|------------|-------------|-----------|-------------------------------------------|---------|
| `*r = o` | | | ❌ no / ❌ no | Assignment doesn't modify objects inside collection |
| `*r = o` | | | ❗ attention / ❗ attention | Defined but an assignment doesn't modify objects inside collection |
| `++r` | `It&` | | ✔️ yes / ✔️ yes | |
| `r++` | Convertible to `const It&` | Same as `It temp = r; ++r; return temp;` | ❌ no / ❌ no | Post-increment not defined |
| `*r++ = o` | | Same as `*r = o; ++r;`| ❌ no / ❌ no | Post-increment not defined |
Expand Down
139 changes: 74 additions & 65 deletions tests/unittests/std_interoperability.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,10 +395,14 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") {
using iterator = CollectionType::iterator;
using const_iterator = CollectionType::const_iterator;
// the checks are duplicated for iterator and const_iterator as expectations on them are slightly different

// nested sections as the requirements make a hierarchy
SECTION("LegacyForwardIterator") {

// LegacyForwardIterator requires LegacyInputIterator
SECTION("LegacyInputIterator") {

// LegacyInputIterator requires LegacyIterator
SECTION("LegacyIterator") {

// CopyConstructible
Expand Down Expand Up @@ -474,7 +478,8 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") {
STATIC_REQUIRE(traits::has_preincrement_v<const_iterator>);
STATIC_REQUIRE(
std::is_same_v<decltype(++std::declval<const_iterator>()), std::add_lvalue_reference_t<const_iterator>>);
}

} // end of LegacyIterator

// EqualityComparable
// iterator
Expand Down Expand Up @@ -552,7 +557,8 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") {
DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v<std::iterator_traits<const_iterator>>);
// STATIC_REQUIRE(std::is_convertible_v < decltype(*std::declval<const_iterator>()++),
// std::iterator_traits<const_iterator>::value_type >>);
}

} // end of LegacyInputIterator

// Mutable iterator: reference same as value_type& or value_type&&
DOCUMENTED_STATIC_FAILURE(traits::has_reference_v<iterator>);
Expand All @@ -576,75 +582,76 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") {
DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v<iterator>);
// const_iterator
DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v<const_iterator>);
}

// Multipass guarantee
// iterator
{
CollectionType coll;
for (int i = 0; i < 3; ++i) {
coll.create();
}
// Multipass guarantee
// iterator
auto a = coll.begin();
auto b = coll.begin();
REQUIRE(a == b);
REQUIRE(*a == *b);
REQUIRE(++a == ++b);
REQUIRE(*a == *b);
DOCUMENTED_STATIC_FAILURE(std::is_copy_constructible_v<iterator>);
// auto a_copy = a;
// ++a_copy;
// REQUIRE(a == b);
// REQUIRE(*a == *b);
{
CollectionType coll;
for (int i = 0; i < 3; ++i) {
coll.create();
}
// iterator
auto a = coll.begin();
auto b = coll.begin();
REQUIRE(a == b);
REQUIRE(*a == *b);
REQUIRE(++a == ++b);
REQUIRE(*a == *b);
DOCUMENTED_STATIC_FAILURE(std::is_copy_constructible_v<iterator>);
// auto a_copy = a;
// ++a_copy;
// REQUIRE(a == b);
// REQUIRE(*a == *b);

// const_iterator
auto ca = coll.cbegin();
auto cb = coll.cbegin();
REQUIRE(ca == cb);
REQUIRE(*ca == *cb);
REQUIRE(++ca == ++cb);
REQUIRE(*ca == *cb);
DOCUMENTED_STATIC_FAILURE(std::is_copy_constructible_v<const_iterator>);
// auto ca_copy = ca;
// ++ca_copy;
// REQUIRE(ca == cb);
// REQUIRE(*ca == *cb);
}

// Singular iterators
// iterator
STATIC_REQUIRE(traits::has_equality_comparator_v<iterator>);
DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v<iterator>);
//{
// REQUIRE(iterator{} == iterator{});
//}
// const_iterator
auto ca = coll.cbegin();
auto cb = coll.cbegin();
REQUIRE(ca == cb);
REQUIRE(*ca == *cb);
REQUIRE(++ca == ++cb);
REQUIRE(*ca == *cb);
DOCUMENTED_STATIC_FAILURE(std::is_copy_constructible_v<const_iterator>);
// auto ca_copy = ca;
// ++ca_copy;
// REQUIRE(ca == cb);
// REQUIRE(*ca == *cb);
}
STATIC_REQUIRE(traits::has_equality_comparator_v<const_iterator>);
DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v<const_iterator>);
//{
// REQUIRE(const_iterator{} == const_iterator{});
//}

// Singular iterators
// iterator
STATIC_REQUIRE(traits::has_equality_comparator_v<iterator>);
DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v<iterator>);
//{
// REQUIRE(iterator{} == iterator{});
//}
// const_iterator
STATIC_REQUIRE(traits::has_equality_comparator_v<const_iterator>);
DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v<const_iterator>);
//{
// REQUIRE(const_iterator{} == const_iterator{});
//}
// i++
// iterator
DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v<iterator>);
// STATIC_REQUIRE(std::is_same_v<decltype(std::declval<iterator>()++), iterator>);
// const_iterator
DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v<const_iterator>);
// STATIC_REQUIRE(std::is_same_v<decltype(std::declval<const_iterator>()++), const_iterator>);

// i++
// iterator
DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v<iterator>);
// STATIC_REQUIRE(std::is_same_v<decltype(std::declval<iterator>()++), iterator>);
// const_iterator
DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v<const_iterator>);
// STATIC_REQUIRE(std::is_same_v<decltype(std::declval<const_iterator>()++), const_iterator>);
// *i++
// iterator
DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v<iterator>);
DOCUMENTED_STATIC_FAILURE(traits::has_reference_v<std::iterator_traits<iterator>>);
// STATIC_REQUIRE(std::is_same_v < decltype(*std::declval<iterator>()++),
// std::iterator_traits<iterator>::reference >>);
// const_iterator
DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v<const_iterator>);
DOCUMENTED_STATIC_FAILURE(traits::has_reference_v<std::iterator_traits<const_iterator>>);
// STATIC_REQUIRE(std::is_same_v < decltype(*std::declval<const_iterator>()++),
// std::iterator_traits<const_iterator>::reference >>);

// *i++
// iterator
DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v<iterator>);
DOCUMENTED_STATIC_FAILURE(traits::has_reference_v<std::iterator_traits<iterator>>);
// STATIC_REQUIRE(std::is_same_v < decltype(*std::declval<iterator>()++),
// std::iterator_traits<iterator>::reference >>);
// const_iterator
DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v<const_iterator>);
DOCUMENTED_STATIC_FAILURE(traits::has_reference_v<std::iterator_traits<const_iterator>>);
// STATIC_REQUIRE(std::is_same_v < decltype(*std::declval<const_iterator>()++),
// std::iterator_traits<const_iterator>::reference >>);
} // end of LegacyForwardIterator

SECTION("LegacyOutputIterator") {

Expand Down Expand Up @@ -712,7 +719,9 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") {
DOCUMENTED_STATIC_FAILURE(
traits::has_dereference_assignment_increment_v<const_iterator, CollectionType::value_type::mutable_type>);
// TODO add runtime check for assignment validity like in '*r = o' case
}

} // end of LegacyOutputIterator

}

TEST_CASE("Collection and std iterator adaptors", "[collection][container][adapter][std]") {
Expand Down

0 comments on commit 5c187de

Please sign in to comment.