Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support encode zero value in protobuf to adjust to go proto2 #274

Merged
merged 3 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 42 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[workspace]
members = ["pilota", "pilota-build", "pilota-thrift-parser"]
members = ["pilota", "pilota-build", "pilota-thrift-parser", "examples"]
resolver = "2"

[profile.bench]
Expand Down
58 changes: 58 additions & 0 deletions examples/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
[package]
name = "examples"
version = "0.1.0"
edition = "2021"
description = "Compile thrift and protobuf idl into rust code at compile-time."
homepage = "https://cloudwego.io/docs/pilota/"
repository = "https://github.com/cloudwego/pilota"
license = "MIT OR Apache-2.0"
authors = ["Pilota Team <pilota@cloudwego.io>"]
keywords = ["serialization", "thrift", "protobuf", "volo"]

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[badges]
maintenance = { status = "actively-developed" }

[dependencies]
pilota = { path = "../pilota", features = ["pb-encode-default-value"] }
pilota-build = { path = "../pilota-build" }
pilota-thrift-parser = { path = "../pilota-thrift-parser", version = "0.11" }

ahash = "0.8"
anyhow = "1"
dashmap = "5"
heck = "0.5"
itertools = "0.13"
normpath = "1"
paste = "1"
petgraph = "0.6"
phf = { version = "0.11", features = ["macros"] }
proc-macro2 = "1"
quote = "1"
rayon = "1"
rustc-hash = "1"
salsa = { version = "0.17.0-pre.2" }
scoped-tls = "1"
serde = { version = "1", features = ["derive"] }
serde_yaml = "0.9"
syn = "2"
toml = "0.8"
tracing = "0.1"
tracing-subscriber = { version = "0.3", features = ["env-filter"] }

# The official rust-protobuf parser currently has some bug.
# We will switch to the official one when https://github.com/stepancheg/rust-protobuf/pull/646 is fixed.
protobuf-parse = { package = "protobuf-parse2", version = "4.0.0-alpha.4" }
protobuf = { package = "protobuf2", version = "4.0.0-alpha.2" }
faststr = "0.2"

[dev-dependencies]

tokio = { version = "1", features = ["io-util"] }
derivative = "2"
tempfile = "3"
diffy = "0.4"
criterion = { version = "0.5", features = ["html_reports"] }
rand = "0.8"
linkedbytes = "0.1"
5 changes: 5 additions & 0 deletions examples/idl/zero_value.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
message A {
map<string, string> str_map = 1;
required string s1 = 2;
optional string s2 = 3;
}
33 changes: 33 additions & 0 deletions examples/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use pilota::prost::Message as _;

Check warning on line 1 in examples/src/lib.rs

View workflow job for this annotation

GitHub Actions / clippy

unused import: `pilota::prost::Message as _`

warning: unused import: `pilota::prost::Message as _` --> examples/src/lib.rs:1:5 | 1 | use pilota::prost::Message as _; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default

mod zero_value;

#[test]
fn test_pb_encode_zero_value() {
let test_data = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("idl")
.join("zero_value.proto");

let out_path = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("src")
.join("zero_value.rs");

pilota_build::Builder::protobuf()
.ignore_unused(false)
.include_dirs(vec![test_data.parent().unwrap().to_path_buf()])
.compile_with_config(
vec![pilota_build::IdlService::from_path(test_data.to_path_buf())],
pilota_build::Output::File(out_path.into()),
);

let mut a = zero_value::zero_value::A::default();

a.str_map.insert("key1".into(), "value".into());
a.str_map.insert("key2".into(), "".into());

let mut buf = pilota::BytesMut::new();
a.encode(&mut buf).unwrap();

println!("{:?}", buf);
println!("{:?}", buf.freeze().as_ref());
}
102 changes: 102 additions & 0 deletions examples/src/zero_value.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
pub mod zero_value {
#![allow(warnings, clippy::all)]
#[derive(Debug, Default, Clone, PartialEq)]
pub struct A {
pub str_map: ::pilota::AHashMap<::pilota::FastStr, ::pilota::FastStr>,

pub s1: ::pilota::FastStr,

pub s2: ::std::option::Option<::pilota::FastStr>,
}
impl ::pilota::prost::Message for A {
#[inline]
fn encoded_len(&self) -> usize {
0 + ::pilota::prost::encoding::hash_map::encoded_len(
::pilota::prost::encoding::faststr::encoded_len,
::pilota::prost::encoding::faststr::encoded_len,
1,
&self.str_map,
) + ::pilota::prost::encoding::faststr::encoded_len(2, &self.s1)
+ self.s2.as_ref().map_or(0, |value| {
::pilota::prost::encoding::faststr::encoded_len(3, value)
})
}

#[allow(unused_variables)]
fn encode_raw<B>(&self, buf: &mut B)
where
B: ::pilota::prost::bytes::BufMut,
{
::pilota::prost::encoding::hash_map::encode(
::pilota::prost::encoding::faststr::encode,
::pilota::prost::encoding::faststr::encoded_len,
::pilota::prost::encoding::faststr::encode,
::pilota::prost::encoding::faststr::encoded_len,
1,
&self.str_map,
buf,
);
::pilota::prost::encoding::faststr::encode(2, &self.s1, buf);
if let Some(_pilota_inner_value) = self.s2.as_ref() {
::pilota::prost::encoding::faststr::encode(3, _pilota_inner_value, buf);
};
}

#[allow(unused_variables)]
fn merge_field<B>(
&mut self,
tag: u32,
wire_type: ::pilota::prost::encoding::WireType,
buf: &mut B,
ctx: ::pilota::prost::encoding::DecodeContext,
) -> ::core::result::Result<(), ::pilota::prost::DecodeError>
where
B: ::pilota::prost::bytes::Buf,
{
const STRUCT_NAME: &'static str = stringify!(A);
match tag {
1 => {
let mut _inner_pilota_value = &mut self.str_map;
::pilota::prost::encoding::hash_map::merge(
::pilota::prost::encoding::faststr::merge,
::pilota::prost::encoding::faststr::merge,
&mut _inner_pilota_value,
buf,
ctx,
)
.map_err(|mut error| {
error.push(STRUCT_NAME, stringify!(str_map));
error
})
}
2 => {
let mut _inner_pilota_value = &mut self.s1;
::pilota::prost::encoding::faststr::merge(
wire_type,
_inner_pilota_value,
buf,
ctx,
)
.map_err(|mut error| {
error.push(STRUCT_NAME, stringify!(s1));
error
})
}
3 => {
let mut _inner_pilota_value = &mut self.s2;
::pilota::prost::encoding::faststr::merge(
wire_type,
_inner_pilota_value.get_or_insert_with(::core::default::Default::default),
buf,
ctx,
)
.map_err(|mut error| {
error.push(STRUCT_NAME, stringify!(s2));
error
})
}
_ => ::pilota::prost::encoding::skip_field(wire_type, tag, buf, ctx),
}
}
}
}
3 changes: 2 additions & 1 deletion pilota/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "pilota"
version = "0.11.3"
version = "0.11.4"
edition = "2021"
description = "Pilota is a thrift and protobuf implementation in pure rust with high performance and extensibility."
documentation = "https://docs.rs/pilota"
Expand Down Expand Up @@ -41,6 +41,7 @@ rand = "0.8"

[features]
unstable = []
pb-encode-default-value = []

[[bench]]
name = "faststr"
Expand Down
10 changes: 8 additions & 2 deletions pilota/src/prost/encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
extern crate alloc;

use alloc::{collections::BTreeMap, format, string::String, vec::Vec};
use core::{cmp::min, convert::TryFrom, mem, str, u32, usize};

Check warning on line 9 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

importing legacy numeric constants

warning: importing legacy numeric constants --> pilota/src/prost/encoding.rs:9:55 | 9 | use core::{cmp::min, convert::TryFrom, mem, str, u32, usize}; | ^^^^^ | = help: remove this import = note: then `usize::<CONST>` will resolve to the respective associated constant = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants

Check warning on line 9 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

importing legacy numeric constants

warning: importing legacy numeric constants --> pilota/src/prost/encoding.rs:9:50 | 9 | use core::{cmp::min, convert::TryFrom, mem, str, u32, usize}; | ^^^ | = help: remove this import = note: then `u32::<CONST>` will resolve to the respective associated constant = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants

use ::bytes::{Buf, BufMut, Bytes};

Expand Down Expand Up @@ -185,7 +185,7 @@
/// The context should be passed by value and can be freely cloned. When passing
/// to a function which is decoding a nested object, then use `enter_recursion`.
#[derive(Clone, Debug)]
#[cfg_attr(feature = "no-recursion-limit", derive(Default))]

Check warning on line 188 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

unexpected `cfg` condition value: `no-recursion-limit`

