pw_protobuf: Add inline option support
Adds inline field option support for max_size and max_count fields, as
recently discussed in PW Live. This makes a distinction between
protocol-level parameters vs codegen configuration, where specifying
these fields inline should only be used in cases where they affect
protocol validity.
Change-Id: I02ebf55bda21910037ae118a3c14fca960e759f4
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/119114
Reviewed-by: Ted Pudlik <tpudlik@google.com>
Commit-Queue: Wyatt Hepler <hepler@google.com>
diff --git a/pw_protobuf/BUILD.bazel b/pw_protobuf/BUILD.bazel
index 61ed8bb..da1b4b8 100644
--- a/pw_protobuf/BUILD.bazel
+++ b/pw_protobuf/BUILD.bazel
@@ -210,6 +210,27 @@
)
proto_library(
+ name = "field_options_proto",
+ srcs = [
+ "pw_protobuf_protos/field_options.proto",
+ ],
+ strip_import_prefix = "//pw_protobuf",
+ deps = [
+ "@com_google_protobuf//:descriptor_proto",
+ ],
+)
+
+py_proto_library(
+ name = "field_options_proto_pb2",
+ srcs = [
+ "pw_protobuf_protos/field_options.proto",
+ ],
+ deps = [
+ "@com_google_protobuf//:protobuf_python",
+ ],
+)
+
+proto_library(
name = "status_proto",
srcs = [
"pw_protobuf_protos/status.proto",
diff --git a/pw_protobuf/BUILD.gn b/pw_protobuf/BUILD.gn
index c8a765e..4463f13 100644
--- a/pw_protobuf/BUILD.gn
+++ b/pw_protobuf/BUILD.gn
@@ -218,6 +218,7 @@
pw_proto_library("common_protos") {
sources = [
"pw_protobuf_protos/common.proto",
+ "pw_protobuf_protos/field_options.proto",
"pw_protobuf_protos/status.proto",
]
}
diff --git a/pw_protobuf/docs.rst b/pw_protobuf/docs.rst
index 7b2cc69..787f150 100644
--- a/pw_protobuf/docs.rst
+++ b/pw_protobuf/docs.rst
@@ -581,6 +581,33 @@
| 5 bytes | 4,294,967,295 or < 4GiB (max uint32_t) |
+-------------------+----------------------------------------+
+Field Options
+=============
+``pw_protobuf`` supports the following field options for specifying
+protocol-level limitations, rather than code generation parameters (although
+they do influence code generation):
+
+
+* ``max_count``:
+ Maximum number of entries for repeated fields.
+
+* ``max_size``:
+ Maximum size of `bytes` or `string` fields.
+
+Even though other proto codegen implementations do not respect these field
+options, they can still compile protos which use these options. This is
+especially useful for host builds using upstream protoc code generation, where
+host software can use the reflection API to query for the options and validate
+messages comply with the specified limitations.
+
+.. code::
+
+ import "pw_protobuf_protos/field_options.proto";
+
+ message Demo {
+ string size_limited_string = 1 [(pw.protobuf.pwpb).max_size = 16];
+ };
+
Options Files
=============
Code generation can be configured using a separate ``.options`` file placed
diff --git a/pw_protobuf/pw_protobuf_protos/field_options.proto b/pw_protobuf/pw_protobuf_protos/field_options.proto
new file mode 100644
index 0000000..5d15677
--- /dev/null
+++ b/pw_protobuf/pw_protobuf_protos/field_options.proto
@@ -0,0 +1,37 @@
+// Copyright 2022 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+syntax = "proto3";
+
+import "google/protobuf/descriptor.proto";
+
+package pw.protobuf;
+
+// Options supported for inline usage in .proto files.
+//
+// These options are for use when there are true limitations intended to be part
+// of the protocol specification and are not specific to any particular protobuf
+// code generator.
+message FieldOptions {
+ // Maximum number of entries for repeated fields.
+ optional int32 max_count = 1;
+
+ // Maximum size for bytes and string fields (excludes null terminator).
+ optional int32 max_size = 2;
+}
+
+extend google.protobuf.FieldOptions {
+ // Extension 1155 is reserved upstream for PW protobuf compiler.
+ // https://github.com/protocolbuffers/protobuf/blob/main/docs/options.md
+ optional FieldOptions pwpb = 1155;
+}
diff --git a/pw_protobuf/py/BUILD.bazel b/pw_protobuf/py/BUILD.bazel
index e273026..c366833 100644
--- a/pw_protobuf/py/BUILD.bazel
+++ b/pw_protobuf/py/BUILD.bazel
@@ -37,6 +37,7 @@
imports = ["."],
deps = [
"//pw_protobuf:codegen_protos_pb2",
+ "//pw_protobuf:field_options_proto_pb2",
"@com_google_protobuf//:protobuf_python",
],
)
diff --git a/pw_protobuf/py/BUILD.gn b/pw_protobuf/py/BUILD.gn
index 3fa3fe5..c9e4019 100644
--- a/pw_protobuf/py/BUILD.gn
+++ b/pw_protobuf/py/BUILD.gn
@@ -34,6 +34,7 @@
python_deps = [
"$dir_pw_cli/py",
"..:codegen_protos.python",
+ "..:common_protos.python",
]
pylintrc = "$dir_pigweed/.pylintrc"
mypy_ini = "$dir_pigweed/.mypy.ini"
diff --git a/pw_protobuf/py/pw_protobuf/options.py b/pw_protobuf/py/pw_protobuf/options.py
index 9a3654d..61cb6f1 100644
--- a/pw_protobuf/py/pw_protobuf/options.py
+++ b/pw_protobuf/py/pw_protobuf/options.py
@@ -21,6 +21,7 @@
from google.protobuf import text_format
from pw_protobuf_codegen_protos.codegen_options_pb2 import CodegenOptions
+from pw_protobuf_protos.field_options_pb2 import FieldOptions
_MULTI_LINE_COMMENT_RE = re.compile(r'/\*.*?\*/', flags=re.MULTILINE)
_SINGLE_LINE_COMMENT_RE = re.compile(r'//.*?$', flags=re.MULTILINE)
@@ -77,3 +78,37 @@
matched.MergeFrom(mask_options)
return matched
+
+
+def create_from_field_options(
+ field_options: FieldOptions,
+) -> CodegenOptions:
+ """Create a CodegenOptions from a FieldOptions."""
+ codegen_options = CodegenOptions()
+
+ if field_options.HasField('max_count'):
+ codegen_options.max_count = field_options.max_count
+
+ if field_options.HasField('max_size'):
+ codegen_options.max_size = field_options.max_size
+
+ return codegen_options
+
+
+def merge_field_and_codegen_options(
+ field_options: CodegenOptions, codegen_options: CodegenOptions
+) -> CodegenOptions:
+ """Merge inline field_options and options file codegen_options."""
+ # The field options specify protocol-level requirements. Therefore, any
+ # codegen options should not violate those protocol-level requirements.
+ if field_options.max_count > 0 and codegen_options.max_count > 0:
+ assert field_options.max_count == codegen_options.max_count
+
+ if field_options.max_size > 0 and codegen_options.max_size > 0:
+ assert field_options.max_size == codegen_options.max_size
+
+ merged_options = CodegenOptions()
+ merged_options.CopyFrom(field_options)
+ merged_options.MergeFrom(codegen_options)
+
+ return merged_options
diff --git a/pw_protobuf/py/pw_protobuf/proto_tree.py b/pw_protobuf/py/pw_protobuf/proto_tree.py
index 5617828..44c8804 100644
--- a/pw_protobuf/py/pw_protobuf/proto_tree.py
+++ b/pw_protobuf/py/pw_protobuf/proto_tree.py
@@ -33,6 +33,7 @@
from pw_protobuf import options, symbol_name_mapping
from pw_protobuf_codegen_protos.codegen_options_pb2 import CodegenOptions
+from pw_protobuf_protos.field_options_pb2 import pwpb as pwpb_field_options
T = TypeVar('T') # pylint: disable=invalid-name
@@ -667,6 +668,7 @@
repeated = (
field.label == descriptor_pb2.FieldDescriptorProto.LABEL_REPEATED
)
+
codegen_options = (
options.match_options(
'.'.join((message.proto_path(), field.name)), proto_options
@@ -674,6 +676,26 @@
if proto_options is not None
else None
)
+
+ field_options = (
+ options.create_from_field_options(
+ field.options.Extensions[pwpb_field_options]
+ )
+ if field.options.HasExtension(pwpb_field_options)
+ else None
+ )
+
+ merged_options = None
+
+ if field_options and codegen_options:
+ merged_options = options.merge_field_and_codegen_options(
+ field_options, codegen_options
+ )
+ elif field_options:
+ merged_options = field_options
+ elif codegen_options:
+ merged_options = codegen_options
+
message.add_field(
ProtoMessageField(
field.name,
@@ -682,7 +704,7 @@
type_node,
optional,
repeated,
- codegen_options,
+ merged_options,
)
)
diff --git a/pw_protobuf_compiler/BUILD.gn b/pw_protobuf_compiler/BUILD.gn
index 9305af5..22f097c 100644
--- a/pw_protobuf_compiler/BUILD.gn
+++ b/pw_protobuf_compiler/BUILD.gn
@@ -54,6 +54,7 @@
pw_proto_library("pwpb_test_protos") {
sources = [ "pw_protobuf_compiler_pwpb_protos/pwpb_test.proto" ]
inputs = [ "pw_protobuf_compiler_pwpb_protos/pwpb_test.options" ]
+ deps = [ "$dir_pw_protobuf:common_protos" ]
}
pw_proto_library("test_protos") {
diff --git a/pw_protobuf_compiler/pw_protobuf_compiler_pwpb_protos/BUILD.bazel b/pw_protobuf_compiler/pw_protobuf_compiler_pwpb_protos/BUILD.bazel
index 70c134c..280c01f 100644
--- a/pw_protobuf_compiler/pw_protobuf_compiler_pwpb_protos/BUILD.bazel
+++ b/pw_protobuf_compiler/pw_protobuf_compiler_pwpb_protos/BUILD.bazel
@@ -37,6 +37,9 @@
proto_library(
name = "pwpb_test_proto",
srcs = [":pwpb_test_proto_and_options_files"],
+ deps = [
+ "//pw_protobuf:field_options_proto",
+ ],
)
pw_proto_library(
diff --git a/pw_protobuf_compiler/pw_protobuf_compiler_pwpb_protos/pwpb_test.options b/pw_protobuf_compiler/pw_protobuf_compiler_pwpb_protos/pwpb_test.options
index 98bb8da..f7e21b9 100644
--- a/pw_protobuf_compiler/pw_protobuf_compiler_pwpb_protos/pwpb_test.options
+++ b/pw_protobuf_compiler/pw_protobuf_compiler_pwpb_protos/pwpb_test.options
@@ -17,3 +17,5 @@
pw.protobuf_compiler.OptionsFileExample.thirty_two_chars max_size:32
pw.protobuf_compiler.OptionsFileExample.forty_two_chars max_size:42
// The `max_size` of the string `unspecified_length` is of course unspecified.
+
+pw.protobuf_compiler.InlineOptionsExample.ten_chars_inline fixed_size:True
diff --git a/pw_protobuf_compiler/pw_protobuf_compiler_pwpb_protos/pwpb_test.proto b/pw_protobuf_compiler/pw_protobuf_compiler_pwpb_protos/pwpb_test.proto
index e176cb0..f93f5ee 100644
--- a/pw_protobuf_compiler/pw_protobuf_compiler_pwpb_protos/pwpb_test.proto
+++ b/pw_protobuf_compiler/pw_protobuf_compiler_pwpb_protos/pwpb_test.proto
@@ -15,6 +15,8 @@
package pw.protobuf_compiler;
+import "pw_protobuf_protos/field_options.proto";
+
message Point {
uint32 x = 1;
uint32 y = 2;
@@ -29,3 +31,7 @@
string forty_two_chars = 2;
string unspecified_length = 3;
};
+
+message InlineOptionsExample {
+ string ten_chars_inline = 1 [(pw.protobuf.pwpb).max_size = 10];
+}
diff --git a/pw_protobuf_compiler/pwpb_test.cc b/pw_protobuf_compiler/pwpb_test.cc
index a7d2d35..ae5ffa7 100644
--- a/pw_protobuf_compiler/pwpb_test.cc
+++ b/pw_protobuf_compiler/pwpb_test.cc
@@ -48,5 +48,14 @@
"The field `unspecified_length` should be a `pw::protobuf::Callback`.");
}
+TEST(Pwpb, InlineOptionsAppliedAndOverridden) {
+ pw::protobuf_compiler::InlineOptionsExample::Message inline_options_example;
+
+ static_assert(
+ std::is_same_v<decltype(inline_options_example.ten_chars_inline),
+ pw::InlineString<10>>,
+ "Field `ten_chars_inline` should be a `pw::InlineString<10>`.");
+}
+
} // namespace
} // namespace pw::protobuf_compiler
