-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
libstd: Remove ~str
from all libstd
modules except fmt
and str
.
#14255
Conversation
#[allow(missing_doc)] | ||
pub fn args() -> ::realstd::vec::Vec<::realstd::strbuf::StrBuf> { | ||
::realstd::os::args() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is somewhat alarming, why was the split necessary? This split of test/not test should be isolated to std::rt::args
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the code injected by the test harness makes a reference to std::strbuf::StrBuf
, which will be the wrong StrBuf
when testing libstd
. (Remember, when testing libstd
there are two StrBuf
s.) I tried several approaches to combat this and this was the one that was the cleanest.
I'm very much less than enthused with the amount of r=me modulo nits |
Out of curiosity, what would be a better alternative? |
There could perhaps be some auto{ref,deref} action going on, but there's been a lot of pushback from that recently. I hear that there are some musings of more principled alternatives, but they make some time to surface. I think we should get some usage with the new system before we rush to change it. |
Theoretically DST could allow implementing |
Closing in favor of #14310 |
Bring back the hex in const hover
r? @alexcrichton