warning: unexpected `cfg` condition value: `no-recursion-limit` --> pilota/src/prost/encoding.rs:188:12 | 188 | #[cfg_attr(feature = "no-recursion-limit", derive(Default))] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: expected values for `feature` are: `pb-encode-default-value` and `unstable` = help: consider adding `no-recursion-limit` as a feature in `Cargo.toml` = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
pub struct DecodeContext {
/// How many times we can recurse in the current decode stack before we hit
/// the recursion limit.
Expand All @@ -193,11 +193,11 @@
/// The recursion limit is defined by `RECURSION_LIMIT` and cannot be
/// customized. The recursion limit can be ignored by building the Prost
/// crate with the `no-recursion-limit` feature.
#[cfg(not(feature = "no-recursion-limit"))]

Check warning on line 196 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

unexpected `cfg` condition value: `no-recursion-limit`

warning: unexpected `cfg` condition value: `no-recursion-limit` --> pilota/src/prost/encoding.rs:196:15 | 196 | #[cfg(not(feature = "no-recursion-limit"))] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: expected values for `feature` are: `pb-encode-default-value` and `unstable` = help: consider adding `no-recursion-limit` as a feature in `Cargo.toml` = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
recurse_count: u32,
}

#[cfg(not(feature = "no-recursion-limit"))]

Check warning on line 200 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

unexpected `cfg` condition value: `no-recursion-limit`

