From 31153de546165b36ef649b0a5f70b88851537695 Mon Sep 17 00:00:00 2001 From: Cary Yang Date: Wed, 1 Dec 2021 16:52:38 -0800 Subject: [PATCH] Avoid extra logic for oneofs with only 1 possible value --- CHANGELOG.md | 2 +- pb-jelly-gen/codegen/codegen.py | 33 +++-- .../proto_pbtest/src/pbtest2.rs.expected | 124 ++++++++++++++++++ pb-test/proto/packages/pbtest/pbtest2.proto | 7 + 4 files changed, 152 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a736f6..9b31146 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,5 @@ # Unreleased -... everything has been released! +* Avoid extra logic for oneofs with only 1 possible value. (#133) # 0.0.10 ### November 30, 2021 diff --git a/pb-jelly-gen/codegen/codegen.py b/pb-jelly-gen/codegen/codegen.py index 3f0721f..cef8732 100755 --- a/pb-jelly-gen/codegen/codegen.py +++ b/pb-jelly-gen/codegen/codegen.py @@ -1382,21 +1382,28 @@ def gen_msg( typ = self.rust_type(msg_type, field) with block(self, '"%s" =>' % field.name): if typ.oneof: - with block(self, "match self.%s" % typ.oneof.name): - self.write("%s => ()," % typ.oneof_val(name, "_")) - with block(self, "_ =>", start=" {", end="},"): - # If this oneof is not currently set to this variant, we explicitly - # set it to this variant. + if len( + oneof_fields[typ.oneof.name] + ) > 1 or oneof_nullable(typ.oneof): + # Only useful to generate this logic if there is more than one + # possible value for this oneof. + with block(self, "match self.%s" % typ.oneof.name): self.write( - "self.%s = %s;" - % ( - typ.oneof.name, - typ.oneof_val( - name, - "::std::default::Default::default()", - ), - ) + "%s => ()," % typ.oneof_val(name, "_") ) + with block(self, "_ =>", start=" {", end="},"): + # If this oneof is not currently set to this variant, we explicitly + # set it to this variant. + self.write( + "self.%s = %s;" + % ( + typ.oneof.name, + typ.oneof_val( + name, + "::std::default::Default::default()", + ), + ) + ) if typ.is_empty_oneof_field(): self.write( "return ::pb_jelly::reflection::FieldMut::Empty;" diff --git a/pb-test/gen/pb-jelly/proto_pbtest/src/pbtest2.rs.expected b/pb-test/gen/pb-jelly/proto_pbtest/src/pbtest2.rs.expected index a1c0831..3c19aec 100644 --- a/pb-test/gen/pb-jelly/proto_pbtest/src/pbtest2.rs.expected +++ b/pb-test/gen/pb-jelly/proto_pbtest/src/pbtest2.rs.expected @@ -698,6 +698,130 @@ impl ::pb_jelly::Reflection for String { } } +#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub struct Version0OneOfNoneNullable { + pub test_oneof: Version0OneOfNoneNullable_TestOneof, +} +#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub enum Version0OneOfNoneNullable_TestOneof { + StringOneOf(::std::string::String), +} +impl Version0OneOfNoneNullable { +} +impl ::std::default::Default for Version0OneOfNoneNullable { + fn default() -> Self { + Version0OneOfNoneNullable { + test_oneof: Version0OneOfNoneNullable_TestOneof::StringOneOf(::std::default::Default::default()), + } + } +} +lazy_static! { + pub static ref Version0OneOfNoneNullable_default: Version0OneOfNoneNullable = Version0OneOfNoneNullable::default(); +} +impl ::pb_jelly::Message for Version0OneOfNoneNullable { + fn descriptor(&self) -> ::std::option::Option<::pb_jelly::MessageDescriptor> { + Some(::pb_jelly::MessageDescriptor { + name: "Version0OneOfNoneNullable", + full_name: "pbtest.Version0OneOfNoneNullable", + fields: &[ + ::pb_jelly::FieldDescriptor { + name: "string_one_of", + full_name: "pbtest.Version0OneOfNoneNullable.string_one_of", + index: 0, + number: 1, + typ: ::pb_jelly::wire_format::Type::LengthDelimited, + label: ::pb_jelly::Label::Optional, + oneof_index: Some(0), + }, + ], + oneofs: &[ + ::pb_jelly::OneofDescriptor { + name: "test_oneof", + }, + ], + }) + } + fn compute_size(&self) -> usize { + let mut size = 0; + let mut string_one_of_size = 0; + if let Version0OneOfNoneNullable_TestOneof::StringOneOf(ref val) = self.test_oneof { + let l = ::pb_jelly::Message::compute_size(val); + string_one_of_size += ::pb_jelly::wire_format::serialized_length(1); + string_one_of_size += ::pb_jelly::varint::serialized_length(l as u64); + string_one_of_size += l; + } + size += string_one_of_size; + size + } + fn compute_grpc_slices_size(&self) -> usize { + let mut size = 0; + if let Version0OneOfNoneNullable_TestOneof::StringOneOf(ref val) = self.test_oneof { + size += ::pb_jelly::Message::compute_grpc_slices_size(val); + } + size + } + fn serialize(&self, w: &mut W) -> ::std::io::Result<()> { + if let Version0OneOfNoneNullable_TestOneof::StringOneOf(ref val) = self.test_oneof { + ::pb_jelly::wire_format::write(1, ::pb_jelly::wire_format::Type::LengthDelimited, w)?; + let l = ::pb_jelly::Message::compute_size(val); + ::pb_jelly::varint::write(l as u64, w)?; + ::pb_jelly::Message::serialize(val, w)?; + } + Ok(()) + } + fn deserialize(&mut self, mut buf: &mut B) -> ::std::io::Result<()> { + let mut oneof_test_oneof: ::std::option::Option = None; + while let Some((field_number, typ)) = ::pb_jelly::wire_format::read(&mut buf)? { + match field_number { + 1 => { + ::pb_jelly::ensure_wire_format(typ, ::pb_jelly::wire_format::Type::LengthDelimited, "Version0OneOfNoneNullable", 1)?; + let len = ::pb_jelly::varint::ensure_read(&mut buf)?; + let mut next = ::pb_jelly::ensure_split(buf, len as usize)?; + let mut val: ::std::string::String = ::std::default::Default::default(); + ::pb_jelly::Message::deserialize(&mut val, &mut next)?; + oneof_test_oneof = Some(Version0OneOfNoneNullable_TestOneof::StringOneOf(val)); + } + _ => { + ::pb_jelly::skip(typ, &mut buf)?; + } + } + } + match oneof_test_oneof { + Some(v) => self.test_oneof = v, + None => return Err(::std::io::Error::new(::std::io::ErrorKind::InvalidInput, "missing value for non-nullable oneof 'test_oneof' while parsing message pbtest.Version0OneOfNoneNullable")), + } + Ok(()) + } +} +impl ::pb_jelly::Reflection for Version0OneOfNoneNullable { + fn which_one_of(&self, oneof_name: &str) -> ::std::option::Option<&'static str> { + match oneof_name { + "test_oneof" => { + if let Version0OneOfNoneNullable_TestOneof::StringOneOf(ref val) = self.test_oneof { + return Some("string_one_of"); + } + return None; + } + _ => { + panic!("unknown oneof name given"); + } + } + } + fn get_field_mut(&mut self, field_name: &str) -> ::pb_jelly::reflection::FieldMut<'_> { + match field_name { + "string_one_of" => { + if let Version0OneOfNoneNullable_TestOneof::StringOneOf(ref mut val) = self.test_oneof { + return ::pb_jelly::reflection::FieldMut::Value(val); + } + unreachable!() + } + _ => { + panic!("unknown field name given"); + } + } + } +} + #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] pub struct Version1OneOfNoneNullable { pub test_oneof: Version1OneOfNoneNullable_TestOneof, diff --git a/pb-test/proto/packages/pbtest/pbtest2.proto b/pb-test/proto/packages/pbtest/pbtest2.proto index fa797ea..c15267e 100644 --- a/pb-test/proto/packages/pbtest/pbtest2.proto +++ b/pb-test/proto/packages/pbtest/pbtest2.proto @@ -11,6 +11,13 @@ message Vec {} message Default {} message String {} +message Version0OneOfNoneNullable { + oneof test_oneof { + option (rust.nullable) = false; + string string_one_of = 1; + } +} + message Version1OneOfNoneNullable { oneof test_oneof { option (rust.nullable) = false;