Skip to content

fix Rfp_wrapped_option (num optional > max_bits)#263

Merged
c-cube merged 4 commits intomransan:masterfrom
tbrk:too-many-optional
Dec 31, 2025
Merged

fix Rfp_wrapped_option (num optional > max_bits)#263
c-cube merged 4 commits intomransan:masterfrom
tbrk:too-many-optional

Conversation

@tbrk
Copy link
Contributor

@tbrk tbrk commented Dec 24, 2025

When the number of optional fields exceeds the size of a bitfield, presence is encoded using the option type (Rfp_wrapped_option). The generated OCaml code does not compile. An example is given below; see also src/tests/integration-tests/test28.proto.

syntax = "proto2";

message Person {
    optional bool ofield00 = 100 [default=true];
    optional bool ofield01 = 101 [default=true];
    ...
    optional bool ofield61 = 161 [default=true];
    optional bool ofield62 = 162 [default=true];
}

This commit fixes the problems.

When the number of optional fields exceeds the size of a bitfield,
presence is encoded using the option type (`Rfp_wrapped_option`). The
generated OCaml code does not compile. An example is given below; see
also `src/tests/integration-tests/test28.proto`.

```
syntax = "proto2";

message Person {
    optional bool ofield00 = 100 [default=true];
    optional bool ofield01 = 101 [default=true];
    ...
    optional bool ofield61 = 161 [default=true];
    optional bool ofield62 = 162 [default=true];
}
```

This commit fixes the problems.
@tbrk
Copy link
Contributor Author

tbrk commented Dec 24, 2025

Thank you for ocaml-protoc and all the maintenance work. It is a very useful project.

I found a bug for protocols with many optional fields. This pull request is my attempted fix, but I'm new to the project and possibly made mistakes.

I know that a special case is needed for Rft_nolabel, but I'm not convinced it's necessary for Rft_required.

@c-cube c-cube merged commit 89e1cb9 into mransan:master Dec 31, 2025
5 checks passed
@c-cube
Copy link
Collaborator

c-cube commented Dec 31, 2025

Thank you, I failed to check this particular corner case!

@tbrk
Copy link
Contributor Author

tbrk commented Dec 31, 2025

Thanks @c-cube . It came up for the Google OR-Tools cp_model.proto definition, while I was working on https://github.com/INRIA/ocaml-ortools.

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.

2 participants