Skip to content

Commit 60af6f0

Browse files
committed
[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.
1 parent b8c7056 commit 60af6f0

File tree

3 files changed

+203
-6
lines changed

3 files changed

+203
-6
lines changed

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

Lines changed: 129 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;
@@ -1612,6 +1614,23 @@ private static CharSequence generateArrayFieldNotPresentCondition(final int sinc
16121614
sinceVersion);
16131615
}
16141616

1617+
private static CharSequence generateSpanFieldNotPresentCondition(
1618+
final int sinceVersion, final String indent, final String cppTypeName)
1619+
{
1620+
if (0 == sinceVersion)
1621+
{
1622+
return "";
1623+
}
1624+
1625+
return String.format(
1626+
indent + " if (m_actingVersion < %1$d)\n" +
1627+
indent + " {\n" +
1628+
indent + " return std::span<const $2%s>();\n" +
1629+
indent + " }\n\n",
1630+
sinceVersion,
1631+
cppTypeName);
1632+
}
1633+
16151634
private static CharSequence generateStringNotPresentCondition(final int sinceVersion, final String indent)
16161635
{
16171636
if (0 == sinceVersion)
@@ -1681,10 +1700,16 @@ private CharSequence generateFileHeader(
16811700
"#if __cplusplus >= 201703L\n" +
16821701
"# include <string_view>\n" +
16831702
"# define SBE_NODISCARD [[nodiscard]]\n" +
1703+
"# define SBE_USE_STRING_VIEW 1\n" +
16841704
"#else\n" +
16851705
"# define SBE_NODISCARD\n" +
16861706
"#endif\n\n" +
16871707

1708+
"#if __cplusplus >= 202002L\n" +
1709+
"# include <span>\n" +
1710+
"# define SBE_USE_SPAN 1\n" +
1711+
"#endif\n\n" +
1712+
16881713
"#if !defined(__STDC_LIMIT_MACROS)\n" +
16891714
"# define __STDC_LIMIT_MACROS 1\n" +
16901715
"#endif\n\n" +
@@ -2236,12 +2261,38 @@ private void generateArrayProperty(
22362261
accessOrderListenerCall);
22372262

22382263
new Formatter(sb).format("\n" +
2239-
indent + " %1$s &put%2$s(const char *const src)%7$s\n" +
2264+
indent + " #ifdef SBE_USE_SPAN\n" +
2265+
indent + " SBE_NODISCARD std::span<const %5$s> get%1$sAsSpan() const%7$s\n" +
2266+
indent + " {\n" +
2267+
"%3$s" +
2268+
"%6$s" +
2269+
indent + " const char *buffer = m_buffer + m_offset + %4$d;\n" +
2270+
indent + " return std::span<const %5$s>(reinterpret_cast<const %5$s*>(buffer), %2$d);\n" +
2271+
indent + " }\n" +
2272+
indent + " #endif\n",
2273+
toUpperFirstChar(propertyName),
2274+
arrayLength,
2275+
generateSpanFieldNotPresentCondition(propertyToken.version(), indent, cppTypeName),
2276+
offset,
2277+
cppTypeName,
2278+
accessOrderListenerCall,
2279+
noexceptDeclaration);
2280+
2281+
new Formatter(sb).format("\n" +
2282+
indent + " #ifdef SBE_USE_SPAN\n" +
2283+
indent + " template <std::size_t N>\n" +
2284+
indent + " %1$s &put%2$s(std::span<const %4$s, N> src)%7$s\n" +
22402285
indent + " {\n" +
2286+
indent + " static_assert(N <= %5$d, \"array too large for put%2$s\");\n\n" +
22412287
"%6$s" +
2242-
indent + " std::memcpy(m_buffer + m_offset + %3$d, src, sizeof(%4$s) * %5$d);\n" +
2288+
indent + " std::memcpy(m_buffer + m_offset + %3$d, src.data(), sizeof(%4$s) * N);\n" +
2289+
indent + " for (std::size_t start = N; start < %5$d; ++start)\n" +
2290+
indent + " {\n" +
2291+
indent + " m_buffer[m_offset + %3$d + start] = 0;\n" +
2292+
indent + " }\n\n" +
22432293
indent + " return *this;\n" +
2244-
indent + " }\n",
2294+
indent + " }\n" +
2295+
indent + " #endif\n",
22452296
containingClassName,
22462297
toUpperFirstChar(propertyName),
22472298
offset,
@@ -2250,6 +2301,79 @@ private void generateArrayProperty(
22502301
accessOrderListenerCall,
22512302
noexceptDeclaration);
22522303

2304+
if (encodingToken.encoding().primitiveType() != PrimitiveType.CHAR)
2305+
{
2306+
new Formatter(sb).format("\n" +
2307+
indent + " #ifdef SBE_USE_SPAN\n" +
2308+
indent + " %1$s &put%2$s(std::span<const %4$s> src)\n" +
2309+
indent + " {\n" +
2310+
indent + " const std::size_t srcLength = src.size();\n" +
2311+
indent + " if (srcLength > %5$d)\n" +
2312+
indent + " {\n" +
2313+
indent + " throw std::runtime_error(\"array too large for put%2$s [E106]\");\n" +
2314+
indent + " }\n\n" +
2315+
2316+
"%6$s" +
2317+
indent + " std::memcpy(m_buffer + m_offset + %3$d, src.data(), sizeof(%4$s) * srcLength);\n" +
2318+
indent + " for (std::size_t start = srcLength; start < %5$d; ++start)\n" +
2319+
indent + " {\n" +
2320+
indent + " m_buffer[m_offset + %3$d + start] = 0;\n" +
2321+
indent + " }\n\n" +
2322+
indent + " return *this;\n" +
2323+
indent + " }\n" +
2324+
indent + " #endif\n",
2325+
containingClassName,
2326+
toUpperFirstChar(propertyName),
2327+
offset,
2328+
cppTypeName,
2329+
arrayLength,
2330+
accessOrderListenerCall);
2331+
}
2332+
2333+
if (!DISABLE_RAW_ARRAYS)
2334+
{
2335+
new Formatter(sb).format("\n" +
2336+
indent + " #ifdef SBE_USE_SPAN\n" +
2337+
indent + " template <typename T>\n" +
2338+
// If std::span is available, redirect string literals to the std::span-accepting overload,
2339+
// where we can do compile-time bounds checking.
2340+
indent + " %1$s &put%2$s(T&& src) %7$s requires\n" +
2341+
indent + " (std::is_pointer_v<std::remove_reference_t<T>> &&\n" +
2342+
indent + " !std::is_array_v<std::remove_reference_t<T>>)\n" +
2343+
indent + " #else\n" +
2344+
indent + " %1$s &put%2$s(const char *const src)%7$s\n" +
2345+
indent + " #endif\n" +
2346+
indent + " {\n" +
2347+
"%6$s" +
2348+
indent + " std::memcpy(m_buffer + m_offset + %3$d, src, sizeof(%4$s) * %5$d);\n" +
2349+
indent + " return *this;\n" +
2350+
indent + " }\n",
2351+
containingClassName,
2352+
toUpperFirstChar(propertyName),
2353+
offset,
2354+
cppTypeName,
2355+
arrayLength,
2356+
accessOrderListenerCall,
2357+
noexceptDeclaration);
2358+
2359+
}
2360+
if (encodingToken.encoding().primitiveType() == PrimitiveType.CHAR)
2361+
{
2362+
// Resolve ambiguity of string literal arguments, which match both
2363+
// std::span<const char, N> and std::string_view overloads.
2364+
new Formatter(sb).format("\n" +
2365+
indent + " #ifdef SBE_USE_SPAN\n" +
2366+
indent + " template <std::size_t N>\n" +
2367+
indent + " %1$s &put%2$s(const char (&src)[N])%3$s\n" +
2368+
indent + " {\n" +
2369+
indent + " return put%2$s(std::span<const char, N>(src));\n" +
2370+
indent + " }\n" +
2371+
indent + " #endif\n",
2372+
containingClassName,
2373+
toUpperFirstChar(propertyName),
2374+
noexceptDeclaration);
2375+
}
2376+
22532377
if (arrayLength > 1 && arrayLength <= 4)
22542378
{
22552379
sb.append("\n").append(indent).append(" ")
@@ -2310,7 +2434,7 @@ private void generateArrayProperty(
23102434
generateJsonEscapedStringGetter(sb, encodingToken, indent, propertyName, accessOrderListenerCall);
23112435

23122436
new Formatter(sb).format("\n" +
2313-
indent + " #if __cplusplus >= 201703L\n" +
2437+
indent + " #ifdef SBE_USE_STRING_VIEW\n" +
23142438
indent + " SBE_NODISCARD std::string_view get%1$sAsStringView() const%6$s\n" +
23152439
indent + " {\n" +
23162440
"%4$s" +
@@ -2332,7 +2456,7 @@ private void generateArrayProperty(
23322456
noexceptDeclaration);
23332457

23342458
new Formatter(sb).format("\n" +
2335-
indent + " #if __cplusplus >= 201703L\n" +
2459+
indent + " #ifdef SBE_USE_STRING_VIEW\n" +
23362460
indent + " %1$s &put%2$s(const std::string_view str)\n" +
23372461
indent + " {\n" +
23382462
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)