Update hyper to latest nightly #324

renato-zannon · 2015-02-19T21:10:56Z

This makes hyper compile again under the recent Send changes, which just landed on nightly. I also addressed a few new warnings.

I tried to address the comment on server/acceptor.rs:26, but was unable to do so. It seems that I need to improve my lifetime-fu :)

Send no longer implies 'static; update needed lifetime bounds.

renato-zannon · 2015-02-19T21:17:16Z

Seems that travis' rust is outdated. I tested on rustc 1.0.0-nightly (dfc5c0f1e 2015-02-18) (built 2015-02-19)

Byron · 2015-02-20T10:33:47Z

When using the compiler version mentioned by @renato-zannon, the build, tests and benchmarks work for me too.

Byron · 2015-02-20T10:36:35Z

Oh, actually, the benchmarks seem to hang, like a deadlock of some sort. I am on OSX 10.10.2, in case it matters. Also it's the first time I am running hyper benchmarks, and thus can't tell if this hang is new.
I will keep going, in the hopes that this is just a bug in the benchmarks.

reem · 2015-02-20T21:28:20Z

@renato-zannon I may have been premature with that comment. Unless we want accept to block and for us to run it in another thread explicitly, we need to use Arc.

reem · 2015-02-20T21:29:33Z

src/server/acceptor.rs

-                                                  work: F,
-                                                  threads: usize) -> JoinGuard<'static, ()> {
+    pub fn accept<F>(self, work: F, threads: usize) -> JoinGuard<'static, ()>
+        where F: Fn(A::Stream) + Send + Sync + 'static {


Additional advantage of making this blocking: 'a here instead of 'static, so this can be used with non-'static data.

