Skip to content

Enforce parent tag type when creating a pbf_builder submessage - #159

Open
kkaefer wants to merge 1 commit into
kk/fix-ci-imagesfrom
kk/strict-builder-parent-tag
Open

kkaefer wants to merge 1 commit into
kk/fix-ci-imagesfrom
kk/strict-builder-parent-tag

Conversation

@kkaefer

@kkaefer kkaefer commented Oct 8, 2026

Copy link
Copy Markdown
Member

Fixes #155.

The submessage constructor of basic_pbf_builder takes the parent as a basic_pbf_writer& and accepts a tag of any type. A tag from the wrong enum therefore compiles silently and writes the wrong field number:

protozero::pbf_builder<Outer> outer{buffer};
protozero::pbf_builder<Inner> inner{outer, Inner::x}; // compiled previously, now an error

This PR adds an overload for basic_pbf_builder parents. It is an exact match, so it wins over the derived-to-base conversion of the existing constructor, and a static_assert requires the tag to be of the parent's enum type. Plain pbf_writer parents work as before.

I also added compile_file ttests that are excluded from the normal build and check that the output contains the static_assert message for code that should trigger it.

Since this change could technically break some existing code, should we bump the minor version for this rather than release it as a patch to the 1.8 series?

The submessage constructor of basic_pbf_builder took the parent as a
basic_pbf_writer& and accepted a tag of any type, so a tag from the wrong
enum compiled silently and wrote the wrong field number.

Add an overload taking a basic_pbf_builder parent. It is an exact match
and therefore preferred over the derived-to-base conversion of the existing
constructor, and a static_assert requires the tag to have the parent's enum
type. Plain pbf_writer parents keep working unchanged.

Adds compile-fail tests run through ctest to verify mismatched enum and
plain integer tags are rejected.

Fixes #155
@kkaefer
kkaefer requested a review from a team as a code owner October 8, 2026 14:00
* parent message.
*
* @param parent_writer The parent pbf_message or pbf_writer
* @param parent_writer The parent pbf_writer

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now have an explicit basic_pbf_builder template overload, so we don't have implicit casts anymore.

@kkaefer

kkaefer commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

...or should we shelve this until we have more changes that would warrant a minor version bump?

@kkaefer
kkaefer requested a review from joto October 8, 2026 15:26
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

Successfully merging this pull request may close these issues.

1 participant