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.

Sign up for GitHub

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

Jump to bottom

Simplified slice #2674

Merged
merged 2 commits into from
Nov 8, 2024
Merged

Simplified slice #2674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
12e33ac
simplify slice some more
neheb Jul 1, 2023
e13c982
Add [[nodiscard]] and suppress warnings in unit test file
kevinbackhouse Nov 5, 2024
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
43 changes: 15 additions & 28 deletions include/exiv2/slice.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ struct ConstSliceBase : SliceBase {
if (new_end > this->end_) {
throw std::out_of_range("Invalid input parameters to slice");
}
return slice_type(storage_.data_, new_begin, new_end);
return {storage_.data_, new_begin, new_end};
}

protected:
Expand Down Expand Up @@ -176,21 +176,21 @@ struct MutableSliceBase : public ConstSliceBase<storage_type, data_type> {
return this->storage_.unsafeAt(this->begin_ + index);
}

const value_type& at(size_t index) const {
[[nodiscard]] const value_type& at(size_t index) const {
return base_type::at(index);
}

/*!
* Obtain an iterator to the first element in the slice.
*/
iterator begin() noexcept {
[[nodiscard]] iterator begin() noexcept {
return this->storage_.unsafeGetIteratorAt(this->begin_);
}

/*!
* Obtain an iterator to the first element beyond the slice.
*/
iterator end() noexcept {
[[nodiscard]] iterator end() noexcept {
return this->storage_.unsafeGetIteratorAt(this->end_);
}

Expand Down Expand Up @@ -227,7 +227,7 @@ struct MutableSliceBase : public ConstSliceBase<storage_type, data_type> {
* mutable_slice_base.
*/
template <typename slice_type>
slice_type subSlice(size_t begin, size_t end) {
[[nodiscard]] slice_type subSlice(size_t begin, size_t end) {
this->rangeCheck(begin);
// end == size() is a legal value, since end is the first
// element beyond the slice
Expand All @@ -241,7 +241,7 @@ struct MutableSliceBase : public ConstSliceBase<storage_type, data_type> {
if (new_end > this->end_) {
throw std::out_of_range("Invalid input parameters to slice");
}
return slice_type(this->storage_.data_, new_begin, new_end);
return {this->storage_.data_, new_begin, new_end};
}
};

Expand Down Expand Up @@ -429,19 +429,6 @@ struct Slice : public Internal::MutableSliceBase<Internal::ContainerStorage, con
using value_type = typename std::remove_cv<typename container::value_type>::type;
#endif

/*!
* Construct a sub-slice of this slice with the given bounds. The bounds
* are evaluated with respect to the current slice.
*
* @param[in] begin First element in the new slice.
* @param[in] end First element beyond the new slice.
*
* @throw std::out_of_range when begin or end are invalid
*/
Slice subSlice(size_t begin, size_t end) {
return Internal::MutableSliceBase<Internal::ContainerStorage, container>::template subSlice<Slice>(begin, end);
}

/*!
* Constructs a new constant subSlice. Behaves otherwise exactly like
* the non-const version.
Expand All @@ -466,7 +453,7 @@ struct Slice<const container> : public Internal::ConstSliceBase<Internal::Contai
using value_type = typename std::remove_cv<typename container::value_type>::type;
#endif

Slice subSlice(size_t begin, size_t end) const {
[[nodiscard]] Slice subSlice(size_t begin, size_t end) const {
return Internal::ConstSliceBase<Internal::ContainerStorage,
const container>::template subSlice<Slice<const container>>(begin, end);
}
Expand Down Expand Up @@ -499,7 +486,7 @@ struct Slice<const T*> : public Internal::ConstSliceBase<Internal::PtrSliceStora
// TODO: use using in C++11
}

Slice<const T*> subSlice(size_t begin, size_t end) const {
[[nodiscard]] Slice<const T*> subSlice(size_t begin, size_t end) const {
return Internal::ConstSliceBase<Internal::PtrSliceStorage, const T*>::template subSlice<Slice<const T*>>(begin,
end);
}
Expand All @@ -514,7 +501,7 @@ struct Slice<T*> : public Internal::MutableSliceBase<Internal::PtrSliceStorage,
// TODO: use using in C++11
}

Slice<T*> subSlice(size_t begin, size_t end) {
[[nodiscard]] Slice<T*> subSlice(size_t begin, size_t end) {
return Internal::MutableSliceBase<Internal::PtrSliceStorage, T*>::template subSlice<Slice<T*>>(begin, end);
}

Expand All @@ -530,23 +517,23 @@ struct Slice<T*> : public Internal::MutableSliceBase<Internal::PtrSliceStorage,
* parameter deduction.
*/
template <typename T>
Slice<T> makeSlice(T& cont, size_t begin, size_t end) {
[[nodiscard]] Slice<T> makeSlice(T& cont, size_t begin, size_t end) {
return {cont, begin, end};
}

/*!
* Overload of makeSlice for slices of C-arrays.
*/
template <typename T>
Slice<T*> makeSlice(T* ptr, size_t begin, size_t end) {
[[nodiscard]] Slice<T*> makeSlice(T* ptr, size_t begin, size_t end) {
return {ptr, begin, end};
}

/*!
* @brief Return a new slice spanning the whole container.
*/
template <typename container>
Slice<container> makeSlice(container& cont) {
[[nodiscard]] Slice<container> makeSlice(container& cont) {
return {cont, 0, cont.size()};
}

Expand All @@ -555,23 +542,23 @@ Slice<container> makeSlice(container& cont) {
* container.
*/
template <typename container>
Slice<container> makeSliceFrom(container& cont, size_t begin) {
[[nodiscard]] Slice<container> makeSliceFrom(container& cont, size_t begin) {
return {cont, begin, cont.size()};
}

/*!
* @brief Return a new slice spanning until `end`.
*/
template <typename container>
Slice<container> makeSliceUntil(container& cont, size_t end) {
[[nodiscard]] Slice<container> makeSliceUntil(container& cont, size_t end) {
return {cont, 0, end};
}

/*!
* Overload of makeSliceUntil for pointer based slices.
*/
template <typename T>
Slice<T*> makeSliceUntil(T* ptr, size_t end) {
[[nodiscard]] Slice<T*> makeSliceUntil(T* ptr, size_t end) {
return {ptr, 0, end};
}

Expand Down
42 changes: 18 additions & 24 deletions unitTests/test_slice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,20 +97,18 @@ TYPED_TEST_P(slice, constructionFailsWithZeroLength) {
* Test the construction of subSlices and their behavior.
*/
TYPED_TEST_P(slice, subSliceSuccessfulConstruction) {
using slice_t = Slice<TypeParam>;

// 0 1 2 3 4 5 6 7 8 9
// | | center_vals
// | | middle
slice_t center_vals = this->getTestSlice(3, 7);
auto center_vals = this->getTestSlice(3, 7);
ASSERT_EQ(center_vals.size(), static_cast<size_t>(4));
ASSERT_NO_THROW(center_vals.subSlice(1, 3));
ASSERT_NO_THROW(static_cast<void>(center_vals.subSlice(1, 3)));

ASSERT_NO_THROW(center_vals.subSlice(1, center_vals.size()));
ASSERT_NO_THROW(static_cast<void>(center_vals.subSlice(1, center_vals.size())));
}

TYPED_TEST_P(slice, subSliceFunctions) {
Slice<TypeParam> middle = this->getTestSlice(3, 7).subSlice(1, 3);
auto middle = this->getTestSlice(3, 7).subSlice(1, 3);

ASSERT_EQ(middle.size(), static_cast<size_t>(2));
ASSERT_EQ(middle.at(1), static_cast<typename Slice<TypeParam>::value_type>(5));
Expand All @@ -121,17 +119,17 @@ TYPED_TEST_P(slice, subSliceFailedConstruction) {
// | | middle
auto middle = this->getTestSlice(4, 6);

ASSERT_THROW(middle.subSlice(1, 5), std::out_of_range);
ASSERT_THROW(middle.subSlice(2, 1), std::out_of_range);
ASSERT_THROW(middle.subSlice(2, 2), std::out_of_range);
ASSERT_THROW(static_cast<void>(middle.subSlice(1, 5)), std::out_of_range);
ASSERT_THROW(static_cast<void>(middle.subSlice(2, 1)), std::out_of_range);
ASSERT_THROW(static_cast<void>(middle.subSlice(2, 2)), std::out_of_range);
}

/*! try to cause integer overflows in a sub-optimal implementation */
TYPED_TEST_P(slice, subSliceConstructionOverflowResistance) {
auto center_vals = this->getTestSlice(3, 7);

ASSERT_THROW(center_vals.subSlice(std::numeric_limits<size_t>::max() - 2, 3), std::out_of_range);
ASSERT_THROW(center_vals.subSlice(2, std::numeric_limits<size_t>::max() - 1), std::out_of_range);
ASSERT_THROW(static_cast<void>(center_vals.subSlice(std::numeric_limits<size_t>::max() - 2, 3)), std::out_of_range);
ASSERT_THROW(static_cast<void>(center_vals.subSlice(2, std::numeric_limits<size_t>::max() - 1)), std::out_of_range);
}

/*!
Expand Down Expand Up @@ -163,11 +161,9 @@ void checkSubSlice(const Slice<T>& sl) {
* Test that all slices can be also passed as const references and still work
*/
TYPED_TEST_P(slice, constMethodsPreserveConst) {
using slice_t = Slice<TypeParam>;

// 0 1 2 3 4 5 6 7 8 9
// | | center_vals
slice_t center_vals = this->getTestSlice(3, 7);
auto center_vals = this->getTestSlice(3, 7);

// check at() const works
checkConstSliceValueAt(center_vals, 4, 1);
Expand All @@ -181,30 +177,28 @@ TYPED_TEST_P(slice, constMethodsPreserveConst) {
* Test the non-const iterators
*/
TYPED_TEST_P(mutableSlice, iterators) {
using slice_t = Slice<TypeParam>;
slice_t sl = this->getTestSlice();
auto sl = this->getTestSlice();

ASSERT_EQ(*sl.begin(), static_cast<typename slice_t::value_type>(1));
ASSERT_EQ(*sl.end(), static_cast<typename slice_t::value_type>(this->vec_size - 1));
ASSERT_EQ(*sl.begin(), static_cast<typename decltype(sl)::value_type>(1));
ASSERT_EQ(*sl.end(), static_cast<typename decltype(sl)::value_type>(this->vec_size - 1));

for (auto it = sl.begin(); it < sl.end(); ++it) {
*it = 2 * (*it);
*it *= 2;
}

ASSERT_EQ(this->vec_.at(0), 0);
for (size_t j = 1; j < this->vec_size - 1; ++j) {
ASSERT_EQ(this->vec_.at(j), static_cast<typename slice_t::value_type>(2 * j));
ASSERT_EQ(this->vec_.at(j), static_cast<typename decltype(sl)::value_type>(2 * j));
ASSERT_EQ(this->vec_.at(j), sl.at(j - 1));
}
ASSERT_EQ(this->vec_.at(this->vec_size - 1), static_cast<typename slice_t::value_type>(this->vec_size - 1));
ASSERT_EQ(this->vec_.at(this->vec_size - 1), static_cast<typename decltype(sl)::value_type>(this->vec_size - 1));
}

/*!
* Test the non-const version of at()
*/
TYPED_TEST_P(mutableSlice, at) {
using slice_t = Slice<TypeParam>;
slice_t sl = this->getTestSlice(2, 4);
auto sl = this->getTestSlice(2, 4);

sl.at(0) = 6;
sl.at(1) = 12;
Expand All @@ -215,7 +209,7 @@ TYPED_TEST_P(mutableSlice, at) {
if (j == 2 || j == 3) {
continue;
}
ASSERT_EQ(this->vec_.at(j), static_cast<typename slice_t::value_type>(j));
ASSERT_EQ(this->vec_.at(j), static_cast<typename decltype(sl)::value_type>(j));
}
}

Expand Down
Loading