From 4463087223983c40a6d67beab0513fba7cdb7538 Mon Sep 17 00:00:00 2001
From: Jacques Lucke <jacques@blender.org>
Date: Wed, 16 Dec 2020 15:59:58 +0100
Subject: BLI: remove implicit casts between some span types

Casting pointers from one type to another does change the
value of the pointer in some cases. Therefore, casting a span
that contains pointers of one type to a span that contains
pointers of another type, is not generally safe. In practice, this
issue mainly comes up when dealing with classes that have a
vtable.

There are some special cases that are still allowed. For example,
adding const to the pointer does not change the address.
Also, casting to a void pointer is fine.

In cases where implicit conversion is disabled, but one is sure
that the cast is valid, an explicit call of `span.cast<NewType>()`
can be used.
---
 source/blender/blenlib/BLI_array.hh                   |  4 ++--
 source/blender/blenlib/BLI_memory_utils.hh            | 19 +++++++++++++++++++
 source/blender/blenlib/BLI_span.hh                    |  5 +++--
 source/blender/blenlib/BLI_vector.hh                  |  4 ++--
 source/blender/blenlib/tests/BLI_memory_utils_test.cc | 11 +++++++++++
 5 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/source/blender/blenlib/BLI_array.hh b/source/blender/blenlib/BLI_array.hh
index 8a7dcb7ffaa..284d62fb876 100644
--- a/source/blender/blenlib/BLI_array.hh
+++ b/source/blender/blenlib/BLI_array.hh
@@ -220,13 +220,13 @@ class Array {
     return MutableSpan<T>(data_, size_);
   }
 
-  template<typename U, typename std::enable_if_t<is_convertible_pointer_v<T, U>> * = nullptr>
+  template<typename U, typename std::enable_if_t<is_span_convertible_pointer_v<T, U>> * = nullptr>
   operator Span<U>() const
   {
     return Span<U>(data_, size_);
   }
 
