Skip to content

Commit ec42a79

Browse files
nbradacmspdt22
nbradac
and
mspdt22
authored
Std span (#1038)
* [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 <[email protected]>
1 parent 1d6e3f1 commit ec42a79

File tree

3 files changed

+207
-6
lines changed

3 files changed

+207
-6
lines changed

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java

Lines changed: 133 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ public class CppGenerator implements CodeGenerator
4949
{
5050
private static final boolean DISABLE_IMPLICIT_COPYING = Boolean.parseBoolean(
5151
System.getProperty("sbe.cpp.disable.implicit.copying", "false"));
52+
private static final boolean DISABLE_RAW_ARRAYS = Boolean.parseBoolean(
53+
System.getProperty("sbe.cpp.disable.raw.arrays", "false"));
5254
private static final String BASE_INDENT = "";
5355
private static final String INDENT = " ";
5456
private static final String TWO_INDENT = INDENT + INDENT;
@@ -1634,6 +1636,23 @@ private static CharSequence generateArrayFieldNotPresentCondition(final int sinc
16341636
sinceVersion);
16351637
}
16361638

1639+
private static CharSequence generateSpanFieldNotPresentCondition(
1640+
final int sinceVersion, final String indent, final String cppTypeName)
1641+
{
1642+
if (0 == sinceVersion)
1643+
{
1644+
return "";
1645+
}
1646+
1647+
return String.format(
1648+
indent + " if (m_actingVersion < %1$d)\n" +
1649+
indent + " {\n" +
1650+
indent + " return std::span<const %2$s>();\n" +
1651+
indent + " }\n\n",
1652+
sinceVersion,
1653+
cppTypeName);
1654+
}
1655+
16371656
private static CharSequence generateStringNotPresentCondition(final int sinceVersion, final String indent)
16381657
{
16391658
if (0 == sinceVersion)
@@ -1703,10 +1722,20 @@ private CharSequence generateFileHeader(
17031722
"#if __cplusplus >= 201703L\n" +
17041723
"# include <string_view>\n" +
17051724
"# define SBE_NODISCARD [[nodiscard]]\n" +
1725+
"# if !defined(SBE_USE_STRING_VIEW)\n" +
1726+
"# define SBE_USE_STRING_VIEW 1\n" +
1727+
"# endif\n" +
17061728
"#else\n" +
17071729
"# define SBE_NODISCARD\n" +
17081730
"#endif\n\n" +
17091731

1732+
"#if __cplusplus >= 202002L\n" +
1733+
"# include <span>\n" +
1734+
"# if !defined(SBE_USE_SPAN)\n" +
1735+
"# define SBE_USE_SPAN 1\n" +
1736+
"# endif\n" +
1737+
"#endif\n\n" +
1738+
17101739
"#if !defined(__STDC_LIMIT_MACROS)\n" +
17111740
"# define __STDC_LIMIT_MACROS 1\n" +
17121741
"#endif\n\n" +
@@ -2258,12 +2287,38 @@ private void generateArrayProperty(
22582287
accessOrderListenerCall);
22592288

22602289
new Formatter(sb).format("\n" +
2261-
indent + " %1$s &put%2$s(const char *const src)%7$s\n" +
2290+
indent + " #ifdef SBE_USE_SPAN\n" +
2291+
indent + " SBE_NODISCARD std::span<const %5$s> get%1$sAsSpan() const%7$s\n" +
2292+
indent + " {\n" +
2293+
"%3$s" +
2294+
"%6$s" +
2295+
indent + " const char *buffer = m_buffer + m_offset + %4$d;\n" +
2296+
indent + " return std::span<const %5$s>(reinterpret_cast<const %5$s*>(buffer), %2$d);\n" +
2297+
indent + " }\n" +
2298+
indent + " #endif\n",
2299+
toUpperFirstChar(propertyName),
2300+
arrayLength,
2301+
generateSpanFieldNotPresentCondition(propertyToken.version(), indent, cppTypeName),
2302+
offset,
2303+
cppTypeName,
2304+
accessOrderListenerCall,
2305+
noexceptDeclaration);
2306+
2307+
new Formatter(sb).format("\n" +
2308+
indent + " #ifdef SBE_USE_SPAN\n" +
2309+
indent + " template <std::size_t N>\n" +
2310+
indent + " %1$s &put%2$s(std::span<const %4$s, N> src)%7$s\n" +
22622311
indent + " {\n" +
2312+
indent + " static_assert(N <= %5$d, \"array too large for put%2$s\");\n\n" +
22632313
"%6$s" +
2264-
indent + " std::memcpy(m_buffer + m_offset + %3$d, src, sizeof(%4$s) * %5$d);\n" +
2314+
indent + " std::memcpy(m_buffer + m_offset + %3$d, src.data(), sizeof(%4$s) * N);\n" +
2315+
indent + " for (std::size_t start = N; start < %5$d; ++start)\n" +
2316+
indent + " {\n" +
2317+
indent + " m_buffer[m_offset + %3$d + start] = 0;\n" +
2318+
indent + " }\n\n" +
22652319
indent + " return *this;\n" +
2266-
indent + " }\n",
2320+
indent + " }\n" +
2321+
indent + " #endif\n",
22672322
containingClassName,
22682323
toUpperFirstChar(propertyName),
22692324
offset,
@@ -2272,6 +2327,79 @@ private void generateArrayProperty(
22722327
accessOrderListenerCall,
22732328
noexceptDeclaration);
22742329

2330+
if (encodingToken.encoding().primitiveType() != PrimitiveType.CHAR)
2331+
{
2332+
new Formatter(sb).format("\n" +
2333+
indent + " #ifdef SBE_USE_SPAN\n" +
2334+
indent + " %1$s &put%2$s(std::span<const %4$s> src)\n" +
2335+
indent + " {\n" +
2336+
indent + " const std::size_t srcLength = src.size();\n" +
2337+
indent + " if (srcLength > %5$d)\n" +
2338+
indent + " {\n" +
2339+
indent + " throw std::runtime_error(\"array too large for put%2$s [E106]\");\n" +
2340+
indent + " }\n\n" +
2341+
2342+
"%6$s" +
2343+
indent + " std::memcpy(m_buffer + m_offset + %3$d, src.data(), sizeof(%4$s) * srcLength);\n" +
2344+
indent + " for (std::size_t start = srcLength; start < %5$d; ++start)\n" +
2345+
indent + " {\n" +
2346+
indent + " m_buffer[m_offset + %3$d + start] = 0;\n" +
2347+
indent + " }\n\n" +
2348+
indent + " return *this;\n" +
2349+
indent + " }\n" +
2350+
indent + " #endif\n",
2351+
containingClassName,
2352+
toUpperFirstChar(propertyName),
2353+
offset,
2354+
cppTypeName,
2355+
arrayLength,
2356+
accessOrderListenerCall);
2357+
}
2358+
2359+
if (!DISABLE_RAW_ARRAYS)
2360+
{
2361+
new Formatter(sb).format("\n" +
2362+
indent + " #ifdef SBE_USE_SPAN\n" +
2363+
indent + " template <typename T>\n" +
2364+
// If std::span is available, redirect string literals to the std::span-accepting overload,
2365+
// where we can do compile-time bounds checking.
2366+
indent + " %1$s &put%2$s(T&& src) %7$s requires\n" +
2367+
indent + " (std::is_pointer_v<std::remove_reference_t<T>> &&\n" +
2368+
indent + " !std::is_array_v<std::remove_reference_t<T>>)\n" +
2369+
indent + " #else\n" +
2370+
indent + " %1$s &put%2$s(const char *const src)%7$s\n" +
2371+
indent + " #endif\n" +
2372+
indent + " {\n" +
2373+
"%6$s" +
2374+
indent + " std::memcpy(m_buffer + m_offset + %3$d, src, sizeof(%4$s) * %5$d);\n" +
2375+
indent + " return *this;\n" +
2376+
indent + " }\n",
2377+
containingClassName,
2378+
toUpperFirstChar(propertyName),
2379+
offset,
2380+
cppTypeName,
2381+
arrayLength,
2382+
accessOrderListenerCall,
2383+
noexceptDeclaration);
2384+
2385+
}
2386+
if (encodingToken.encoding().primitiveType() == PrimitiveType.CHAR)
2387+
{
2388+
// Resolve ambiguity of string literal arguments, which match both
2389+
// std::span<const char, N> and std::string_view overloads.
2390+
new Formatter(sb).format("\n" +
2391+
indent + " #ifdef SBE_USE_SPAN\n" +
2392+
indent + " template <std::size_t N>\n" +
2393+
indent + " %1$s &put%2$s(const char (&src)[N])%3$s\n" +
2394+
indent + " {\n" +
2395+
indent + " return put%2$s(std::span<const char, N>(src));\n" +
2396+
indent + " }\n" +
2397+
indent + " #endif\n",
2398+
containingClassName,
2399+
toUpperFirstChar(propertyName),
2400+
noexceptDeclaration);
2401+
}
2402+
22752403
if (arrayLength > 1 && arrayLength <= 4)
22762404
{
22772405
sb.append("\n").append(indent).append(" ")
@@ -2332,7 +2460,7 @@ private void generateArrayProperty(
23322460
generateJsonEscapedStringGetter(sb, encodingToken, indent, propertyName);
23332461

23342462
new Formatter(sb).format("\n" +
2335-
indent + " #if __cplusplus >= 201703L\n" +
2463+
indent + " #ifdef SBE_USE_STRING_VIEW\n" +
23362464
indent + " SBE_NODISCARD std::string_view get%1$sAsStringView() const%6$s\n" +
23372465
indent + " {\n" +
23382466
"%4$s" +
@@ -2354,7 +2482,7 @@ private void generateArrayProperty(
23542482
noexceptDeclaration);
23552483

23562484
new Formatter(sb).format("\n" +
2357-
indent + " #if __cplusplus >= 201703L\n" +
2485+
indent + " #ifdef SBE_USE_STRING_VIEW\n" +
23582486
indent + " %1$s &put%2$s(const std::string_view str)\n" +
23592487
indent + " {\n" +
23602488
indent + " const std::size_t srcLength = str.length();\n" +

‎sbe-tool/src/test/cpp/CMakeLists.txt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,11 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
103103
sbe_test(DtoTest codecs)
104104
target_compile_features(DtoTest PRIVATE cxx_std_17)
105105
endif()
106+
107+
# Check if the GCC version supports C++20
108+
if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "11.0")
109+
target_compile_features(CodeGenTest PRIVATE cxx_std_20)
110+
endif()
106111
endif()
107112

108113
if (CMAKE_CXX_COMPILER_ID STREQUAL "CLang")
@@ -111,12 +116,18 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "CLang")
111116
sbe_test(DtoTest codecs)
112117
target_compile_features(DtoTest PRIVATE cxx_std_17)
113118
endif()
119+
120+
# Check if CLang version supports C++20
121+
if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "10.0")
122+
target_compile_features(CodeGenTest PRIVATE cxx_std_20)
123+
endif()
114124
endif()
115125

116126
if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
117-
# Check if MSVC version supports C++17
127+
# Check if MSVC version supports C++17 / C++20
118128
if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "19.14")
119129
sbe_test(DtoTest codecs)
120130
target_compile_options(DtoTest PRIVATE /std:c++17)
131+
target_compile_options(CodeGenTest PRIVATE /std:c++20)
121132
endif()
122133
endif()

‎sbe-tool/src/test/cpp/CodeGenTest.cpp

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,3 +1083,65 @@ TEST_F(CodeGenTest, shouldAllowForMultipleIterations)
10831083
std::string passFive = walkCar(carDecoder);
10841084
EXPECT_EQ(passOne, passFive);
10851085
}
1086+
1087+
#ifdef SBE_USE_STRING_VIEW
1088+
TEST_F(CodeGenTest, shouldBeAbleToUseStdStringViewMethods)
1089+
{
1090+
std::string vehicleCode(VEHICLE_CODE, Car::vehicleCodeLength());
1091+
1092+
char buffer[BUFFER_LEN] = {};
1093+
std::uint64_t baseOffset = MessageHeader::encodedLength();
1094+
Car car;
1095+
car.wrapForEncode(buffer, baseOffset, sizeof(buffer));
1096+
car.putVehicleCode(std::string_view(vehicleCode));
1097+
1098+
car.sbeRewind();
1099+
EXPECT_EQ(car.getVehicleCodeAsStringView(), vehicleCode);
1100+
}
1101+
#endif
1102+
1103+
#ifdef SBE_USE_SPAN
1104+
TEST_F(CodeGenTest, shouldBeAbleToUseStdSpanViewMethods)
1105+
{
1106+
char buffer[BUFFER_LEN] = {};
1107+
1108+
std::uint64_t baseOffset = MessageHeader::encodedLength();
1109+
Car car;
1110+
car.wrapForEncode(buffer, baseOffset, sizeof(buffer));
1111+
1112+
{
1113+
std::array<std::int32_t, Car::someNumbersLength()> numbers;
1114+
for (std::uint64_t i = 0; i < Car::someNumbersLength(); i++)
1115+
{
1116+
numbers[i] = static_cast<std::int32_t>(i);
1117+
}
1118+
car.putSomeNumbers(numbers);
1119+
}
1120+
1121+
car.sbeRewind();
1122+
1123+
{
1124+
std::span<const std::int32_t> numbers = car.getSomeNumbersAsSpan();
1125+
ASSERT_EQ(numbers.size(), Car::someNumbersLength());
1126+
for (std::uint64_t i = 0; i < numbers.size(); i++)
1127+
{
1128+
EXPECT_EQ(numbers[i], static_cast<std::int32_t>(i));
1129+
}
1130+
}
1131+
}
1132+
#endif
1133+
1134+
#ifdef SBE_USE_SPAN
1135+
TEST_F(CodeGenTest, shouldBeAbleToResolveStringLiterals)
1136+
{
1137+
char buffer[BUFFER_LEN] = {};
1138+
1139+
std::uint64_t baseOffset = MessageHeader::encodedLength();
1140+
Car car;
1141+
car.wrapForEncode(buffer, baseOffset, sizeof(buffer));
1142+
car.putVehicleCode("ABCDE");
1143+
1144+
car.sbeRewind();
1145+
EXPECT_EQ(car.getVehicleCodeAsStringView(), "ABCDE");
1146+
}
1147+
#endif

0 commit comments

Comments
 (0)