You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "fgerlits (via GitHub)" <gi...@apache.org> on 2023/06/07 11:58:08 UTC

[GitHub] [nifi-minifi-cpp] fgerlits commented on a diff in pull request #1584: MINIFICPP-1755 - Use std::span instead of gsl::span

fgerlits commented on code in PR #1584:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1584#discussion_r1221440952


##########
libminifi/src/utils/LineByLineInputOutputStreamCallback.cpp:
##########
@@ -67,7 +67,7 @@ void LineByLineInputOutputStreamCallback::readLine() {
   if (end_of_line != input_.end()) { ++end_of_line; }
 
   current_line_ = next_line_;
-  next_line_ = utils::span_to<std::string>(gsl::make_span(&*current_pos_, &*end_of_line).as_span<char>());
+  next_line_ = utils::span_to<std::string>(utils::as_span<char>(std::span(&*current_pos_, &*end_of_line)));

Review Comment:
   I think we could remove the `&*`s:
   ```suggestion
     next_line_ = utils::span_to<std::string>(utils::as_span<char>(std::span(current_pos_, end_of_line)));
   ```
   as there is a `span` constructor which takes a pair of iterators



##########
libminifi/include/utils/gsl.h:
##########
@@ -32,18 +33,23 @@ using remove_cvref_t = typename std::remove_cv<typename std::remove_reference<T>
 }  // namespace detail
 
 template<typename Container, typename T>
-Container span_to(gsl::span<T> span) {
+Container span_to(std::span<T> span) {
   static_assert(std::is_constructible<Container, typename gsl::span<T>::iterator, typename gsl::span<T>::iterator>::value,
       "The destination container must have an iterator (pointer) range constructor");
   return Container(std::begin(span), std::end(span));
 }
 template<template<typename...> class Container, typename T>
-Container<detail::remove_cvref_t<T>> span_to(gsl::span<T> span) {
+Container<detail::remove_cvref_t<T>> span_to(std::span<T> span) {
   static_assert(std::is_constructible<Container<detail::remove_cvref_t<T>>, typename gsl::span<T>::iterator, typename gsl::span<T>::iterator>::value,
       "The destination container must have an iterator (pointer) range constructor");
   return span_to<Container<detail::remove_cvref_t<T>>>(span);
 }
 
+template<typename T, typename U>
+std::span<T> as_span(std::span<U> value) {
+  return std::span{reinterpret_cast<T*>(value.data()), value.size_bytes() / sizeof(T)};
+}

Review Comment:
   the `span_to` and `as_span` functions could be moved to a new file, eg. `utils/span.h`, as they don't belong in `gsl.h` any longer



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org