Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++] Fix field precedence check issue #1031. #1033

Merged
merged 1 commit into from
Dec 11, 2024
Merged

[C++] Fix field precedence check issue #1031. #1033

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 27 additions & 52 deletions sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1201,7 +1201,7 @@ private void generateVarData(
lengthCppType,
accessOrderListenerCall);

generateJsonEscapedStringGetter(sb, token, indent, propertyName, accessOrderListenerCall);
generateJsonEscapedStringGetter(sb, token, indent, propertyName);

new Formatter(sb).format("\n" +
indent + " #if __cplusplus >= 201703L\n" +
Expand Down Expand Up @@ -2307,7 +2307,7 @@ private void generateArrayProperty(
generateStringNotPresentCondition(propertyToken.version(), indent),
accessOrderListenerCall);

generateJsonEscapedStringGetter(sb, encodingToken, indent, propertyName, accessOrderListenerCall);
generateJsonEscapedStringGetter(sb, encodingToken, indent, propertyName);

new Formatter(sb).format("\n" +
indent + " #if __cplusplus >= 201703L\n" +
Expand Down Expand Up @@ -2381,14 +2381,12 @@ private void generateJsonEscapedStringGetter(
final StringBuilder sb,
final Token token,
final String indent,
final String propertyName,
final CharSequence accessOrderListenerCall)
final String propertyName)
{
new Formatter(sb).format("\n" +
indent + " std::string get%1$sAsJsonEscapedString()\n" +
indent + " {\n" +
"%2$s" +
"%3$s" +
indent + " std::ostringstream oss;\n" +
indent + " std::string s = get%1$sAsString();\n\n" +
indent + " for (const auto c : s)\n" +
Expand Down Expand Up @@ -2417,8 +2415,7 @@ private void generateJsonEscapedStringGetter(
indent + " return oss.str();\n" +
indent + " }\n",
toUpperFirstChar(propertyName),
generateStringNotPresentCondition(token.version(), indent),
accessOrderListenerCall);
generateStringNotPresentCondition(token.version(), indent));
}

private void generateConstPropertyMethods(
Expand Down Expand Up @@ -2505,7 +2502,7 @@ private void generateConstPropertyMethods(
values,
constantValue.length);

generateJsonEscapedStringGetter(sb, token, indent, propertyName, "");
generateJsonEscapedStringGetter(sb, token, indent, propertyName);
}

private CharSequence generateFixedFlyweightCode(final String className, final int size)
Expand Down Expand Up @@ -2654,34 +2651,13 @@ private static String generateHiddenCopyConstructor(final String indent, final S
return DISABLE_IMPLICIT_COPYING ? ctorAndCopyAssignmentDeletion : "";
}

private static CharSequence generateConstructorsAndOperators(
private CharSequence generateConstructorsAndOperators(
final String className,
final FieldPrecedenceModel fieldPrecedenceModel)
{
final String constructorWithCodecState = null == fieldPrecedenceModel ? "" : String.format(
" %1$s(\n" +
" char *buffer,\n" +
" const std::uint64_t offset,\n" +
" const std::uint64_t bufferLength,\n" +
" const std::uint64_t actingBlockLength,\n" +
" const std::uint64_t actingVersion,\n" +
" CodecState codecState) :\n" +
" m_buffer(buffer),\n" +
" m_bufferLength(bufferLength),\n" +
" m_offset(offset),\n" +
" m_position(sbeCheckPosition(offset + actingBlockLength)),\n" +
" m_actingBlockLength(actingBlockLength),\n" +
" m_actingVersion(actingVersion),\n" +
" m_codecState(codecState)\n" +
" {\n" +
" }\n\n",
className);

return String.format(
" %1$s() = default;\n\n" +

"%2$s" +

" %1$s(\n" +
" char *buffer,\n" +
" const std::uint64_t offset,\n" +
Expand All @@ -2695,6 +2671,7 @@ private static CharSequence generateConstructorsAndOperators(
" m_actingBlockLength(actingBlockLength),\n" +
" m_actingVersion(actingVersion)\n" +
" {\n" +
"%2$s" +
" }\n\n" +

" %1$s(char *buffer, const std::uint64_t bufferLength) :\n" +
Expand All @@ -2711,7 +2688,7 @@ private static CharSequence generateConstructorsAndOperators(
" {\n" +
" }\n\n",
className,
constructorWithCodecState);
generateCodecStateTransitionForWrapping(fieldPrecedenceModel));
}

private CharSequence generateMessageFlyweightCode(
Expand All @@ -2728,26 +2705,25 @@ private CharSequence generateMessageFlyweightCode(
final String semanticVersion = ir.semanticVersion() == null ? "" : ir.semanticVersion();


final String codecStateArgument = null == fieldPrecedenceModel ? "" : ", m_codecState";

return String.format(
"private:\n" +
"%14$s" +
"%15$s" +
"%16$s" +
" char *m_buffer = nullptr;\n" +
" std::uint64_t m_bufferLength = 0;\n" +
" std::uint64_t m_offset = 0;\n" +
" std::uint64_t m_position = 0;\n" +
" std::uint64_t m_actingBlockLength = 0;\n" +
" std::uint64_t m_actingVersion = 0;\n" +
"%17$s" +
"%16$s" +

" inline std::uint64_t *sbePositionPtr() SBE_NOEXCEPT\n" +
" {\n" +
" return &m_position;\n" +
" }\n\n" +

"%22$s" +
"%19$s" +
"%21$s" +

"public:\n" +
" static constexpr %1$s SBE_BLOCK_LENGTH = %2$s;\n" +
Expand Down Expand Up @@ -2775,7 +2751,7 @@ private CharSequence generateMessageFlyweightCode(

" using messageHeader = %12$s;\n\n" +

"%18$s" +
"%17$s" +
"%11$s" +

" SBE_NODISCARD static SBE_CONSTEXPR %1$s sbeBlockLength() SBE_NOEXCEPT\n" +
Expand Down Expand Up @@ -2826,7 +2802,7 @@ private CharSequence generateMessageFlyweightCode(
" m_actingBlockLength = sbeBlockLength();\n" +
" m_actingVersion = sbeSchemaVersion();\n" +
" m_position = sbeCheckPosition(m_offset + m_actingBlockLength);\n" +
"%19$s" +
"%18$s" +
" return *this;\n" +
" }\n\n" +

Expand All @@ -2847,12 +2823,10 @@ private CharSequence generateMessageFlyweightCode(
" m_actingBlockLength = sbeBlockLength();\n" +
" m_actingVersion = sbeSchemaVersion();\n" +
" m_position = sbeCheckPosition(m_offset + m_actingBlockLength);\n" +
"%19$s" +
"%18$s" +
" return *this;\n" +
" }\n\n" +

"%20$s" +

" %10$s &wrapForDecode(\n" +
" char *buffer,\n" +
" const std::uint64_t offset,\n" +
Expand All @@ -2866,7 +2840,7 @@ private CharSequence generateMessageFlyweightCode(
" m_actingBlockLength = actingBlockLength;\n" +
" m_actingVersion = actingVersion;\n" +
" m_position = sbeCheckPosition(m_offset + m_actingBlockLength);\n" +
"%21$s" +
"%20$s" +
" return *this;\n" +
" }\n\n" +

Expand Down Expand Up @@ -2903,7 +2877,7 @@ private CharSequence generateMessageFlyweightCode(

" SBE_NODISCARD std::uint64_t decodeLength() const\n" +
" {\n" +
" %10$s skipper(m_buffer, m_offset, m_bufferLength, sbeBlockLength(), m_actingVersion%14$s);\n" +
" %10$s skipper(m_buffer, m_offset, m_bufferLength, sbeBlockLength(), m_actingVersion);\n" +
" skipper.skip();\n" +
" return skipper.encodedLength();\n" +
" }\n\n" +
Expand Down Expand Up @@ -2940,14 +2914,13 @@ private CharSequence generateMessageFlyweightCode(
generateConstructorsAndOperators(className, fieldPrecedenceModel),
formatClassName(headerType),
semanticVersion,
codecStateArgument,
generateFieldOrderStateEnum(fieldPrecedenceModel),
generateLookupTableDeclarations(fieldPrecedenceModel),
generateFieldOrderStateMember(fieldPrecedenceModel),
generateAccessOrderErrorType(fieldPrecedenceModel),
generateEncoderWrapListener(fieldPrecedenceModel),
generateDecoderWrapListener(fieldPrecedenceModel),
generateDecoderWrapListenerCall(fieldPrecedenceModel),
generateCodecStateTransitionForWrappingLatestVersion(fieldPrecedenceModel),
generateOnWrappedListener(fieldPrecedenceModel),
generateCodecStateTransitionForWrapping(fieldPrecedenceModel),
generateHiddenCopyConstructor(" ", className));
}

Expand Down Expand Up @@ -3118,7 +3091,7 @@ private static CharSequence generateFieldOrderStateMember(final FieldPrecedenceM
return sb;
}

private static CharSequence generateDecoderWrapListener(final FieldPrecedenceModel fieldPrecedenceModel)
private static CharSequence generateOnWrappedListener(final FieldPrecedenceModel fieldPrecedenceModel)
{
if (null == fieldPrecedenceModel)
{
Expand All @@ -3131,7 +3104,7 @@ private static CharSequence generateDecoderWrapListener(final FieldPrecedenceMod
}

final StringBuilder sb = new StringBuilder();
sb.append(INDENT).append("void onWrapForDecode(std::uint64_t actingVersion)\n")
sb.append(INDENT).append("void onWrapped(std::uint64_t actingVersion)\n")
.append(INDENT).append("{\n");

final MutableBoolean actingVersionCanBeTooLowToBeValid = new MutableBoolean(true);
Expand Down Expand Up @@ -3168,7 +3141,7 @@ private static CharSequence generateDecoderWrapListener(final FieldPrecedenceMod
}


private CharSequence generateDecoderWrapListenerCall(final FieldPrecedenceModel fieldPrecedenceModel)
private CharSequence generateCodecStateTransitionForWrapping(final FieldPrecedenceModel fieldPrecedenceModel)
{
if (null == fieldPrecedenceModel)
{
Expand All @@ -3185,10 +3158,12 @@ private CharSequence generateDecoderWrapListenerCall(final FieldPrecedenceModel
return sb;
}

return generateAccessOrderListenerCall(fieldPrecedenceModel, TWO_INDENT, "onWrapForDecode", "actingVersion");
return generateAccessOrderListenerCall(fieldPrecedenceModel, TWO_INDENT, "onWrapped", "actingVersion");
}

private CharSequence generateEncoderWrapListener(final FieldPrecedenceModel fieldPrecedenceModel)
private CharSequence generateCodecStateTransitionForWrappingLatestVersion(
final FieldPrecedenceModel fieldPrecedenceModel
)
{
if (null == fieldPrecedenceModel)
{
Expand Down
93 changes: 93 additions & 0 deletions sbe-tool/src/test/cpp/FieldAccessOrderCheckTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

#include <sstream>
#include "gtest/gtest.h"
#include "gmock/gmock.h"
#include "order_check/MultipleVarLength.h"
Expand Down Expand Up @@ -4955,3 +4956,95 @@ TEST_F(FieldAccessOrderCheckTest, allowsSkippingFutureGroupWhenDecodingFromVersi
EXPECT_EQ(decoder.a(), 42);
EXPECT_EQ(decoder.getBAsString(), "abc");
}

TEST_F(FieldAccessOrderCheckTest, worksWithWrappingConstructors1)
{
MultipleVarLength encoder(m_buffer, BUFFER_LEN);
encoder.a(42);
encoder.putB("abc");
encoder.putC("def");
encoder.checkEncodingIsComplete();

MultipleVarLength decoder(m_buffer, BUFFER_LEN);
EXPECT_EQ(decoder.a(), 42);
EXPECT_EQ(decoder.getBAsString(), "abc");
EXPECT_EQ(decoder.getCAsString(), "def");
}

TEST_F(FieldAccessOrderCheckTest, worksWithWrappingConstructors2)
{
MultipleVarLength encoder(
m_buffer,
BUFFER_LEN,
MultipleVarLength::sbeBlockLength(),
MultipleVarLength::sbeSchemaVersion()
);
encoder.a(42);
encoder.putB("abc");
encoder.putC("def");
encoder.checkEncodingIsComplete();

MultipleVarLength decoder(
m_buffer,
BUFFER_LEN,
MultipleVarLength::sbeBlockLength(),
MultipleVarLength::sbeSchemaVersion()
);
EXPECT_EQ(decoder.a(), 42);
EXPECT_EQ(decoder.getBAsString(), "abc");
EXPECT_EQ(decoder.getCAsString(), "def");
}

TEST_F(FieldAccessOrderCheckTest, worksWithWrappingConstructors3)
{
MultipleVarLength encoder(
m_buffer,
OFFSET,
BUFFER_LEN,
MultipleVarLength::sbeBlockLength(),
MultipleVarLength::sbeSchemaVersion()
);
encoder.a(42);
encoder.putB("abc");
encoder.putC("def");
encoder.checkEncodingIsComplete();

MultipleVarLength decoder(
m_buffer,
OFFSET,
BUFFER_LEN,
MultipleVarLength::sbeBlockLength(),
MultipleVarLength::sbeSchemaVersion()
);
EXPECT_EQ(decoder.a(), 42);
EXPECT_EQ(decoder.getBAsString(), "abc");
EXPECT_EQ(decoder.getCAsString(), "def");
}

TEST_F(FieldAccessOrderCheckTest, worksWithInsertionOperator)
{
MultipleVarLength encoder(
m_buffer,
OFFSET,
BUFFER_LEN,
MultipleVarLength::sbeBlockLength(),
MultipleVarLength::sbeSchemaVersion()
);
encoder.a(42);
encoder.putB("abc");
encoder.putC("def");
encoder.checkEncodingIsComplete();

MultipleVarLength decoder(
m_buffer,
BUFFER_LEN
);
std::stringstream stream;
stream << decoder;

const std::string expected = "\"a\": 42, \"b\": \"abc\", \"c\": \"def\"";
EXPECT_THAT(stream.str(), HasSubstr(expected));
EXPECT_EQ(decoder.a(), 42);
EXPECT_EQ(decoder.getBAsString(), "abc");
EXPECT_EQ(decoder.getCAsString(), "def");
}
Loading