[Bf-blender-cvs] [6c2e1f33983] master: BLI: cleanup StringRef and Span and improve parameter validation
Jacques Lucke
noreply at git.blender.org
Sat Feb 20 22:07:25 CET 2021
Commit: 6c2e1f33983766ee5ee028e14a24e36c28d0a566
Author: Jacques Lucke
Date: Sat Feb 20 22:05:27 2021 +0100
Branches: master
https://developer.blender.org/rB6c2e1f33983766ee5ee028e14a24e36c28d0a566
BLI: cleanup StringRef and Span and improve parameter validation
Previously, methods like `Span.drop_front` would crash when more
elements would be dropped than are available. While this is most
efficient, it is not very practical in some use cases. Also other languages
silently clamp the index, so one can easily write wrong code accidentally.
Now, `Span.drop_front` and similar methods will only crash when n
is negative. Too large values will be clamped down to their maximum
possible value. While this is slightly less efficient, I did not have a case
where this actually mattered yet. If it does matter in the future, we can
add a separate `*_unchecked` method.
This should not change the behavior of existing code.
===================================================================
M source/blender/blenlib/BLI_span.hh
M source/blender/blenlib/BLI_string_ref.hh
M source/blender/blenlib/tests/BLI_span_test.cc
M source/blender/blenlib/tests/BLI_string_ref_test.cc
===================================================================
diff --git a/source/blender/blenlib/BLI_span.hh b/source/blender/blenlib/BLI_span.hh
index 8011b2f9abc..5f55efe3f63 100644
--- a/source/blender/blenlib/BLI_span.hh
+++ b/source/blender/blenlib/BLI_span.hh
@@ -142,15 +142,15 @@ template<typename T> class Span {
}
/**
- * Returns a contiguous part of the array. This invokes undefined behavior when the slice does
- * not stay within the bounds of the array.
+ * Returns a contiguous part of the array. This invokes undefined behavior when the start or size
+ * is negative.
*/
constexpr Span slice(int64_t start, int64_t size) const
{
BLI_assert(start >= 0);
BLI_assert(size >= 0);
- BLI_assert(start + size <= this->size() || size == 0);
- return Span(data_ + start, size);
+ const int64_t new_size = std::max<int64_t>(0, std::min(size, size_ - start));
+ return Span(data_ + start, new_size);
}
constexpr Span slice(IndexRange range) const
@@ -160,46 +160,46 @@ template<typename T> class Span {
/**
* Returns a new Span with n elements removed from the beginning. This invokes undefined
- * behavior when the array is too small.
+ * behavior when n is negative.
*/
constexpr Span drop_front(int64_t n) const
{
BLI_assert(n >= 0);
- BLI_assert(n <= this->size());
- return this->slice(n, this->size() - n);
+ const int64_t new_size = std::max<int64_t>(0, size_ - n);
+ return Span(data_ + n, new_size);
}
/**
* Returns a new Span with n elements removed from the beginning. This invokes undefined
- * behavior when the array is too small.
+ * behavior when n is negative.
*/
constexpr Span drop_back(int64_t n) const
{
BLI_assert(n >= 0);
- BLI_assert(n <= this->size());
- return this->slice(0, this->size() - n);
+ const int64_t new_size = std::max<int64_t>(0, size_ - n);
+ return Span(data_, new_size);
}
/**
* Returns a new Span that only contains the first n elements. This invokes undefined
- * behavior when the array is too small.
+ * behavior when n is negative.
*/
constexpr Span take_front(int64_t n) const
{
BLI_assert(n >= 0);
- BLI_assert(n <= this->size());
- return this->slice(0, n);
+ const int64_t new_size = std::min<int64_t>(size_, n);
+ return Span(data_, new_size);
}
/**
* Returns a new Span that only contains the last n elements. This invokes undefined
- * behavior when the array is too small.
+ * behavior when n is negative.
*/
constexpr Span take_back(int64_t n) const
{
BLI_assert(n >= 0);
- BLI_assert(n <= this->size());
- return this->slice(this->size() - n, n);
+ const int64_t new_size = std::min<int64_t>(size_, n);
+ return Span(data_ + size_ - new_size, new_size);
}
/**
@@ -480,6 +480,14 @@ template<typename T> class MutableSpan {
return size_;
}
+ /**
+ * Returns true if the size is zero.
+ */
+ constexpr bool is_empty() const
+ {
+ return size_ == 0;
+ }
+
/**
* Replace all elements in the referenced array with the given value.
*/
@@ -534,53 +542,59 @@ template<typename T> class MutableSpan {
}
/**
- * Returns a contiguous part of the array. This invokes undefined behavior when the slice would
- * go out of bounds.
+ * Returns a contiguous part of the array. This invokes undefined behavior when the start or size
+ * is negative.
*/
- constexpr MutableSpan slice(const int64_t start, const int64_t length) const
+ constexpr MutableSpan slice(const int64_t start, const int64_t size) const
{
- BLI_assert(start + length <= this->size());
- return MutableSpan(data_ + start, length);
+ BLI_assert(start >= 0);
+ BLI_assert(size >= 0);
+ const int64_t new_size = std::max<int64_t>(0, std::min(size, size_ - start));
+ return MutableSpan(data_ + start, new_size);
}
/**
* Returns a new MutableSpan with n elements removed from the beginning. This invokes
- * undefined behavior when the array is too small.
+ * undefined behavior when n is negative.
*/
constexpr MutableSpan drop_front(const int64_t n) const
{
- BLI_assert(n <= this->size());
- return this->slice(n, this->size() - n);
+ BLI_assert(n >= 0);
+ const int64_t new_size = std::max<int64_t>(0, size_ - n);
+ return MutableSpan(data_ + n, new_size);
}
/**
* Returns a new MutableSpan with n elements removed from the end. This invokes undefined
- * behavior when the array is too small.
+ * behavior when n is negative.
*/
constexpr MutableSpan drop_back(const int64_t n) const
{
- BLI_assert(n <= this->size());
- return this->slice(0, this->size() - n);
+ BLI_assert(n >= 0);
+ const int64_t new_size = std::max<int64_t>(0, size_ - n);
+ return MutableSpan(data_, new_size);
}
/**
* Returns a new MutableSpan that only contains the first n elements. This invokes undefined
- * behavior when the array is too small.
+ * behavior when n is negative.
*/
constexpr MutableSpan take_front(const int64_t n) const
{
- BLI_assert(n <= this->size());
- return this->slice(0, n);
+ BLI_assert(n >= 0);
+ const int64_t new_size = std::min<int64_t>(size_, n);
+ return MutableSpan(data_, new_size);
}
/**
* Return a new MutableSpan that only contains the last n elements. This invokes undefined
- * behavior when the array is too small.
+ * behavior when n is negative.
*/
constexpr MutableSpan take_back(const int64_t n) const
{
- BLI_assert(n <= this->size());
- return this->slice(this->size() - n, n);
+ BLI_assert(n >= 0);
+ const int64_t new_size = std::min<int64_t>(size_, n);
+ return MutableSpan(data_ + size_ - new_size, new_size);
}
/**
diff --git a/source/blender/blenlib/BLI_string_ref.hh b/source/blender/blenlib/BLI_string_ref.hh
index a2562c6100a..0cff8fa7fb4 100644
--- a/source/blender/blenlib/BLI_string_ref.hh
+++ b/source/blender/blenlib/BLI_string_ref.hh
@@ -321,37 +321,36 @@ class StringRef : public StringRefBase {
}
/**
- * Returns a new StringRef that does not contain the first n chars.
- *
- * This is similar to std::string_view::remove_prefix.
+ * Returns a new StringRef that does not contain the first n chars. This invokes undefined
+ * behavior when n is negative.
*/
constexpr StringRef drop_prefix(const int64_t n) const
{
BLI_assert(n >= 0);
- BLI_assert(n <= size_);
- return StringRef(data_ + n, size_ - n);
+ const int64_t clamped_n = std::min(n, size_);
+ const int64_t new_size = size_ - clamped_n;
+ return StringRef(data_ + clamped_n, new_size);
}
/**
* Return a new StringRef with the given prefix being skipped. This invokes undefined behavior if
* the string does not begin with the given prefix.
*/
- constexpr StringRef drop_prefix(StringRef prefix) const
+ constexpr StringRef drop_known_prefix(StringRef prefix) const
{
BLI_assert(this->startswith(prefix));
return this->drop_prefix(prefix.size());
}
/**
- * Return a new StringRef that does not contain the last n chars.
- *
- * This is similar to std::string_view::remove_suffix.
+ * Return a new StringRef that does not contain the last n chars. This invokes undefined behavior
+ * when n is negative.
*/
constexpr StringRef drop_suffix(const int64_t n) const
{
BLI_assert(n >= 0);
- BLI_assert(n <= size_);
- return StringRef(data_, size_ - n);
+ const int64_t new_size = std::max<int64_t>(0, size_ - n);
+ return StringRef(data_, new_size);
}
/**
@@ -460,7 +459,8 @@ constexpr inline bool StringRefBase::endswith(StringRef suffix) const
}
/**
- * Return a new #StringRef containing only a sub-string of the original string.
+ * Return a new #StringRef containing only a sub-string of the original string. This invokes
+ * undefined if the start or max_size is negative.
*/
constexpr inline StringRef StringRefBase::substr(const int64_t start,
const int64_t max_size = INT64_MAX) const
diff --git a/source/blender/blenlib/tests/BLI_span_test.cc b/source/blender/blenlib/tests/BLI_span_test.cc
index d1c9f312b97..002c97b0c7d 100644
--- a/source/blender/blenlib/tests/BLI_span_test.cc
+++ b/source/blender/blenlib/tests/BLI_span_test.cc
@@ -62,6 +62,15 @@ TEST(span, DropFront)
EXPECT_EQ(slice[2], 7);
}
+TEST(span, DropFrontLargeN)
+{
+ Vector<int> a = {1, 2, 3, 4, 5};
+ Span<int> slice1 = Span<int>(a).drop_front(100);
+ MutableSpan<int> slice2 = MutableSpan<int>(a).drop_front(100);
+ EXPECT_TRUE(slice1.is_empty());
+ EXPECT_TRUE(slice2.is_empty());
+}
+
TEST(span, DropFrontAll)
{
Vector<int> a = {4, 5, 6, 7};
@@ -78,6 +87,15 @@ TEST(span, TakeFront)
EXPECT_EQ(slice[1], 5);
}
+TEST(span, TakeFrontLargeN)
+{
+ Vector<int> a = {4, 5, 6, 7};
+ Span<int> slice1 = Span<int>(a).take_front(100);
+ MutableSpan<int> slice2 = MutableSpan<int>(a).take_front(100);
+ EXPECT_EQ(slice1.size(), 4);
+ EXPECT_EQ(slice2.size(), 4);
+}
+
TEST(span, TakeBack)
{
Vector<int> a = {5, 6, 7, 8};
@@ -87,6 +105,15 @@ TEST(span, TakeBack)
EXPECT_EQ(slice[1], 8);
}
+TEST(span, TakeBackLargeN)
+{
+ Vector<int> a = {3, 4, 5, 6};
+ Span<int> slice1 = Span<int>(a).take_back(100);
+ MutableSpan<int> slice2 = MutableSpan<int>(a).take_back(100);
+ EXPECT_EQ(slice1.size(), 4);
+ EXPECT_EQ(slice2.size(), 4);
+}
+
TEST(span, Slice)
{
Vector<int> a = {4, 5, 6, 7};
@@ -112,6 +139,19 @@ TEST(span, SliceRange)
EXPECT_EQ(slice[1], 4);
}
+TEST(span, SliceLargeN)
+{
+ Vector<int> a = {1, 2, 3, 4, 5};
+ Span<int> slice1 = Span<int>(a).slice(3, 100);
+ MutableSpan<int> slice2 = MutableSpan<int>(a).slice(3, 100);
+ EXPECT_EQ(slice1.size(), 2);
+ EXPECT_EQ(slice2.size(), 2);
+ EXPECT_EQ(slice1[0], 4);
+ EXPECT_EQ(slice2[0], 4);
+ EXPECT_EQ(slice1[1], 5);
+ EXPECT_EQ(slice2[1], 5);
+}
+
TEST(span, Contains)
{
Vector<int> a = {4, 5, 6, 7};
@@ -337,7 +377,7 @@ TEST(span, MutableReverseIterator)
EXPECT_EQ_ARRAY(src.data(), Span({14, 15, 16, 17}).data(), 4);
}
-TEST(span, constexpr_)
+TEST(span, Constexpr)
{
static constexpr std::array<int, 3> src = {3,
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list