tidy: Fix paths to `coretests` and `alloctests`
Following `#135937` and `#136642`, tests for core and alloc are in coretests and alloctests. Fix tidy to lint for the new paths. Also, update comments referring to the old locations.
Some context for changes which don't match that pattern:
- `library/std/src/thread/local/dynamic_tests.rs` and `library/std/src/sync/mpsc/sync_tests.rs` were moved under `library/std/tests/` in 332fb7e6f1 (Move std::thread_local unit tests to integration tests, 2025-01-17) and b8ae372e48 (Move std::sync unit tests to integration tests, 2025-01-17), respectively, so are no longer special cases.
- There never was a `library/core/tests/fmt.rs` file. That comment previously referred to `src/test/ui/ifmt.rs`, which was folded into `library/alloc/tests/fmt.rs` in 949c96660c (move format! interface tests, 2020-09-08).
Now, the only matches for `(alloc|core)/tests` are in `compiler/rustc_codegen_{cranelift,gcc}/patches`. I don't know why CI hasn't broken because those patches can't apply. Or maybe they somehow still can apply?
r? `@bjorn3`
Following `#135937` and `#136642`, tests for core and alloc are in
coretests and alloctests. Fix tidy to lint for the new paths. Also,
update comments referring to the old locations.
Some context for changes which don't match that pattern:
* library/std/src/thread/local/dynamic_tests.rs and
library/std/src/sync/mpsc/sync_tests.rs were moved under
library/std/tests/ in 332fb7e6f1 (Move std::thread_local unit tests
to integration tests, 2025-01-17) and b8ae372e48 (Move std::sync unit
tests to integration tests, 2025-01-17), respectively, so are no
longer special cases.
* There never was a library/core/tests/fmt.rs file. That comment
previously referred to src/test/ui/ifmt.rs, which was folded into
library/alloc/tests/fmt.rs in 949c96660c (move format! interface
tests, 2020-09-08).
Instead of calling new(), we can just use a struct expression directly.
Before:
Placeholder::new(…, …, …, …)
After:
Placeholder {
position: …,
flags: …,
width: …,
precision: …,
}
Reduce FormattingOptions to 64 bits
This is part of https://github.com/rust-lang/rust/issues/99012
This reduces FormattingOptions from 6-7 machine words (384 bits on 64-bit platforms, 224 bits on 32-bit platforms) to just 64 bits (a single register on 64-bit platforms).
Before:
```rust
pub struct FormattingOptions {
flags: u32, // only 6 bits used
fill: char,
align: Option<Alignment>,
width: Option<usize>,
precision: Option<usize>,
}
```
After:
```rust
pub struct FormattingOptions {
/// Bits:
/// - 0-20: fill character (21 bits, a full `char`)
/// - 21: `+` flag
/// - 22: `-` flag
/// - 23: `#` flag
/// - 24: `0` flag
/// - 25: `x?` flag
/// - 26: `X?` flag
/// - 27: Width flag (if set, the width field below is used)
/// - 28: Precision flag (if set, the precision field below is used)
/// - 29-30: Alignment (0: Left, 1: Right, 2: Center, 3: Unknown)
/// - 31: Always set to 1
flags: u32,
/// Width if width flag above is set. Otherwise, always 0.
width: u16,
/// Precision if precision flag above is set. Otherwise, always 0.
precision: u16,
}
```
When the formatting args to `fmt::Write::write_fmt` are a statically
known string, it simplifies to only calling `write_str` without a
runtime branch. Do the same in `io::Write::write_fmt` with `write_all`.
Also, match the convention of `fmt::Write` for the name of `args`.
core: Make `Debug` impl of raw pointers print metadata if present
Make Rust pointers appear less magic by including metadata information in their `Debug` output.
This does not break Rust stability guarantees because `Debug` impl are explicitly exempted from stability:
https://doc.rust-lang.org/std/fmt/trait.Debug.html#stability
> ## Stability
>
> Derived `Debug` formats are not stable, and so may change with future Rust versions. Additionally, `Debug` implementations of types provided by the standard library (`std`, `core`, `alloc`, etc.) are not stable, and may also change with future Rust versions.
Note that a regression test is added as a separate commit to make it clear what impact the last commit has on the output.
Closes#128684 because the output of that code now becomes:
```
thread 'main' panicked at src/main.rs:5:5:
assertion `left == right` failed
left: Pointer { addr: 0x7ffd45c6fc6b, metadata: 5 }
right: Pointer { addr: 0x7ffd45c6fc6b, metadata: 3 }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
Remove `#[cfg(not(test))]` gates in `core`
These gates are unnecessary now that unit tests for `core` are in a separate package, `coretests`, instead of in the same files as the source code. They previously prevented the two `core` versions from conflicting with each other.
These gates are unnecessary now that unit tests for `core` are in a
separate package, `coretests`, instead of in the same files as the
source code. They previously prevented the two `core` versions from
conflicting with each other.
When both width and precision flags are specified, then the character
width is counted twice. Instead, record the character width when
truncating it to the precision, so it does not need to be recomputed.
Simplify control flow so the cases are more clear.
Add `MAX_LEN_UTF8` and `MAX_LEN_UTF16` Constants
This pull request adds the `MAX_LEN_UTF8` and `MAX_LEN_UTF16` constants as per #45795, gated behind the `char_max_len` feature.
The constants are currently applied in the `alloc`, `core` and `std` libraries.
Make Rust pointers less magic by including metadata information in their
`Debug` output.
This does not break Rust stability guarantees because `Debug` output is
explicitly exempted from stability:
https://doc.rust-lang.org/std/fmt/trait.Debug.html#stability
Co-authored-by: Lukas <26522220+lukas-code@users.noreply.github.com>
Co-authored-by: Josh Stone <cuviper@gmail.com>
Mark `std::fmt::from_fn` as `#[must_use]`
While working on #135494 I managed to shoot my own foot a few times by forgetting to actually use the result of `fmt::from_fn`, so I think a `#[must_use]` could be appropriate!
Didn't have a good message to put in the attr so left it blank, still unstable so we can come back to it I guess?
cc #117729 (and a huge +1 for getting it stabilized, it's very useful IMHO)
Formatter::with_options takes self as a mutable reference (`&'a mut
Formatter<'b>`). `'a` and `'b` need to be different lifetimes. Just taking `&'a
mut Formatter<'a>` and trusting in Rust being able to implicitely convert from
`&'a mut Formatter<'b>` if necessary (after all, `'a` must be smaller than `'b`
anyway) fails because `'b` is behind a *mutable* reference. For background on
on this behavior, see https://doc.rust-lang.org/nomicon/subtyping.html#variance.
The idea behind this is to make implementing `fmt::FormattingOptions` (as well
as any future changes to `std::Formatter`) easier.
In theory, this might have a negative performance impact because of the
additional function calls. However, I strongly believe that those will be
inlined anyway, thereby producing assembly code that has comparable performance.
Rollup of 7 pull requests
Successful merges:
- #131304 (float types: move copysign, abs, signum to libcore)
- #132907 (Change intrinsic declarations to new style)
- #132971 (Handle infer vars in anon consts on stable)
- #133003 (Make `CloneToUninit` dyn-compatible)
- #133004 (btree: simplify the backdoor between set and map)
- #133008 (update outdated comment about test-float-parse)
- #133012 (Add test cases for #125918)
r? `@ghost`
`@rustbot` modify labels: rollup
float types: move copysign, abs, signum to libcore
These operations are explicitly specified to act "bitwise", i.e. they just act on the sign bit and do not even quiet signaling NaNs. We also list them as ["non-arithmetic operations"](https://doc.rust-lang.org/nightly/std/primitive.f32.html#nan-bit-patterns), and all the other non-arithmetic operations are in libcore. There's no reason to expect them to require any sort of runtime support, and from [these experiments](https://github.com/rust-lang/rust/issues/50145#issuecomment-997301250) it seems like LLVM indeed compiles them in a way that does not require any sort of runtime support.
Nominating for `@rust-lang/libs-api` since this change takes immediate effect on stable.
Part of https://github.com/rust-lang/rust/issues/50145.
improve codegen of fmt_num to delete unreachable panic
it seems LLVM doesn't realize that `curr` is always decremented at least once in either loop formatting characters of the input string by their appropriate radix, and so the later `&buf[curr..]` generates a check for out-of-bounds access and panic. this is unreachable in reality as even for `x == T::zero()` we'll produce at least the character `Self::digit(T::zero())`, yielding at least one character output, and `curr` will always be at least one below `buf.len()`.
adjust `fmt_int` to make this fact more obvious to the compiler, which fortunately (or unfortunately) results in a measurable performance improvement for workloads heavy on formatting integers.
in the program i'd noticed this in, you can see the `cmp $0x80,%rdi; ja 7c` here, which branches to a slice index fail helper:
<img width="660" alt="before" src="https://github.com/rust-lang/rust/assets/4615790/ac482d54-21f8-494b-9c83-4beadc3ca0ef">
where after this change the function is broadly similar, but smaller, with one fewer registers updated in each pass through the loop in addition the never-taken `cmp/ja` being gone:
<img width="646" alt="after" src="https://github.com/rust-lang/rust/assets/4615790/1bee1d76-b674-43ec-9b21-4587364563aa">
this represents a ~2-3% difference in runtime in my [admittedly comically i32-formatting-bound](https://github.com/athre0z/disas-bench/blob/master/bench/yaxpeax/src/main.rs#L58-L67) use case (printing x86 instructions, including i32 displacements and immediates) as measured on a ryzen 9 3950x.
the impact on `<impl LowerHex for i8>::fmt` is both more dramatic and less impactful: it continues to have a loop that is evaluated at most twice, though the compiler doesn't know that to unroll it. the generated code there is identical to the impl for `i32`. there, the smaller loop body has less effect on runtime, and removing the never-taken slice bounds check is offset by whatever address recalculation is happening with the `lea/add/neg` at the end of the loop. it behaves about the same before and after.
---
i initially measured slightly better outcomes using `unreachable_unchecked()` here instead, but that was hacking on std and rebuilding with `-Z build-std` on an older rustc (nightly 5b377cece, 2023-06-30). it does not yield better outcomes now, so i see no reason to proceed with that approach at all.
<details>
<summary>initial notes about that, seemingly irrelevant on modern rustc</summary>
i went through a few tries at getting llvm to understand the bounds check isn't necessary, but i should mention the _best_ i'd seen here was actually from the existing `fmt_int` with a diff like
```diff
if x == zero {
// No more digits left to accumulate.
break;
};
}
}
+
+ if curr >= buf.len() {
+ unsafe { core::hint::unreachable_unchecked(); }
+ }
let buf = &buf[curr..];
```
posting a random PR to `rust-lang/rust` to do that without a really really compelling reason seemed a bit absurd, so i tried to work that into something that seems more palatable at a glance. but if you're interested, that certainly produced better (x86_64) code through LLVM. in that case with `buf.iter_mut().rev()` as the iterator, `<impl LowerHex for i8>::fmt` actually unrolls into something like
```
put_char(x & 0xf);
let mut len = 1;
if x > 0xf {
put_char((x >> 4) & 0xf);
len = 2;
}
pad_integral(buf[buf.len() - len..]);
```
it's pretty cool! `<impl LowerHex for i32>::fmt` also was slightly better. that all resulted in closer to an 6% difference in my use case.
</details>
---
i have not looked at formatters other than LowerHex/UpperHex with this change, though i'd be a bit shocked if any were _worse_.
(i have absolutely _no_ idea how you'd regression test this, but that might be just my not knowing what the right tool for that would be in rust-lang/rust. i'm of half a mind that this is small and fiddly enough to not be worth landing lest it quietly regress in the future anyway. but i didn't want to discard the idea without at least offering it upstream here)