-  template<typename U, typename std::enable_if_t<is_convertible_pointer_v<T, U>> * = nullptr>
+  template<typename U, typename std::enable_if_t<is_span_convertible_pointer_v<T, U>> * = nullptr>
   operator MutableSpan<U>()
   {
     return MutableSpan<U>(data_, size_);
diff --git a/source/blender/blenlib/BLI_memory_utils.hh b/source/blender/blenlib/BLI_memory_utils.hh
index 49076bb1aae..b3b6855089e 100644
--- a/source/blender/blenlib/BLI_memory_utils.hh
+++ b/source/blender/blenlib/BLI_memory_utils.hh
@@ -427,6 +427,25 @@ template<typename From, typename To>
 inline constexpr bool is_convertible_pointer_v =
     std::is_convertible_v<From, To> &&std::is_pointer_v<From> &&std::is_pointer_v<To>;
 
+/**
+ * Helper variable that checks if a Span<From> can be converted to Span<To> safely, whereby From
+ * and To are pointers. Adding const and casting to a void pointer is allowed.
+ * Casting up and down a class hierarchy generally is not allowed, because this might change the
+ * pointer under some circumstances.
+ */
+template<typename From, typename To>
+inline constexpr bool is_span_convertible_pointer_v =
+    /* Make sure we are working with pointers. */
+    std::is_pointer_v<From> &&std::is_pointer_v<To> &&
+    (/* No casting is necessary when both types are the same. */
+     std::is_same_v<From, To> ||
+     /* Allow adding const to the underlying type. */
+     std::is_same_v<const std::remove_pointer_t<From>, std::remove_pointer_t<To>> ||
+     /* Allow casting non-const pointers to void pointers. */
+     (!std::is_const_v<std::remove_pointer_t<From>> && std::is_same_v<To, void *>) ||
+     /* Allow casting any pointer to const void pointers. */
+     std::is_same_v<To, const void *>);
+
 /**
  * Inline buffers for small-object-optimization should be disable by default. Otherwise we might
  * get large unexpected allocations on the stack.
diff --git a/source/blender/blenlib/BLI_span.hh b/source/blender/blenlib/BLI_span.hh
index 3f410efe908..8011b2f9abc 100644
--- a/source/blender/blenlib/BLI_span.hh
+++ b/source/blender/blenlib/BLI_span.hh
@@ -100,7 +100,7 @@ template<typename T> class Span {
     BLI_assert(size >= 0);
   }
 
-  template<typename U, typename std::enable_if_t<is_convertible_pointer_v<U, T>> * = nullptr>
+  template<typename U, typename std::enable_if_t<is_span_convertible_pointer_v<U, T>> * = nullptr>
   constexpr Span(const U *start, int64_t size) : data_(static_cast<const T *>(start)), size_(size)
   {
     BLI_assert(size >= 0);
@@ -135,7 +135,8 @@ template<typename T> class Span {
    * Support implicit conversions like the ones below:
    *   Span<T *> -> Span<const T *>
    */
-  template<typename U, typename std::enable_if_t<is_convertible_pointer_v<U, T>> * = nullptr>
+
+  template<typename U, typename std::enable_if_t<is_span_convertible_pointer_v<U, T>> * = nullptr>
   constexpr Span(Span<U> array) : data_(static_cast<const T *>(array.data())), size_(array.size())
   {
   }
diff --git a/source/blender/blenlib/BLI_vector.hh b/source/blender/blenlib/BLI_vector.hh
index 053dcb6faea..fe6d54ae9e5 100644
--- a/source/blender/blenlib/BLI_vector.hh
+++ b/source/blender/blenlib/BLI_vector.hh
@@ -315,13 +315,13 @@ class Vector {
     return MutableSpan<T>(begin_, this->size());
   }
 
-  template<typename U, typename std::enable_if_t<is_convertible_pointer_v<T, U>> * = nullptr>
+  template<typename U, typename std::enable_if_t<is_span_convertible_pointer_v<T, U>> * = nullptr>
   operator Span<U>() const
   {
     return Span<U>(begin_, this->size());
   }
 
-  template<typename U, typename std::enable_if_t<is_convertible_pointer_v<T, U>> * = nullptr>
+  template<typename U, typename std::enable_if_t<is_span_convertible_pointer_v<T, U>> * = nullptr>
   operator MutableSpan<U>()
   {
     return MutableSpan<U>(begin_, this->size());
diff --git a/source/blender/blenlib/tests/BLI_memory_utils_test.cc b/source/blender/blenlib/tests/BLI_memory_utils_test.cc
index fcef2f8688a..23415e69b04 100644
--- a/source/blender/blenlib/tests/BLI_memory_utils_test.cc
+++ b/source/blender/blenlib/tests/BLI_memory_utils_test.cc
@@ -158,4 +158,15 @@ static_assert(is_convertible_pointer_v<int **, int **const>);
 static_assert(is_convertible_pointer_v<int **, int *const *>);
 static_assert(is_convertible_pointer_v<int **, int const *const *>);
 
+static_assert(is_span_convertible_pointer_v<int *, int *>);
+static_assert(is_span_convertible_pointer_v<int *, const int *>);
+static_assert(!is_span_convertible_pointer_v<const int *, int *>);
+static_assert(is_span_convertible_pointer_v<const int *, const int *>);
+static_assert(is_span_convertible_pointer_v<const int *, const void *>);
+static_assert(!is_span_convertible_pointer_v<const int *, void *>);
+static_assert(is_span_convertible_pointer_v<int *, void *>);
+static_assert(is_span_convertible_pointer_v<int *, const void *>);
+static_assert(!is_span_convertible_pointer_v<TestBaseClass *, TestChildClass *>);
+static_assert(!is_span_convertible_pointer_v<TestChildClass *, TestBaseClass *>);
+
 }  // namespace blender::tests
-- 
cgit v1.2.3