Needs rebase.

Also why did you convert this function to a macro?

Also, do these new methods need to be pub?

Also why did you convert this function to a macro?

Also, do these new methods need to be pub?

Answer in 2 parts:

• I made multiple methods to remove the Into<InputString> trait bound
• I made a private macro to be called by the new public methods because I couldn't think how to handle the T: Integer type bound without too much trickery
• I made the methods public because they are called from other crates. I think these should however go into the new private module (Introduce private modules and infrastructure #3844) if it has legs.

Woops that's 3 parts.

Needs another rebase, sorry.

Re "without too much trickery", I'll give it a shot when I do a full review.

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Check semver breaks / PR Semver - stable toolchain" job then expand "Run semver checker script" and scroll to the end of the section.

Re the macro, it seems it can be replaced with the function

fn to_int<T: FromStr<Err = core::num::ParseIntError> + TryFrom<i8>>(s: &str) -> Result<T, ParseIntError> {
        s.parse().map_err(|error| {
            ParseIntError {
                input: s.into(),
                bits: u8::try_from(core::mem::size_of::<T>() * 8).expect("max is 128 bits for u128"),
                // We detect if the type is signed by checking if -1 can be represented by it
                // this way we don't have to implement special traits and optimizer will get rid of the
                // computation.
                is_signed: T::try_from(-1i8).is_ok(),
                source: error,
            }
        })
}

(though you should fix the formatting). This is just a copy/paste of your macro but with $s replaced with s.

Mad, thanks. Will re-spin.

Ah woops, my bad. The proper answer to your original question of why did I use a macro is that using a function (like you posted) introduces an allocation in the error path but the macro moves the input string if its a String or Box<str>.

Then we can make s also a generic Into<String> which will then do the right thing.

We can't do that because we don't have String, that is why we are using InputString, right. But we can't use Into<InputString> either hence the macro. Its private to the file so its not too much of a bother, right?

You can do it by taking S: Into<InputString> + Borrow<str>. Then $s.parse() becomes s.borrow().parse().

Having said that, ok, I'm okay with the macro since the signature of the function winds up being a real mouthful. But if we stick with it, I'd like a more explicit comment on it explaining why we're using the macro rather than a generic function. The complicated function signature at least has the benefit of demonstrating exactly what we're trying to do.

Oooo I missed that its private so fine to use Into<InputString>. I'll have another play.

No more macro. I totally overcomplicated this PR by failing to realize there was nothing wrong with the current code except that it was public.

I did not close, bad GitHub - no biscuit.

I clicked re-open and its not open, what is going on here?

open sesame

Will try again later. Please click re-open @apoelstra if you read this.

You force-pushed while it was closed, and this perma-closes PRs because Github is idiotic.

O, TIL. Cheers.