… data Change AcceptorPool to not spawn detached threads anymore. This, together with the recent `Send` changes, allows the `work` closure to close over non-`'static` data. This doesn't change the high-level `Server` interface, because that would make it's `listen` a blocking call (it's currently non-blocking) - which would be a breaking change.

renato-zannon · 2015-02-21T04:52:32Z

@reem I tried to address your comments on the last commit. I refrained from changing the non-blocking behavior of Server::listen - even though that would be necessary to make it possible to close over non-``static` data - because that would be a breaking change. It should be relatively straightforward to do though, and I would be happy to do it.

reem · 2015-02-21T04:58:33Z

Looks like good work, but there is still another error.

seanmonstar · 2015-02-21T05:15:43Z

An error about IntoCow, it looks like? And all those warnings about the
changed slice syntax will be errors in 'cargo test'...

On Fri, Feb 20, 2015, 11:58 PM Jonathan Reem notifications@github.com
wrote:

Looks like good work, but there is still another error.

—
Reply to this email directly or view it on GitHub
#324 (comment).

renato-zannon · 2015-02-21T05:20:49Z

Those are from a recent breaking change. I'll download the new nightly and fix that!

Necessary since [RFC 738](https://github.com/rust-lang/rfcs/blob/master/text/0738-variance.md).

renato-zannon · 2015-02-21T06:09:16Z

@reem I've updated the PR again even though it is still WIP, because there's an an error I haven't been able to figure out by myself - from lines 252 to 256 on src/lib.rs:

#[allow(unconditional_recursion)]
fn _assert_send<T: Send>() {
    _assert_send::<client::Request<net::Fresh>>();
    _assert_send::<client::Response>();
}

Error:

src/lib.rs:254:5: 254:48 error: the trait `core::marker::Send` is not implemented for the type `<str as collections::borrow::ToOwned>::Owned` [E0277]
src/lib.rs:254     _assert_send::<client::Request<net::Fresh>>();
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/lib.rs:254:5: 254:48 note: `<str as collections::borrow::ToOwned>::Owned` cannot be sent between threads safely
src/lib.rs:254     _assert_send::<client::Request<net::Fresh>>();
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

If I comment these lines out, hyper compiles, and all tests pass.

BTW: turns out there were lots of breaking changes on the nightly. Let me know if I should squash all these fix(rustup) commits.

reem · 2015-02-21T06:12:17Z

After we're done reviewing and are ready to merge you'll squash.

renato-zannon · 2015-02-21T12:02:08Z

Boiled this down to Cow<'static, str>. This doesn't compile:

#[allow(unconditional_recursion)]
fn _assert_send<T: Send>() {
    use std::borrow::Cow;
    _assert_send::<Cow<'static, str>>();
}

Fails with:

src/lib.rs:256:5: 256:38 error: the trait `core::marker::Send` is not implemented for the type `<str as collections::borrow::ToOwned>::Owned` [E0277]
src/lib.rs:256     _assert_send::<Cow<'static, str>>();
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/lib.rs:256:5: 256:38 note: `<str as collections::borrow::ToOwned>::Owned` cannot be sent between threads safely
src/lib.rs:256     _assert_send::<Cow<'static, str>>();
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
error: aborting due to previous error
Could not compile `hyper`.

Even though this compiles fine:

#[allow(unconditional_recursion)]
fn _assert_send<T: Send>() {
    use std::borrow::ToOwned;
    _assert_send::<<str as ToOwned>::Owned>();
}

Which seems contradictory to me.

softprops · 2015-02-21T19:16:59Z

I updated my cargo config to point to

[dependencies.hyper]
git = "https://github.com/renato-zannon/hyper.git"
rev = "124a31fb9b7363960bebbdcf71df9e1e1cb0e075"

My compiler errors/warnings went down to the following afterwards

error: the trait `core::marker::Send` is not implemented for the type `<str as collections::borrow::ToOwned>::Owned` [E0277]
 _assert_send::<client::Request<net::Fresh>>();
note: `<str as collections::borrow::ToOwned>::Owned` cannot be sent between threads safely
 _assert_send::<client::Request<net::Fresh>>();

seanmonstar · 2015-02-21T22:59:51Z

src/header/common/authorization.rs

@@ -22,14 +22,14 @@ impl<S: Scheme> DerefMut for Authorization<S> {
    }
 }

-impl<S: Scheme> Header for Authorization<S> {
+impl<S: Scheme + 'static> Header for Authorization<S> where <S as FromStr>::Err: 'static {


Huh, seems odd to need this bound, but I see why. Not something I would immediately think of...

Yes, I found this odd too. Took me a while to figure out where to put the bound

seanmonstar · 2015-02-21T23:10:18Z

@renato-zannon This is awesome, thanks! I realize this can be tedious work (updating for syntax changes).

I've fixed the last remaining thing, and will merge the new PR that includes your commits and mine. I feel like the commits are sufficiently separated that they can be merged as-is, without squashing.

seanmonstar · 2015-02-21T23:13:22Z

Merged as #329

renato-zannon · 2015-02-22T01:16:27Z

@seanmonstar Cool :) Even though they can be a bit tedious, these changes are a great way to get early exposure to the codebase. Maybe in the future I'll be able to submit more interesting PRs :)

softprops · 2015-02-22T02:44:23Z

Thanks for patching this up quickly. You guys are awesome

renato-zannon added 3 commits February 19, 2015 19:00

fix(rustup): update lifetime bounds

4ad95be

Send no longer implies 'static; update needed lifetime bounds.

fix(rustup): use module-level thread functions

bf6a51a

fix(rustup): update feature flags

c76fa20

renato-zannon mentioned this pull request Feb 19, 2015

fix(headers): use $crate when referring to hyper modules on macros #325

Merged

reem reviewed Feb 20, 2015
View reviewed changes

renato-zannon added 5 commits February 21, 2015 03:25

fix(rustup): Fix signature of IntoCow

40f3042

fix(rustup): Remove uses of the obsolete &a[] syntax

1fd8c26

fix(rustup): Add PhantomData markers to phantom type users

ad70106

Necessary since [RFC 738](https://github.com/rust-lang/rfcs/blob/master/text/0738-variance.md).

fix(rustup): Extend now takes an IntoIterator

e36c1b8

fix(rustup): CowString is gone

124a31f

seanmonstar mentioned this pull request Feb 21, 2015

Build failure with rust nightly #328

Closed

seanmonstar reviewed Feb 21, 2015
View reviewed changes

seanmonstar closed this Feb 21, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update hyper to latest nightly #324

Update hyper to latest nightly #324

renato-zannon commented Feb 19, 2015

renato-zannon commented Feb 19, 2015

Byron commented Feb 20, 2015

Byron commented Feb 20, 2015

reem commented Feb 20, 2015

reem Feb 20, 2015

renato-zannon commented Feb 21, 2015

reem commented Feb 21, 2015

seanmonstar commented Feb 21, 2015

renato-zannon commented Feb 21, 2015

renato-zannon commented Feb 21, 2015

reem commented Feb 21, 2015

renato-zannon commented Feb 21, 2015

softprops commented Feb 21, 2015

seanmonstar Feb 21, 2015

renato-zannon Feb 22, 2015

seanmonstar commented Feb 21, 2015

seanmonstar commented Feb 21, 2015

renato-zannon commented Feb 22, 2015

softprops commented Feb 22, 2015

Update hyper to latest nightly #324

Update hyper to latest nightly #324

Conversation

renato-zannon commented Feb 19, 2015

renato-zannon commented Feb 19, 2015

Byron commented Feb 20, 2015

Byron commented Feb 20, 2015

reem commented Feb 20, 2015

reem Feb 20, 2015

Choose a reason for hiding this comment

renato-zannon commented Feb 21, 2015

reem commented Feb 21, 2015

seanmonstar commented Feb 21, 2015

renato-zannon commented Feb 21, 2015

renato-zannon commented Feb 21, 2015

reem commented Feb 21, 2015

renato-zannon commented Feb 21, 2015

softprops commented Feb 21, 2015

seanmonstar Feb 21, 2015

Choose a reason for hiding this comment

renato-zannon Feb 22, 2015

Choose a reason for hiding this comment

seanmonstar commented Feb 21, 2015

seanmonstar commented Feb 21, 2015

renato-zannon commented Feb 22, 2015

softprops commented Feb 22, 2015