From e988b31c4cf25e169409d90f5b5fce749ebd531e Mon Sep 17 00:00:00 2001 From: Artem Date: Thu, 21 Dec 2023 14:03:46 +0100 Subject: [PATCH] Codegen: option to use BTreeMap as map representation As of now, protobuf maps are represented with [std::collections::HashMap](https://doc.rust-lang.org/stable/std/collections/struct.HashMap.html), which serves as a robust default. In rarer instances, opting for a different implementation, such as BTreeMap, might be desirable, e.g. for deterministic serialization (#496). This change * adds `btreemaps` codegen option to generate [std::collections::BTreeMap](https://doc.rust-lang.org/std/collections/struct.BTreeMap.html) for maps representation. --- Cargo.toml | 1 + protobuf-codegen/src/customize/mod.rs | 13 ++ .../src/customize/rustproto_proto.rs | 6 + protobuf-codegen/src/gen/field/accessor.rs | 2 +- protobuf-codegen/src/gen/field/mod.rs | 2 +- protobuf-codegen/src/gen/rust_types_values.rs | 31 +++- protobuf/src/reflect/acc/v2/map.rs | 57 +++++-- protobuf/src/reflect/map/generated.rs | 65 ++++++-- protobuf/src/reflect/map/mod.rs | 2 +- protobuf/src/reflect/runtime_types.rs | 96 ++++++++--- protobuf/src/well_known_types/struct_.rs | 2 +- .../protobuf-codegen-btreemap-test/.gitignore | 2 + .../protobuf-codegen-btreemap-test/Cargo.toml | 31 ++++ .../protobuf-codegen-btreemap-test/README.md | 1 + .../protobuf-codegen-btreemap-test/build.rs | 156 ++++++++++++++++++ .../src/common/mod.rs | 3 + .../src/common/v2/.gitignore | 2 + .../src/common/v3/.gitignore | 2 + .../src/google/protobuf/.gitignore | 2 + .../src/include_generated/.gitignore | 1 + .../src/interop/.gitignore | 1 + .../protobuf-codegen-btreemap-test/src/lib.rs | 10 ++ .../src/v2/.gitignore | 2 + .../src/v3/.gitignore | 2 + .../protobuf-codegen-btreemap-test/test.sh | 7 + .../src/common/v2/test_reflect_clear.rs | 9 +- 26 files changed, 441 insertions(+), 67 deletions(-) create mode 100644 test-crates/protobuf-codegen-btreemap-test/.gitignore create mode 100644 test-crates/protobuf-codegen-btreemap-test/Cargo.toml create mode 100644 test-crates/protobuf-codegen-btreemap-test/README.md create mode 100644 test-crates/protobuf-codegen-btreemap-test/build.rs create mode 100644 test-crates/protobuf-codegen-btreemap-test/src/common/mod.rs create mode 100644 test-crates/protobuf-codegen-btreemap-test/src/common/v2/.gitignore create mode 100644 test-crates/protobuf-codegen-btreemap-test/src/common/v3/.gitignore create mode 100644 test-crates/protobuf-codegen-btreemap-test/src/google/protobuf/.gitignore create mode 100644 test-crates/protobuf-codegen-btreemap-test/src/include_generated/.gitignore create mode 100644 test-crates/protobuf-codegen-btreemap-test/src/interop/.gitignore create mode 100644 test-crates/protobuf-codegen-btreemap-test/src/lib.rs create mode 100644 test-crates/protobuf-codegen-btreemap-test/src/v2/.gitignore create mode 100644 test-crates/protobuf-codegen-btreemap-test/src/v3/.gitignore create mode 100755 test-crates/protobuf-codegen-btreemap-test/test.sh diff --git a/Cargo.toml b/Cargo.toml index 812e5f2c8..0a5c6dbc5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ members = [ "test-crates/perftest/bytes", "test-crates/perftest/misc", "test-crates/perftest/vs-cxx", + "test-crates/protobuf-codegen-btreemap-test", "test-crates/protobuf-codegen-identical-test", "test-crates/protobuf-codegen-protoc-test", "test-crates/protobuf-codegen-pure-test", diff --git a/protobuf-codegen/src/customize/mod.rs b/protobuf-codegen/src/customize/mod.rs index 826bd3f6d..ad2a539b2 100644 --- a/protobuf-codegen/src/customize/mod.rs +++ b/protobuf-codegen/src/customize/mod.rs @@ -109,6 +109,8 @@ pub struct Customize { /// Used internally to generate protos bundled in protobuf crate /// like `descriptor.proto` pub(crate) inside_protobuf: Option, + /// When true, protobuf maps are represented with `std::collections::BTreeMap` + pub(crate) btreemaps: Option, } #[derive(Debug, thiserror::Error)] @@ -171,6 +173,14 @@ impl Customize { self } + /// Use btreemaps for maps representation + pub fn btreemaps(self, use_btreemaps: bool) -> Self { + Self { + btreemaps: Some(use_btreemaps), + ..self + } + } + /// Update fields of self with fields defined in other customize pub fn update_with(&mut self, that: &Customize) { if let Some(v) = &that.before { @@ -197,6 +207,9 @@ impl Customize { if let Some(v) = that.inside_protobuf { self.inside_protobuf = Some(v); } + if let Some(v) = that.btreemaps { + self.btreemaps = Some(v); + } } /// Update unset fields of self with fields from other customize diff --git a/protobuf-codegen/src/customize/rustproto_proto.rs b/protobuf-codegen/src/customize/rustproto_proto.rs index 3d9a77b2f..2e3427b5c 100644 --- a/protobuf-codegen/src/customize/rustproto_proto.rs +++ b/protobuf-codegen/src/customize/rustproto_proto.rs @@ -15,6 +15,7 @@ pub(crate) fn customize_from_rustproto_for_message(source: &MessageOptions) -> C let lite_runtime = None; let gen_mod_rs = None; let inside_protobuf = None; + let btreemaps = None; Customize { before, generate_accessors, @@ -24,6 +25,7 @@ pub(crate) fn customize_from_rustproto_for_message(source: &MessageOptions) -> C lite_runtime, gen_mod_rs, inside_protobuf, + btreemaps, } } @@ -40,6 +42,7 @@ pub(crate) fn customize_from_rustproto_for_field(source: &FieldOptions) -> Custo let lite_runtime = None; let gen_mod_rs = None; let inside_protobuf = None; + let btreemaps = None; Customize { before, generate_accessors, @@ -49,6 +52,7 @@ pub(crate) fn customize_from_rustproto_for_field(source: &FieldOptions) -> Custo lite_runtime, gen_mod_rs, inside_protobuf, + btreemaps, } } @@ -61,6 +65,7 @@ pub(crate) fn customize_from_rustproto_for_file(source: &FileOptions) -> Customi let lite_runtime = rustproto::exts::lite_runtime_all.get(source); let gen_mod_rs = None; let inside_protobuf = None; + let btreemaps = None; Customize { before, generate_accessors, @@ -70,5 +75,6 @@ pub(crate) fn customize_from_rustproto_for_file(source: &FileOptions) -> Customi lite_runtime, inside_protobuf, gen_mod_rs, + btreemaps, } } diff --git a/protobuf-codegen/src/gen/field/accessor.rs b/protobuf-codegen/src/gen/field/accessor.rs index 26f0ef48a..acded86fd 100644 --- a/protobuf-codegen/src/gen/field/accessor.rs +++ b/protobuf-codegen/src/gen/field/accessor.rs @@ -66,7 +66,7 @@ impl FieldGen<'_> { let MapField { .. } = map_field; AccessorFn { name: "make_map_simpler_accessor".to_owned(), - type_params: vec![format!("_"), format!("_")], + type_params: vec![format!("_")], callback_params: self.make_accessor_fns_lambda(), } } diff --git a/protobuf-codegen/src/gen/field/mod.rs b/protobuf-codegen/src/gen/field/mod.rs index d6a5e44a4..c4d5d5c5c 100644 --- a/protobuf-codegen/src/gen/field/mod.rs +++ b/protobuf-codegen/src/gen/field/mod.rs @@ -276,7 +276,7 @@ impl<'a> FieldGen<'a> { FieldKind::Repeated(ref repeated) => repeated.rust_type(reference), FieldKind::Map(MapField { ref key, ref value, .. - }) => RustType::HashMap( + }) => RustType::Map( Box::new(key.rust_storage_elem_type(reference)), Box::new(value.rust_storage_elem_type(reference)), ), diff --git a/protobuf-codegen/src/gen/rust_types_values.rs b/protobuf-codegen/src/gen/rust_types_values.rs index cd67f2b35..68f879346 100644 --- a/protobuf-codegen/src/gen/rust_types_values.rs +++ b/protobuf-codegen/src/gen/rust_types_values.rs @@ -33,7 +33,7 @@ pub(crate) enum RustType { Float(u32), Bool, Vec(Box), - HashMap(Box, Box), + Map(Box, Box), String, // [T], not &[T] Slice(Box), @@ -70,11 +70,19 @@ impl RustType { RustType::Float(bits) => format!("f{}", bits), RustType::Bool => format!("bool"), RustType::Vec(ref param) => format!("::std::vec::Vec<{}>", param.to_code(customize)), - RustType::HashMap(ref key, ref value) => format!( - "::std::collections::HashMap<{}, {}>", - key.to_code(customize), - value.to_code(customize) - ), + RustType::Map(ref key, ref value) => { + let ty = if let Some(true) = customize.btreemaps { + "::std::collections::BTreeMap" + } else { + "::std::collections::HashMap" + }; + format!( + "{}<{}, {}>", + ty, + key.to_code(customize), + value.to_code(customize) + ) + } RustType::String => format!("::std::string::String"), RustType::Slice(ref param) => format!("[{}]", param.to_code(customize)), RustType::Str => format!("str"), @@ -220,7 +228,14 @@ impl RustType { RustType::Float(..) => "0.".to_string(), RustType::Bool => "false".to_string(), RustType::Vec(..) => EXPR_VEC_NEW.to_string(), - RustType::HashMap(..) => "::std::collections::HashMap::new()".to_string(), + RustType::Map(..) => { + let ty = if let Some(true) = customize.btreemaps { + "::std::collections::BTreeMap" + } else { + "::std::collections::HashMap" + }; + format!("{}::new()", ty) + } RustType::String => "::std::string::String::new()".to_string(), RustType::Bytes => "::bytes::Bytes::new()".to_string(), RustType::Chars => format!("{}::Chars::new()", protobuf_crate_path(customize)), @@ -266,7 +281,7 @@ impl RustType { | RustType::Chars | RustType::String | RustType::MessageField(..) - | RustType::HashMap(..) => format!("{}.clear()", v), + | RustType::Map(..) => format!("{}.clear()", v), RustType::Bool | RustType::Float(..) | RustType::Int(..) diff --git a/protobuf/src/reflect/acc/v2/map.rs b/protobuf/src/reflect/acc/v2/map.rs index 3d8df8b81..44e0d51ed 100644 --- a/protobuf/src/reflect/acc/v2/map.rs +++ b/protobuf/src/reflect/acc/v2/map.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::fmt; use std::hash::Hash; @@ -8,7 +8,7 @@ use crate::reflect::acc::v2::AccessorV2; use crate::reflect::acc::FieldAccessor; use crate::reflect::map::ReflectMapMut; use crate::reflect::map::ReflectMapRef; -use crate::reflect::runtime_types::RuntimeTypeHashable; +use crate::reflect::runtime_types::RuntimeTypeMapKey; use crate::reflect::runtime_types::RuntimeTypeTrait; use crate::reflect::ProtobufValue; use crate::reflect::RuntimeType; @@ -29,21 +29,46 @@ impl<'a> fmt::Debug for MapFieldAccessorHolder { } } -struct MapFieldAccessorImpl +struct MapFieldAccessorImpl where M: MessageFull, - K: ProtobufValue, - V: ProtobufValue, { - get_field: fn(&M) -> &HashMap, - mut_field: fn(&mut M) -> &mut HashMap, + get_field: fn(&M) -> &T, + mut_field: fn(&mut M) -> &mut T, } -impl MapFieldAccessor for MapFieldAccessorImpl +impl MapFieldAccessor for MapFieldAccessorImpl> where M: MessageFull, K: ProtobufValue + Eq + Hash, - K::RuntimeType: RuntimeTypeHashable, + K::RuntimeType: RuntimeTypeMapKey, + V: ProtobufValue, +{ + fn get_reflect<'a>(&self, m: &'a dyn MessageDyn) -> ReflectMapRef<'a> { + let m = m.downcast_ref().unwrap(); + let map = (self.get_field)(m); + ReflectMapRef::new(map) + } + + fn mut_reflect<'a>(&self, m: &'a mut dyn MessageDyn) -> ReflectMapMut<'a> { + let m = m.downcast_mut().unwrap(); + let map = (self.mut_field)(m); + ReflectMapMut::new(map) + } + + fn element_type(&self) -> (RuntimeType, RuntimeType) { + ( + K::RuntimeType::runtime_type_box(), + V::RuntimeType::runtime_type_box(), + ) + } +} + +impl MapFieldAccessor for MapFieldAccessorImpl> +where + M: MessageFull, + K: ProtobufValue + Ord, + K::RuntimeType: RuntimeTypeMapKey, V: ProtobufValue, { fn get_reflect<'a>(&self, m: &'a dyn MessageDyn) -> ReflectMapRef<'a> { @@ -67,21 +92,19 @@ where } /// Make accessor for map field -pub fn make_map_simpler_accessor( +pub fn make_map_simpler_accessor( name: &'static str, - get_field: for<'a> fn(&'a M) -> &'a HashMap, - mut_field: for<'a> fn(&'a mut M) -> &'a mut HashMap, + get_field: for<'a> fn(&'a M) -> &'a T, + mut_field: for<'a> fn(&'a mut M) -> &'a mut T, ) -> FieldAccessor where - M: MessageFull + 'static, - K: ProtobufValue + Hash + Eq, - K::RuntimeType: RuntimeTypeHashable, - V: ProtobufValue, + M: MessageFull, + MapFieldAccessorImpl: MapFieldAccessor, { FieldAccessor::new( name, AccessorV2::Map(MapFieldAccessorHolder { - accessor: Box::new(MapFieldAccessorImpl:: { + accessor: Box::new(MapFieldAccessorImpl:: { get_field, mut_field, }), diff --git a/protobuf/src/reflect/map/generated.rs b/protobuf/src/reflect/map/generated.rs index ccece690f..88988fe8a 100644 --- a/protobuf/src/reflect/map/generated.rs +++ b/protobuf/src/reflect/map/generated.rs @@ -1,11 +1,11 @@ -use std::collections::hash_map; -use std::collections::HashMap; +use std::collections::{btree_map, hash_map}; +use std::collections::{BTreeMap, HashMap}; use std::hash::Hash; use crate::reflect::map::ReflectMap; use crate::reflect::map::ReflectMapIter; use crate::reflect::map::ReflectMapIterTrait; -use crate::reflect::runtime_types::RuntimeTypeHashable; +use crate::reflect::runtime_types::RuntimeTypeMapKey; use crate::reflect::runtime_types::RuntimeTypeTrait; use crate::reflect::ProtobufValue; use crate::reflect::ReflectValueBox; @@ -16,10 +16,12 @@ impl ReflectMap for HashMap where K: ProtobufValue + Eq + Hash, V: ProtobufValue, - K::RuntimeType: RuntimeTypeHashable, + K::RuntimeType: RuntimeTypeMapKey, { fn reflect_iter<'a>(&'a self) -> ReflectMapIter<'a> { - ReflectMapIter::new(GeneratedMapIterImpl::<'a, K, V> { iter: self.iter() }) + ReflectMapIter::new(GeneratedMapIterImpl::<'a, K, V, hash_map::Iter<'a, K, V>> { + iter: self.iter(), + }) } fn len(&self) -> usize { @@ -31,7 +33,7 @@ where } fn get<'a>(&'a self, key: ReflectValueRef) -> Option> { - ::hash_map_get(self, key).map(V::RuntimeType::as_ref) + ::hash_map_get(self, key).map(V::RuntimeType::as_ref) } fn insert(&mut self, key: ReflectValueBox, value: ReflectValueBox) { @@ -53,12 +55,55 @@ where } } -struct GeneratedMapIterImpl<'a, K: Eq + Hash + 'static, V: 'static> { - iter: hash_map::Iter<'a, K, V>, +impl ReflectMap for BTreeMap +where + K: ProtobufValue + Ord, + V: ProtobufValue, + K::RuntimeType: RuntimeTypeMapKey, +{ + fn reflect_iter<'a>(&'a self) -> ReflectMapIter<'a> { + ReflectMapIter::new( + GeneratedMapIterImpl::<'a, K, V, btree_map::Iter<'a, K, V>> { iter: self.iter() }, + ) + } + + fn len(&self) -> usize { + BTreeMap::len(self) + } + + fn is_empty(&self) -> bool { + self.is_empty() + } + + fn get<'a>(&'a self, key: ReflectValueRef) -> Option> { + ::btree_map_get(self, key).map(V::RuntimeType::as_ref) + } + + fn insert(&mut self, key: ReflectValueBox, value: ReflectValueBox) { + let key: K = key.downcast().expect("wrong key type"); + let value: V = value.downcast().expect("wrong value type"); + self.insert(key, value); + } + + fn clear(&mut self) { + self.clear(); + } + + fn key_type(&self) -> RuntimeType { + K::RuntimeType::runtime_type_box() + } + + fn value_type(&self) -> RuntimeType { + V::RuntimeType::runtime_type_box() + } +} + +struct GeneratedMapIterImpl<'a, K: 'static, V: 'static, I: Iterator> { + iter: I, } -impl<'a, K: ProtobufValue + Eq + Hash, V: ProtobufValue> ReflectMapIterTrait<'a> - for GeneratedMapIterImpl<'a, K, V> +impl<'a, K: ProtobufValue, V: ProtobufValue, I: Iterator> + ReflectMapIterTrait<'a> for GeneratedMapIterImpl<'a, K, V, I> { fn next(&mut self) -> Option<(ReflectValueRef<'a>, ReflectValueRef<'a>)> { match self.iter.next() { diff --git a/protobuf/src/reflect/map/mod.rs b/protobuf/src/reflect/map/mod.rs index 5ec3fd8ab..c927473fa 100644 --- a/protobuf/src/reflect/map/mod.rs +++ b/protobuf/src/reflect/map/mod.rs @@ -12,7 +12,7 @@ use crate::reflect::RuntimeType; mod empty; mod generated; -/// Implemented for `HashMap` with appropriate keys and values +/// Implemented for `HashMap`, `BTreeMap` with appropriate keys and values pub(crate) trait ReflectMap: Debug + Send + Sync + 'static { fn reflect_iter(&self) -> ReflectMapIter; diff --git a/protobuf/src/reflect/runtime_types.rs b/protobuf/src/reflect/runtime_types.rs index 26bfe9669..c4f6add7f 100644 --- a/protobuf/src/reflect/runtime_types.rs +++ b/protobuf/src/reflect/runtime_types.rs @@ -1,6 +1,6 @@ //! Implementations of `RuntimeType` for all types. -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::fmt; use std::marker; @@ -118,13 +118,29 @@ pub trait RuntimeTypeWithDeref: RuntimeTypeTrait { fn deref_as_ref(value: &Self::DerefTarget) -> ReflectValueRef; } -/// Types which can be hashmap keys. -pub trait RuntimeTypeHashable: RuntimeTypeTrait { +/// Types which can be map keys. +pub trait RuntimeTypeMapKey: RuntimeTypeTrait { + /// Try to extract the value from the given ref + fn try_deref_key(key: ReflectValueRef) -> Option; /// Query hash map with a given key. - fn hash_map_get<'a, V>(map: &'a HashMap, key: ReflectValueRef) - -> Option<&'a V>; -} + fn hash_map_get<'a, V>(map: &'a HashMap, key: ReflectValueRef) -> Option<&'a V> + where + Self::Value: std::hash::Hash + Eq, + { + Self::try_deref_key(key).and_then(|key| map.get(&key)) + } + /// Query btree map with a given key. + fn btree_map_get<'a, V>( + map: &'a BTreeMap, + key: ReflectValueRef, + ) -> Option<&'a V> + where + Self::Value: Ord, + { + Self::try_deref_key(key).and_then(|key| map.get(&key)) + } +} /// Implementation for `f32` #[derive(Debug, Copy, Clone)] pub struct RuntimeTypeF32; @@ -311,10 +327,10 @@ impl RuntimeTypeTrait for RuntimeTypeI32 { } } } -impl RuntimeTypeHashable for RuntimeTypeI32 { - fn hash_map_get<'a, V>(map: &'a HashMap, key: ReflectValueRef) -> Option<&'a V> { +impl RuntimeTypeMapKey for RuntimeTypeI32 { + fn try_deref_key(key: ReflectValueRef) -> Option { match key { - ReflectValueRef::I32(i) => map.get(&i), + ReflectValueRef::I32(i) => Some(i), _ => None, } } @@ -370,10 +386,10 @@ impl RuntimeTypeTrait for RuntimeTypeI64 { } } } -impl RuntimeTypeHashable for RuntimeTypeI64 { - fn hash_map_get<'a, V>(map: &'a HashMap, key: ReflectValueRef) -> Option<&'a V> { +impl RuntimeTypeMapKey for RuntimeTypeI64 { + fn try_deref_key(key: ReflectValueRef) -> Option { match key { - ReflectValueRef::I64(i) => map.get(&i), + ReflectValueRef::I64(i) => Some(i), _ => None, } } @@ -428,10 +444,10 @@ impl RuntimeTypeTrait for RuntimeTypeU32 { } } } -impl RuntimeTypeHashable for RuntimeTypeU32 { - fn hash_map_get<'a, V>(map: &'a HashMap, key: ReflectValueRef) -> Option<&'a V> { +impl RuntimeTypeMapKey for RuntimeTypeU32 { + fn try_deref_key(key: ReflectValueRef) -> Option { match key { - ReflectValueRef::U32(i) => map.get(&i), + ReflectValueRef::U32(i) => Some(i), _ => None, } } @@ -486,10 +502,10 @@ impl RuntimeTypeTrait for RuntimeTypeU64 { } } } -impl RuntimeTypeHashable for RuntimeTypeU64 { - fn hash_map_get<'a, V>(map: &'a HashMap, key: ReflectValueRef) -> Option<&'a V> { +impl RuntimeTypeMapKey for RuntimeTypeU64 { + fn try_deref_key(key: ReflectValueRef) -> Option { match key { - ReflectValueRef::U64(i) => map.get(&i), + ReflectValueRef::U64(u) => Some(u), _ => None, } } @@ -541,10 +557,10 @@ impl RuntimeTypeTrait for RuntimeTypeBool { ProtobufTypeBool::get_from_unknown(unknown) } } -impl RuntimeTypeHashable for RuntimeTypeBool { - fn hash_map_get<'a, V>(map: &'a HashMap, key: ReflectValueRef) -> Option<&'a V> { +impl RuntimeTypeMapKey for RuntimeTypeBool { + fn try_deref_key(key: ReflectValueRef) -> Option { match key { - ReflectValueRef::Bool(i) => map.get(&i), + ReflectValueRef::Bool(b) => Some(b), _ => None, } } @@ -599,10 +615,31 @@ impl RuntimeTypeWithDeref for RuntimeTypeString { ReflectValueRef::String(value) } } -impl RuntimeTypeHashable for RuntimeTypeString { - fn hash_map_get<'a, V>(map: &'a HashMap, key: ReflectValueRef) -> Option<&'a V> { +impl RuntimeTypeMapKey for RuntimeTypeString { + fn try_deref_key(_key: ReflectValueRef) -> Option { + unimplemented!() + } + + fn hash_map_get<'a, V>(map: &'a HashMap, key: ReflectValueRef) -> Option<&'a V> + where + Self::Value: std::hash::Hash + Eq, + { match key { - ReflectValueRef::String(s) => map.get(*&s), + ReflectValueRef::String(s) => map.get(s), + _ => None, + } + } + + /// Query btree map with a given key. + fn btree_map_get<'a, V>( + map: &'a BTreeMap, + key: ReflectValueRef, + ) -> Option<&'a V> + where + Self::Value: Ord, + { + match key { + ReflectValueRef::String(s) => map.get(s), _ => None, } } @@ -763,13 +800,22 @@ impl RuntimeTypeWithDeref for RuntimeTypeTokioChars { } } #[cfg(feature = "bytes")] -impl RuntimeTypeHashable for RuntimeTypeTokioChars { +impl RuntimeTypeMapKey for RuntimeTypeTokioChars { + fn try_deref_key(_key: ReflectValueRef) -> Option { + unimplemented!() + } fn hash_map_get<'a, V>(map: &'a HashMap, key: ReflectValueRef) -> Option<&'a V> { match key { ReflectValueRef::String(s) => map.get(&*s), _ => None, } } + fn btree_map_get<'a, V>(map: &'a BTreeMap, key: ReflectValueRef) -> Option<&'a V> { + match key { + ReflectValueRef::String(s) => map.get(&*s), + _ => None, + } + } } impl RuntimeTypeTrait for RuntimeTypeEnumOrUnknown diff --git a/protobuf/src/well_known_types/struct_.rs b/protobuf/src/well_known_types/struct_.rs index c2666695e..71a2f894a 100644 --- a/protobuf/src/well_known_types/struct_.rs +++ b/protobuf/src/well_known_types/struct_.rs @@ -55,7 +55,7 @@ impl Struct { fn generated_message_descriptor_data() -> crate::reflect::GeneratedMessageDescriptorData { let mut fields = ::std::vec::Vec::with_capacity(1); let mut oneofs = ::std::vec::Vec::with_capacity(0); - fields.push(crate::reflect::rt::v2::make_map_simpler_accessor::<_, _, _>( + fields.push(crate::reflect::rt::v2::make_map_simpler_accessor::<_, _>( "fields", |m: &Struct| { &m.fields }, |m: &mut Struct| { &mut m.fields }, diff --git a/test-crates/protobuf-codegen-btreemap-test/.gitignore b/test-crates/protobuf-codegen-btreemap-test/.gitignore new file mode 100644 index 000000000..03ef71b23 --- /dev/null +++ b/test-crates/protobuf-codegen-btreemap-test/.gitignore @@ -0,0 +1,2 @@ +*_pb.rs +*_pb_proto3.rs diff --git a/test-crates/protobuf-codegen-btreemap-test/Cargo.toml b/test-crates/protobuf-codegen-btreemap-test/Cargo.toml new file mode 100644 index 000000000..d03632ddb --- /dev/null +++ b/test-crates/protobuf-codegen-btreemap-test/Cargo.toml @@ -0,0 +1,31 @@ +[package] +name = "protobuf-codegen-btreemap-test" +version = "0.0.0" +authors = ["Stepan Koltsov "] +publish = false +edition = "2021" +description = "Codegen btreemaps option tests" + +[lib] +doctest = false +bench = false + +[features] +default = ["test_with_btreemaps"] +test_with_btreemaps = [] +proto3 = [] +with-bytes = ["bytes", "protobuf/with-bytes", "protobuf-test-common/with-bytes"] + +[build-dependencies] +protobuf-codegen = { path = "../../protobuf-codegen" } +protobuf-test-common = { path = "../../test-crates/protobuf-test-common" } +glob = "0.2" +log = "0.4" +env_logger = "0.5.*" + +[dependencies] +bytes = { version = "1.1", optional = true } + +protobuf = { path = "../../protobuf" } +protobuf-test-common = { path = "../../test-crates/protobuf-test-common" } +protobuf-json-mapping = { path = "../../protobuf-json-mapping" } diff --git a/test-crates/protobuf-codegen-btreemap-test/README.md b/test-crates/protobuf-codegen-btreemap-test/README.md new file mode 100644 index 000000000..4e975cb06 --- /dev/null +++ b/test-crates/protobuf-codegen-btreemap-test/README.md @@ -0,0 +1 @@ +# Tests for codegen with the btreemap option enabled diff --git a/test-crates/protobuf-codegen-btreemap-test/build.rs b/test-crates/protobuf-codegen-btreemap-test/build.rs new file mode 100644 index 000000000..c19ef1af0 --- /dev/null +++ b/test-crates/protobuf-codegen-btreemap-test/build.rs @@ -0,0 +1,156 @@ +use std::env; +use std::fs; +use std::io::Read; +use std::io::Write; +use std::path::Path; + +use protobuf_codegen::Codegen; +use protobuf_test_common::build::*; +use protobuf_test_common::print_rerun_if_changed_recursively; + +fn copy_test, P2: AsRef>(src: P1, dst: P2) { + eprintln!("copy {:?} to {:?}", src.as_ref(), dst.as_ref()); + let mut content = Vec::new(); + fs::File::open(src.as_ref()) + .expect(&format!("open {}", src.as_ref().display())) + .read_to_end(&mut content) + .expect(&format!("read_to_end {}", src.as_ref().display())); + + let mut write = fs::File::create(dst).expect("create"); + writeln!(write, "// @generated").expect("write"); + writeln!(write, "// copied from {}", src.as_ref().display()).expect("write"); + writeln!(write, "").expect("write"); + write.write_all(&content).expect("write_all"); + // Print generated twice to avoid overlooking it accidentally + writeln!(write, "// @generated").expect("write"); + write.flush().expect("flush"); +} + +fn copy_from_protobuf_test(path: &str) { + copy_test( + &format!("../../test-crates/protobuf-codegen-protoc-test/{}", path), + &format!("{}", path), + ) +} + +enum FileNameClass { + ModRs, + TestRs, + Proto, + GeneratedRs, + Ignore, +} + +fn classify_file_name(dir: &str, name: &str) -> FileNameClass { + if name.starts_with(".") || name.ends_with(".md") || name.ends_with(".sh") { + FileNameClass::Ignore + } else if name.ends_with("_pb.rs") || name.ends_with("_pb_proto3.rs") { + FileNameClass::GeneratedRs + } else if name == "mod.rs" { + FileNameClass::ModRs + } else if name.ends_with(".proto") || name.ends_with(".proto3") { + FileNameClass::Proto + } else if name.ends_with(".rs") { + if dir == "src/google/protobuf" { + FileNameClass::GeneratedRs + } else { + FileNameClass::TestRs + } + } else { + panic!("unknown test file: {}", name); + } +} + +// Copy tests from `protobuf-test` directory to the same directory here +fn copy_tests(dir: &str) { + let src_dir = format!("../../test-crates/protobuf-codegen-protoc-test/{}", dir); + for entry in fs::read_dir(&src_dir).expect(&format!("read_dir {}", src_dir)) { + let file_name = entry.expect("entry").file_name().into_string().unwrap(); + + match classify_file_name(dir, &file_name) { + FileNameClass::ModRs | FileNameClass::Ignore | FileNameClass::GeneratedRs => {} + FileNameClass::TestRs | FileNameClass::Proto => { + copy_from_protobuf_test(&format!("{}/{}", dir, file_name)) + } + } + } +} + +fn gen_in_dir(dir: &str, include_dir: &str) { + gen_in_dir_impl( + dir, + |GenInDirArgs { + out_dir, + input, + customize, + }| { + Codegen::new() + .pure() + .out_dir(out_dir) + .inputs(input) + .includes(&[include_dir]) + .customize(customize.btreemaps(true)) + .run_from_script() + }, + ); +} + +fn generate_interop() { + copy_from_protobuf_test("src/interop/mod.rs"); + copy_from_protobuf_test("src/interop/json.rs"); + copy_from_protobuf_test("src/interop/bin.rs"); + + Codegen::new() + .pure() + .out_dir("src/interop") + .includes(&["../../test-crates/interop/cxx", "../../proto"]) + .input("../../test-crates/interop/cxx/interop_pb.proto") + .run_from_script(); +} + +fn generate_include_generated() { + copy_from_protobuf_test("src/include_generated/mod.rs"); + + let dir = format!("{}/include_generated", env::var("OUT_DIR").unwrap()); + if Path::new(&dir).exists() { + fs::remove_dir_all(&dir).unwrap(); + } + fs::create_dir(&dir).unwrap(); + Codegen::new() + .pure() + .out_dir(dir) + .input("../../test-crates/protobuf-codegen-protoc-test/src/include_generated/v2.proto") + .input("../../test-crates/protobuf-codegen-protoc-test/src/include_generated/v3.proto") + .include("../../test-crates/protobuf-codegen-protoc-test/src/include_generated") + .run_from_script(); +} + +fn generate_pb_rs() { + print_rerun_if_changed_recursively("../../test-crates/protobuf-codegen-protoc-test"); + + copy_tests("src/v2"); + gen_in_dir("src/v2", "src/v2"); + + copy_tests("src/v3"); + gen_in_dir("src/v3", "src/v3"); + + copy_tests("src/common/v2"); + gen_in_dir("src/common/v2", "src/common/v2"); + + copy_tests_v2_v3("src/common/v2", "src/common/v3"); + gen_in_dir("src/common/v3", "src/common/v3"); + + copy_tests("src/google/protobuf"); + gen_in_dir("src/google/protobuf", "src"); + + generate_interop(); + + generate_include_generated(); +} + +fn main() { + env_logger::init(); + + clean_old_files(); + generate_pb_rs(); +} diff --git a/test-crates/protobuf-codegen-btreemap-test/src/common/mod.rs b/test-crates/protobuf-codegen-btreemap-test/src/common/mod.rs new file mode 100644 index 000000000..4b842adda --- /dev/null +++ b/test-crates/protobuf-codegen-btreemap-test/src/common/mod.rs @@ -0,0 +1,3 @@ +mod v2; + +mod v3; diff --git a/test-crates/protobuf-codegen-btreemap-test/src/common/v2/.gitignore b/test-crates/protobuf-codegen-btreemap-test/src/common/v2/.gitignore new file mode 100644 index 000000000..81eea527a --- /dev/null +++ b/test-crates/protobuf-codegen-btreemap-test/src/common/v2/.gitignore @@ -0,0 +1,2 @@ +# All files in this directory are generated +* diff --git a/test-crates/protobuf-codegen-btreemap-test/src/common/v3/.gitignore b/test-crates/protobuf-codegen-btreemap-test/src/common/v3/.gitignore new file mode 100644 index 000000000..81eea527a --- /dev/null +++ b/test-crates/protobuf-codegen-btreemap-test/src/common/v3/.gitignore @@ -0,0 +1,2 @@ +# All files in this directory are generated +* diff --git a/test-crates/protobuf-codegen-btreemap-test/src/google/protobuf/.gitignore b/test-crates/protobuf-codegen-btreemap-test/src/google/protobuf/.gitignore new file mode 100644 index 000000000..81eea527a --- /dev/null +++ b/test-crates/protobuf-codegen-btreemap-test/src/google/protobuf/.gitignore @@ -0,0 +1,2 @@ +# All files in this directory are generated +* diff --git a/test-crates/protobuf-codegen-btreemap-test/src/include_generated/.gitignore b/test-crates/protobuf-codegen-btreemap-test/src/include_generated/.gitignore new file mode 100644 index 000000000..72e8ffc0d --- /dev/null +++ b/test-crates/protobuf-codegen-btreemap-test/src/include_generated/.gitignore @@ -0,0 +1 @@ +* diff --git a/test-crates/protobuf-codegen-btreemap-test/src/interop/.gitignore b/test-crates/protobuf-codegen-btreemap-test/src/interop/.gitignore new file mode 100644 index 000000000..72e8ffc0d --- /dev/null +++ b/test-crates/protobuf-codegen-btreemap-test/src/interop/.gitignore @@ -0,0 +1 @@ +* diff --git a/test-crates/protobuf-codegen-btreemap-test/src/lib.rs b/test-crates/protobuf-codegen-btreemap-test/src/lib.rs new file mode 100644 index 000000000..593428f0a --- /dev/null +++ b/test-crates/protobuf-codegen-btreemap-test/src/lib.rs @@ -0,0 +1,10 @@ +#![cfg(test)] + +mod v2; +mod v3; + +mod common; + +mod interop; + +mod include_generated; diff --git a/test-crates/protobuf-codegen-btreemap-test/src/v2/.gitignore b/test-crates/protobuf-codegen-btreemap-test/src/v2/.gitignore new file mode 100644 index 000000000..81eea527a --- /dev/null +++ b/test-crates/protobuf-codegen-btreemap-test/src/v2/.gitignore @@ -0,0 +1,2 @@ +# All files in this directory are generated +* diff --git a/test-crates/protobuf-codegen-btreemap-test/src/v3/.gitignore b/test-crates/protobuf-codegen-btreemap-test/src/v3/.gitignore new file mode 100644 index 000000000..81eea527a --- /dev/null +++ b/test-crates/protobuf-codegen-btreemap-test/src/v3/.gitignore @@ -0,0 +1,2 @@ +# All files in this directory are generated +* diff --git a/test-crates/protobuf-codegen-btreemap-test/test.sh b/test-crates/protobuf-codegen-btreemap-test/test.sh new file mode 100755 index 000000000..368546669 --- /dev/null +++ b/test-crates/protobuf-codegen-btreemap-test/test.sh @@ -0,0 +1,7 @@ +#!/bin/sh -ex + +cd "$(dirname "$0")" + +cargo test --features="$RUST_PROTOBUF_FEATURES" + +# vim: set ts=4 sw=4 et: diff --git a/test-crates/protobuf-codegen-protoc-test/src/common/v2/test_reflect_clear.rs b/test-crates/protobuf-codegen-protoc-test/src/common/v2/test_reflect_clear.rs index 2e0affafa..27c8de195 100644 --- a/test-crates/protobuf-codegen-protoc-test/src/common/v2/test_reflect_clear.rs +++ b/test-crates/protobuf-codegen-protoc-test/src/common/v2/test_reflect_clear.rs @@ -1,4 +1,7 @@ -use std::collections::HashMap; +#[cfg(feature = "test_with_btreemaps")] +use std::collections::BTreeMap as Map; +#[cfg(not(feature = "test_with_btreemaps"))] +use std::collections::HashMap as Map; use protobuf::reflect::ReflectValueBox; use protobuf::MessageFull; @@ -7,7 +10,7 @@ use super::test_reflect_clear_pb::*; #[test] fn test_generated() { - let mut map = HashMap::new(); + let mut map = Map::new(); map.insert("key".to_string(), "value".to_string()); let mut msg = TestMessage::default(); @@ -45,7 +48,7 @@ fn test_dynamic() { let mut msg = msg_desc.new_instance(); - let mut map = HashMap::new(); + let mut map = Map::new(); map.insert("key".to_string(), "value".to_string()); a_field.set_singular_field(msg.as_mut(), 1.into());