From f4b553b7f6eeb526d6096f5a2373311a264548ac Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Tue, 25 Sep 2018 14:11:00 +0200 Subject: [PATCH 1/5] Improve Travis-CI config Several improvements: - Split "without nightly feature" and "with nightly feature" into two jobs - Add a job that builds on beta - Fix `RUST_FLAGS` => `RUSTFLAGS` typo - Remove cargo cache. Caching is not always good. Download the cache takes time and since we compile with different feature sets, the `target` folder cannot really be reused anyway. We should keep an eye on this, though. - Early exit in scripts (fail when first command fails) --- .travis.yml | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index 396d127..5509cc4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,14 +1,21 @@ language: rust -rust: - - nightly -script: - - cargo build --verbose - - cargo test --verbose - - cargo build --verbose --features=nightly - - cargo test --verbose --features=nightly +matrix: + include: + - name: "Build & test on nightly WITH nightly feature" + rust: nightly + script: + - cargo build --verbose --features=nightly || travis_terminate 1 + - cargo test --verbose --features=nightly || travis_terminate 1 + - name: "Build & test on nightly WITHOUT nightly feature" + rust: nightly + script: + - cargo build --verbose || travis_terminate 1 + - cargo test --verbose || travis_terminate 1 + - name: "Build on beta" + rust: beta + script: cargo build --verbose env: - - RUST_FLAGS="--deny warnings" - -cache: cargo + global: + - RUSTFLAGS="--deny warnings" From c15616528154a4efc688b63117acb4d22de391c5 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Thu, 4 Oct 2018 16:09:14 +0200 Subject: [PATCH 2/5] Fix late-bound lifetime warning PR #35 introduced a warning in one of the examples. We were passing all generic parameters to the inner function, including lifetimes. But passing late-bound lifetimes won't be allowed in the future. This commit simply ignores lifetime parameters. AFAICT this should never be a problem. Passing lifetimes explicitly hardly ever makes sense. The only case I can think of where the changes of this commit could break anything is in this function: fn foo<'a>() -> &'a i32 { ... } But functions with an early-bound lifetime (return type) and no late-bound ones are extremely rare and special. So for now we can ignore them. In particular, determining which lifetime parameters are early- and which are late-bound is extremely difficult. Lastly, const parameters are not yet correctly passed to the inner function. This needs to be fixed in the future, when const parameters are a thing. --- src/gen.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/gen.rs b/src/gen.rs index f725b4e..5971871 100644 --- a/src/gen.rs +++ b/src/gen.rs @@ -430,9 +430,19 @@ fn gen_method_item( // Generate the list of argument used to call the method. let args = get_arg_list(sig.decl.inputs.iter())?; - // Builds turbofish with generic types - let (_, generic_types, _) = sig.decl.generics.split_for_impl(); - let generic_types = generic_types.as_turbofish(); + // Builds turbofish with generic types (without lifetimes) + let generic_types = sig.decl.generics + .type_params() + .map(|param| { + let name = ¶m.ident; + quote! { #name , } + }) + .collect::(); + let generic_types = if generic_types.is_empty() { + generic_types + } else { + quote ! { ::<#generic_types> } + }; // Generate the body of the function. This mainly depends on the self type, // but also on the proxy type. @@ -452,14 +462,14 @@ fn gen_method_item( // Receiver `self` (by value) SelfType::Value => { // The proxy type is a Box. - quote! { (*self).#name#generic_types(#args) } + quote! { (*self).#name #generic_types(#args) } } // `&self` or `&mut self` receiver SelfType::Ref | SelfType::Mut => { // The proxy type could be anything in the `Ref` case, and `&mut` // or Box in the `Mut` case. - quote! { (*self).#name#generic_types(#args) } + quote! { (*self).#name #generic_types(#args) } } }; From 3bbe127a52ed404dbbc4b36349e3a3c07436b5ca Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Thu, 4 Oct 2018 16:17:03 +0200 Subject: [PATCH 3/5] Add `proc_macro_def_site` feature in `nightly` mode --- src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 109b025..f1dede1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,7 +2,10 @@ //! references, some common smart pointers and closures. -#![cfg_attr(feature = "nightly", feature(proc_macro_diagnostic, proc_macro_span))] +#![cfg_attr( + feature = "nightly", + feature(proc_macro_diagnostic, proc_macro_span, proc_macro_def_site) +)] extern crate proc_macro; #[macro_use] From fe83565f58b56439ec2616cb43226412d16f3d89 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Thu, 4 Oct 2018 17:15:39 +0200 Subject: [PATCH 4/5] Fix build-plan generation with `nightly` feature --- tests/util/mod.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/util/mod.rs b/tests/util/mod.rs index 06686ce..b849a54 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -21,11 +21,14 @@ pub(crate) fn get_dep_path() -> PathBuf { // Obtain the build plan from `cargo build`. This JSON plan will tell us // several things, including the path of the output of `auto_impl` (usually // an .so file on Linux). - let output = Command::new(env!("CARGO")) - .args(&["build", "-Z", "unstable-options", "--build-plan"]) - .stderr(Stdio::inherit()) - .output() - .expect("failed to run `cargo build`"); + let mut command = Command::new(env!("CARGO")); + command.args(&["build", "-Z", "unstable-options", "--build-plan"]); + command.stderr(Stdio::inherit()); + + #[cfg(feature = "nightly")] + command.arg("--features=nightly"); + + let output = command.output().expect("failed to run `cargo build`"); if !output.status.success() { panic!("failed to run `cargo build`"); From 0f63c858cb7ff204b201dcc437af0b32a1bf0b7b Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Fri, 5 Oct 2018 12:11:13 +0200 Subject: [PATCH 5/5] Add comment explaining not explicitly passing lifetime parameters to inner functions --- src/gen.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/gen.rs b/src/gen.rs index 5971871..a4d08eb 100644 --- a/src/gen.rs +++ b/src/gen.rs @@ -430,7 +430,27 @@ fn gen_method_item( // Generate the list of argument used to call the method. let args = get_arg_list(sig.decl.inputs.iter())?; - // Builds turbofish with generic types (without lifetimes) + // Build the turbofish type parameters. We need to pass type parameters + // explicitly as they cannot be inferred in all cases (e.g. something like + // `mem::size_of`). However, we don't explicitly specify lifetime + // parameters. Most lifetime parameters are so called late-bound lifetimes + // (ones that stick to input parameters) and Rust prohibits us from + // specifying late-bound lifetimes explicitly (which is not a problem, + // because those can always be correctly inferred). It would be possible to + // explicitly specify early-bound lifetimes, but this is hardly useful. + // Early-bound lifetimes are lifetimes that are only attached to the return + // type. Something like: + // + // fn foo<'a>() -> &'a i32 + // + // It's hard to imagine how such a function would even work. So since those + // functions are really rare and special, we won't support them. In + // particular, for us to determine if a lifetime parameter is early- or + // late-bound would be *really* difficult. + // + // So we just specify type parameters. In the future, however, we need to + // add support for const parameters. But those are not remotely stable yet, + // so we can wait a bit still. let generic_types = sig.decl.generics .type_params() .map(|param| {