warning: unexpected `cfg` condition value: `no-recursion-limit` --> pilota/src/prost/encoding.rs:200:11 | 200 | #[cfg(not(feature = "no-recursion-limit"))] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: expected values for `feature` are: `pb-encode-default-value` and `unstable` = help: consider adding `no-recursion-limit` as a feature in `Cargo.toml` = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
impl Default for DecodeContext {
#[inline]
fn default() -> DecodeContext {
Expand All @@ -214,7 +214,7 @@
/// `DecodeContext` to be used at the next level of recursion. Continue
/// to use the old context
// at the previous level of recursion.
#[cfg(not(feature = "no-recursion-limit"))]

Check warning on line 217 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

unexpected `cfg` condition value: `no-recursion-limit`

warning: unexpected `cfg` condition value: `no-recursion-limit` --> pilota/src/prost/encoding.rs:217:15 | 217 | #[cfg(not(feature = "no-recursion-limit"))] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: expected values for `feature` are: `pb-encode-default-value` and `unstable` = help: consider adding `no-recursion-limit` as a feature in `Cargo.toml` = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
#[inline]
pub(crate) fn enter_recursion(&self) -> DecodeContext {
DecodeContext {
Expand All @@ -222,7 +222,7 @@
}
}

#[cfg(feature = "no-recursion-limit")]

Check warning on line 225 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

unexpected `cfg` condition value: `no-recursion-limit`

warning: unexpected `cfg` condition value: `no-recursion-limit` --> pilota/src/prost/encoding.rs:225:11 | 225 | #[cfg(feature = "no-recursion-limit")] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: expected values for `feature` are: `pb-encode-default-value` and `unstable` = help: consider adding `no-recursion-limit` as a feature in `Cargo.toml` = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
#[inline]
pub(crate) fn enter_recursion(&self) -> DecodeContext {
DecodeContext {}
Expand All @@ -233,7 +233,7 @@
///
/// Returns `Ok<()>` if it is ok to continue recursing.
/// Returns `Err<DecodeError>` if the recursion limit has been reached.
#[cfg(not(feature = "no-recursion-limit"))]

Check warning on line 236 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

unexpected `cfg` condition value: `no-recursion-limit`

warning: unexpected `cfg` condition value: `no-recursion-limit` --> pilota/src/prost/encoding.rs:236:15 | 236 | #[cfg(not(feature = "no-recursion-limit"))] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: expected values for `feature` are: `pb-encode-default-value` and `unstable` = help: consider adding `no-recursion-limit` as a feature in `Cargo.toml` = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
#[inline]
pub(crate) fn limit_reached(&self) -> Result<(), DecodeError> {
if self.recurse_count == 0 {
Expand All @@ -243,7 +243,7 @@
}
}

#[cfg(feature = "no-recursion-limit")]

Check warning on line 246 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

unexpected `cfg` condition value: `no-recursion-limit`

warning: unexpected `cfg` condition value: `no-recursion-limit` --> pilota/src/prost/encoding.rs:246:11 | 246 | #[cfg(feature = "no-recursion-limit")] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: expected values for `feature` are: `pb-encode-default-value` and `unstable` = help: consider adding `no-recursion-limit` as a feature in `Cargo.toml` = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
#[inline]
#[allow(clippy::unnecessary_wraps)] // needed in other features
pub(crate) fn limit_reached(&self) -> Result<(), DecodeError> {
Expand Down Expand Up @@ -952,7 +952,7 @@
// guard is used.
unsafe {
struct DropGuard<'a>(&'a mut Vec<u8>);
impl<'a> Drop for DropGuard<'a> {

Check warning on line 955 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

the following explicit lifetimes could be elided: 'a

warning: the following explicit lifetimes could be elided: 'a --> pilota/src/prost/encoding.rs:955:18 | 955 | impl<'a> Drop for DropGuard<'a> { | ^^ ^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes = note: `#[warn(clippy::needless_lifetimes)]` on by default help: elide the lifetimes | 955 - impl<'a> Drop for DropGuard<'a> { 955 + impl Drop for DropGuard<'_> { |
#[inline]
fn drop(&mut self) {
self.0.clear();
Expand Down Expand Up @@ -1579,8 +1579,14 @@
VL: Fn(u32, &V) -> usize,
{
for (key, val) in values.iter() {
let skip_key = key == &K::default();
let skip_val = val == val_default;
let mut skip_key = key == &K::default();

Check warning on line 1582 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

variable does not need to be mutable

warning: variable does not need to be mutable --> pilota/src/prost/encoding.rs:1582:21 | 1582 | let mut skip_key = key == &K::default(); | ----^^^^^^^^ | | | help: remove this `mut` ... 1687 | map!(BTreeMap); | -------------- in this macro invocation | = note: this warning originates in the macro `map` (in Nightly builds, run with -Z macro-backtrace for more info)

Check warning on line 1582 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

variable does not need to be mutable

warning: variable does not need to be mutable --> pilota/src/prost/encoding.rs:1582:21 | 1582 | let mut skip_key = key == &K::default(); | ----^^^^^^^^ | | | help: remove this `mut` ... 1683 | map!(AHashMap); | -------------- in this macro invocation | = note: `#[warn(unused_mut)]` on by default = note: this warning originates in the macro `map` (in Nightly builds, run with -Z macro-backtrace for more info)
let mut skip_val = val == val_default;

Check warning on line 1583 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

variable does not need to be mutable

warning: variable does not need to be mutable --> pilota/src/prost/encoding.rs:1583:21 | 1583 | let mut skip_val = val == val_default; | ----^^^^^^^^ | | | help: remove this `mut` ... 1687 | map!(BTreeMap); | -------------- in this macro invocation | = note: this warning originates in the macro `map` (in Nightly builds, run with -Z macro-backtrace for more info)

Check warning on line 1583 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

variable does not need to be mutable

warning: variable does not need to be mutable --> pilota/src/prost/encoding.rs:1583:21 | 1583 | let mut skip_val = val == val_default; | ----^^^^^^^^ | | | help: remove this `mut` ... 1683 | map!(AHashMap); | -------------- in this macro invocation | = note: this warning originates in the macro `map` (in Nightly builds, run with -Z macro-backtrace for more info)

#[cfg(feature = "pb-encode-zero-value")]

Check warning on line 1585 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

unexpected `cfg` condition value: `pb-encode-zero-value`

warning: unexpected `cfg` condition value: `pb-encode-zero-value` --> pilota/src/prost/encoding.rs:1585:23 | 1585 | #[cfg(feature = "pb-encode-zero-value")] | ^^^^^^^^^^---------------------- | | | help: there is a expected value with a similar name: `"pb-encode-default-value"` ... 1687 | map!(BTreeMap); | -------------- in this macro invocation | = note: expected values for `feature` are: `pb-encode-default-value` and `unstable` = help: consider adding `pb-encode-zero-value` as a feature in `Cargo.toml` = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration = note: this warning originates in the macro `map` (in Nightly builds, run with -Z macro-backtrace for more info)

Check warning on line 1585 in pilota/src/prost/encoding.rs

View workflow job for this annotation

GitHub Actions / clippy

unexpected `cfg` condition value: `pb-encode-zero-value`

warning: unexpected `cfg` condition value: `pb-encode-zero-value` --> pilota/src/prost/encoding.rs:1585:23 | 1585 | #[cfg(feature = "pb-encode-zero-value")] | ^^^^^^^^^^---------------------- | | | help: there is a expected value with a similar name: `"pb-encode-default-value"` ... 1683 | map!(AHashMap); | -------------- in this macro invocation | = note: expected values for `feature` are: `pb-encode-default-value` and `unstable` = help: consider adding `pb-encode-zero-value` as a feature in `Cargo.toml` = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration = note: this warning originates in the macro `map` (in Nightly builds, run with -Z macro-backtrace for more info)
{
skip_key = false;
skip_val = false;
}

let len = (if skip_key { 0 } else { key_encoded_len(1, key) })
+ (if skip_val { 0 } else { val_encoded_len(2, val) });
Expand Down
Loading