join: "support" field numbers larger than usize::MAX #2882
Conversation
| @@ -775,8 +775,11 @@ fn get_field_number(keys: Option<usize>, key: Option<usize>) -> UResult<usize> { | |||
| /// Parse the specified field string as a natural number and return | |||
| /// the zero-based field number. | |||
| fn parse_field_number(value: &str) -> UResult<usize> { | |||
| // TODO: use ParseIntError.kind() once MSRV >= 1.55 | |||
| let overflow = "184467440737095516150".parse::<usize>().err().unwrap(); | |||
There was a problem hiding this comment.
This hack could potentially add some overhead, since we're probably doing this every time a field number gets parsed (I don't know how expensive these operations are, and I'm not sure how aggressive the compiler's optimizations are). In the typical case, this should be negligible, since this only happens a linear number of times in the size of the arguments, but someone could possibly be relying on some crazy xargs that causes it to show up. Possible workarounds would be passing overflow as an arg, storing it in global scope, or maybe a const fn (I'd have to check when things were added vs. MSRV), but my inclination is to keep the hack scoped to these lines, and just accept whatever overhead until either someone complains, or MSRV gets bumped enough we can use the proper solution.
There was a problem hiding this comment.
That's a clever trick, hahaha. I can't believe we cannot properly introspect the kind until Rust 1.55. Even though it's not a const fn, it might get optimized out anyway. I think we can accept this if you add a bit of documentation on how this workaround works.
There was a problem hiding this comment.
I've added a bit of explanation, and a link to where I found the trick. But yeah, I suspect the actual runtime cost is either tiny or non-existent, I just haven't done any real investigation or benchmarking to confirm that.
jtracey
force-pushed
the
join-bigfields-compat
branch
from
9d111ec to
4d35f2e
Compare
They silently get folded to usize::MAX, which is the official GNU behavior.
jtracey
force-pushed
the
join-bigfields-compat
branch
from
4d35f2e to
ce3df12
Compare
Summary: Here we improve the following: * The test case for long integers (big integers) * We have to use an unusual workaround to detect integer overflow of the `parse::<isize>` function as Meta is on a version of rust < 2022. * The workaround is documented and there is a todo as follows: ``` // TODO: use ParseIntError.kind() to detect integer overflow of // parse of const_value when Meta is on rust 2022. // In rust 2021 ParseIntError.kind is private // For now, store an overflow Err from parsing a large integer // Adapted from rust-lang/rust#22639 // and uutils/coreutils#2882 ``` This would seem to be the recommended workaround for detecting integer overflow. Reviewed By: stroxler Differential Revision: D42407915 Privacy Context Container: L1152058 fbshipit-source-id: 049dd6c74305fe193131fd1ea2ad72a0965d253a
They silently get folded to usize::MAX, which is the official GNU behavior.