CppGenerator: operator<< take messages as const-ref

[SBAM]

Dear all,

Following commit 1973c86, using generated operator<< would require a mutable ref message.
This is unnecessary and can be problematic.
For example when using wrappers such as fmt::streamed ( https://fmt.dev/latest/api.html#std-ostream-support ) that enforce const-correctness.

[ksergey]

Your patch breaks formatting messages with repeating groups, because access for reading repeating groups fields is non-const.

Previous commit also could break exists code, because operator<< modifies internal offsets of a message.

[SBAM]

Hello @ksergey

Apologies if I misunderstood your comment but method generateGroupDisplay was not modified. Dumping a Group should still require a mutable ref, should it not ?

[ksergey]

Hello @SBAM !

Dumping a Group should still require a mutable ref, should it not ?

Yes, require a mutable ref

Example (sbe-benchmarks/src/main/resources/car.xml):

...
private:
    FuelFigures m_fuelFigures;

public:
    SBE_NODISCARD static SBE_CONSTEXPR std::uint16_t fuelFiguresId() SBE_NOEXCEPT
    {
        return 9;
    }

    SBE_NODISCARD inline FuelFigures &fuelFigures()
    {
        m_fuelFigures.wrapForDecode(m_buffer, sbePositionPtr(), m_actingVersion, m_bufferLength);
        return m_fuelFigures;
    }

    FuelFigures &fuelFiguresCount(const std::uint16_t count)
    {
        m_fuelFigures.wrapForEncode(m_buffer, count, sbePositionPtr(), m_actingVersion, m_bufferLength);
        return m_fuelFigures;
    }

    SBE_NODISCARD static SBE_CONSTEXPR std::uint64_t fuelFiguresSinceVersion() SBE_NOEXCEPT
    {
        return 0;
    }

    SBE_NODISCARD bool fuelFiguresInActingVersion() const SBE_NOEXCEPT
    {
#if defined(__clang__)
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wtautological-compare"
#endif
        return m_actingVersion >= fuelFiguresSinceVersion();
#if defined(__clang__)
#pragma clang diagnostic pop
#endif
    }
...

[SBAM]

Dumping directly a Group indeed mutates it since 1973c86. I assume that people who did that copied Group first.

However, dumping a regular message copies initial data and passes to cascading groups dumpers that copy.

template<typename CharT, typename Traits>
friend std::basic_ostream<CharT, Traits> & operator << (
    std::basic_ostream<CharT, Traits> &builder, const MDIncrementalRefreshBook46 &_writer)
{
    MDIncrementalRefreshBook46 writer(
        _writer.m_buffer,
        _writer.m_offset,
        _writer.m_bufferLength,
        _writer.m_actingBlockLength,
        _writer.m_actingVersion);

    builder << '{';

Are you suggesting to revert 1973c86 completely ?

[ksergey]

I think current implementation works pretty well

[SBAM]

I still can't quite grasp how adding that const will make current implementation work less well.
Does it generate issues that are not already there ?

[ksergey]

@SBAM
sorry, looks like there is no issue with const/non-const, because inside operator<< input sbe object copied into local variable.