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

Codegen: option to use BTreeMap as map representation #700

Closed
wants to merge 1 commit into from
Closed
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
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 @@ -119,7 +119,7 @@

// Representation of map entry: key type and value type
#[derive(Clone, Debug)]
pub struct EntryKeyValue<'a>(FieldElem<'a>, FieldElem<'a>);

Check warning on line 122 in protobuf-codegen/src/gen/field/mod.rs

View workflow job for this annotation

GitHub Actions / linux nightly (all features) (hashmap)

fields `0` and `1` are never read

Check warning on line 122 in protobuf-codegen/src/gen/field/mod.rs

View workflow job for this annotation

GitHub Actions / linux nightly (all features) (btreemap)

fields `0` and `1` are never read

#[derive(Clone)]
pub(crate) struct FieldGen<'a> {
Expand Down Expand Up @@ -276,7 +276,7 @@
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
Loading
Loading