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

Serialized newtype structs in serde #526

Closed
Relrin opened this issue Aug 24, 2016 · 9 comments
Closed

Serialized newtype structs in serde #526

Relrin opened this issue Aug 24, 2016 · 9 comments
Labels

Comments

@Relrin
Copy link

Relrin commented Aug 24, 2016

Currently I'm trying to implement serializing newtype structs. So, we have the next code:

#[inline]
fn serialize_newtype_struct<T>(
    &mut self, _name: &'static str, value: T
) -> Result<()> where T: ser::Serialize {
    let header = vec![BertTag::SmallTuple as u8];
    try!(self.writer.write_all(header.as_slice()));

    let structure_name_atom = self.get_atom(_name);
    try!(self.writer.write_all(structure_name_atom.as_slice()));

    value.serialize(self)
}

And simple test for this stuff:

#[test]
fn serialize_newtype_struct() {

    struct Point(i32, i32);
    let point = Point(1, 2);

    assert_eq!(
        term_to_binary(&point).unwrap(),
       // ... some result there
    )
}

term_to_binary function it's no more than a little wrapper, which let so use BERT serializer more easier way. It has the next signature:

/// Encode the passed value into a `[u8]` writer.
#[inline]
pub fn to_writer<W, T>(
        writer: &mut W, value: &T
    ) -> Result<()> where W: io::Write, T: ser::Serialize {
    let mut ser = Serializer::new(writer);
    try!(value.serialize(&mut ser));
    Ok(())
}


/// Encode the specified struct into a `[u8]` buffer.
#[inline]
pub fn to_vec<T>(value: &T) -> Result<Vec<u8>> where T: ser::Serialize {
    let mut writer = Vec::with_capacity(128);
    try!(to_writer(&mut writer, value));
    Ok(writer)
}


/// Convert passed value to a BERT representation.
#[inline]
pub fn term_to_binary<T> (
        value: &T
    ) -> Result<Vec<u8>> where T: ser::Serialize {
    let mut binary = vec![EXT_VERSION];
    let data = try!(to_vec(value));
    binary.extend(data.iter().clone());
    Ok(binary)
}

So, how can I invoke the serialize_newtype_struct method with passed into term_to_binary newtype struct from test?

@dtolnay
Copy link
Member

dtolnay commented Aug 24, 2016

struct Point(i32, i32) is a tuple struct, not a newtype struct. Try using something like struct Meters(i32).

See https://serde.rs/impl-serialize.html#serializing-a-struct and The Book for the different types of structs.

@dtolnay
Copy link
Member

dtolnay commented Aug 24, 2016

You also need to set up Serde to generate a Serialize implementation for your struct. As an example see how Bincode does this, and it is documented at https://serde.rs/codegen.html.

In Cargo.toml add:

[dev-dependencies]
serde_macros = "0.8"

In tests/tests.rs add:

#![feature(plugin, custom_derive)]
#![plugin(serde_macros)]

#[test]
fn serialize_newtype_struct() {
    #[derive(Serialize, Deserialize)]
    struct Meters(i32);
    let km = Meters(1000);

    assert_eq!(
        term_to_binary(&km).unwrap(),
        // ... some result there
    );
}

You need to move your tests out of src and into a tests directory, the way Bincode has them. Otherwise your crate will depend on serde_macros even when not running tests.

Also this setup requires that your tests are run with a nightly compiler only (which is good enough for Bincode). There is a Rust PR rust-lang/rust#35957 which will allow it to work on stable so we are not that far away, but if you want it to work on a stable compiler now, there is a more complicated setup using a build script - see serde-yaml as an example.

EDIT: never mind about the crossed out part. You can put this in src/lib.rs and keep the tests where they are.

#![cfg_attr(test, feature(plugin, custom_derive))]
#![cfg_attr(test, plugin(serde_macros))]

@Relrin
Copy link
Author

Relrin commented Aug 24, 2016

So, I have separated my module into two different: bert and bert_tests.
In the bert module my Cargo.toml looks like this:

[dependencies]
serde = "0.8.3"
num = "0.1.34"
byteorder = "0.5.3"

[dev-dependencies]
serde_macros = "0.8.*"

And in bert_tests Cargo.toml file:

[features]
default = ["serde_macros"]
with-syntex = ["syntex", "serde_codegen", "indoc/with-syntex"]

