Support for 2018 edition

[FallingSnow]

Currently capnpc generates incompatible code with rust 2018.

Seems to be an issue with using :: to refer to the crate root. This has been changed to crate:: in 2018.

2015 Edition:

#[inline]
pub fn get_currency(self) -> ::std::result::Result<::account_capnp::Currency,::capnp::NotInSchema> {
  ::capnp::traits::FromU16::from_u16(self.reader.get_data_field::<u16>(16))
}

2018 Edition:

#[inline]
pub fn get_currency(self) -> ::std::result::Result<crate::account_capnp::Currency,::capnp::NotInSchema> {
  ::capnp::traits::FromU16::from_u16(self.reader.get_data_field::<u16>(16))
}

[dwrensha]

Thanks for the report.
I suppose probably the easiest way to go would be to add a rust_edition field to the CompilerCommand struct. It could have type:

enum RustEdition {
    Rust2015,
    Rust2018
}

[FallingSnow]

I agree.

Is there an easy way to write a test to test this?

[FallingSnow]

I found passing another argument (RustEdition) around codegen.rs a little complex so I went for the minimum viable product.

Proof of concept: FallingSnow@2b0fcfe

[FallingSnow]

Nightly (8-23-2018) also seems to have an issue with the generated try!() statements for some odd reason so I updated every try!() I could find to ?. The changes can be found in the same repo as above.

All tests pass and I'm willing to submit a pull request if you would like.

[dwrensha]

Cool! I think before getting this landed it would be good to avoid the unsafe static mutable variable. Probably there should be a new field of type RustEdition on GeneratorContext. Unfortunately, we can't change the signature of codegen::main() in a version-compatible way, so we would need to write a new function that run_command() could call. Maybe something like generate_code<T>(inp: T, out_dir: &Path, edition: RustEdition) -> Result<()> where T: Reader, and the existing codegen::main() function could delegate to that.

For testing, I think it would make sense to update one of the examples to use 2018 edition.

[FallingSnow]

Cool! I think before getting this landed it would be good to avoid the unsafe static mutable variable.

I agree.

For testing, I think it would make sense to update one of the examples to use 2018 edition.

As far as I'm aware you have to define the rust version as the crate level. Is it possible to use 2018 in a single example?

[dwrensha]

All of the capnp examples are in their own crates because they require code generation. I think the addressbook_send example would be a good candidate for testing with the 2018 edition. We would also need to find a way to tell travis to only try to compile that example on nightly.

[FallingSnow]

I got it mostly setup. Rust edition is now passed to the generator context. FallingSnow@96b8c13

I just don't know where to add crate:: if necessary. I found

capnproto-rust/capnpc/src/codegen.rs

Lines 53 to 68 in 3502749

	for requested_file in try!(gen.request.get_requested_files()).iter() {
	let id = requested_file.get_id();
	let imports = try!(requested_file.get_imports());
	for import in imports.iter() {
	let importpath = ::std::path::Path::new(try!(import.get_name()));
	let root_name: String = format!(
	"::{}_capnp",
	try!(path_to_stem_string(importpath)).replace("-", "_"));
	try!(populate_scope_map(&gen.node_map, &mut gen.scope_map, vec!(root_name), import.get_id()));
	}
	let root_name = try!(path_to_stem_string(try!(requested_file.get_filename())));
	let root_mod = format!("::{}_capnp", root_name.replace("-", "_"));
	try!(populate_scope_map(&gen.node_map, &mut gen.scope_map, vec!(root_mod), id));
	}

which seems close, but this adds crate:: to every path, not just the generated code paths.

[dwrensha]

Does this work?

diff --git a/capnpc/src/codegen.rs b/capnpc/src/codegen.rs
index fd1e50a..71e9a07 100644
--- a/capnpc/src/codegen.rs
+++ b/capnpc/src/codegen.rs
@@ -43,6 +43,17 @@ pub struct GeneratorContext<'a> {
     pub rust_edition: RustEdition,
 }
 
+fn root_scope(rust_edition: RustEdition, root_name: String) -> Vec<String> {
+    match rust_edition {
+        RustEdition::Rust2015 => {
+            vec!["crate".into(), root_name]
+        }
+        RustEdition::Rust2018 => {
+            vec![format!("::{}", root_name)]
+        }
+    }
+}
+
 impl <'a> GeneratorContext<'a> {
     pub fn new(message:&'a capnp::message::Reader<capnp::serialize::OwnedSegments>, rust_edition: RustEdition)
             -> ::capnp::Result<GeneratorContext<'a>> {
@@ -65,14 +76,20 @@ impl <'a> GeneratorContext<'a> {
             for import in imports.iter() {
                 let importpath = ::std::path::Path::new(import.get_name()?);
                 let root_name: String = format!(
-                    "::{}_capnp",
+                    "{}_capnp",
                     path_to_stem_string(importpath)?.replace("-", "_"));
-                populate_scope_map(&gen.node_map, &mut gen.scope_map, vec!(root_name), import.get_id())?;
+                populate_scope_map(&gen.node_map,
+                                   &mut gen.scope_map,
+                                   root_scope(rust_edition, root_name),
+                                   import.get_id())?;
             }
 
             let root_name = path_to_stem_string(requested_file.get_filename()?)?;
-            let root_mod = format!("::{}_capnp", root_name.replace("-", "_"));
-            populate_scope_map(&gen.node_map, &mut gen.scope_map, vec!(root_mod), id)?;
+            let root_mod = format!("{}_capnp", root_name.replace("-", "_"));
+            populate_scope_map(&gen.node_map,
+                               &mut gen.scope_map,
+                               root_scope(rust_edition, root_mod),
+                               id)?;
         }
         Ok(gen)
     }

[FallingSnow]

Yes. I flip flopped the output of 2015 and 2018 and it appears to be working. Fork

I've just been running cargo test --all to see if it works. Is that sufficient?

[dwrensha]

Yep. You could also try running the example with cargo run -p addressbook_send.

I predict that cargo test --all will fail if you switch to a stable or beta rustc. We'll need to update the travis config to account for that. I'm not sure what the best way would be, but I did just notice that cargo build has an --exclude flag that could help.

[FallingSnow]

$ cargo +stable build --all --exclude addressbook_send
error: failed to parse manifest at `/home/ayrton/Documents/capnproto-rust/example/addressbook_send/Cargo.toml`

Caused by:
  the cargo feature `edition` requires a nightly version of Cargo, but this is the `stable` channel

Hmm. Removing addressbook_send from the top level cargo.toml and adding a command to test it explicitly is my best idea at the moment.

[dwrensha]

I'd be okay with not checking in anything that tests the new edition until it's actually on a stable rustc release.

[FallingSnow]

We'll do that then.

[athre0z]

Any update on wrapping this into a PR and getting it merged? The fork seems to be working well for me!

[dwrensha]

Resolved in 322becc

[FallingSnow]

:D

[athre0z]

Nice, thanks!

[Restioson]

Perhaps it would be good to push a new version to cargo?

[dwrensha]

@Restioson Support for Rust 2018 was included in the 0.9.1 version of the capnpc crate, published 12 November 2018.

[dwrensha]

With more recent versions of rustc, we can generate code that works for both editions. I've pushed a change to take advantage of that: 6f09e5f