Repository navigation
Conversation
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
commented
Oct 8, 2026
| * parent message. | ||
| * | ||
| * @param parent_writer The parent pbf_message or pbf_writer | ||
| * @param parent_writer The parent pbf_writer |
Member
Author
There was a problem hiding this comment.
We now have an explicit basic_pbf_builder template overload, so we don't have implicit casts anymore.
Member
Author
|
...or should we shelve this until we have more changes that would warrant a minor version bump? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #155.
The submessage constructor of
basic_pbf_buildertakes the parent as abasic_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 errorThis PR adds an overload for
basic_pbf_builderparents. It is an exact match, so it wins over the derived-to-base conversion of the existing constructor, and astatic_assertrequires the tag to be of the parent's enum type. Plainpbf_writerparents work as before.I also added
compile_filettests that are excluded from the normal build and check that the output contains thestatic_assertmessage 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?