Merge pull request #133 from cyang1/reflection-oneof

Avoid extra logic for oneofs with only 1 possible value
dropbox · Dec 7, 2021 · 4d3aaef · 4d3aaef
2 parents 9f02bdf + 31153de
commit 4d3aaef
Show file tree

Hide file tree

Showing 4 changed files with 152 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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<W: ::pb_jelly::PbBufferWriter>(&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<B: ::pb_jelly::PbBufferReader>(&mut self, mut buf: &mut B) -> ::std::io::Result<()> {
+    let mut oneof_test_oneof: ::std::option::Option<Version0OneOfNoneNullable_TestOneof> = 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
@@ -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;