Add support for span<T> as a sequence view-type #1969
Conversation
| @@ -81,7 +81,7 @@ namespace Ice | |||
| * You can supply a communicator later by calling initialize(). | |||
| * @param bytes The encoded data. | |||
| */ | |||
| InputStream(const std::pair<const std::byte*, const std::byte*>& bytes); | |||
| InputStream(std::pair<const std::byte*, const std::byte*> bytes); | |||
There was a problem hiding this comment.
We now pass pairs of pointers by value instead of const&, just like other "view types".
| @@ -9,7 +9,6 @@ | |||
| #include "ProxyF.h" | |||
| #include "ValueF.h" | |||
| #include "Exception.h" | |||
| #include "StreamHelpers.h" | |||
There was a problem hiding this comment.
Unrelated cleanup - should have been included in a separate PR.
| @@ -13,6 +13,10 @@ | |||
| #include <optional> | |||
| #include <string_view> | |||
|
|
|||
| #if __has_include(<span>) | |||
There was a problem hiding this comment.
That's the preferred and portable way to conditionally include span. Span is a c++20 and up feature.
| * Helper for span (C++20 or later). | ||
| * \headerfile Ice/Ice.h | ||
| */ | ||
| template<typename T> struct StreamHelper<std::span<T>, StreamHelperCategorySequence> |
There was a problem hiding this comment.
That's the actual core for this PR - not much. The bulk of the additional code is the new span test.
| @@ -94,6 +94,51 @@ namespace | |||
| } | |||
| } | |||
|
|
|||
| // Do we pass this type by value when it's an input parameter? | |||
| bool inputParamByValue(const TypePtr& type, const StringList& metaData) | |||
There was a problem hiding this comment.
The idea is to pass "small" inexpensive param by value, including string_view, pairs of pointers, and spans.
shared_ptr are passed by const& to avoid incrementing and then decrementing the refcount atomically.
There was a problem hiding this comment.
Sounds fine, assuming the most common is to use the shared_ptr and not store it.
| # Copyright (c) ZeroC, Inc. All rights reserved. | ||
| # | ||
|
|
||
| $(test)_cppflags += -std=c++20 |
There was a problem hiding this comment.
The last -std= wins.
| @@ -94,6 +94,51 @@ namespace | |||
| } | |||
| } | |||
|
|
|||
| // Do we pass this type by value when it's an input parameter? | |||
| bool inputParamByValue(const TypePtr& type, const StringList& metaData) | |||
There was a problem hiding this comment.
Sounds fine, assuming the most common is to use the shared_ptr and not store it.
This PR adds support for std::span as a sequence view-type in Ice C++.
It also updates the mapping for view-type from const T& to T, i.e. view types are always passed by value (not const reference). And for consistency, "array" parameters are now also passed by value, e.g. we now transmit a byte array as
pair<byte, byte>instead ofconst pair<byte, byte>&.See https://doc.zeroc.com/ice/3.7/language-mappings/c++11-mapping/customizing-the-c++11-mapping/the-cpp-type-and-cpp-view-type-metadata-directives-with-c++11#id-.Thecpp:typeandcpp:viewtypeMetadataDirectiveswithC++11v3.7-Avoidingcopieswithcpp:view-type
Note that in Ice 3.8, view-type no longer applies when unmarshaling, while array still does.