[build-dependencies]
syntex = { version = "*", optional = true }
serde_codegen = { version = "0.8.0", optional = true }
indoc = "*"

[dependencies]
bert = { path = "../bert" }
serde = "0.8.3"
serde_macros = { version = "0.8.0", optional = true }

[[test]]
name = "test"
path = "tests/test.rs"

In the test.rs of bert_tests crate:

#![cfg_attr(not(feature = "with-syntex"), feature(custom_derive, plugin))]
#![cfg_attr(not(feature = "with-syntex"), plugin(serde_macros))]

extern crate serde;
extern crate bert;

#[cfg(feature = "with-syntex")]
include!(concat!(env!("OUT_DIR"), "/test.rs"));

#[cfg(not(feature = "with-syntex"))]
include!("test.rs.in");

But in the result I'd taken an error from the Rust compiler:

relrin at MacBook-Relrin in ~/code/bert-rs/bert_tests on git:serde-rs-support+?
=> cargo test
Compiling aster v0.25.0
Compiling rustc-serialize v0.3.19
Compiling num-traits v0.1.35
Compiling serde_codegen v0.8.4
Compiling quasi v0.18.0
/Users/savicvalera/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/aster-0.25.0/src/lib.rs:1:43: 1:66 error: #[feature] may not be used on the stable release channel
/Users/savicvalera/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/aster-0.25.0/src/lib.rs:1 #![cfg_attr(not(feature = "with-syntex"), feature(rustc_private))]
^~~~~~~~~~~~~~~~~~~~~~~
error: aborting due to previous error
Build failed, waiting for other jobs to finish...
/Users/savicvalera/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/quasi-0.18.0/src/lib.rs:11:43: 11:66 error: #[feature] may not be used on the stable release channel
/Users/savicvalera/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/quasi-0.18.0/src/lib.rs:11 #![cfg_attr(not(feature = "with-syntex"), feature(rustc_private))]
^~~~~~~~~~~~~~~~~~~~~~~
error: aborting due to previous error
error: Could not compile aster.

To learn more, run the command again with --verbose.

@dtolnay
Copy link
Member

dtolnay commented Aug 24, 2016

Running the tests with serde_macros (the default) requires a nightly compiler.

$ rustup run nightly cargo test

Running them without serde_macros on a stable compiler:

$ rustup run stable cargo test --no-default-features --features with-syntex

  # or if you are using stable by default, just:
$ cargo test --no-default-features --features with-syntex

In bert_tests/Cargo.toml you can set stable to be the default by doing this, although typically it will compile faster on nightly so I would recommend keeping the default as nightly.

[features]
default = ["with-syntex"]  # instead of "serde-macros"

Then on stable:

$ rustup run stable cargo test

And on nightly:

$ rustup run nightly cargo test --no-default-features --features serde_macros

@Relrin
Copy link
Author

Relrin commented Aug 24, 2016

I don't understand this clearly, because I'm new in Rust lang. Could you say, whether I'm doing it right? Maybe I'm doing it wrong:

  1. Installed rustup toolchain on my Mac.
  2. Set default in rustup toolchain to stable and downloaded nightly build also.
  3. I've fixed my test.rs and Cargo.toml files in the bert_tests crate to this:

test.rs:

#![cfg_attr(not(feature = "with-syntex"), feature(custom_derive, plugin))]
#![cfg_attr(not(feature = "with-syntex"), plugin(serde_macros))]

extern crate serde;
extern crate bert;

#[cfg(feature = "with-syntex")]
include!(concat!(env!("OUT_DIR"), "/test.rs"));

#[cfg(not(feature = "with-syntex"))]
include!("test.rs.in");

Cargo.toml

[features]
default = ["with-syntex"]
with-syntex = ["syntex", "serde_codegen"]

[build-dependencies]
syntex = { version = "*", optional = true }
serde_codegen = { version = "0.8.0", optional = true }

[dependencies]
bert = { path = "../bert" }
serde = "0.8.3"
serde_macros = { version = "0.8.0", optional = true }

[[test]]
name = "test"
path = "tests/test.rs"
  1. Fixed bert/src/lib.rs to this:
#![cfg_attr(test, feature(plugin, custom_derive))]
#![cfg_attr(test, plugin(serde_macros))]

extern crate byteorder;
extern crate num;
extern crate serde;

