Skip to content

Commit

Permalink
Codegen: option to use BTreeMap as map representation
Browse files Browse the repository at this point in the history
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.

This change

* adds `btreemaps` codegen option to generate
  [std::collections::BTreeMap](https://doc.rust-lang.org/std/collections/struct.BTreeMap.html)
  for maps representation.
  • Loading branch information
akhramov committed Jan 12, 2024
1 parent 4cffee8 commit fbc04d4
Show file tree
Hide file tree
Showing 15 changed files with 291 additions and 71 deletions.
30 changes: 30 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,14 @@ jobs:
linux-stable-default-features:
name: linux stable (default features)
runs-on: ubuntu-latest
strategy:
matrix:
map_representation:
- btreemap
- hashmap
env:
RUST_BACKTRACE: 1
MAP_REPRESENTATION: ${{ matrix.map_representation }}
steps:
- name: Checkout sources
uses: actions/checkout@v2
Expand Down Expand Up @@ -53,8 +59,14 @@ jobs:
linux-beta-default-features:
name: linux beta (default features)
runs-on: ubuntu-latest
strategy:
matrix:
map_representation:
- btreemap
- hashmap
env:
RUST_BACKTRACE: 1
MAP_REPRESENTATION: ${{ matrix.map_representation }}
steps:
- name: Checkout sources
uses: actions/checkout@v2
Expand Down Expand Up @@ -98,8 +110,14 @@ jobs:
linux-stable-with-bytes:
name: linux stable (with-bytes)
runs-on: ubuntu-latest
strategy:
matrix:
map_representation:
- btreemap
- hashmap
env:
RUST_BACKTRACE: 1
MAP_REPRESENTATION: ${{ matrix.map_representation }}
steps:
- name: Checkout sources
uses: actions/checkout@v2
Expand Down Expand Up @@ -140,8 +158,14 @@ jobs:
linux-nightly-all-features:
name: linux nightly (all features)
runs-on: ubuntu-latest
strategy:
matrix:
map_representation:
- btreemap
- hashmap
env:
RUST_BACKTRACE: 1
MAP_REPRESENTATION: ${{ matrix.map_representation }}
steps:
- name: Checkout sources
uses: actions/checkout@v2
Expand Down Expand Up @@ -183,8 +207,14 @@ jobs:
windows-stable-default-features:
name: windows stable (default features)
runs-on: windows-latest
strategy:
matrix:
map_representation:
- btreemap
- hashmap
env:
RUST_BACKTRACE: 1
MAP_REPRESENTATION: ${{ matrix.map_representation }}
VCPKGRS_DYNAMIC: 1
steps:
- name: Checkout sources
Expand Down
27 changes: 27 additions & 0 deletions ci-gen/src/ghwf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ pub struct Job {
pub id: String,
pub name: String,
pub runs_on: Env,
pub strategy: Option<Strategy>,
pub steps: Vec<Step>,
pub timeout_minutes: Option<u64>,
pub env: Vec<(String, String)>,
Expand All @@ -141,6 +142,9 @@ impl Into<(String, Yaml)> for Job {
("name", Yaml::string(self.name)),
("runs-on", Yaml::string(format!("{}", self.runs_on))),
];
if let Some(strategy) = self.strategy {
entries.push(("strategy", strategy.into()));
}
if let Some(timeout_minutes) = self.timeout_minutes {
entries.push((
"timeout-minutes",
Expand All @@ -154,3 +158,26 @@ impl Into<(String, Yaml)> for Job {
(self.id, Yaml::map(entries))
}
}

pub struct Strategy {
pub matrix: Matrix,
}

impl Into<Yaml> for Strategy {
fn into(self) -> Yaml {
Yaml::map(vec![("matrix".to_owned(), self.matrix)])
}
}

pub struct Matrix {
pub map_representation: Vec<String>,
}

impl Into<Yaml> for Matrix {
fn into(self) -> Yaml {
Yaml::map(vec![(
"map_representation".to_owned(),
Yaml::list(self.map_representation),
)])
}
}
15 changes: 14 additions & 1 deletion ci-gen/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ use crate::actions::RustToolchain;
use crate::cargo_sync_readme::cargo_sync_readme_job;
use crate::ghwf::Env;
use crate::ghwf::Job;
use crate::ghwf::Matrix;
use crate::ghwf::Step;
use crate::ghwf::Strategy;
use crate::yaml::Yaml;
use crate::yaml::YamlWriter;

Expand Down Expand Up @@ -155,7 +157,13 @@ fn job(channel: RustToolchain, os: Os, features: Features) -> Job {
}
}

let mut env = vec![("RUST_BACKTRACE".to_owned(), "1".to_owned())];
let mut env = vec![
("RUST_BACKTRACE".to_owned(), "1".to_owned()),
(
"MAP_REPRESENTATION".to_owned(),
"${{ matrix.map_representation }}".to_owned(),
),
];
if os == WINDOWS {
env.push(("VCPKGRS_DYNAMIC".to_owned(), "1".to_owned()));
};
Expand All @@ -167,6 +175,11 @@ fn job(channel: RustToolchain, os: Os, features: Features) -> Job {
runs_on: os.ghwf.to_owned(),
steps,
env,
strategy: Some(Strategy {
matrix: Matrix {
map_representation: vec!["btreemap".to_owned(), "hashmap".to_owned()],
},
}),
..Default::default()
}
}
Expand Down
13 changes: 13 additions & 0 deletions protobuf-codegen/src/customize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ pub struct Customize {
/// Used internally to generate protos bundled in protobuf crate
/// like `descriptor.proto`
pub(crate) inside_protobuf: Option<bool>,
/// When true, protobuf maps are represented with `std::collections::BTreeMap`
pub(crate) btreemaps: Option<bool>,
}

#[derive(Debug, thiserror::Error)]
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions protobuf-codegen/src/customize/rustproto_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -24,6 +25,7 @@ pub(crate) fn customize_from_rustproto_for_message(source: &MessageOptions) -> C
lite_runtime,
gen_mod_rs,
inside_protobuf,
btreemaps,
}
}

Expand All @@ -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,
Expand All @@ -49,6 +52,7 @@ pub(crate) fn customize_from_rustproto_for_field(source: &FieldOptions) -> Custo
lite_runtime,
gen_mod_rs,
inside_protobuf,
btreemaps,
}
}

Expand All @@ -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,
Expand All @@ -70,5 +75,6 @@ pub(crate) fn customize_from_rustproto_for_file(source: &FileOptions) -> Customi
lite_runtime,
inside_protobuf,
gen_mod_rs,
btreemaps,
}
}
2 changes: 1 addition & 1 deletion protobuf-codegen/src/gen/field/accessor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
}
Expand Down
2 changes: 1 addition & 1 deletion protobuf-codegen/src/gen/field/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
),
Expand Down
31 changes: 23 additions & 8 deletions protobuf-codegen/src/gen/rust_types_values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub(crate) enum RustType {
Float(u32),
Bool,
Vec(Box<RustType>),
HashMap(Box<RustType>, Box<RustType>),
Map(Box<RustType>, Box<RustType>),
String,
// [T], not &[T]
Slice(Box<RustType>),
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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)),
Expand Down Expand Up @@ -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(..)
Expand Down

0 comments on commit fbc04d4

Please sign in to comment.