diff --git a/README.md b/README.md index d5de8a649..c99219d31 100644 --- a/README.md +++ b/README.md @@ -107,10 +107,91 @@ Scalar value types are converted as follows: #### Enumerations All `.proto` enumeration types convert to the Rust `i32` type. Additionally, -each enumeration type gets a corresponding Rust `enum` type, with helper methods -to convert `i32` values to the enum type. The `enum` type isn't used directly as -a field, because the Protobuf spec mandates that enumerations values are 'open', -and decoding unrecognized enumeration values must be possible. +each enumeration type gets a corresponding Rust `enum` type. For example, this +`proto` enum: + +```proto +enum PhoneType { + MOBILE = 0; + HOME = 1; + WORK = 2; +} +``` + +gets this corresponding Rust enum [1]: + +```rust +pub enum PhoneType { + Mobile = 0, + Home = 1, + Work = 2, +} +``` + +You can convert a `PhoneType` value to an `i32` by doing: + +```rust +PhoneType::Mobile as i32 +``` + +The `#[derive(::prost::Enumeration)]` annotation added to the generated +`PhoneType` adds these associated functions to the type: + +```rust +impl PhoneType { + pub fn is_valid(value: i32) -> bool { ... } + pub fn from_i32(value: i32) -> Option { ... } +} +``` + +so you can convert an `i32` to its corresponding `PhoneType` value by doing, +for example: + +```rust +let phone_type = 2i32; + +match PhoneType::from_i32(phone_type) { + Some(PhoneType::Mobile) => ..., + Some(PhoneType::Home) => ..., + Some(PhoneType::Work) => ..., + None => ..., +} +``` + +Additionally, wherever a `proto` enum is used as a field in a `Message`, the +message will have 'accessor' methods to get/set the value of the field as the +Rust enum type. For instance, this proto `PhoneNumber` message that has a field +named `type` of type `PhoneType`: + +```proto +message PhoneNumber { + string number = 1; + PhoneType type = 2; +} +``` + +will become the following Rust type [1] with methods `type` and `set_type`: + +```rust +pub struct PhoneNumber { + pub number: String, + pub r#type: i32, // the `r#` is needed because `type` is a Rust keyword +} + +impl PhoneNumber { + pub fn r#type(&self) -> PhoneType { ... } + pub fn set_type(&mut self, value: PhoneType) { ... } +} +``` + +Note that the getter methods will return the Rust enum's default value if the +field has an invalid `i32` value. + +The `enum` type isn't used directly as a field, because the Protobuf spec +mandates that enumerations values are 'open', and decoding unrecognized +enumeration values must be possible. + +[1] Annotations have been elided for clarity. See below for a full example. #### Field Modifiers @@ -215,27 +296,29 @@ message AddressBook { and the generated Rust code (`tutorial.rs`): ```rust -#[derive(Clone, Debug, PartialEq, Message)] +#[derive(Clone, PartialEq, ::prost::Message)] pub struct Person { #[prost(string, tag="1")] - pub name: String, + pub name: ::prost::alloc::string::String, /// Unique ID number for this person. #[prost(int32, tag="2")] pub id: i32, #[prost(string, tag="3")] - pub email: String, + pub email: ::prost::alloc::string::String, #[prost(message, repeated, tag="4")] - pub phones: Vec, + pub phones: ::prost::alloc::vec::Vec, } +/// Nested message and enum types in `Person`. pub mod person { - #[derive(Clone, Debug, PartialEq, Message)] + #[derive(Clone, PartialEq, ::prost::Message)] pub struct PhoneNumber { #[prost(string, tag="1")] - pub number: String, + pub number: ::prost::alloc::string::String, #[prost(enumeration="PhoneType", tag="2")] - pub type_: i32, + pub r#type: i32, } - #[derive(Clone, Copy, Debug, PartialEq, Eq, Enumeration)] + #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, ::prost::Enumeration)] + #[repr(i32)] pub enum PhoneType { Mobile = 0, Home = 1, @@ -243,10 +326,10 @@ pub mod person { } } /// Our address book file is just one of these. -#[derive(Clone, Debug, PartialEq, Message)] +#[derive(Clone, PartialEq, ::prost::Message)] pub struct AddressBook { #[prost(message, repeated, tag="1")] - pub people: Vec, + pub people: ::prost::alloc::vec::Vec, } ``` @@ -269,7 +352,7 @@ prost = { version = "0.6", default-features = false, features = ["prost-derive"] prost-types = { version = "0.6", default-features = false } ``` -Additionally, configure `prost-buid` to output `BTreeMap`s instead of `HashMap`s +Additionally, configure `prost-build` to output `BTreeMap`s instead of `HashMap`s for all Protobuf `map` fields in your `build.rs`: ```rust @@ -349,7 +432,7 @@ pub enum Gender { There are two complications with trying to serialize Protobuf messages with Serde: - - Protobuf fields require a numbered tag, and curently there appears to be no + - Protobuf fields require a numbered tag, and currently there appears to be no mechanism suitable for this in `serde`. - The mapping of Protobuf type to Rust type is not 1-to-1. As a result, trait-based approaches to dispatching don't work very well. Example: six diff --git a/benches/varint.rs b/benches/varint.rs index b2bc8ca37..34951f5ea 100644 --- a/benches/varint.rs +++ b/benches/varint.rs @@ -1,13 +1,14 @@ use std::mem; use bytes::Buf; -use criterion::{Benchmark, Criterion, Throughput}; +use criterion::{Criterion, Throughput}; use prost::encoding::{decode_varint, encode_varint, encoded_len_varint}; use rand::{rngs::StdRng, seq::SliceRandom, SeedableRng}; fn benchmark_varint(criterion: &mut Criterion, name: &str, mut values: Vec) { // Shuffle the values in a stable order. values.shuffle(&mut StdRng::seed_from_u64(0)); + let name = format!("varint/{}", name); let encoded_len = values .iter() @@ -16,53 +17,58 @@ fn benchmark_varint(criterion: &mut Criterion, name: &str, mut values: Vec) .sum::() as u64; let decoded_len = (values.len() * mem::size_of::()) as u64; - let encode_values = values.clone(); - let encode = Benchmark::new("encode", move |b| { - let mut buf = Vec::::with_capacity(encode_values.len() * 10); - b.iter(|| { - buf.clear(); - for &value in &encode_values { - encode_varint(value, &mut buf); + criterion + .benchmark_group(&name) + .bench_function("encode", { + let encode_values = values.clone(); + move |b| { + let mut buf = Vec::::with_capacity(encode_values.len() * 10); + b.iter(|| { + buf.clear(); + for &value in &encode_values { + encode_varint(value, &mut buf); + } + criterion::black_box(&buf); + }) } - criterion::black_box(&buf); }) - }) - .throughput(Throughput::Bytes(encoded_len)); + .throughput(Throughput::Bytes(encoded_len)); - let decode_values = values.clone(); - let decode = Benchmark::new("decode", move |b| { - let mut buf = Vec::with_capacity(decode_values.len() * 10); - for &value in &decode_values { - encode_varint(value, &mut buf); - } + criterion + .benchmark_group(&name) + .bench_function("decode", { + let decode_values = values.clone(); - b.iter(|| { - let mut buf = &mut buf.as_slice(); - while buf.has_remaining() { - let result = decode_varint(&mut buf); - debug_assert!(result.is_ok()); - criterion::black_box(&result); - } - }) - }) - .throughput(Throughput::Bytes(decoded_len)); + move |b| { + let mut buf = Vec::with_capacity(decode_values.len() * 10); + for &value in &decode_values { + encode_varint(value, &mut buf); + } - let encoded_len = Benchmark::new("encoded_len", move |b| { - b.iter(|| { - let mut sum = 0; - for &value in &values { - sum += encoded_len_varint(value); + b.iter(|| { + let mut buf = &mut buf.as_slice(); + while buf.has_remaining() { + let result = decode_varint(&mut buf); + debug_assert!(result.is_ok()); + criterion::black_box(&result); + } + }) } - criterion::black_box(sum); }) - }) - .throughput(Throughput::Bytes(decoded_len)); + .throughput(Throughput::Bytes(decoded_len)); - let name = format!("varint/{}", name); criterion - .bench(&name, encode) - .bench(&name, decode) - .bench(&name, encoded_len); + .benchmark_group(&name) + .bench_function("encoded_len", move |b| { + b.iter(|| { + let mut sum = 0; + for &value in &values { + sum += encoded_len_varint(value); + } + criterion::black_box(sum); + }) + }) + .throughput(Throughput::Bytes(decoded_len)); } fn main() { diff --git a/prost-build/Cargo.toml b/prost-build/Cargo.toml index 139a61240..536e316f6 100644 --- a/prost-build/Cargo.toml +++ b/prost-build/Cargo.toml @@ -12,7 +12,7 @@ edition = "2018" [dependencies] bytes = { version = "1", default-features = false } heck = "0.3" -itertools = "0.9" +itertools = "0.10" log = "0.4" multimap = { version = "0.8", default-features = false } petgraph = { version = "0.5", default-features = false } diff --git a/prost-build/src/code_generator.rs b/prost-build/src/code_generator.rs index af50c28c4..156f006dc 100644 --- a/prost-build/src/code_generator.rs +++ b/prost-build/src/code_generator.rs @@ -237,12 +237,12 @@ impl<'a> CodeGenerator<'a> { for (idx, oneof) in message.oneof_decl.into_iter().enumerate() { let idx = idx as i32; - self.append_oneof( - &fq_message_name, - oneof, - idx, - oneof_fields.remove(&idx).unwrap(), - ); + // optional fields create a synthetic oneof that we want to skip + let fields = match oneof_fields.remove(&idx) { + Some(fields) => fields, + None => continue, + }; + self.append_oneof(&fq_message_name, oneof, idx, fields); } self.pop_mod(); diff --git a/prost-build/src/lib.rs b/prost-build/src/lib.rs index 6353b7d0d..041e350e2 100644 --- a/prost-build/src/lib.rs +++ b/prost-build/src/lib.rs @@ -51,10 +51,10 @@ //! `build.rs` build-script: //! //! ```rust,no_run -//! # use std::io::Result; +//! use std::io::Result; //! fn main() -> Result<()> { -//! prost_build::compile_protos(&["src/items.proto"], &["src/"])?; -//! Ok(()) +//! prost_build::compile_protos(&["src/items.proto"], &["src/"])?; +//! Ok(()) //! } //! ``` //! diff --git a/prost-derive/Cargo.toml b/prost-derive/Cargo.toml index 2460bd972..0c0b747ec 100644 --- a/prost-derive/Cargo.toml +++ b/prost-derive/Cargo.toml @@ -14,7 +14,7 @@ proc_macro = true [dependencies] anyhow = "1" -itertools = "0.9" +itertools = "0.10" proc-macro2 = "1" quote = "1" syn = { version = "1", features = [ "extra-traits" ] } diff --git a/prost-derive/src/field/mod.rs b/prost-derive/src/field/mod.rs index 9e2e70c4c..09fef830e 100644 --- a/prost-derive/src/field/mod.rs +++ b/prost-derive/src/field/mod.rs @@ -32,7 +32,7 @@ impl Field { /// If the meta items are invalid, an error will be returned. /// If the field should be ignored, `None` is returned. pub fn new(attrs: Vec, inferred_tag: Option) -> Result, Error> { - let attrs = prost_attrs(attrs)?; + let attrs = prost_attrs(attrs); // TODO: check for ignore attribute. @@ -58,7 +58,7 @@ impl Field { /// If the meta items are invalid, an error will be returned. /// If the field should be ignored, `None` is returned. pub fn new_oneof(attrs: Vec) -> Result, Error> { - let attrs = prost_attrs(attrs)?; + let attrs = prost_attrs(attrs); // TODO: check for ignore attribute. @@ -224,8 +224,8 @@ impl fmt::Display for Label { } /// Get the items belonging to the 'prost' list attribute, e.g. `#[prost(foo, bar="baz")]`. -pub(super) fn prost_attrs(attrs: Vec) -> Result, Error> { - Ok(attrs +fn prost_attrs(attrs: Vec) -> Vec { + attrs .iter() .flat_map(Attribute::parse_meta) .flat_map(|meta| match meta { @@ -244,7 +244,7 @@ pub(super) fn prost_attrs(attrs: Vec) -> Result, Error> { NestedMeta::Lit(lit) => bail!("invalid prost attribute: {:?}", lit), } }) - .collect()) + .collect() } pub fn set_option(option: &mut Option, value: T, message: &str) -> Result<(), Error> diff --git a/protobuf/benches/dataset.rs b/protobuf/benches/dataset.rs index 4572b8c55..847008555 100644 --- a/protobuf/benches/dataset.rs +++ b/protobuf/benches/dataset.rs @@ -84,9 +84,6 @@ dataset!(google_message1_proto2, proto2::GoogleMessage1); dataset!(google_message1_proto3, proto3::GoogleMessage1); dataset!(google_message2, proto2::GoogleMessage2); dataset!(google_message3_1, GoogleMessage3); -dataset!(google_message3_2, GoogleMessage3); -dataset!(google_message3_3, GoogleMessage3); -dataset!(google_message3_4, GoogleMessage3); dataset!(google_message3_5, GoogleMessage3); dataset!(google_message4, GoogleMessage4); @@ -95,9 +92,6 @@ criterion_group!( google_message1_proto2, google_message1_proto3, google_message2, - google_message3_2, - google_message3_3, - google_message3_4, ); criterion_group! { diff --git a/protobuf/build.rs b/protobuf/build.rs index 1f025e0f8..b0c5694d2 100644 --- a/protobuf/build.rs +++ b/protobuf/build.rs @@ -270,8 +270,5 @@ fn install_datasets(src_dir: &Path, prefix_dir: &Path) -> Result<()> { .with_context(|| format!("failed to move {}", dataset.display()))?; } - download_tarball( - "https://storage.googleapis.com/protobuf_opensource_benchmark_data/datasets.tar.gz", - share_dir, - ) + Ok(()) } diff --git a/protobuf/src/lib.rs b/protobuf/src/lib.rs index db083e6ce..819708e6b 100644 --- a/protobuf/src/lib.rs +++ b/protobuf/src/lib.rs @@ -34,27 +34,6 @@ pub mod benchmarks { )) } - pub fn google_message3_2() -> &'static Path { - Path::new(concat!( - env!("PROTOBUF"), - "/share/dataset.google_message3_2.pb" - )) - } - - pub fn google_message3_3() -> &'static Path { - Path::new(concat!( - env!("PROTOBUF"), - "/share/dataset.google_message3_3.pb" - )) - } - - pub fn google_message3_4() -> &'static Path { - Path::new(concat!( - env!("PROTOBUF"), - "/share/dataset.google_message3_4.pb" - )) - } - pub fn google_message3_5() -> &'static Path { Path::new(concat!( env!("PROTOBUF"), diff --git a/src/encoding.rs b/src/encoding.rs index c599f662d..252358685 100644 --- a/src/encoding.rs +++ b/src/encoding.rs @@ -259,6 +259,7 @@ impl DecodeContext { #[cfg(feature = "no-recursion-limit")] #[inline] + #[allow(clippy::unnecessary_wraps)] // needed in other features pub(crate) fn limit_reached(&self) -> Result<(), DecodeError> { Ok(()) } @@ -990,12 +991,7 @@ pub mod bytes { // > last value it sees. // // [1]: https://developers.google.com/protocol-buffers/docs/encoding#optional - - // NOTE: The use of BufExt::take() currently prevents zero-copy decoding - // for bytes fields backed by Bytes when docoding from Bytes. This could - // be addressed in the future by specialization. - // See also: https://github.com/tokio-rs/bytes/issues/374 - value.replace_with(buf.take(len)); + value.replace_with(buf.copy_to_bytes(len)); Ok(()) } diff --git a/src/error.rs b/src/error.rs index 73bf197ca..fc098299c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -30,8 +30,9 @@ impl DecodeError { /// Creates a new `DecodeError` with a 'best effort' root cause description. /// /// Meant to be used only by `Message` implementations. + #[doc(hidden)] #[cold] - pub(crate) fn new(description: impl Into>) -> DecodeError { + pub fn new(description: impl Into>) -> DecodeError { DecodeError { inner: Box::new(Inner { description: description.into(), diff --git a/tests/src/lib.rs b/tests/src/lib.rs index b20bc49bc..f56d73e7a 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -597,7 +597,10 @@ mod tests { #[test] fn test_proto3_presence() { - let msg = proto3::presence::A { b: Some(42) }; + let msg = proto3::presence::A { + b: Some(42), + foo: Some(proto3::presence::a::Foo::C(13)), + }; check_message(&msg); } diff --git a/tests/src/proto3_presence.proto b/tests/src/proto3_presence.proto index d47decce3..1e12c9bcb 100644 --- a/tests/src/proto3_presence.proto +++ b/tests/src/proto3_presence.proto @@ -4,4 +4,8 @@ package proto3.presence; message A { optional int32 b = 1; + + oneof foo { + int32 c = 2; + } }