pub use errors::{Error};
pub use serializers::{Serializer, term_to_binary, to_vec, to_writer};
pub use types::{
    BERT_LABEL, EXT_VERSION,
    BertTag, BertNil, BertTime, BertRegex
};

mod serializers;
mod types;
mod errors;
  1. After that in my terminal I must:
    Run my tests on the stable branch:
$ rustup run stable cargo test

and nightly branch:

$ rustup run nightly cargo test --no-default-features --features serde_macros
  1. All this tests should successfully passed?

If all these steps are right, so... Compiler warn me, that some of functionality are private, not public:

rustup run nightly cargo test
 Downloading syntex v0.42.2
 Downloading syntex_syntax v0.42.0
 Downloading syntex_errors v0.42.0
 Downloading log v0.3.6
 Downloading syntex_pos v0.42.0
 Downloading term v0.4.4
 Downloading unicode-xid v0.0.3
 Downloading winapi v0.2.8
 Downloading kernel32-sys v0.2.2
 Downloading winapi-build v0.1.1
 Downloading bitflags v0.5.0
   Compiling winapi-build v0.1.1
   Compiling unicode-xid v0.0.3
   Compiling log v0.3.6
   Compiling syntex_pos v0.42.0
   Compiling bitflags v0.5.0
   Compiling winapi v0.2.8
   Compiling kernel32-sys v0.2.2
   Compiling term v0.4.4
   Compiling syntex_errors v0.42.0
   Compiling syntex_syntax v0.42.0
   Compiling serde_codegen_internals v0.7.0
   Compiling syntex v0.42.2
   Compiling aster v0.25.0
   Compiling quasi v0.18.0
   Compiling quasi_codegen v0.18.0
   Compiling serde_codegen v0.8.4
   Compiling bert_tests v0.1.0 (file:///Users/savicvalera/code/bert-rs/bert_tests)
error: module `serializers` is private
 --> /Users/savicvalera/code/bert-rs/bert_tests/target/debug/build/bert_tests-b995ea8bc9a96268/out/test.rs:5:29
  |
5 |     use bert::serializers::{Serializer, term_to_binary, to_vec};
  |                             ^^^^^^^^^^

error: module `serializers` is private
 --> /Users/savicvalera/code/bert-rs/bert_tests/target/debug/build/bert_tests-b995ea8bc9a96268/out/test.rs:5:41
  |
5 |     use bert::serializers::{Serializer, term_to_binary, to_vec};
  |                                         ^^^^^^^^^^^^^^

error: module `serializers` is private
 --> /Users/savicvalera/code/bert-rs/bert_tests/target/debug/build/bert_tests-b995ea8bc9a96268/out/test.rs:5:57
  |
5 |     use bert::serializers::{Serializer, term_to_binary, to_vec};
  |                                                         ^^^^^^

error: module `types` is private
 --> /Users/savicvalera/code/bert-rs/bert_tests/target/debug/build/bert_tests-b995ea8bc9a96268/out/test.rs:6:23
  |
6 |     use bert::types::{BertTag};
  |                       ^^^^^^^

error: aborting due to 4 previous errors

error: Could not compile `bert_tests`.

To learn more, run the command again with --verbose.

@dtolnay
Copy link
Member

dtolnay commented Aug 24, 2016

See Crates and Modules: Exporting a Public Interface.

In bert/src/lib.rs you have mod serializers and pub use serializers::{Serializer, term_to_binary, to_vec, to_writer}, which means bert::serializers is private but bert::serializers::Serializer is exposed publically as just bert::Serializer (and similarly for the other types/functions/modules). Your tests (and other crates) should be using the public interface: bert::{Serializer, term_to_binary, to_vec, to_writer}.

@Relrin
Copy link
Author

Relrin commented Aug 24, 2016

I've fixed this issue. Just add the pub before the mod keyword. Now all tests have passed and works fine. Thanks!

@dtolnay
Copy link
Member

dtolnay commented Aug 24, 2016

You shouldn't have both pub mod serializers and pub use serializers::{...} because then the types and functions are exposed publically in two places which is confusing: bert::term_to_binary and bert::serializers::term_to_binary. I would recommend using just mod serializers like you had before, but fixing the tests to use bert::{...} instead of bert::serializers::{...}.

@Relrin
Copy link
Author

Relrin commented Aug 24, 2016

I've got it! Thanks for the useful tip. 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants