Compile causes warnings about macros

[kpcyrd]

Compiling my project results in a bunch of warnings:

warning: code relies on type inference rules which are likely to change
  --> src/main.rs:83:21
   |
83 |       server.get("/", middleware! { |_, res|
   |  _____________________^ starting here...
84 | |         let mut data = HashMap::new();
85 | |         data.insert("name", "user");
86 | |         return res.render("templates/index.html", &data);
87 | |     });
   | |_____^ ...ending here
   |
   = note: #[warn(resolve_trait_on_defaulted_unit)] on by default
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #39216 <https://github.com/rust-lang/rust/issues/39216>
   = note: this error originates in a macro outside of the current crate

warning: unreachable expression
  --> src/main.rs:83:39
   |
83 |     server.get("/", middleware! { |_, res|
   |                                       ^^^
   |
   = note: #[warn(unreachable_code)] on by default

warning: code relies on type inference rules which are likely to change
  --> src/main.rs:89:28
   |
89 |       server.get("/e/:hash", middleware! { |req, res|
   |  ____________________________^ starting here...
90 | |         let hash = req.param("hash").unwrap();
91 | |
92 | |         let mut data = HashMap::new();
93 | |         data.insert("hash", hash);
94 | |         return res.render("templates/view.html", &data);
95 | |     });
   | |_____^ ...ending here
   |
   = note: #[warn(resolve_trait_on_defaulted_unit)] on by default
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #39216 <https://github.com/rust-lang/rust/issues/39216>
   = note: this error originates in a macro outside of the current crate

warning: unreachable expression
  --> src/main.rs:89:48
   |
89 |     server.get("/e/:hash", middleware! { |req, res|
   |                                                ^^^
   |
   = note: #[warn(unreachable_code)] on by default

This happens with router! as well. My code is very similar to the examples on http://nickel.rs, I'm not sure if I'm doing something wrong.

[davidpodhola]

Same here, related to rust-lang/rust#39216

[jolhoeft]

This appears to be causing four tests to fail during cargo test.

[jolhoeft]

This seems to be only a problem when using the middleware macro. Implementing the Middleware trait bypasses this issue.

[jolhoeft]

The problem arises in the middleware macro (in
https://github.com/nickel-org/nickel.rs/blob/master/src/macros/middleware.rs),
specifically in the _middleware_inner support macro. This snippet of
code from the form_data example:

server.get("/", middleware! { |_, res|
    let mut data = HashMap::new();
    data.insert("title","Contact");

    return res.render("examples/form_data/views/contact.html", &data)
});

expands to (via cargo expand):

server.get("/", {
    // use ::{MiddlewareResult, Responder, Response, Request};
    // #[inline(always)]
    fn restrict<'mw, D, R: Responder<D>>(r: R, res: Response<'mw, D>) -> MiddlewareResult<'mw, D> {
        res.send(r)
    }
    // #[inline(always)]
    fn restrict_closure<F, D>(f: F) -> F
        where F: for<'r, 'mw, 'conn>Fn(&'r mut Request<'mw, 'conn, D>, Response<'mw, D>)-> MiddlewareResult<'mw, D> + Send + Sync {
        f
    }
    restrict_closure(move |_, res| {
        restrict({
            let mut data = HashMap::new();
            data.insert("title", "Contact");
            return res.render("examples/form_data/views/contact.html", &data)
        }, res)
    })
});

The compiler warning is on the call to the restrict method, which is
triggering the resolve_trait_on_defaulted_unit warning. Apparently we
not being explicit enough in types. My initial thought was to
explicitly set the type parameter D to () (the correct value in this
case) in the expanded code, but that failed to fix the warning.

[lawliet89]

So I was trying to build the form_data example and I kept getting:

warning: unreachable expression
  --> examples/form_data/form_data.rs:8:21
   |
8  |       server.get("/", middleware! { |_, res|
   |  _____________________^
9  | |         let mut data = HashMap::new();
10 | |         data.insert("title","Contact");
11 | |
12 | |         return res.render("examples/form_data/views/contact.html", &data)
13 | |     });
   | |_____^

Expanded, it becomes:

warning: unreachable expression
  --> examples/form_data/form_data.rs:34:17
   |
34 |                 res,
   |                 ^^^
   |
   = note: #[warn(unreachable_code)] on by default

I realised what's probably happening:

1. The return res.render("examples/form_data/views/contact.html", &data) is causing your expanded macro to never invoke restrict.
2. Therefore the compiler cannot infer the type of res.
3. According to Tracking issue for future-incompatibility lint resolve_trait_on_defaulted_unit rust-lang/rust#39216, it used to infer this as () but no longer.

If you change your expanded code to something like:

#[macro_use] extern crate nickel;
use nickel::{Nickel, HttpRouter, FormBody};
use std::collections::HashMap;

fn main() {
    let mut server = Nickel::new();

    server.get("/", {
        use nickel::{MiddlewareResult, Responder, Response, Request};
        #[inline(always)]
        fn restrict<'mw, D, R: Responder<D>>(
            r: R,
            res: Response<'mw, D>,
        ) -> MiddlewareResult<'mw, D> {
            res.send(r)
        }
        #[inline(always)]
        fn restrict_closure<F, D>(f: F) -> F
        where
            F: for<'r, 'mw, 'conn> Fn(&'r mut Request<'mw, 'conn, D>, Response<'mw, D>)
                                   -> MiddlewareResult<'mw, D>
                + Send
                + Sync,
        {
            f
        }
        restrict_closure(move |_, res| {
            restrict::<_, ()>(
                {
                    let mut data = HashMap::new();
                    data.insert("title", "Contact");
                    return res.render("examples/form_data/views/contact.html", &data);
                },
                res,
            )
        })

    });

    server.post("/confirmation", {
        use nickel::{MiddlewareResult, Responder, Response, Request};
        #[inline(always)]
        fn restrict<'mw, D, R: Responder<D>>(
            r: R,
            res: Response<'mw, D>,
        ) -> MiddlewareResult<'mw, D> {
            res.send(r)
        }
        #[inline(always)]
        fn restrict_closure<F, D>(f: F) -> F
        where
            F: for<'r, 'mw, 'conn> Fn(&'r mut Request<'mw, 'conn, D>, Response<'mw, D>)
                                   -> MiddlewareResult<'mw, D>
                + Send
                + Sync,
        {
            f
        }
        restrict_closure(move |req, res| {
            restrict::<_, ()>(
                {
                    let form_data = {
                        match req.form_body() {
                            ::std::result::Result::Ok(val) => val,
                            ::std::result::Result::Err(e) => return Err(From::from((res, e))),
                        }
                    };
                    println!("{:?}", form_data);
                    let mut data = HashMap::new();
                    data.insert("title", "Confirmation");
                    data.insert(
                        "firstname",
                        form_data.get("firstname").unwrap_or("First name?"),
                    );
                    data.insert(
                        "lastname",
                        form_data.get("lastname").unwrap_or("Last name?"),
                    );
                    data.insert("phone", form_data.get("phone").unwrap_or("Phone?"));
                    data.insert("email", form_data.get("email").unwrap_or("Email?"));
                    return res.render("examples/form_data/views/confirmation.html", &data);
                },
                res,
            )
        })
    });


    server.listen("0.0.0.0:8080").unwrap();
}

it builds. The change was to explicitly state the type parameter R of restrict is ().

P.S.: I thought the return line in your macro invocation was quite unidiomatic and very unexpected.

[jolhoeft]

The unidiomatic return does seem part of the underlying cause. All the places where this bug turns up so far have that pattern. The cases where it doesn't work fine, since the final res sets the type of the return.

Unfortunately adding the type signature to the macro is breaking the use cases without the return.

[jolhoeft]

I am starting to understand. The middleware macro looks like it takes a closure, but actually is unpacking what looks like one. The block is supposed to return something implementing Responder, which is then passed to the Response. res.render doesn't, but the return statement breaks out of the macro expansion before the type issues manifest, except as the inference failure we are seeing here. This seems to be rather fragile.

I think the middleware macro will need to be reworked in an API breaking way. Currently it is being used in two different ways. One, to emulate a closure that returns aResponder, and the other to emulate a closure that returns a MiddlewareResult.

[jolhoeft]

I am not seeing a way to fix this without breaking existing code. Since the migration to hyper 0.11.a in #402 is going to change the API anyway, I plan on addressing this as part of the eventual nickel 0.11.

[jolhoeft]

PR #414 addresses this issue partially, in that it fixes currently breaking examples and documents the macro's limitations.