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

Can you make "somestr".trim_matches(char::is_ascii_punctuation) compile too ? it compiles with char::is_numeric #57307

Open
ghost opened this issue Jan 3, 2019 · 3 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@ghost
Copy link

ghost commented Jan 3, 2019

this works:

assert_eq!("123foo1bar123".trim_matches(char::is_numeric), "foo1bar"); //ok

but this doesn't:

assert_eq!(
        ".,\"foo1bar\".,';".trim_matches(char::is_ascii_punctuation),
        "foo1bar"
    ); //XXX fail
    // expected signature of `fn(char) -> _`
    // found signature of `for<'r> fn(&'r char) -> _`
    //   = note: required because of the requirements on the impl of `std::str::pattern::Pattern<'_>` for `for<'r> fn(&'r char) -> bool {std::char::methods::<impl char>::is_ascii_punctuation}`

sig for is_numeric is:

pub fn is_numeric(self) -> bool

And for is_ascii_punctuation is:

pub fn is_ascii_punctuation(&self) -> bool

So in order to make that work, I've had to do this:

pub trait Man {
    fn manual_is_ascii_punctuation(self) -> bool;
}

impl Man for char {
#[inline]
    fn manual_is_ascii_punctuation(self) -> bool {
        self.is_ascii() && (self as u8).is_ascii_punctuation()
    }
}
fn main() {
assert_eq!(
        ".,\"foo1bar\".,';".trim_matches(char::manual_is_ascii_punctuation),
        "foo1bar"
    ); //works because the func sig matches
}

Full code (playground):

#![allow(unused)]

pub trait Man {
    fn manual_is_ascii_punctuation(self) -> bool;
}

impl Man for char {
#[inline]
    fn manual_is_ascii_punctuation(self) -> bool {
        self.is_ascii() && (self as u8).is_ascii_punctuation()
    }
}

fn main() {
    assert_eq!("11foo1bar11".trim_matches('1'), "foo1bar");
    assert_eq!("123foo1bar123".trim_matches(char::is_numeric), "foo1bar"); //ok
    assert_eq!(
        ".,\"foo1bar\".,';".trim_matches(char::manual_is_ascii_punctuation),
        "foo1bar"
    ); //works because the func sig matches
    assert_eq!(
        ".,\"foo1bar\".,';".trim_matches(char::is_ascii_punctuation),
        "foo1bar"
    ); //XXX fail
    // expected signature of `fn(char) -> _`
    // found signature of `for<'r> fn(&'r char) -> _`
    //   = note: required because of the requirements on the impl of `std::str::pattern::Pattern<'_>` for `for<'r> fn(&'r char) -> bool {std::char::methods::<impl char>::is_ascii_punctuation}`

    assert_eq!("\"123foo1bar\"".trim_matches(|x| x == '"'), "123foo1bar"); //ok

    let x: &[_] = &['1', '2'];
    assert_eq!("12foo1bar12".trim_matches(x), "foo1bar");
}
Compiling playground v0.0.1 (/playground)
error[E0631]: type mismatch in function arguments
  --> src/main.rs:22:29
   |
22 |         ".,\"foo1bar\".,';".trim_matches(char::is_ascii_punctuation),
   |                             ^^^^^^^^^^^^
   |                             |
   |                             expected signature of `fn(char) -> _`
   |                             found signature of `for<'r> fn(&'r char) -> _`
   |
   = note: required because of the requirements on the impl of `std::str::pattern::Pattern<'_>` for `for<'r> fn(&'r char) -> bool {std::char::methods::<impl char>::is_ascii_punctuation}`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0631`.
error: Could not compile `playground`.

To learn more, run the command again with --verbose.
@varkor
Copy link
Member

varkor commented Jan 3, 2019

So in order to make that work, I've had to do this:

You can just use a closure instead:

assert_eq!(
        ".,\"foo1bar\".,';".trim_matches(|c| char::is_ascii_punctuation(&c)),
        "foo1bar"
    );

@ExpHP
Copy link
Contributor

ExpHP commented Jan 3, 2019

Ouch. Looks like this is an unfortunate holdover from when these methods were a part of std::ascii::AsciiExt.

Technically speaking, I believe the answer is yes; this is technically possible. However, it cannot be done by implementing Pattern for F: FnMut(&char) -> bool, because that would overlap with the impl for F: FnMut(char) -> bool. There would have to be two impls that differ in a very subtle way:

// This would work for any closure.
impl<'a, F> Pattern<'a> for F where F: FnMut(char) -> bool { ... }

// This would only work for path expressions to fn items
//    and type-annotated locals.
// (even annotated closures like `|c: &char| true` will select the `for F` impl
//  during type inference and cause type-checking to fail, as matching this
//  second impl would require a coercion)
impl<'a, F> Pattern<'a> for fn(&char) -> bool { ... }

This unfortunate characteristic of these methods was mentioned on the PR that moved them, but I can't find any existing discussion on the possibility of adding impl<'a> Pattern<'a> for fn(&char) -> bool.

@estebank estebank added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jan 3, 2019
@caemor
Copy link

caemor commented Dec 8, 2020

Could we maybe change the documenation whenever char::is_aynthing is used to instead use the variant with the closure instead that is working for every (I only tried 2) char::is_anything function?

Benefit would be less confusion when mixing/trying the different char functions.

Disadvantage would be slightly longer (and therefore possible a bit more confusing?) documentation code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants