Std span (#1038)

nbradac · mspdt22 · web-flow · commit ec42a79fab4e · 2024-12-16T21:09:02.000+01:00
* [C++] Integrate std::span support for flyweight API

The impetus was a bug that we ran into when writing a string-literal to a fixed-width char field:

```c++
flyweight.putFixedChar("hello");
```

This is unsafe:
- If the field size is less than 6, we overrun the buffer and corrupt it.
- If the field size is more than 6, we don't zero pad the rest of it.

Instead, we build on support for the std::string_view getters and setters, which do length checking.
std::span generalizes this to fixed-width fields of all types. Notably, if the size of the std::span
is knowable at compile time, we pay no runtime cost for the length checking, and we should get
similar performance to the existing API which takes a raw pointer.

Further, we add a sbetool option to disable accepting arrays by raw pointer, which should prevent
memcpy operation without bounds checking. This is off by default to avoid a breaking change.

* [C++] hide USE_SPAN behind ENABLE_SPAN

* [C++] add macro guards

---------

Co-authored-by: Matt Stern &lt;stern@pdtpartners.com&gt;
diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java
@@ -49,6 +49,8 @@ public class CppGenerator implements CodeGenerator
 {
     private static final boolean DISABLE_IMPLICIT_COPYING = Boolean.parseBoolean(
         System.getProperty("sbe.cpp.disable.implicit.copying", "false"));
+    private static final boolean DISABLE_RAW_ARRAYS = Boolean.parseBoolean(
+        System.getProperty("sbe.cpp.disable.raw.arrays", "false"));
     private static final String BASE_INDENT = "";
     private static final String INDENT = "    ";
     private static final String TWO_INDENT = INDENT + INDENT;
@@ -1634,6 +1636,23 @@ private static CharSequence generateArrayFieldNotPresentCondition(final int sinc
             sinceVersion);
     }
 
+    private static CharSequence generateSpanFieldNotPresentCondition(
+        final int sinceVersion, final String indent, final String cppTypeName)
+    {
+        if (0 == sinceVersion)
+        {
+            return "";
+        }
+
+        return String.format(
+            indent + "        if (m_actingVersion < %1$d)\n" +
+            indent + "        {\n" +
+            indent + "            return std::span<const %2$s>();\n" +
+            indent + "        }\n\n",
+            sinceVersion,
+            cppTypeName);
+    }
+
     private static CharSequence generateStringNotPresentCondition(final int sinceVersion, final String indent)
     {
         if (0 == sinceVersion)
@@ -1703,10 +1722,20 @@ private CharSequence generateFileHeader(
             "#if __cplusplus >= 201703L\n" +
             "#  include <string_view>\n" +
             "#  define SBE_NODISCARD [[nodiscard]]\n" +
+            "#  if !defined(SBE_USE_STRING_VIEW)\n" +
+            "#    define SBE_USE_STRING_VIEW 1\n" +
+            "#  endif\n" +
             "#else\n" +
             "#  define SBE_NODISCARD\n" +
             "#endif\n\n" +
 
+            "#if __cplusplus >= 202002L\n" +
+            "#  include <span>\n" +
+            "#  if !defined(SBE_USE_SPAN)\n" +
+            "#    define SBE_USE_SPAN 1\n" +
+            "#  endif\n" +
+            "#endif\n\n" +
+
             "#if !defined(__STDC_LIMIT_MACROS)\n" +
             "#  define __STDC_LIMIT_MACROS 1\n" +
             "#endif\n\n" +
@@ -2258,12 +2287,38 @@ private void generateArrayProperty(
             accessOrderListenerCall);
 
         new Formatter(sb).format("\n" +
-            indent + "    %1$s &put%2$s(const char *const src)%7$s\n" +
+            indent + "    #ifdef SBE_USE_SPAN\n" +
+            indent + "    SBE_NODISCARD std::span<const %5$s> get%1$sAsSpan() const%7$s\n" +
+            indent + "    {\n" +
+            "%3$s" +
+            "%6$s" +
+            indent + "        const char *buffer = m_buffer + m_offset + %4$d;\n" +
+            indent + "        return std::span<const %5$s>(reinterpret_cast<const %5$s*>(buffer), %2$d);\n" +
+            indent + "    }\n" +
+            indent + "    #endif\n",
+            toUpperFirstChar(propertyName),
+            arrayLength,
+            generateSpanFieldNotPresentCondition(propertyToken.version(), indent, cppTypeName),
+            offset,
+            cppTypeName,
+            accessOrderListenerCall,
+            noexceptDeclaration);
+
+        new Formatter(sb).format("\n" +
+            indent + "    #ifdef SBE_USE_SPAN\n" +
+            indent + "    template <std::size_t N>\n" +
+            indent + "    %1$s &put%2$s(std::span<const %4$s, N> src)%7$s\n" +
             indent + "    {\n" +
+            indent + "        static_assert(N <= %5$d, \"array too large for put%2$s\");\n\n" +
             "%6$s" +
-            indent + "        std::memcpy(m_buffer + m_offset + %3$d, src, sizeof(%4$s) * %5$d);\n" +
+            indent + "        std::memcpy(m_buffer + m_offset + %3$d, src.data(), sizeof(%4$s) * N);\n" +
+            indent + "        for (std::size_t start = N; start < %5$d; ++start)\n" +
+            indent + "        {\n" +
+            indent + "            m_buffer[m_offset + %3$d + start] = 0;\n" +
+            indent + "        }\n\n" +
             indent + "        return *this;\n" +
-            indent + "    }\n",
+            indent + "    }\n" +
+            indent + "    #endif\n",
             containingClassName,
             toUpperFirstChar(propertyName),
             offset,
@@ -2272,6 +2327,79 @@ private void generateArrayProperty(
             accessOrderListenerCall,
             noexceptDeclaration);
 
+        if (encodingToken.encoding().primitiveType() != PrimitiveType.CHAR)
+        {
+            new Formatter(sb).format("\n" +
+                indent + "    #ifdef SBE_USE_SPAN\n" +
+                indent + "    %1$s &put%2$s(std::span<const %4$s> src)\n" +
+                indent + "    {\n" +
+                indent + "        const std::size_t srcLength = src.size();\n" +
+                indent + "        if (srcLength > %5$d)\n" +
+                indent + "        {\n" +
+                indent + "            throw std::runtime_error(\"array too large for put%2$s [E106]\");\n" +
+                indent + "        }\n\n" +
+
+                "%6$s" +
+                indent + "        std::memcpy(m_buffer + m_offset + %3$d, src.data(), sizeof(%4$s) * srcLength);\n" +
+                indent + "        for (std::size_t start = srcLength; start < %5$d; ++start)\n" +
+                indent + "        {\n" +
+                indent + "            m_buffer[m_offset + %3$d + start] = 0;\n" +
+                indent + "        }\n\n" +
+                indent + "        return *this;\n" +
+                indent + "    }\n" +
+                indent + "    #endif\n",
+                containingClassName,
+                toUpperFirstChar(propertyName),
+                offset,
+                cppTypeName,
+                arrayLength,
+                accessOrderListenerCall);
+        }
+
+        if (!DISABLE_RAW_ARRAYS)
+        {
+            new Formatter(sb).format("\n" +
+                indent + "    #ifdef SBE_USE_SPAN\n" +
+                indent + "    template <typename T>\n" +
+                // If std::span is available, redirect string literals to the std::span-accepting overload,
+                // where we can do compile-time bounds checking.
+                indent + "    %1$s &put%2$s(T&& src) %7$s requires\n" +
+                indent + "        (std::is_pointer_v<std::remove_reference_t<T>> &&\n" +
+                indent + "         !std::is_array_v<std::remove_reference_t<T>>)\n" +
+                indent + "    #else\n" +
+                indent + "    %1$s &put%2$s(const char *const src)%7$s\n" +
+                indent + "    #endif\n" +
+                indent + "    {\n" +
+                "%6$s" +
+                indent + "        std::memcpy(m_buffer + m_offset + %3$d, src, sizeof(%4$s) * %5$d);\n" +
+                indent + "        return *this;\n" +
+                indent + "    }\n",
+                containingClassName,
+                toUpperFirstChar(propertyName),
+                offset,
+                cppTypeName,
+                arrayLength,
+                accessOrderListenerCall,
+                noexceptDeclaration);
+
+        }
+        if (encodingToken.encoding().primitiveType() == PrimitiveType.CHAR)
+        {
+            // Resolve ambiguity of string literal arguments, which match both
+            // std::span<const char, N> and std::string_view overloads.
+            new Formatter(sb).format("\n" +
+                indent + "    #ifdef SBE_USE_SPAN\n" +
+                indent + "    template <std::size_t N>\n" +
+                indent + "    %1$s &put%2$s(const char (&src)[N])%3$s\n" +
+                indent + "    {\n" +
+                indent + "        return put%2$s(std::span<const char, N>(src));\n" +
+                indent + "    }\n" +
+                indent + "    #endif\n",
+                containingClassName,
+                toUpperFirstChar(propertyName),
+                noexceptDeclaration);
+        }
+
         if (arrayLength > 1 && arrayLength <= 4)
         {
             sb.append("\n").append(indent).append("    ")
@@ -2332,7 +2460,7 @@ private void generateArrayProperty(
             generateJsonEscapedStringGetter(sb, encodingToken, indent, propertyName);
 
             new Formatter(sb).format("\n" +
-                indent + "    #if __cplusplus >= 201703L\n" +
+                indent + "    #ifdef SBE_USE_STRING_VIEW\n" +
                 indent + "    SBE_NODISCARD std::string_view get%1$sAsStringView() const%6$s\n" +
                 indent + "    {\n" +
                 "%4$s" +
@@ -2354,7 +2482,7 @@ private void generateArrayProperty(
                 noexceptDeclaration);
 
             new Formatter(sb).format("\n" +
-                indent + "    #if __cplusplus >= 201703L\n" +
+                indent + "    #ifdef SBE_USE_STRING_VIEW\n" +
                 indent + "    %1$s &put%2$s(const std::string_view str)\n" +
                 indent + "    {\n" +
                 indent + "        const std::size_t srcLength = str.length();\n" +
diff --git a/sbe-tool/src/test/cpp/CMakeLists.txt b/sbe-tool/src/test/cpp/CMakeLists.txt
@@ -103,6 +103,11 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
         sbe_test(DtoTest codecs)
         target_compile_features(DtoTest PRIVATE cxx_std_17)
     endif()
+
+    # Check if the GCC version supports C++20
+    if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "11.0")
+        target_compile_features(CodeGenTest PRIVATE cxx_std_20)
+    endif()
 endif()
 
 if (CMAKE_CXX_COMPILER_ID STREQUAL "CLang")
@@ -111,12 +116,18 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "CLang")
         sbe_test(DtoTest codecs)
         target_compile_features(DtoTest PRIVATE cxx_std_17)
     endif()
+
+    # Check if CLang version supports C++20
+    if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "10.0")
+        target_compile_features(CodeGenTest PRIVATE cxx_std_20)
+    endif()
 endif()
 
 if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
-    # Check if MSVC version supports C++17
+    # Check if MSVC version supports C++17 / C++20
     if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "19.14")
         sbe_test(DtoTest codecs)
         target_compile_options(DtoTest PRIVATE /std:c++17)
+        target_compile_options(CodeGenTest PRIVATE /std:c++20)
     endif()
 endif()
diff --git a/sbe-tool/src/test/cpp/CodeGenTest.cpp b/sbe-tool/src/test/cpp/CodeGenTest.cpp
@@ -1083,3 +1083,65 @@ TEST_F(CodeGenTest, shouldAllowForMultipleIterations)
     std::string passFive = walkCar(carDecoder);
     EXPECT_EQ(passOne, passFive);
 }
+
+#ifdef SBE_USE_STRING_VIEW
+TEST_F(CodeGenTest, shouldBeAbleToUseStdStringViewMethods)
+{
+    std::string vehicleCode(VEHICLE_CODE, Car::vehicleCodeLength());
+
+    char buffer[BUFFER_LEN] = {};
+    std::uint64_t baseOffset = MessageHeader::encodedLength();
+    Car car;
+    car.wrapForEncode(buffer, baseOffset, sizeof(buffer));
+    car.putVehicleCode(std::string_view(vehicleCode));
+
+    car.sbeRewind();
+    EXPECT_EQ(car.getVehicleCodeAsStringView(), vehicleCode);
+}
+#endif
+
+#ifdef SBE_USE_SPAN
+TEST_F(CodeGenTest, shouldBeAbleToUseStdSpanViewMethods)
+{
+    char buffer[BUFFER_LEN] = {};
+
+    std::uint64_t baseOffset = MessageHeader::encodedLength();
+    Car car;
+    car.wrapForEncode(buffer, baseOffset, sizeof(buffer));
+
+    {
+        std::array<std::int32_t, Car::someNumbersLength()> numbers;
+        for (std::uint64_t i = 0; i < Car::someNumbersLength(); i++)
+        {
+            numbers[i] = static_cast<std::int32_t>(i);
+        }
+        car.putSomeNumbers(numbers);
+    }
+
+    car.sbeRewind();
+
+    {
+        std::span<const std::int32_t> numbers = car.getSomeNumbersAsSpan();
+        ASSERT_EQ(numbers.size(), Car::someNumbersLength());
+        for (std::uint64_t i = 0; i < numbers.size(); i++)
+        {
+            EXPECT_EQ(numbers[i], static_cast<std::int32_t>(i));
+        }
+    }
+}
+#endif
+
+#ifdef SBE_USE_SPAN
+TEST_F(CodeGenTest, shouldBeAbleToResolveStringLiterals)
+{
+    char buffer[BUFFER_LEN] = {};
+
+    std::uint64_t baseOffset = MessageHeader::encodedLength();
+    Car car;
+    car.wrapForEncode(buffer, baseOffset, sizeof(buffer));
+    car.putVehicleCode("ABCDE");
+
+    car.sbeRewind();
+    EXPECT_EQ(car.getVehicleCodeAsStringView(), "ABCDE");
+}
+#endif
