Skip to content

Conversation

pcwalton
Copy link
Contributor

Copy link
Member

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

Copy link
Contributor Author

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 StrBufs.) I tried several approaches to combat this and this was the one that was the cleanest.

@alexcrichton
Copy link
Member

I'm very much less than enthused with the amount of as_slice() action going on.

r=me modulo nits

@hatahet
Copy link
Contributor

hatahet commented May 20, 2014

I'm very much less than enthused with the amount of as_slice() action going on.

Out of curiosity, what would be a better alternative?

@alexcrichton
Copy link
Member

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.

@huonw
Copy link
Contributor

huonw commented May 20, 2014

Theoretically DST could allow implementing Deref for StrBuf to map to &str (with no other language changes), which would mean things like strbuf.trim_char() etc. work automatically, although s.push_str(strbuf.as_slice()) would still need an explicit borrow via s.push_str(&*strbuf).

@alexcrichton
Copy link
Member

Closing in favor of #14310

lnicola pushed a commit to lnicola/rust that referenced this pull request Mar 13, 2023
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.

4 participants