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.

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

BUG: C++ constant value gets serialized #529

Closed
arobenko opened this issue Nov 9, 2017 · 2 comments
Closed

BUG: C++ constant value gets serialized #529

arobenko opened this issue Nov 9, 2017 · 2 comments

Comments

@arobenko
Copy link

arobenko commented Nov 9, 2017

Having the following enum definition

<enum name="Enum1" encodingType="uint8">
    <validValue name="Val1">0</validValue>
    <validValue name="Val2">1</validValue>
    <validValue name="Val3">2</validValue>
</enum> 

and message with single constant field

<message name="Msg1" id="1" description="TestMessage">
    <field name="field1" id="20" type="uint8" presence="constant" valueRef="Enum1.Val2"/>
</message>

generates code that serializes the field:

std::uint8_t field1() const
{
    return (*((std::uint8_t *)(m_buffer + m_offset + 0)));
}

Msg1 &field1(const std::uint8_t value)
{
    *((std::uint8_t *)(m_buffer + m_offset + 0)) = (value);
    return *this;
}

while realizing that the field is constant:

    static const char *field1MetaAttribute(const MetaAttribute::Attribute metaAttribute) SBE_NOEXCEPT
    {
        switch (metaAttribute)
        {
            case MetaAttribute::EPOCH: return "unix";
            case MetaAttribute::TIME_UNIT: return "nanosecond";
            case MetaAttribute::SEMANTIC_TYPE: return "";
            case MetaAttribute::PRESENCE: return "constant";   // <----- HERE
        }

        return "";
    }

The only difference from #528 is that the type of the field is specified "uint8" directly instead of "Enum1".

Reproduced with v1.7.4

@mjpt777
Copy link
Contributor

mjpt777 commented Nov 10, 2017

The type provided for the constant is incorrect and thus was generating the wrong code. I've pushed a change to validate the types match.

@mjpt777 mjpt777 closed this as completed Nov 10, 2017
@arobenko
Copy link
Author

Hi there,
At this moment I'm working on my own "schema to C++11 only" compiler, which generates implementation of the protocol in terms of my COMMS Library. I'm using code generated by SBE as a reference implementation and in my unit-testing, to make sure that a message serialized by one generated code can be correctly deserialized by another. I assumed that any example encountered in the spec is a valid one and the compiler is expected to understand it. The spec contains the following one:

<composite name="UTCTimestampNanos" description="UTC timestamp with nanosecond precision" semanticType="UTCTimestamp" >
    <type name="time" primitiveType="uint64" />
    <type name="unit" primitiveType="uint8" presence="constant" valueRef="TimeUnit.nanosecond" />
</composite>

The "unit" is regular "uint8" constant type, but uses "valueRef" to specify its numeric value. I think other developers who read the spec may consider it to be valid and decide to use the same constructs in their schema. Maybe it's a good idea to support it in your compiler as well. I do support it in mine.

mjpt777 added a commit that referenced this issue Nov 11, 2017
mjpt777 added a commit that referenced this issue Nov 12, 2017
… basic types and fix composite length calculation when using them. Issue #529.
mjpt777 added a commit that referenced this issue Nov 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants