Merge bitcoin#19387: span: update constructors to match c++20 draft spec and add lifetimebound attribute

MarcoFalke · MarcoFalke · commit 5c0aebfcd42a · 2020-11-25T15:18:33.000+01:00
e3e7446 Add lifetimebound to attributes for general-purpose usage (Cory Fields) 1d58cc7 span: add lifetimebound attribute (Cory Fields) 62733fe span: (almost) match std::span's constructor behavior (Cory Fields) Pull request description: Replaces bitcoin#19382 with a different approach. See [this comment](bitcoin#19382 (comment)) for the reasoning behind the switch. -- Description from bitcoin#19382: See [here](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0936r0.pdf) for more detail on lifetimebound. This is implemented using preprocesor macros rather than configure checks in order to keep span.h self-contained. The ```[[clang::lifetimebound]]``` syntax was chosen over ```__attribute__((lifetimebound))``` because the former is more flexible and works to guard ```this``` as well as function parameters, and also because at least for now, it's available only in clang. There are currently no violations in our codebase, but this can easily be tested by inserting one like this somewhere and compiling with a modern clang: ```c++ Span<const int> bad(std::vector<int>{1,2,3}); ``` The result: > warning: temporary whose address is used as value of local variable 'bad' will be destroyed at the end of the full-expression [-Wdangling] Span<const int> bad(std::vector<int>{1,2,3}); ``` ACKs for top commit: sipa: ACK e3e7446 ajtowns: ACK e3e7446 (drive by; only a quick skim of code and some basic sanity checks) MarcoFalke: review ACK e3e7446 🔗 jonatack: ACK e3e7446 change since last review is adding `[[clang::lifetimebound]]` as `LIFETIMEBOUND` to src/attributes.h as suggested in bitcoin#19387 (comment). Tree-SHA512: 05a3440ee595ef0e8d693a2820b360707695c016a68e15df47c20cd8d053646cc6c8cca8addd7db40e72b3fce208879a41c8102ba7ae9223e4366e5de1175211
diff --git a/src/attributes.h b/src/attributes.h
@@ -19,4 +19,14 @@
 #  endif
 #endif
 
+#if defined(__clang__)
+#  if __has_attribute(lifetimebound)
+#    define LIFETIMEBOUND [[clang::lifetimebound]]
+#  else
+#    define LIFETIMEBOUND
+#  endif
+#else
+#  define LIFETIMEBOUND
+#endif
+
 #endif // BITCOIN_ATTRIBUTES_H
diff --git a/src/span.h b/src/span.h
@@ -18,6 +18,16 @@
 #define ASSERT_IF_DEBUG(x)
 #endif
 
+#if defined(__clang__)
+#if __has_attribute(lifetimebound)
+#define SPAN_ATTR_LIFETIMEBOUND [[clang::lifetimebound]]
+#else
+#define SPAN_ATTR_LIFETIMEBOUND
+#endif
+#else
+#define SPAN_ATTR_LIFETIMEBOUND
+#endif
+
 /** A Span is an object that can refer to a contiguous sequence of objects.
  *
  * It implements a subset of C++20's std::span.
@@ -84,6 +94,14 @@ class Span
     C* m_data;
     std::size_t m_size;
 
+    template <class T>
+    struct is_Span_int : public std::false_type {};
+    template <class T>
+    struct is_Span_int<Span<T>> : public std::true_type {};
+    template <class T>
+    struct is_Span : public is_Span_int<typename std::remove_cv<T>::type>{};
+
+
 public:
     constexpr Span() noexcept : m_data(nullptr), m_size(0) {}
 
@@ -134,8 +152,19 @@ class Span
      * To prevent surprises, only Spans for constant value types are supported when passing in temporaries.
      * Note that this restriction does not exist when converting arrays or other Spans (see above).
      */
-    template <typename V, typename std::enable_if<(std::is_const<C>::value || std::is_lvalue_reference<V>::value) && std::is_convertible<typename std::remove_pointer<decltype(std::declval<V&>().data())>::type (*)[], C (*)[]>::value && std::is_convertible<decltype(std::declval<V&>().size()), std::size_t>::value, int>::type = 0>
-    constexpr Span(V&& v) noexcept : m_data(v.data()), m_size(v.size()) {}
+    template <typename V>
+    constexpr Span(V& other SPAN_ATTR_LIFETIMEBOUND,
+        typename std::enable_if<!is_Span<V>::value &&
+                                std::is_convertible<typename std::remove_pointer<decltype(std::declval<V&>().data())>::type (*)[], C (*)[]>::value &&
+                                std::is_convertible<decltype(std::declval<V&>().size()), std::size_t>::value, std::nullptr_t>::type = nullptr)
+        : m_data(other.data()), m_size(other.size()){}
+
+    template <typename V>
+    constexpr Span(const V& other SPAN_ATTR_LIFETIMEBOUND,
+        typename std::enable_if<!is_Span<V>::value &&
+                                std::is_convertible<typename std::remove_pointer<decltype(std::declval<const V&>().data())>::type (*)[], C (*)[]>::value &&
+                                std::is_convertible<decltype(std::declval<const V&>().size()), std::size_t>::value, std::nullptr_t>::type = nullptr)
+        : m_data(other.data()), m_size(other.size()){}
 
     constexpr C* data() const noexcept { return m_data; }
     constexpr C* begin() const noexcept { return m_data; }
@@ -192,9 +221,9 @@ class Span
 /** MakeSpan for arrays: */
 template <typename A, int N> Span<A> constexpr MakeSpan(A (&a)[N]) { return Span<A>(a, N); }
 /** MakeSpan for temporaries / rvalue references, only supporting const output. */
-template <typename V> constexpr auto MakeSpan(V&& v) -> typename std::enable_if<!std::is_lvalue_reference<V>::value, Span<const typename std::remove_pointer<decltype(v.data())>::type>>::type { return std::forward<V>(v); }
+template <typename V> constexpr auto MakeSpan(V&& v SPAN_ATTR_LIFETIMEBOUND) -> typename std::enable_if<!std::is_lvalue_reference<V>::value, Span<const typename std::remove_pointer<decltype(v.data())>::type>>::type { return std::forward<V>(v); }
 /** MakeSpan for (lvalue) references, supporting mutable output. */
-template <typename V> constexpr auto MakeSpan(V& v) -> Span<typename std::remove_pointer<decltype(v.data())>::type> { return v; }
+template <typename V> constexpr auto MakeSpan(V& v SPAN_ATTR_LIFETIMEBOUND) -> Span<typename std::remove_pointer<decltype(v.data())>::type> { return v; }
 
 /** Pop the last element off a span, and return a reference to that element. */
 template <typename T>
