Skip to content

Commit

Permalink
[#667] Fix choice tag clashing in packing context (Java & C++)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mi-La committed Dec 4, 2024
1 parent 9ed1b81 commit df12750
Show file tree
Hide file tree
Showing 12 changed files with 176 additions and 13 deletions.
2 changes: 1 addition & 1 deletion compiler/extensions/cpp/freemarker/CompoundField.inc.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,7 @@ ${I}context.${field.getterName}().init<<@array_traits_type_name field/>>(<#rt>
<#if hasChoiceTag || uses_packing_context(fieldList)>
public:
<#if hasChoiceTag>
::zserio::DeltaContext& getChoiceTag()
::zserio::DeltaContext& choiceTag()
{
return m_choiceTag;
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/extensions/cpp/freemarker/Union.cpp.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ ${name}::ChoiceTag ${name}::choiceTag() const

void ${name}::initPackingContext(${name}::ZserioPackingContext& context) const
{
context.getChoiceTag().init<${choiceTagArrayTraits}>(static_cast<uint32_t>(m_choiceTag));
context.choiceTag().init<${choiceTagArrayTraits}>(static_cast<uint32_t>(m_choiceTag));

switch (m_choiceTag)
{
Expand Down Expand Up @@ -510,7 +510,7 @@ size_t ${name}::bitSizeOf(${name}::ZserioPackingContext& context, size_t bitPosi
{
size_t endBitPosition = bitPosition;

endBitPosition += context.getChoiceTag().bitSizeOf<${choiceTagArrayTraits}>(static_cast<uint32_t>(m_choiceTag));
endBitPosition += context.choiceTag().bitSizeOf<${choiceTagArrayTraits}>(static_cast<uint32_t>(m_choiceTag));

switch (m_choiceTag)
{
Expand Down Expand Up @@ -557,7 +557,7 @@ size_t ${name}::initializeOffsets(${name}::ZserioPackingContext& context, size_t
{
size_t endBitPosition = bitPosition;

endBitPosition += context.getChoiceTag().bitSizeOf<${choiceTagArrayTraits}>(static_cast<uint32_t>(m_choiceTag));
endBitPosition += context.choiceTag().bitSizeOf<${choiceTagArrayTraits}>(static_cast<uint32_t>(m_choiceTag));

switch (m_choiceTag)
{
Expand Down Expand Up @@ -701,7 +701,7 @@ void ${name}::write(::zserio::BitStreamWriter&<#if fieldList?has_content> out</#

void ${name}::write(${name}::ZserioPackingContext& context, ::zserio::BitStreamWriter& out) const
{
context.getChoiceTag().write<${choiceTagArrayTraits}>(out, static_cast<uint32_t>(m_choiceTag));
context.choiceTag().write<${choiceTagArrayTraits}>(out, static_cast<uint32_t>(m_choiceTag));

switch (m_choiceTag)
{
Expand Down Expand Up @@ -734,7 +734,7 @@ ${name}::ChoiceTag ${name}::readChoiceTag(::zserio::BitStreamReader& in)

${name}::ChoiceTag ${name}::readChoiceTag(${name}::ZserioPackingContext& context, ::zserio::BitStreamReader& in)
{
return static_cast<${name}::ChoiceTag>(static_cast<int32_t>(context.getChoiceTag().read<${choiceTagArrayTraits}>(in)));
return static_cast<${name}::ChoiceTag>(static_cast<int32_t>(context.choiceTag().read<${choiceTagArrayTraits}>(in)));
}
</#if>

Expand Down
2 changes: 1 addition & 1 deletion compiler/extensions/java/freemarker/CompoundField.inc.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ ${I}<@compound_get_field field/>.initPackingContext(zserioContext.${field.getter
}

<#if hasChoiceTag>
public zserio.runtime.array.DeltaContext getChoiceTag()
public zserio.runtime.array.DeltaContext choiceTag()
{
return choiceTag;
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/extensions/java/freemarker/Union.java.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public class ${name} implements <#rt>
public void initPackingContext(zserio.runtime.array.PackingContext context)
{
final ZserioPackingContext zserioContext = context.cast();
zserioContext.getChoiceTag().init(
zserioContext.choiceTag().init(
new ${choiceTagArrayTraits}(),
new ${choiceTagArrayElement}(choiceTag));

Expand Down Expand Up @@ -190,7 +190,7 @@ public class ${name} implements <#rt>
final ZserioPackingContext zserioContext = context.cast();
long endBitPosition = bitPosition;

endBitPosition += zserioContext.getChoiceTag().bitSizeOf(
endBitPosition += zserioContext.choiceTag().bitSizeOf(
new ${choiceTagArrayTraits}(),
new ${choiceTagArrayElement}(choiceTag));

Expand Down Expand Up @@ -370,7 +370,7 @@ public class ${name} implements <#rt>
{
final ZserioPackingContext zserioContext = context.cast();
choiceTag = ((${choiceTagArrayElement})
zserioContext.getChoiceTag().read(
zserioContext.choiceTag().read(
new ${choiceTagArrayTraits}(), in)).get();

switch (choiceTag)
Expand Down Expand Up @@ -426,7 +426,7 @@ public class ${name} implements <#rt>
final ZserioPackingContext zserioContext = context.cast();
long endBitPosition = bitPosition;

endBitPosition += zserioContext.getChoiceTag().bitSizeOf(
endBitPosition += zserioContext.choiceTag().bitSizeOf(
new ${choiceTagArrayTraits}(),
new ${choiceTagArrayElement}(choiceTag));

Expand Down Expand Up @@ -471,7 +471,7 @@ public class ${name} implements <#rt>
throws java.io.IOException
{
final ZserioPackingContext zserioContext = context.cast();
zserioContext.getChoiceTag().write(
zserioContext.choiceTag().write(
new ${choiceTagArrayTraits}(), out,
new ${choiceTagArrayElement}(choiceTag));

Expand Down
2 changes: 1 addition & 1 deletion test/data
20 changes: 20 additions & 0 deletions test/extensions/language/array_types/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,34 @@ if (ZSERIO_LOG)
check_zserio_warnings("${ZSERIO_LOG}" 0)
endif ()

add_library(choice_tag_clash_zs STATIC ${TEST_ZS_ROOT}/choice_tag_clash.zs)
zserio_generate_cpp(
TARGET choice_tag_clash_zs
SRC_DIR ${TEST_ZS_ROOT}
GEN_DIR ${CMAKE_CURRENT_BINARY_DIR}/gen_choice_tag_clash
EXTRA_ARGS -withoutSourcesAmalgamation -withoutCrossExtensionCheck ${ZSERIO_EXTRA_ARGS}
GENERATED_SOURCES_VAR GENERATED_SOURCES_CHOICE_TAG_CLASH
OUTPUT_VAR ZSERIO_LOG_CHOICE_TAG_CLASH
ERROR_VAR ZSERIO_LOG_CHOICE_TAG_CLASH
)
target_link_libraries(choice_tag_clash_zs PUBLIC ZserioCppRuntime)
if (ZSERIO_LOG_CHOICE_TAG_CLASH)
file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/zserio_log_choice_tag_clash.txt
"${ZSERIO_LOG_CHOICE_TAG_CLASH}")
check_zserio_warnings("${ZSERIO_LOG_CHOICE_TAG_CLASH}" 0)
endif ()

add_custom_test(array_types
DEPENDS
array_types_zs
choice_tag_clash_zs
SOURCES
${CMAKE_CURRENT_SOURCE_DIR}/cpp/ArraysMappingTest.cpp
${CMAKE_CURRENT_SOURCE_DIR}/cpp/AutoArrayBitfieldParamTest.cpp
${CMAKE_CURRENT_SOURCE_DIR}/cpp/AutoArrayStructRecursionTest.cpp
${CMAKE_CURRENT_SOURCE_DIR}/cpp/AutoArraySubtypedUInt8Test.cpp
${CMAKE_CURRENT_SOURCE_DIR}/cpp/AutoArrayUInt8Test.cpp
${CMAKE_CURRENT_SOURCE_DIR}/cpp/ChoiceTagClashTest.cpp
${CMAKE_CURRENT_SOURCE_DIR}/cpp/FixedArrayUInt8Test.cpp
${CMAKE_CURRENT_SOURCE_DIR}/cpp/PackedArraysMappingTest.cpp
${CMAKE_CURRENT_SOURCE_DIR}/cpp/PackedAutoArrayBitfieldParamTest.cpp
Expand All @@ -44,4 +63,5 @@ add_custom_test(array_types
${CMAKE_CURRENT_SOURCE_DIR}/cpp/VariableArrayTernaryOperatorTest.cpp
GENERATED_SOURCES
${GENERATED_SOURCES}
${GENERATED_SOURCES_CHOICE_TAG_CLASH}
)
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ cppcoreguidelines-pro-type-member-init:gen/array_types/packing_interface_optimiz
cppcoreguidelines-pro-type-member-init:gen/array_types/packing_interface_optimization/PackedColorChoice.cpp
cppcoreguidelines-pro-type-member-init:gen/array_types/packing_interface_optimization/UnpackedColorChoice.cpp
cppcoreguidelines-pro-type-member-init:gen/array_types/variable_array_ternary_operator/VariableArrayElement.cpp
cppcoreguidelines-pro-type-member-init:gen_choice_tag_clash/choice_tag_clash/TestChoice.cpp

google-explicit-constructor:gen/array_types/arrays_mapping/TestBitmask.h
google-explicit-constructor:gen/array_types/packed_arrays_mapping/TestBitmask.h
Expand Down Expand Up @@ -99,9 +100,11 @@ readability-make-member-function-const:gen/array_types/packing_interface_optimiz
readability-make-member-function-const:gen/array_types/packing_interface_optimization/UnpackedColorChoice.cpp
readability-make-member-function-const:gen/array_types/packing_interface_optimization/UnpackedColorStruct.cpp
readability-make-member-function-const:gen/array_types/variable_array_ternary_operator/VariableArrayElement.cpp
readability-make-member-function-const:gen_choice_tag_clash/choice_tag_clash/TestChoice.cpp

readability-simplify-boolean-expr:gen/array_types/packed_auto_array_empty_compounds/EmptyChoice.cpp
readability-simplify-boolean-expr:gen/array_types/packed_auto_array_empty_compounds/EmptyUnion.cpp
readability-simplify-boolean-expr:gen/array_types/packing_interface_optimization/MixedColorChoice.cpp
readability-simplify-boolean-expr:gen/array_types/packing_interface_optimization/PackedColorChoice.cpp
readability-simplify-boolean-expr:gen/array_types/packing_interface_optimization/UnpackedColorChoice.cpp
readability-simplify-boolean-expr:gen_choice_tag_clash/choice_tag_clash/TestChoice.cpp
3 changes: 3 additions & 0 deletions test/extensions/language/array_types/build.xml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
<target name="gen" depends="gen.check" unless="array_types.zs.gen_is_uptodate">
<testGenClean testName="array_types"/>
<testGen testName="array_types" zsFile="array_types.zs"/>
<testGen testName="array_types" zsFile="choice_tag_clash.zs">
<arg name="withoutCrossExtensionCheck"/>
</testGen>
</target>

<target name="gen.checkWarnings" depends="gen">
Expand Down
74 changes: 74 additions & 0 deletions test/extensions/language/array_types/cpp/ChoiceTagClashTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
#include "choice_tag_clash/ChoiceTagClash.h"
#include "gtest/gtest.h"
#include "zserio/SerializeUtil.h"

namespace choice_tag_clash
{

using allocator_type = ChoiceTagClash::allocator_type;
template <typename T>
using vector_type = zserio::vector<T, allocator_type>;
using BitBuffer = zserio::BasicBitBuffer<zserio::RebindAlloc<allocator_type, uint8_t>>;

class ChoiceTagClashTest : public ::testing::Test
{
protected:
ChoiceTagClash createChoiceTagClash()
{
ChoiceTagClash choiceTagClash;
fillChoices(choiceTagClash.getChoices());
fillUnions(choiceTagClash.getUnions());
return choiceTagClash;
}

static void fillChoices(vector_type<TestChoice>& choices)
{
for (size_t i = 0; i < NUM_CHOICES; ++i)
{
choices.emplace_back();
if (i % 2 == 0)
{
choices.back().setChoiceTag(static_cast<uint32_t>(i));
}
else
{
choices.back().setStringField("text " + zserio::toString<allocator_type>(i));
}
}
}

static void fillUnions(vector_type<TestUnion>& unions)
{
for (size_t i = 0; i < NUM_UNIONS; ++i)
{
unions.emplace_back();
if (i % 2 == 0)
{
unions.back().setChoiceTag(static_cast<uint32_t>(i));
}
else
{
unions.back().setStringField("text " + zserio::toString<allocator_type>(i));
}
}
}

private:
static constexpr size_t NUM_CHOICES = 10;
static constexpr size_t NUM_UNIONS = 13;
};

constexpr size_t ChoiceTagClashTest::NUM_CHOICES;
constexpr size_t ChoiceTagClashTest::NUM_UNIONS;

TEST_F(ChoiceTagClashTest, writeRead)
{
ChoiceTagClash choiceTagClash = createChoiceTagClash();

BitBuffer bitBuffer = zserio::serialize(choiceTagClash);
ChoiceTagClash readChoiceTagClash = zserio::deserialize<ChoiceTagClash>(bitBuffer);

ASSERT_EQ(choiceTagClash, readChoiceTagClash);
}

} // namespace choice_tag_clash
1 change: 1 addition & 0 deletions test/extensions/language/array_types/doc_options.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ZSERIO_ARGS=-withoutCrossExtensionCheck
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package choice_tag_clash;

import static org.junit.jupiter.api.Assertions.*;

import org.junit.jupiter.api.Test;

import zserio.runtime.io.BitBuffer;
import zserio.runtime.io.SerializeUtil;

public class ChoiceTagClashTest
{
@Test
public void writeRead()
{
final ChoiceTagClash choiceTagClash = new ChoiceTagClash(createChoices(), createUnions());

final BitBuffer bitBuffer = SerializeUtil.serialize(choiceTagClash);
final ChoiceTagClash readChoiceTagClash = SerializeUtil.deserialize(ChoiceTagClash.class, bitBuffer);

assertEquals(choiceTagClash, readChoiceTagClash);
}

private TestChoice[] createChoices()
{
final TestChoice[] choices = new TestChoice[NUM_CHOICES];
for (int i = 0; i < NUM_CHOICES; ++i)
{
choices[i] = new TestChoice(i % 2 == 0);
if (i % 2 == 0)
{
choices[i].setChoiceTag(i);
}
else
{
choices[i].setStringField("text " + i);
}
}
return choices;
}

private TestUnion[] createUnions()
{
final TestUnion[] unions = new TestUnion[NUM_UNIONS];
for (int i = 0; i < NUM_UNIONS; ++i)
{
unions[i] = new TestUnion();
if (i % 2 == 0)
{
unions[i].setChoiceTag(i);
}
else
{
unions[i].setStringField("text " + i);
}
}
return unions;
}

private static final int NUM_CHOICES = 10;
private static final int NUM_UNIONS = 13;
}
1 change: 1 addition & 0 deletions test/extensions/language/array_types/xml_options.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ZSERIO_ARGS=-withoutCrossExtensionCheck

0 comments on commit df12750

Please sign in to comment.