Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

sielicki
Copy link

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

@sielicki
Copy link
Author

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
Copy link
Owner

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

@sielicki
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

(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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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.

@sielicki sielicki closed this Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants