-
Notifications
You must be signed in to change notification settings - Fork 306
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
Future of StatusCode::as_str #468
Comments
Thanks for taking the time to write up your thoughts. FWIW: I just want to (re-)state that the rationale and spirit of [#438] -> #443 (comment) was that the ship has sailed on the feature, and its costly for downstream to remove it, with little practical gain in removing it achieved here (since we improved it in #443, anyway). Now, that said, I totally get that the feature as it stands may have very limited value when you consider that you are just sidestepping it when in hyper, for example, you have a static constant for Your writeup also suggests that it was perhaps in some ways wise that I didn't just add the change for 'static to #443, in the hope of getting it merged with the larger changes! (I'm almost always surprised with what actually gets merged and what gets ignored for 2 years, here! And then there is release numbers!) |
I use If it was removed from |
If you really want a use std::io::Write;
use std::str;
use http::StatusCode;
let mut buf = [0u8; 3];
write!(&mut buf[..], "{:03}", StatusCode::OK.as_u16()).unwrap();
let s = str::from_utf8(&buf).unwrap();
assert_eq!(s, "200"); Edit: By the way, I've found that you can get a use http::StatusCode;
pub fn status_code_as_static_str(s: &StatusCode) -> Option<&'static str> {
match *s {
StatusCode::CONTINUE => Some(StatusCode::CONTINUE.as_str()),
// ...
_ => None,
}
} Is this an expected (or desirable) behavior? |
As I said, "doesn't require a formatter or allocating a String". |
FWIW, Actix Web doesn't use this method and opts for I think if someone really wants to avoid using a formatter then going with |
My use case has not changed since my first comment. let io_slices = [
std::io::IoSlice::new(b"HTTP/1.1 "),
std::io::IoSlice::new(status.as_str().as_bytes()),
std::io::IoSlice::new(b" \r\n"),
/* headers */,
std::io::IoSlice::new(b"\r\n"),
/* body */,
];
stream.write_all_vectored(io_slices);
Does the overall efficiency matter significantly? Not really; CPUs are very fast, and serializing the first line of an HTTP response is a tiny share of work that a web server does for generating that response. But |
4th PR to change return type to |
Do we actually know the cost of this? |
I wanted to write-up some thoughts on the future direction for the
StatusCode::as_str
method. Theas_str
method returns the numeric part of the status code as a string slice. So,StatusCode::OK.as_str()
returns"200"
, andStatusCode::NOT_FOUND.as_str()
returns"404"
, and so forth. This works for allStatusCode
s, from 100-999. Since theStatusCode
only stores an integer, there's no field to actually borrow the string from, so it is in reality returning a&'static str
, even though the method signature doesn't expose that.There have been several PRs filed to make that part of the signature (that is, change from
fn as_str(&self) -> &str
tofn as_str(&self) -> &'static str
) (#380 #436 #466).I've personally questioned the value of the method in it's entirety:
That comment was prompted as I wondered about the cost of including all the static strings in the binary, when certainly most applications will never need them. Because I don't see value in the method, I've held off from changing it return a
&'static str
. Instead, I've thought we may just want to deprecate it and chop it for 1.0.However, I realize I could be wrong, and not considering a large use case for this method. If so, we can just merge one of the PRs and carry on. If I have overlooked something, hopefully we can collect the reasons why to keep the method, with some details about the value it provides versus the cost of having a bunch of extra strings in the binary for everyone.
The text was updated successfully, but these errors were encountered: