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

Cleanup strs #7996

Closed
wants to merge 17 commits into from
Closed

Cleanup strs #7996

wants to merge 17 commits into from

Conversation

erickt
Copy link
Contributor

@erickt erickt commented Jul 23, 2013

This is a cleanup pull request that does:

  • removes os::as_c_charp
  • moves str::as_buf and str::as_c_str into StrSlice
  • converts some functions from StrSlice::as_buf to StrSlice::as_c_str
  • renames StrSlice::as_buf to StrSlice::as_imm_buf (and adds StrSlice::as_mut_buf to match vec.rs.
  • renames UniqueStr::as_bytes_with_null_consume to UniqueStr::to_bytes
  • and other misc cleanups and minor optimizations

* to full strings, or suffixes of them.
*/
#[inline]
fn as_mut_buf<T>(&self, f: &fn(*mut u8, uint) -> T) -> T {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably have a warning about maintaining the invariant that the string is valid UTF8, and should definitely take &mut self rather than &self.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that. I'll add the &mut self and a warning.

@erickt
Copy link
Contributor Author

erickt commented Jul 23, 2013

@huonw: Updated the pull request address your suggestion, and also added a couple more cleanup functions.

@graydon
Copy link
Contributor

graydon commented Jul 23, 2013

I would think to_bytes should remain to_bytes_with_null or such, no? We named those APIs so they're clear about null termination.

(The whole matter of null termination remains, to this day, contentious. As you know, in #7235)

@erickt
Copy link
Contributor Author

erickt commented Jul 23, 2013

@graydon: Most of this pull request was extracted from the non-controversial parts of #7172, so my original intention was that there would no longer be strings with nulls in them :) I can rename to_bytes to to_bytes_with_null for now, and continue the debate for my next round of #7172.

@erickt
Copy link
Contributor Author

erickt commented Jul 24, 2013

@graydon: Updated the pull request to add str.to_bytes_with_null().

bors added a commit that referenced this pull request Jul 24, 2013
This is a cleanup pull request that does:

* removes `os::as_c_charp`
* moves `str::as_buf` and `str::as_c_str` into `StrSlice`
* converts some functions from `StrSlice::as_buf` to `StrSlice::as_c_str`
* renames `StrSlice::as_buf` to `StrSlice::as_imm_buf` (and adds `StrSlice::as_mut_buf` to match `vec.rs`.
* renames `UniqueStr::as_bytes_with_null_consume` to `UniqueStr::to_bytes`
* and other misc cleanups and minor optimizations
@bors bors closed this Jul 24, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 23, 2021
…arth

Add test case for RESULT_MAP_OR_INTO_OPTION

just added test case for RESULT_MAP_OR_INTO_OPTION.
changelog: none
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.

None yet

4 participants