Skip to content

std::str: Remove functions count_chars, count_bytes #8857

New issue

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

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

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from
Closed

std::str: Remove functions count_chars, count_bytes #8857

wants to merge 1 commit into from

Conversation

bluss
Copy link
Member

@bluss bluss commented Aug 29, 2013

These are very easy to replace with methods on string slices, basically
.char_len() and .len().

These are the replacement implementations I did to clean these
functions up, but seeing this I propose removal:

/// ...
pub fn count_chars(s: &str, begin: uint, end: uint) -> uint {
// .slice() checks the char boundaries
s.slice(begin, end).char_len()
}

/// Counts the number of bytes taken by the first n chars in s
/// starting from byte index begin.
///
/// Fails if there are less than n chars past begin
pub fn count_bytes<'b>(s: &'b str, begin: uint, n: uint) -> uint {
s.slice_from(begin).slice_chars(0, n).len()
}

These are very easy to replace with methods on string slices, basically
`.char_len()` and `.len()`.

These are the replacement implementations I did to clean these
functions up, but seeing this I propose removal:

/// ...
pub fn count_chars(s: &str, begin: uint, end: uint) -> uint {
    // .slice() checks the char boundaries
    s.slice(begin, end).char_len()
}

/// Counts the number of bytes taken by the first `n` chars in `s`
/// starting from byte index `begin`.
///
/// Fails if there are less than `n` chars past `begin`
pub fn count_bytes<'b>(s: &'b str, begin: uint, n: uint) -> uint {
    s.slice_from(begin).slice_chars(0, n).len()
}
bors added a commit that referenced this pull request Aug 30, 2013
These are very easy to replace with methods on string slices, basically
`.char_len()` and `.len()`.

These are the replacement implementations I did to clean these
functions up, but seeing this I propose removal:

/// ...
pub fn count_chars(s: &str, begin: uint, end: uint) -> uint {
    // .slice() checks the char boundaries
    s.slice(begin, end).char_len()
}

/// Counts the number of bytes taken by the first `n` chars in `s`
/// starting from byte index `begin`.
///
/// Fails if there are less than `n` chars past `begin`
pub fn count_bytes<'b>(s: &'b str, begin: uint, n: uint) -> uint {
    s.slice_from(begin).slice_chars(0, n).len()
}
@bors bors closed this Aug 30, 2013
Jarcho pushed a commit to Jarcho/rust that referenced this pull request Aug 29, 2022
Add test for rust-lang#8855

Fix rust-lang#8855

Here is what I think is going on.

First, the expression `format!("{:>6} {:>6}", a, b.to_string())` expands to:
```rust
{
    let res =
        ::alloc::fmt::format(::core::fmt::Arguments::new_v1_formatted(&["",
                            " "],
                &[::core::fmt::ArgumentV1::new_display(&a),
                            ::core::fmt::ArgumentV1::new_display(&b.to_string())],
                &[::core::fmt::rt::v1::Argument {
                                position: 0usize,
                                format: ::core::fmt::rt::v1::FormatSpec {
                                    fill: ' ',
                                    align: ::core::fmt::rt::v1::Alignment::Right,
                                    flags: 0u32,
                                    precision: ::core::fmt::rt::v1::Count::Implied,
                                    width: ::core::fmt::rt::v1::Count::Is(6usize),
                                },
                            },
                            ::core::fmt::rt::v1::Argument {
                                position: 1usize,
                                format: ::core::fmt::rt::v1::FormatSpec {
                                    fill: ' ',
                                    align: ::core::fmt::rt::v1::Alignment::Right,
                                    flags: 0u32,
                                    precision: ::core::fmt::rt::v1::Count::Implied,
                                    width: ::core::fmt::rt::v1::Count::Is(6usize),
                                },
                            }], unsafe { ::core::fmt::UnsafeArg::new() }));
    res
}
```
When I dump the expressions that get past the call to `has_string_formatting` [here](https://github.com/rust-lang/rust-clippy/blob/b312ad7d0cf0f30be2bd4658b71a3520a2e76709/clippy_lints/src/format_args.rs#L83), I see more than I would expect.

In particular, I see this subexpression of the above:
```
                &[::core::fmt::ArgumentV1::new_display(&a),
                            ::core::fmt::ArgumentV1::new_display(&b.to_string())],
```

This suggests to me that more expressions are getting past [this call](https://github.com/rust-lang/rust-clippy/blob/b312ad7d0cf0f30be2bd4658b71a3520a2e76709/clippy_lints/src/format_args.rs#L71) to `FormatArgsExpn::parse` than should.

Those expressions are then visited, but no `::core::fmt::rt::v1::Argument`s are found and pushed [here](https://github.com/rust-lang/rust-clippy/blob/b312ad7d0cf0f30be2bd4658b71a3520a2e76709/clippy_utils/src/macros.rs#L407).

As a result, the expressions appear unformatted, hence, the false positive.

My proposed fix is to restrict `FormatArgsExpn::parse` so that it only matches `Call` expressions.

cc: `@akanalytics`

changelog: none
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants