Support custom options without optional keyword (proto3)

[twam]

When I use custom options in a proto3 file nanopb adds a note in the generated header file that they're not supported:

/* Extension field foo_options was skipped because only "optional"
   type of extension fields is currently supported. */

I would be awesome if those were supported as well.

Minimum (non-)working example:

syntax = "proto3";
 
import "google/protobuf/descriptor.proto";

message FooOptions {
  int32 opt1 = 1;
  string opt2 = 2;
}

extend google.protobuf.FieldOptions {
  FooOptions foo_options = 50000;
}

message Bar {
  int32 value = 1 [(foo_options).opt1 = 123, (foo_options).opt2 = "baz"];
}

[PetteriAimonen]

Hmm, extensions only exist for custom options in proto3, which is why by default it skips them.

It's simple enough to allow extensions in proto3 mode in nanopb, but this wouldn't get you very far, because nanopb has no reflection mechanism that would let to access the actual option values. However adding real support for custom options seems like a reasonable idea.

What would be your preferred way to access such options? I see two straightforward options:

1. As defines, like:

#define Bar_value_foo_options_opt1 123
#define Bar_value_foo_options_opt2 "baz"

2. As static protobuf object:

extern cost FooOptions Bar_value_foo_options;
..
const FooOptions Bar_value_foo_options = {123, "baz"};

Of these, 1) seems to me like it would be more useful in most cases.

[twam]

I haven't tried with a proto2 file, so my guess was that it would work after added it in proto3 mode. It would be awesome if this could be added to nanopb!

I personally prefer 2) as things are typed and can be more easily passed as a pointer, but I could live with both options.

[anton-danielsson]

Would be really cool indeed. I also prefer option 2).
If it's added it should have an option to disable it though, too keep the code size down.
Maybe have it disabled by default even?
We use "custom options" to annotate and reflect over things in C++/Python code. Currently we don't need it in the C counterpart.

[anton-danielsson]

Now we also need it in the C counterpart.

[maxmclau]

Any plans to implement this?

[PetteriAimonen]

I do not currently have plans to do so. It is a relatively simple task (only generator code changes needed), and pull requests are welcome.