Replace try! macro with '?' operator.

seanmonstar / httparse

A push parser for the HTTP 1.x protocol in Rust.

https://docs.rs/httparse

Apache License 2.0

569 stars 110 forks source link

Replace try! macro with '?' operator. #94

Closed sielicki closed 3 years ago

sielicki commented 3 years ago

Building with 'cargo build -vv' complains about try! being deprecated. Replace with functionally-identical '?' operator.

sielicki commented 3 years ago

It's failing because this isn't stable on 1.10.0 (current MSRV), feel free to close if that's non-negotiable.

Otherwise, if you'd like, I can bump MSRV to like 1.31.0 in travis and rebase this change on that.

seanmonstar commented 3 years ago

I think this can be fixed with a #![allow(deprecated)] at the top of the build file instead...

sielicki commented 3 years ago

IMO there are good reasons to actually go through with removing the try macro and to replace it with ? given here

restated:

? was stabilized in 1.13, November 2016, (timeline wise, this is not some bleeding edge feature.)
try is a reserved keyword in rust 2018 onward, so I think try! invocations will need to be written as r#try to be future proof.

seanmonstar commented 3 years ago

Yea, normally I'd say moving to ? makes sense. However, httparse for good or bad chose to be very strict with its MSRV, and so far hasn't had the need to change it. try becoming a keyword is only a problem if opting in to the 2018 edition, which httparse does not do (because of its old MSRV).

So in this case, the warning can be safely ignored.

sielicki commented 3 years ago

(Apologies for the noise if this doesn't work)

I force pushed a different patch that adds #[allow(deprecated)] and does raw identifier r#try. If calling try! via raw identifier doesn't work in Rust 2015, I'll abandon.

sielicki commented 3 years ago

Off-topic, but I'm somewhat annoyed that you have to put that #[allow(deprecated)] block above the whole function definition/signature, and you can't suppress the warning with more granularity, ie:

let major = { 
        #[allow(deprecated)]
        r#try!(num.parse::<u32>().map_err(|e| e.to_string()))
};

etc.

seanmonstar commented 3 years ago

Heh, yea, but then attributes on expressions wasn't stable back then (is it now finally?). But in this case, it's safe to just mark the whole file, since any other deprecation we couldn't do anything about either.

sielicki commented 3 years ago

Okay cool, it failed CI on 1.10.0 again. So as far as I can tell, there is no mechanism to call the try macro in both Rust 2015 and Rust 2018 in a way that avoids the possibility of conflicting with a future try keyword.

In that case, this entire PR just boils down to adding #[allow(deprecated)]. I'm going to abandon it. Sorry for the noise.