Skip to content

Commit

Permalink
WIP fixes to prost-derive, and porting of tests to alloc configuration
Browse files Browse the repository at this point in the history
  • Loading branch information
cbeck88 committed Sep 12, 2019
1 parent fc7a772 commit cc51c9d
Show file tree
Hide file tree
Showing 19 changed files with 1,146 additions and 189 deletions.
1 change: 1 addition & 0 deletions .travis.yml
Expand Up @@ -21,6 +21,7 @@ rust:
script:
- cargo build --verbose --all --exclude benchmarks
- cargo test --verbose --all --exclude benchmarks
- cargo run -p tests-alloc --verbose
- if [[ $TRAVIS_RUST_VERSION = nightly* ]]; then
cargo bench --verbose --no-run;
fi
3 changes: 2 additions & 1 deletion Cargo.toml
Expand Up @@ -36,7 +36,7 @@ exclude = [
[features]
default = ["prost-derive", "std"]
no-recursion-limit = []
std = []
std = ["prost-derive/std"]
# When std is disabled, we attempt to provide no_std support in prost
# Config::use_alloc_collections() should be set when using prost_build in your build.rs
# so that generated files will not have std:: either, and will use alloc crate instead
Expand All @@ -60,5 +60,6 @@ name = "varint"
name = "benchmark"
harness = false

# TODO: Remove this after bytes releases 0.5, and before we make a release
[patch.crates-io]
bytes = { git = "https://github.com/tokio-rs/bytes", rev = "c17e40115f5bb2a2db71ed90dceae6ec643dc024" }
13 changes: 12 additions & 1 deletion prost-derive/Cargo.toml
Expand Up @@ -13,8 +13,19 @@ edition = "2018"
[lib]
proc_macro = true

[features]
std = [ "failure/std" ]
# std feature means that prost-derive attempts to use std collections
#
# When this feature is disabled, we attempt to use alloc.
# This should be similar to putting `config.use_alloc_collections` in a
# prost-build config
#
# Generated code assumes that user's crate has `extern crate alloc` somewhere
default = ["std"]

[dependencies]
failure = { version = "0.1", default-features = false, features = ["std"] }
failure = { version = "0.1", default-features = false }
itertools = "0.8"
proc-macro2 = "0.4.4"
quote = "0.6.3"
Expand Down
6 changes: 4 additions & 2 deletions prost-derive/src/field/map.rs
Expand Up @@ -240,6 +240,8 @@ impl Field {
/// The Debug tries to convert any enumerations met into the variants if possible, instead of
/// outputting the raw numbers.
pub fn debug(&self, wrapper_name: TokenStream) -> TokenStream {
let libname = super::collections_lib_name();

let type_name = match self.map_ty {
MapTy::HashMap => Ident::new("HashMap", Span::call_site()),
MapTy::BTreeMap => Ident::new("BTreeMap", Span::call_site()),
Expand All @@ -263,14 +265,14 @@ impl Field {
ValueTy::Scalar(ref ty) => {
let value = ty.rust_type();
quote! {
struct #wrapper_name<'a>(&'a ::alloc::collections::#type_name<#key, #value>);
struct #wrapper_name<'a>(&'a ::#libname::collections::#type_name<#key, #value>);
impl<'a> ::core::fmt::Debug for #wrapper_name<'a> {
#fmt
}
}
}
ValueTy::Message => quote! {
struct #wrapper_name<'a, V: 'a>(&'a ::alloc::collections::#type_name<#key, V>);
struct #wrapper_name<'a, V: 'a>(&'a ::#libname::collections::#type_name<#key, V>);
impl<'a, V> ::core::fmt::Debug for #wrapper_name<'a, V>
where
V: ::core::fmt::Debug + 'a,
Expand Down
18 changes: 14 additions & 4 deletions prost-derive/src/field/mod.rs
Expand Up @@ -4,11 +4,11 @@ mod message;
mod oneof;
mod scalar;

use std::fmt;
use std::slice;
use core::fmt;
use core::slice;

use failure::{bail, Error};
use proc_macro2::TokenStream;
use proc_macro2::{Span, TokenStream};
use quote::quote;
use syn::{Attribute, Ident, Lit, LitBool, Meta, MetaList, MetaNameValue, NestedMeta};

Expand Down Expand Up @@ -135,7 +135,7 @@ impl Field {
pub fn default(&self) -> TokenStream {
match *self {
Field::Scalar(ref scalar) => scalar.default(),
_ => quote!(::std::default::Default::default()),
_ => quote!(::core::default::Default::default()),
}
}

Expand Down Expand Up @@ -364,3 +364,13 @@ fn tags_attr(attr: &Meta) -> Result<Option<Vec<u32>>, Error> {
_ => bail!("invalid tag attribute: {:?}", attr),
}
}

// Helper which builds an identifier corresponding `std` or `alloc` depending
// on feature selection
fn collections_lib_name() -> Ident {
if cfg!(std) {
Ident::new("std", Span::call_site())
} else {
Ident::new("alloc", Span::call_site())
}
}
2 changes: 1 addition & 1 deletion prost-derive/src/field/oneof.rs
Expand Up @@ -90,6 +90,6 @@ impl Field {
}

pub fn clear(&self, ident: TokenStream) -> TokenStream {
quote!(#ident = ::std::option::Option::None)
quote!(#ident = ::core::option::Option::None)
}
}
66 changes: 35 additions & 31 deletions prost-derive/src/field/scalar.rs
@@ -1,4 +1,4 @@
use std::fmt;
use core::fmt;

use failure::{bail, format_err, Error};
use proc_macro2::{Span, TokenStream};
Expand Down Expand Up @@ -131,7 +131,7 @@ impl Field {
}
}
Kind::Optional(..) => quote! {
if let ::std::option::Option::Some(ref value) = #ident {
if let ::core::option::Option::Some(ref value) = #ident {
#encode_fn(#tag, value, buf);
}
},
Expand Down Expand Up @@ -204,17 +204,18 @@ impl Field {
_ => quote!(#ident = #default),
}
}
Kind::Optional(_) => quote!(#ident = ::std::option::Option::None),
Kind::Optional(_) => quote!(#ident = ::core::option::Option::None),
Kind::Repeated | Kind::Packed => quote!(#ident.clear()),
}
}

/// Returns an expression which evaluates to the default value of the field.
pub fn default(&self) -> TokenStream {
let libname = super::collections_lib_name();
match self.kind {
Kind::Plain(ref value) | Kind::Required(ref value) => value.owned(),
Kind::Optional(_) => quote!(::std::option::Option::None),
Kind::Repeated | Kind::Packed => quote!(::std::vec::Vec::new()),
Kind::Optional(_) => quote!(::core::option::Option::None),
Kind::Repeated | Kind::Packed => quote!(::#libname::vec::Vec::new()),
}
}

Expand All @@ -223,11 +224,11 @@ impl Field {
if let Ty::Enumeration(ref ty) = self.ty {
quote! {
struct #wrap_name<'a>(&'a i32);
impl<'a> ::std::fmt::Debug for #wrap_name<'a> {
fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
impl<'a> ::core::fmt::Debug for #wrap_name<'a> {
fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
match #ty::from_i32(*self.0) {
None => ::std::fmt::Debug::fmt(&self.0, f),
Some(en) => ::std::fmt::Debug::fmt(&en, f),
None => ::core::fmt::Debug::fmt(&self.0, f),
Some(en) => ::core::fmt::Debug::fmt(&en, f),
}
}
}
Expand All @@ -242,23 +243,24 @@ impl Field {
/// Returns a fragment for formatting the field `ident` in `Debug`.
pub fn debug(&self, wrapper_name: TokenStream) -> TokenStream {
let wrapper = self.debug_inner(quote!(Inner));
let libname = super::collections_lib_name();
let inner_ty = self.ty.rust_type();
match self.kind {
Kind::Plain(_) | Kind::Required(_) => self.debug_inner(wrapper_name),
Kind::Optional(_) => quote! {
struct #wrapper_name<'a>(&'a ::std::option::Option<#inner_ty>);
impl<'a> ::std::fmt::Debug for #wrapper_name<'a> {
fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
struct #wrapper_name<'a>(&'a ::core::option::Option<#inner_ty>);
impl<'a> ::core::fmt::Debug for #wrapper_name<'a> {
fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
#wrapper
::std::fmt::Debug::fmt(&self.0.as_ref().map(Inner), f)
::core::fmt::Debug::fmt(&self.0.as_ref().map(Inner), f)
}
}
},
Kind::Repeated | Kind::Packed => {
quote! {
struct #wrapper_name<'a>(&'a ::std::vec::Vec<#inner_ty>);
impl<'a> ::std::fmt::Debug for #wrapper_name<'a> {
fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
struct #wrapper_name<'a>(&'a ::#libname::vec::Vec<#inner_ty>);
impl<'a> ::core::fmt::Debug for #wrapper_name<'a> {
fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
let mut vec_builder = f.debug_list();
for v in self.0 {
#wrapper
Expand Down Expand Up @@ -300,13 +302,13 @@ impl Field {
}

pub fn #set(&mut self, value: #ty) {
self.#ident = ::std::option::Option::Some(value as i32);
self.#ident = ::core::option::Option::Some(value as i32);
}
}
}
Kind::Repeated | Kind::Packed => {
quote! {
pub fn #ident(&self) -> ::std::iter::FilterMap<::std::iter::Cloned<::std::slice::Iter<i32>>,
pub fn #ident(&self) -> ::core::iter::FilterMap<::core::iter::Cloned<::core::slice::Iter<i32>>,
fn(i32) -> Option<#ty>> {
self.#ident.iter().cloned().filter_map(#ty::from_i32)
}
Expand All @@ -320,16 +322,16 @@ impl Field {
let ty = self.ty.rust_ref_type();

let match_some = if self.ty.is_numeric() {
quote!(::std::option::Option::Some(val) => val,)
quote!(::core::option::Option::Some(val) => val,)
} else {
quote!(::std::option::Option::Some(ref val) => &val[..],)
quote!(::core::option::Option::Some(ref val) => &val[..],)
};

Some(quote! {
pub fn #ident(&self) -> #ty {
match self.#ident {
#match_some
::std::option::Option::None => #default,
::core::option::Option::None => #default,
}
}
})
Expand Down Expand Up @@ -465,9 +467,10 @@ impl Ty {

// TODO: rename to 'owned_type'.
pub fn rust_type(&self) -> TokenStream {
let libname = super::collections_lib_name();
match *self {
Ty::String => quote!(::std::string::String),
Ty::Bytes => quote!(::std::vec::Vec<u8>),
Ty::String => quote!(::#libname::string::String),
Ty::Bytes => quote!(::#libname::vec::Vec<u8>),
_ => self.rust_ref_type(),
}
}
Expand Down Expand Up @@ -628,16 +631,16 @@ impl DefaultValue {
match value {
"inf" => {
return Ok(DefaultValue::Path(parse_str::<Path>(
"::std::f32::INFINITY",
"::core::f32::INFINITY",
)?));
}
"-inf" => {
return Ok(DefaultValue::Path(parse_str::<Path>(
"::std::f32::NEG_INFINITY",
"::core::f32::NEG_INFINITY",
)?));
}
"nan" => {
return Ok(DefaultValue::Path(parse_str::<Path>("::std::f32::NAN")?));
return Ok(DefaultValue::Path(parse_str::<Path>("::core::f32::NAN")?));
}
_ => (),
}
Expand All @@ -646,16 +649,16 @@ impl DefaultValue {
match value {
"inf" => {
return Ok(DefaultValue::Path(parse_str::<Path>(
"::std::f64::INFINITY",
"::core::f64::INFINITY",
)?));
}
"-inf" => {
return Ok(DefaultValue::Path(parse_str::<Path>(
"::std::f64::NEG_INFINITY",
"::core::f64::NEG_INFINITY",
)?));
}
"nan" => {
return Ok(DefaultValue::Path(parse_str::<Path>("::std::f64::NAN")?));
return Ok(DefaultValue::Path(parse_str::<Path>("::core::f64::NAN")?));
}
_ => (),
}
Expand Down Expand Up @@ -745,12 +748,13 @@ impl DefaultValue {
}

pub fn owned(&self) -> TokenStream {
let libname = super::collections_lib_name();
match *self {
DefaultValue::String(ref value) if value.is_empty() => {
quote!(::std::string::String::new())
quote!(::#libname::string::String::new())
}
DefaultValue::String(ref value) => quote!(#value.to_owned()),
DefaultValue::Bytes(ref value) if value.is_empty() => quote!(::std::vec::Vec::new()),
DefaultValue::Bytes(ref value) if value.is_empty() => quote!(::#libname::vec::Vec::new()),
DefaultValue::Bytes(ref value) => {
let lit = LitByteStr::new(value, Span::call_site());
quote!(#lit.to_owned())
Expand Down
1 change: 1 addition & 0 deletions prost-derive/src/lib.rs
Expand Up @@ -3,6 +3,7 @@
#![recursion_limit = "4096"]

extern crate proc_macro;
extern crate alloc;

use failure::bail;
use quote::quote;
Expand Down
2 changes: 1 addition & 1 deletion protobuf/build.rs
Expand Up @@ -174,7 +174,7 @@ fn install_conformance_test_runner(src_dir: &Path, prefix_dir: &Path) {

let rc = Command::new("make")
.arg("-j")
.arg(&num_jobs)
.arg("1") // Note: 1 instead of NUM_JOBS to work around bugs in makefile
.arg("install")
.current_dir(src_dir.join("conformance"))
.status()
Expand Down
1 change: 1 addition & 0 deletions tests-2015/Cargo.toml
Expand Up @@ -21,6 +21,7 @@ cfg-if = "0.1"
prost = { path = ".." }
prost-types = { path = "../prost-types" }
protobuf = { path = "../protobuf" }
tests-infra = { path = "../tests-infra" }

[dev-dependencies]
diff = "0.1"
Expand Down
19 changes: 9 additions & 10 deletions tests-alloc/Cargo.toml
Expand Up @@ -5,21 +5,20 @@ authors = ["Dan Burkert <dan@danburkert.com>"]
publish = false
edition = "2018"

build = "../tests/src/build.rs"
build = "src/build.rs"

[lib]
doctest = false
path = "../tests/src/lib.rs"

[features]
default = ["nostd-collections"]
nostd-collections = []
# The standard libtest relies on std, so if we want to test no_std builds,
# we should make a binary and not use #[test]
[[bin]]
name = "tests-alloc"
path = "src/bin.rs"

[dependencies]
bytes = { version = "0.5", default-features = false }
cfg-if = "0.1"
prost = { path = ".." }
prost-types = { path = "../prost-types" }
prost = { path = "..", default-features = false }
prost-derive = { path = "../prost-derive", default-features = false }
prost-types = { path = "../prost-types", default-features = false }
protobuf = { path = "../protobuf" }

[dev-dependencies]
Expand Down

0 comments on commit cc51c9d

Please sign in to comment.