Make int_format_into API more flexible
#143636
Conversation
rustbot
added
S-waiting-on-review
|
Maybe add some tests showing the error when the size was chosen wrong |
|
Oh right. Adding |
This comment was marked as outdated.
This comment was marked as outdated.
|
So sadly, I couldn't make the |
This comment has been minimized.
This comment has been minimized.
GuillaumeGomez
force-pushed
the
int_format_into-improvement
branch
from
5a05f0f to
25f59e2
Compare
GuillaumeGomez
force-pushed
the
int_format_into-improvement
branch
from
25f59e2 to
82d9d62
Compare
| /// | ||
| #[doc = concat!("let n = 32", stringify!($signed), ";")] | ||
| /// // We use a `NumBuffer` used to store a bigger integer. | ||
| /// let mut buf = NumBuffer::<u128>::new(); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
i128 and u128 are not covered by this macro, they are implemented below. So this example works all the time. :)
Also, if it didn't, the doctest would fail because compilation would fail.
| @@ -338,7 +338,7 @@ macro_rules! impl_Display { | |||
| /// use core::fmt::NumBuffer; | |||
| /// | |||
| #[doc = concat!("let n = 0", stringify!($signed), ";")] | |||
| /// let mut buf = NumBuffer::new(); | |||
| #[doc = concat!("let mut buf = NumBuffer::<", stringify!($signed), ">::new();")] | |||
There was a problem hiding this comment.
Needing to specify/repeat the type when creating the buffer seems like a non-trivial downside. I'm not sure if the added flexibility is worth it. Can you give a semi-realistic example of where you'd want the flexibility of formatting one of several integer types into the same buffer?
There was a problem hiding this comment.
If you want to have only one buffer across your program life to fit all your integers that have different sizes, it's quite convenient.
There was a problem hiding this comment.
When would I want such a long-lived buffer? Every usage of itoa I've seen while researching for the ACP just creates a new temporary buffer on the spot whenever it needs to format an integer. I don't think I've even seen a single buffer used twice for the same integer type. Setting up a buffer doesn't have any cost that needs to be amortized, allocating some extra uninitialized space in your stack frame is as close to free as it gets. If anything, putting the buffer somewhere else has a higher risk that it's moved around, which generally implies copying the whole buffer!
| | | ||
| = note: this note originates in the macro `impl_Display` (in Nightly builds, run with -Z macro-backtrace for more info) | ||
|
|
||
| note: the above error was encountered while instantiating `fn core::fmt::num::imp::<impl u32>::format_into::<u8>` |
There was a problem hiding this comment.
That this is a post-mono error is another small downside of this PR. There's a policy that cargo check isn't required to catch all errors that cargo build catches, but it's still preferable to have it catch as many errors as reasonably possible.
Amanieu
added
the
I-libs-api-nominated
|
We discussed this in the @rust-lang/libs-api meeting. Rather than making Additionally we would prefer to avoid post-monomorphization errors here and instead use trait bounds to limit the allowed conversions. |
Amanieu
removed
the
I-libs-api-nominated
|
@Amanieu Does it allow to have one |
Follow-up of #142098.
Part of #138215.
This change allows to pass
NumBuffer<u128>for any smaller integers, making the API much more flexible overall.r? @Amanieu