This stabilizes the `Path::try_exists()` method which returns
`Result<bool, io::Error>` instead of `bool` allowing handling of errors
unrelated to the file not existing. (e.g permission errors)
Along with the stabilization it also:
* Warns that the `exists()` method is error-prone and suggests to use
the newly stabilized one.
* Suggests it instead of `metadata()` to handle errors.
* Mentions TOCTOU bugs to avoid false assumption that `try_exists()` is
completely safe fixed version of `exists()`.
* Renames the feature of still-unstable `std::fs::try_exists()` to
`fs_try_exists` to avoid name conflict.
The tracking issue #83186 remains open to track `fs_try_exists`.
Integrate measureme's hardware performance counter support.
*Note: this is a companion to https://github.com/rust-lang/measureme/pull/143, and duplicates some information with it for convenience*
**(much later) EDIT**: take any numbers with a grain of salt, they may have changed since initial PR open.
## Credits
I'd like to start by thanking `@alyssais,` `@cuviper,` `@edef1c,` `@glandium,` `@jix,` `@Mark-Simulacrum,` `@m-ou-se,` `@mystor,` `@nagisa,` `@puckipedia,` and `@yorickvP,` for all of their help with testing, and valuable insight and suggestions.
Getting here wouldn't have been possible without you!
(If I've forgotten anyone please let me know, I'm going off memory here, plus some discussion logs)
## Summary
This PR adds support to `-Z self-profile` for counting hardware events such as "instructions retired" (as opposed to being limited to time measurements), using the `rdpmc` instruction on `x86_64` Linux.
While other OSes may eventually be supported, preliminary research suggests some kind of kernel extension/driver is required to enable this, whereas on Linux any user can profile (at least) their own threads.
Supporting Linux on architectures other than x86_64 should be much easier (provided the hardware supports such performance counters), and was mostly not done due to a lack of readily available test hardware.
That said, 32-bit `x86` (aka `i686`) would be almost trivial to add and test once we land the initial `x86_64` version (as all the CPU detection code can be reused).
A new flag `-Z self-profile-counter` was added, to control which of the named `measureme` counters is used, and which defaults to `wall-time`, in order to keep `-Z self-profile`'s current functionality unchanged (at least for now).
The named counters so far are:
* `wall-time`: the existing time measurement
* name chosen for consistency with `perf.rust-lang.org`
* continues to use `std::time::Instant` for a nanosecond-precision "monotonic clock"
* `instructions:u`: the hardware performance counter usually referred to as "Instructions retired"
* here "retired" (roughly) means "fully executed"
* the `:u` suffix is from the Linux `perf` tool and indicates the counter only runs while userspace code is executing, and therefore counts no kernel instructions
* *see [Caveats/Subtracting IRQs](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Subtracting-IRQs) for why this isn't entirely true and why `instructions-minus-irqs:u` should be preferred instead*
* `instructions-minus-irqs:u`: same as `instructions:u`, except the count of hardware interrupts ("IRQs" here for brevity) is subtracted
* *see [Caveats/Subtracting IRQs](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Subtracting-IRQs) for why this should be preferred over `instructions:u`*
* `instructions-minus-r0420:u`: experimental counter, same as `instructions-minus-irqs:u` but subtracting an undocumented counter (`r0420:u`) instead of IRQs
* the `rXXXX` notation is again from Linux `perf`, and indicates a "raw" counter, with a hex representation of the low-level counter configuration - this was picked because we still don't *really* know what it is
* this only exists for (future) testing and isn't included/used in any comparisons/data we've put together so far
* *see [Challenges/Zen's undocumented 420 counter](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Epilogue-Zen’s-undocumented-420-counter) for details on how this counter was found and what it does*
---
There are also some additional commits:
* ~~see [Challenges/Rebasing *shouldn't* affect the results, right?](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Rebasing-*shouldn’t*-affect-the-results,-right) for details on the changes to `rustc_parse` and `rustc_trait_section` (the latter far more dubious, and probably shouldn't be merged, or not as-is)~~
* **EDIT**: the effects of these are no long quantifiable, the PR includes reverts for them
* ~~see [Challenges/`jemalloc`: purging will commence in ten seconds](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#jemalloc-purging-will-commence-in-ten-seconds) for details on the `jemalloc` change~~
* this is also separately found in #77162, and we probably want to avoid doing it by default, ideally we'd use the runtime control API `jemalloc` offers (assuming that can stop the timer that's already running, which I'm not sure about)
* **EDIT**: until we can do this based on `-Z` flags, this commit has also been reverted
* the `proc_macro` change was to avoid randomized hashing and therefore ASLR-like effects
---
**(much later) EDIT**: take any numbers with a grain of salt, they may have changed since initial PR open.
#### Write-up / report
Because of how extensive the full report ended up being, I've kept most of it [on `hackmd.io`](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view), but for convenient access, here are all the sections (with individual links):
<sup>(someone suggested I'd make a backup, so [here it is on the wayback machine](http://web.archive.org/web/20201127164748/https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view) - I'll need to remember to update that if I have to edit the write-up)</sup>
* [**Motivation**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Motivation)
* [**Results**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Results)
* [**Overhead**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Overhead)
*Preview (see the report itself for more details):*
|Counter|Total<br>`instructions-minus-irqs:u`|Overhead from "Baseline"<br>(for all 1903881<br>counter reads)|Overhead from "Baseline"<br>(per each counter read)|
|-|-|-|-|
|Baseline|63637621286 ±6||
|`instructions:u`|63658815885 ±2| +21194599 ±8| +11|
|`instructions-minus-irqs:u`|63680307361 ±13| +42686075 ±19| +22|
|`wall-time`|63951958376 ±10275|+314337090 ±10281|+165|
* [**"Macro" noise (self time)**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#“Macro”-noise-(self-time))
*Preview (see the report itself for more details):*
|| `wall-time` (ns) | `instructions:u` | `instructions-minus-irqs:u`
-: | -: | -: | -:
`typeck` | 5478261360 ±283933373 (±~5.2%) | 17350144522 ±6392 (±~0.00004%) | 17351035832.5 ±4.5 (±~0.00000003%)
`expand_crate` | 2342096719 ±110465856 (±~4.7%) | 8263777916 ±2937 (±~0.00004%) | 8263708389 ±0 (±~0%)
`mir_borrowck` | 2216149671 ±119458444 (±~5.4%) | 8340920100 ±2794 (±~0.00003%) | 8341613983.5 ±2.5 (±~0.00000003%)
`mir_built` | 1269059734 ±91514604 (±~7.2%) | 4454959122 ±1618 (±~0.00004%) | 4455303811 ±1 (±~0.00000002%)
`resolve_crate` | 942154987.5 ±53068423.5 (±~5.6%) | 3951197709 ±39 (±~0.000001%) | 3951196865 ±0 (±~0%)
* [**"Micro" noise (individual sampling intervals)**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#“Micro”-noise-(individual-sampling-intervals))
* [**Caveats**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Caveats)
* [**Disabling ASLR**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Disabling-ASLR)
* [**Non-deterministic proc macros**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Non-deterministic-proc-macros)
* [**Subtracting IRQs**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Subtracting-IRQs)
* [**Lack of support for multiple threads**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Lack-of-support-for-multiple-threads)
* [**Challenges**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Challenges)
* [**How do we even read hardware performance counters?**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#How-do-we-even-read-hardware-performance-counters)
* [**ASLR: it's free entropy**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#ASLR-it’s-free-entropy)
* [**The serializing instruction**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#The-serializing-instruction)
* [**Getting constantly interrupted**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Getting-constantly-interrupted)
* [**AMD patented time-travel and dubbed it `SpecLockMap`<br><sup> or: "how we accidentally unlocked `rr` on AMD Zen"</sup>**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#AMD-patented-time-travel-and-dubbed-it-SpecLockMapnbspnbspnbspnbspnbspnbspnbspnbspor-“how-we-accidentally-unlocked-rr-on-AMD-Zen”)
* [**`jemalloc`: purging will commence in ten seconds**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#jemalloc-purging-will-commence-in-ten-seconds)
* [**Rebasing *shouldn't* affect the results, right?**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Rebasing-*shouldn’t*-affect-the-results,-right)
* [**Epilogue: Zen's undocumented 420 counter**](https://hackmd.io/sH315lO2RuicY-SEt7ynGA?view#Epilogue-Zen’s-undocumented-420-counter)
Our condvar doesn't support setting attributes, like
pthread_condattr_setclock, which the current wait_timeout expects to
have configured.
Switch to a different implementation, following espidf.
line 1352, change `self` to `*self`, other to `*other`
The current code will not results bug, but it difficult to understand. These code result to call &f32::partial_cmp(), and the performance will be lower than the changed code. I'm not sure why the current code don't use (*self) (*other), if you have some idea, please let me know.
Use `fcntl(fd, F_GETFD)` to detect if standard streams are open
In the previous implementation, if the standard streams were open,
but the RLIMIT_NOFILE value was below three, the poll would fail
with EINVAL:
> ERRORS: EINVAL The nfds value exceeds the RLIMIT_NOFILE value.
Switch to the existing fcntl based implementation to avoid the issue.
Fixes#96621.
The current code will not results bug, but it difficult to understand. These code result to call &f32::partial_cmp(), and the performance will be lower than the changed code. I'm not sure why the current code don't use (*self) (*other), if you have some idea, please let me know.
Rollup of 6 pull requests
Successful merges:
- #97718 (Fix `delayed_good_path_bug` ice for expected diagnostics (RFC 2383))
- #97876 (update docs for `std::future::IntoFuture`)
- #97888 (Don't use __gxx_personality_v0 in panic_unwind on emscripten target)
- #97922 (Remove redundant calls to reserve in impl Write for VecDeque)
- #97927 (Do not introduce bindings for types and consts in HRTB.)
- #97937 (Fix a typo in `test/ui/hrtb/hrtb-just-for-static.rs`)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Don't use __gxx_personality_v0 in panic_unwind on emscripten target
This resolves#85821. See also the discussion here:
https://github.com/emscripten-core/emscripten/issues/17128
The consensus seems to be that rust_eh_personality is never invoked.
I patched __gxx_personality_v0 to log invocations and then ran
various panic tests and it was never called, so this analysis matches
what seems to happen in practice. This replaces the definition with
an abort, modeled on the structured exception handling implementation.
update docs for `std::future::IntoFuture`
Ref https://github.com/rust-lang/rust/issues/67644.
This updates the docs for `IntoFuture` providing a bit more guidance on how to use it. Thanks!
std::io: Modify some ReadBuf method signatures to return `&mut Self`
This allows using `ReadBuf` in a builder-like style and to setup a `ReadBuf` and
pass it to `read_buf` in a single expression, e.g.,
```
// With this PR:
reader.read_buf(ReadBuf::uninit(buf).assume_init(init_len))?;
// Previously:
let mut buf = ReadBuf::uninit(buf);
buf.assume_init(init_len);
reader.read_buf(&mut buf)?;
```
r? `@sfackler`
cc https://github.com/rust-lang/rust/issues/78485, https://github.com/rust-lang/rust/issues/94741
Add the Provider api to core::any
This is an implementation of [RFC 3192](https://github.com/rust-lang/rfcs/pull/3192) ~~(which is yet to be merged, thus why this is a draft PR)~~. It adds an API for type-driven requests and provision of data from trait objects. A primary use case is for the `Error` trait, though that is not implemented in this PR. The only major difference to the RFC is that the functionality is added to the `any` module, rather than being in a sibling `provide_any` module (as discussed in the RFC thread).
~~Still todo: improve documentation on items, including adding examples.~~
cc `@yaahc`
`Mutex::lock()` and `RwLock::write()` are poison-guarded against panics,
in that they set the poison flag if a panic occurs while they're locked.
But if we're already in a panic (`thread::panicking()`), they leave the
poison flag alone.
That check is a bit of a waste for methods that never set the poison
flag though, namely `get_mut()`, `into_inner()`, and `RwLock::read()`.
These use-cases are now split to avoid that unnecessary call.
impl Read and Write for VecDeque<u8>
Implementing `Read` and `Write` for `VecDeque<u8>` fills in the VecDeque api surface where `Vec<u8>` and `Cursor<Vec<u8>>` already impl Read and Write. Not only for completeness, but VecDeque in particular is a very handy mock interface for a TCP echo service, if only it supported Read/Write.
Since this PR is just an impl trait, I don't think there is a way to limit it behind a feature flag, so it's "insta-stable". Please correct me if I'm wrong here, not trying to rush stability.
BTreeSet: avoid intermediate sorting when collecting sorted iterators
As [pointed out by droundy](https://users.rust-lang.org/t/question-about-btreeset-implementation/76427), an obvious optimization is to skip the first step introduced by #88448 (creation of a vector and sorting) and it's easy to do so for btree's own iterators. Also, exploit `from` in the examples.
This commit adds a new unstable attribute, `#[doc(tuple_varadic)]`, that
shows a 1-tuple as `(T, ...)` instead of just `(T,)`, and links to a section
in the tuple primitive docs that talks about these.
Suggest using `iter()` or `into_iter()` for `Vec`
We cannot do that for `&Vec` because `#[rustc_on_unimplemented]` is limited (it does not clean generic instantiation for references, only for ADTs).
`@rustbot` label +A-diagnostics
This resolves#85821. See also the discussion here:
https://github.com/emscripten-core/emscripten/issues/17128
The consensus seems to be that rust_eh_personality is never invoked.
I patched __gxx_personality_v0 to log invocations and then ran
various panic tests and it was never called, so this analysis matches
what seems to happen in practice. This replaces the definition with
an abort, modeled on the structured exception handling implementation.
Use repr(C) when depending on struct layout in ptr tests
The test depends on the layout of this struct `Pair`, so it should use `repr(C)` instead of the default `repr(Rust)`.
In some situations it is possible for required functions to be called before they've had a chance to be loaded. Therefore, we make it possible to recover from this situation simply by looking at error codes.
Add documentation to `std::os::unix::io` describing Rust's stance on
`/proc/self/mem`, treating it as an external entity which is outside
the scope of Rust's safety guarantees.
Remove confusing sentence from `Mutex` docs
The docs were saying something about "statically initializing" the
mutex, and it's not clear what this means. Remove that part to avoid
confusion.
Improve the safety docs for `CStr`
Namely, the two functions `from_ptr` and `from_bytes_with_nul_unchecked`.
Before, these functions didn't state the requirements clearly enough,
and I was not immediately able to find them like for other functions.
This doesn't change the content of the docs, but simply rewords them for
clarity.
note: I'm not entirely sure about the '`ptr` must be valid for reads of `u8`.', there might be room for improvement for this (and maybe for the other docs as well 😄)
Namely, the two functions `from_ptr` and `from_bytes_with_nul_unchecked`.
Before, this functions didn't state the requirements clearly enough,
and I was not immediately able to find them like for other functions.
This doesn't change the content of the docs, but simply rewords them for
clarity.
Remove migrate borrowck mode
Closes#58781Closes#43234
# Stabilization proposal
This PR proposes the stabilization of `#![feature(nll)]` and the removal of `-Z borrowck`. Current borrow checking behavior of item bodies is currently done by first infering regions *lexically* and reporting any errors during HIR type checking. If there *are* any errors, then MIR borrowck (NLL) never occurs. If there *aren't* any errors, then MIR borrowck happens and any errors there would be reported. This PR removes the lexical region check of item bodies entirely and only uses MIR borrowck. Because MIR borrowck could never *not* be run for a compiled program, this should not break any programs. It does, however, change diagnostics significantly and allows a slightly larger set of programs to compile.
Tracking issue: #43234
RFC: https://github.com/rust-lang/rfcs/blob/master/text/2094-nll.md
Version: 1.63 (2022-06-30 => beta, 2022-08-11 => stable).
## Motivation
Over time, the Rust borrow checker has become "smarter" and thus allowed more programs to compile. There have been three different implementations: AST borrowck, MIR borrowck, and polonius (well, in progress). Additionally, there is the "lexical region resolver", which (roughly) solves the constraints generated through HIR typeck. It is not a full borrow checker, but does emit some errors.
The AST borrowck was the original implementation of the borrow checker and was part of the initially stabilized Rust 1.0. In mid 2017, work began to implement the current MIR borrow checker and that effort ompleted by the end of 2017, for the most part. During 2018, efforts were made to migrate away from the AST borrow checker to the MIR borrow checker - eventually culminating into "migrate" mode - where HIR typeck with lexical region resolving following by MIR borrow checking - being active by default in the 2018 edition.
In early 2019, migrate mode was turned on by default in the 2015 edition as well, but with MIR borrowck errors emitted as warnings. By late 2019, these warnings were upgraded to full errors. This was followed by the complete removal of the AST borrow checker.
In the period since, various errors emitted by the MIR borrow checker have been improved to the point that they are mostly the same or better than those emitted by the lexical region resolver.
While there do remain some degradations in errors (tracked under the [NLL-diagnostics tag](https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3ANLL-diagnostics), those are sufficiently small and rare enough that increased flexibility of MIR borrow check-only is now a worthwhile tradeoff.
## What is stabilized
As said previously, this does not fundamentally change the landscape of accepted programs. However, there are a [few](https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3ANLL-fixed-by-NLL) cases where programs can compile under `feature(nll)`, but not otherwise.
There are two notable patterns that are "fixed" by this stabilization. First, the `scoped_threads` feature, which is a continutation of a pre-1.0 API, can sometimes emit a [weird lifetime error](https://github.com/rust-lang/rust/issues/95527) without NLL. Second, actually seen in the standard library. In the `Extend` impl for `HashMap`, there is an implied bound of `K: 'a` that is available with NLL on but not without - this is utilized in the impl.
As mentioned before, there are a large number of diagnostic differences. Most of them are better, but some are worse. None are serious or happen often enough to need to block this PR. The biggest change is the loss of error code for a number of lifetime errors in favor of more general "lifetime may not live long enough" error. While this may *seem* bad, the former error codes were just attempts to somewhat-arbitrarily bin together lifetime errors of the same type; however, on paper, they end up being roughly the same with roughly the same kinds of solutions.
## What isn't stabilized
This PR does not completely remove the lexical region resolver. In the future, it may be possible to remove that (while still keeping HIR typeck) or to remove it together with HIR typeck.
## Tests
Many test outputs get updated by this PR. However, there are number of tests specifically geared towards NLL under `src/test/ui/nll`
## History
* On 2017-07-14, [tracking issue opened](https://github.com/rust-lang/rust/issues/43234)
* On 2017-07-20, [initial empty MIR pass added](https://github.com/rust-lang/rust/pull/43271)
* On 2017-08-29, [RFC opened](https://github.com/rust-lang/rfcs/pull/2094)
* On 2017-11-16, [Integrate MIR type-checker with NLL](https://github.com/rust-lang/rust/pull/45825)
* On 2017-12-20, [NLL feature complete](https://github.com/rust-lang/rust/pull/46862)
* On 2018-07-07, [Don't run AST borrowck on mir mode](https://github.com/rust-lang/rust/pull/52083)
* On 2018-07-27, [Add migrate mode](https://github.com/rust-lang/rust/pull/52681)
* On 2019-04-22, [Enable migrate mode on 2015 edition](https://github.com/rust-lang/rust/pull/59114)
* On 2019-08-26, [Don't downgrade errors on 2015 edition](https://github.com/rust-lang/rust/pull/64221)
* On 2019-08-27, [Remove AST borrowck](https://github.com/rust-lang/rust/pull/64790)
Add note to documentation of HashSet::intersection
The functionality of the `std::collections::HashSet::intersection(...)` method was slightly surprising to me so I wanted to take a sec to contribute to the documentation for this method.
I've added a `Note:` section if that is appropriate.
use strict provenance APIs
The stdlib was adjusted to avoid bare int2ptr casts, but recently some casts of that sort have sneaked back in. Let's fix that. :)
implement ptr.addr() via transmute
As per the discussion in https://github.com/rust-lang/unsafe-code-guidelines/issues/286, the semantics for ptr-to-int transmutes that we are going with for now is to make them strip provenance without exposing it. That's exactly what `ptr.addr()` does! So we can implement `ptr.addr()` via `transmute`. This also means that once https://github.com/rust-lang/rust/pull/97684 lands, Miri can distinguish `ptr.addr()` from `ptr.expose_addr()`, and the following code will correctly be called out as having UB (if permissive provenance mode is enabled, which will become the default once the [implementation is complete](https://github.com/rust-lang/miri/issues/2133)):
```rust
fn main() {
let x: i32 = 3;
let x_ptr = &x as *const i32;
let x_usize: usize = x_ptr.addr();
// Cast back an address that did *not* get exposed.
let ptr = std::ptr::from_exposed_addr::<i32>(x_usize);
assert_eq!(unsafe { *ptr }, 3); //~ ERROR Undefined Behavior: dereferencing pointer failed
}
```
This completes the Miri implementation of the new distinctions introduced by strict provenance. :)
Cc `@Gankra` -- for now I left in your `FIXME(strict_provenance_magic)` saying these should be intrinsics, but I do not necessarily agree that they should be. Or if we have an intrinsic, I think it should behave exactly like the `transmute` does, which makes one wonder why the intrinsic should be needed.
Rollup of 6 pull requests
Successful merges:
- #97609 (Iterate over `maybe_unused_trait_imports` when checking dead trait imports)
- #97688 (test const_copy to make sure bytewise pointer copies are working)
- #97707 (Improve soundness of rustc_data_structures)
- #97731 (Add regresion test for #87142)
- #97735 (Don't generate "Impls on Foreign Types" for std)
- #97737 (Fix pretty printing named bound regions under -Zverbose)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Call the OS function to set the main thread's name on program init
Normally, `Thread::spawn` takes care of setting the thread's name, if
one was provided, but since the main thread wasn't created by calling
`Thread::spawn`, we need to call that function in `std::rt::init`.
This is mainly useful for system tools like debuggers and profilers
which might show the thread name to a user. Prior to these changes, gdb
and WinDbg would show all thread names except the main thread's name to
a user. I've validated that this patch resolves the issue for both
debuggers.
Lazily allocate and initialize pthread locks.
Lazily allocate and initialize pthread locks.
This allows {Mutex, Condvar, RwLock}::new() to be const, while still using the platform's native locks for features like priority inheritance and debug tooling. E.g. on macOS, we cannot directly use the (private) APIs that pthread's locks are implemented with, making it impossible for us to use anything other than pthread while still preserving priority inheritance, etc.
This PR doesn't yet make the public APIs const. That's for a separate PR with an FCP.
Tracking issue: https://github.com/rust-lang/rust/issues/93740
Avoid zero-sized allocs in ThinBox if T and H are both ZSTs.
This was surprisingly tricky, and took longer to get right than expected. `ThinBox` is a surprisingly subtle piece of code. That said, in the end, a lot of this was due to overthinking[^overthink] -- ultimately the fix ended up fairly clean and simple.
[^overthink]: Honestly, for a while I was convinced this couldn't be done without allocations or runtime branches in these cases, but that's obviously untrue.
Anyway, as a result of spending all that time debugging, I've extended the tests quite a bit, and also added more debug assertions. Many of these helped for subtle bugs I made in the middle (for example, the alloc/drop tracking is because I ended up double-dropping the value in the case where both were ZSTs), they're arguably a bit of overkill at this point, although I imagine they could help in the future too.
Anyway, these tests cover a wide range of size/align cases, nd fully pass under miri[^1]. They also do some smoke-check asserting that the value has the correct alignment, although in practice it's totally within the compiler's rights to delete these assertions since we'd have already done UB if they get hit. They have more boilerplate than they really need, but it's not *too* bad on a per-test basis.
A notable absence from testing is atypical header types, but at the moment it's impossible to manually implement `Pointee`. It would be really nice to have testing here, since it's not 100% obvious to me that the aligned read/write we use for `H` are correct in the face of arbitrary combinations of `size_of::<H>()`, `align_of::<H>()`, and `align_of::<T>()`. (That said, I spent a while thinking through it and am *pretty* sure it's fine -- I'd just feel... better if we could test some cases for non-ZST headers which have unequal and align).
[^1]: Or at least, they pass under miri if I copy the code and tests into a new crate and run miri on it (after making it less stdlibified).
Fixes#96485.
I'd request review ``@yaahc,`` but I believe you're taking some time away from reviews, so I'll request from the previous PR's reviewer (I think that the context helps, even if the actual change didn't end up being bad here).
r? ``@joshtriplett``
Stabilize `{slice,array}::from_ref`
This PR stabilizes the following APIs as `const` functions in Rust `1.63`:
```rust
// core::array
pub const fn from_ref<T>(s: &T) -> &[T; 1];
// core::slice
pub const fn from_ref<T>(s: &T) -> &[T];
```
Note that the `mut` versions are not stabilized as unique references (`&mut _`) are [unstable in const context].
FCP: https://github.com/rust-lang/rust/issues/90206#issuecomment-1134586665
r? rust-lang/libs-api `@rustbot` label +T-libs-api -T-libs
[unstable in const context]: https://github.com/rust-lang/rust/issues/57349
Improve documentation for constructors of pinned `Box`es
Adds a cross-references between `Box::pin` and `Box::into_pin` (and other related methods, i.e. the equivalent `From` implementation, and the unstable `pin_in` method), in particular now that `into_pin` [was stabilized](https://github.com/rust-lang/rust/pull/97397). The main goal is to further improve visibility of the fact that `Box<T> -> Pin<Box<T>>` conversion exits in the first place, and that `Box::pin(x)` is – essentially – just a convenience function for `Box::into_pin(Box::new(x))`
The motivating context why I think this is important is even experienced Rust users overlooking the existence this kind of conversion, [e.g. in this thread on IRLO](https://internals.rust-lang.org/t/pre-rfc-function-variants/16732/7?u=steffahn); and also the fact that that discussion brought up that there would be a bunch of Box-construction methods "missing" such as e.g. methods with fallible allocation a la "`Box::try_pin`", and similar; while those are in fact *not* necessary, because you can use `Box::into_pin(Box::try_new(x)?)` instead.
I have *not* included explicit mention of methods (e.g. `try_new`) in the docs of stable methods (e.g. `into_pin`). (Referring to unstable API in stable API docs would be bad style IMO.) Stable examples I have in mind with the statement "constructing a (pinned) Box in a different way than with `Box::new`" are things like cloning a `Box`, or `Box::from_raw`. If/when `try_new` would get stabilized, it would become a very good concrete example use-case of `Box::into_pin` IMO.
Revert #96682.
The change was "Show invisible delimiters (within comments) when pretty
printing". It's useful to show these delimiters, but is a breaking
change for some proc macros.
Fixes#97608.
r? ``@petrochenkov``
Add #[rustc_box] and use it inside alloc
This commit adds an alternative content boxing syntax, and uses it inside alloc.
```Rust
#![feature(box_syntax)]
fn foo() {
let foo = box bar;
}
```
is equivalent to
```Rust
#![feature(rustc_attrs)]
fn foo() {
let foo = #[rustc_box] Box::new(bar);
}
```
The usage inside the very performance relevant code in
liballoc is the only remaining relevant usage of box syntax
in the compiler (outside of tests, which are comparatively easy to port).
box syntax was originally designed to be used by all Rust
developers. This introduces a replacement syntax more tailored
to only being used inside the Rust compiler, and with it,
lays the groundwork for eventually removing box syntax.
[Earlier work](https://github.com/rust-lang/rust/pull/87781#issuecomment-894714878) by `@nbdd0121` to lower `Box::new` to `box` during THIR -> MIR building ran into borrow checker problems, requiring the lowering to be adjusted in a way that led to [performance regressions](https://github.com/rust-lang/rust/pull/87781#issuecomment-894872367). The proposed change in this PR lowers `#[rustc_box] Box::new` -> `box` in the AST -> HIR lowering step, which is way earlier in the compiler, and thus should cause less issues both performance wise as well as regarding type inference/borrow checking/etc. Hopefully, future work can move the lowering further back in the compiler, as long as there are no performance regressions.
The change was "Show invisible delimiters (within comments) when pretty
printing". It's useful to show these delimiters, but is a breaking
change for some proc macros.
Fixes#97608.
Additional `*mut [T]` methods
Split out from #94247
This adds the following methods to raw slices that already exist on regular slices
* `*mut [T]::is_empty`
* `*mut [T]::split_at_mut`
* `*mut [T]::split_at_mut_unchecked`
These methods reduce the amount of unsafe code needed to migrate `ChunksMut` and related iterators
to raw slices (#94247)
r? `@m-ou-se`
Tweak insert docs
For `{Hash, BTree}Map::insert`, I always have to take a few extra seconds to think about the slight weirdness about the fact that if we "did not" insert (which "sounds" false), we return true, and if we "did" insert, (which "sounds" true), we return false.
This tweaks the doc comments for the `insert` methods of those types (as well as what looks like a rustc internal data structure that I found just by searching the codebase for "If the set did") to first use the "Returns whether _something_" pattern used in e.g. `remove`, where we say that `remove` "returns whether the value was present".
Corrected EBNF grammar for from_str
Hello! This is my first time contributing to an open-source project. I'm excited to have the chance to contribute to the rust community 🥳
I noticed an issue with the documentation for `from_str` in `f32` and `f64`. It states that "All strings that adhere to the following [EBNF](https://www.w3.org/TR/REC-xml/#sec-notation) grammar when lowercased will result in an `Ok` being returned. I believe this is incorrect for the string `"."`, which is valid for the given EBNF grammar, but does not result in an `Ok` being returned ([playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=09f891aa87963a56d3b0d715d8cbc2b4)). I have simplified the grammar in a way which fixes that, but is otherwise identical.
Previously, the `Number` part of the EBNF grammar had an option for `'.' Digit*`, which would include the string `"."`. This is not valid, and does not return an Ok as stated. The corrected version removes this, and still allows for the `'.' Digit+` case with the already existing `Digit* '.' Digit+` case.
Expose `get_many_mut` and `get_many_unchecked_mut` to HashMap
This pull-request expose the function [`get_many_mut`](https://docs.rs/hashbrown/0.12.0/hashbrown/struct.HashMap.html#method.get_many_mut) and [`get_many_unchecked_mut`](https://docs.rs/hashbrown/0.12.0/hashbrown/struct.HashMap.html#method.get_many_unchecked_mut) from `hashbrown` to the standard library `HashMap` type. They obviously keep the same API and are added under the (new) `map_many_mut` feature.
- `get_many_mut`: Attempts to get mutable references to `N` values in the map at once.
- `get_many_unchecked_mut`: Attempts to get mutable references to `N` values in the map at once, without validating that the values are unique.
library/std: Bump compiler_builtins
Some neat changes include faster float conversions & fixes for AVR 🙂
(note that's it's my first time upgrading `compiler_builtins`, so I'm not 100% sure if bumping `library/std/Cargo.toml` is enough; certainly seems to be so, though.)
Fixup feature name to be more consistent with others
`slice_from_mut_ptr_range_const` -> `const_slice_from_mut_ptr_range`, we usually have `const` in the front.
I've made a typo in #97419
alloc: remove repeated word in comment
Linux's `checkpatch.pl` reports:
```txt
#42544: FILE: rust/alloc/vec/mod.rs:2692:
WARNING: Possible repeated word: 'to'
+ // - Elements are :Copy so it's OK to to copy them, without doing
```
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Put a bound on collection misbehavior
As currently written, when a logic error occurs in a collection's trait parameters, this allows *completely arbitrary* misbehavior, so long as it does not cause undefined behavior in std. However, because the extent of misbehavior is not specified, it is allowed for *any* code in std to start misbehaving in arbitrary ways which are not formally UB; consider the theoretical example of a global which gets set on an observed logic error. Because the misbehavior is only bound by not resulting in UB from safe APIs and the crate-level encapsulation boundary of all of std, this makes writing user unsafe code that utilizes std theoretically impossible, as it now relies on undocumented QOI (quality of implementation) that unrelated parts of std cannot be caused to misbehave by a misuse of std::collections APIs.
In practice, this is a nonconcern, because std has reasonable QOI and an implementation that takes advantage of this freedom is essentially a malicious implementation and only compliant by the most langauage-lawyer reading of the documentation.
To close this hole, we just add a small clause to the existing logic error paragraph that ensures that any misbehavior is limited to the collection which observed the logic error, making it more plausible to prove the soundness of user unsafe code.
This is not meant to be formal; a formal refinement would likely need to mention that values derived from the collection can also misbehave after a logic error is observed, as well as define what it means to "observe" a logic error in the first place. This fix errs on the side of informality in order to close the hole without complicating a normal reading which can assume a reasonable nonmalicious QOI.
See also [discussion on IRLO][1].
[1]: https://internals.rust-lang.org/t/using-std-collections-and-unsafe-anything-can-happen/16640
r? rust-lang/libs-api ```@rustbot``` label +T-libs-api -T-libs
This technically adds a new guarantee to the documentation, though I argue as written it's one already implicitly provided.
Clarify the guarantees of Vec::as_ptr and Vec::as_mut_ptr when there's no allocation
Currently the documentation says they return a pointer to the vector's buffer, which has the implied precondition that the vector allocated some memory. However `Vec`'s documentation also specifies that it won't always allocate, so it's unclear whether the pointer returned is valid in that case. Of course you won't be able to read/write actual bytes to/from it since the capacity is 0, but there's an exception: zero sized read/writes. They are still valid as long as the pointer is not null and the memory it points to wasn't deallocated, but `Vec::as_ptr` and `Vec::as_mut_ptr` don't specify that's not the case. This PR thus specifies they are actually valid for zero sized reads since `Vec` is implemented to hold a dangling pointer in those cases, which is neither null nor was deallocated.
Linux's `checkpatch.pl` reports:
```txt
#42544: FILE: rust/alloc/vec/mod.rs:2692:
WARNING: Possible repeated word: 'to'
+ // - Elements are :Copy so it's OK to to copy them, without doing
```
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Add unicode fast path to `is_printable`
Before, it would enter the full expensive check even for normal ascii characters. Now, it skips the check for the ascii characters in `32..127`. This range was checked manually from the current behavior.
I ran the `tracing` test suite in miri, and it was really slow. I looked at a profile, and miri spent most of the time in `core::char::methods::escape_debug_ext`, where half of that was dominated by `core::unicode::printable::is_printable`. So I optimized it here.
The tracing profile:
Before, it would enter the full expensive check even for normal ascii
characters. Now, it skips the check for the ascii characters in
`32..127`. This range was checked manually from the current behavior.
Rollup of 6 pull requests
Successful merges:
- #97089 (Improve settings theme display)
- #97229 (Document the current aliasing rules for `Box<T>`.)
- #97371 (Suggest adding a semicolon to a closure without block)
- #97455 (Stabilize `toowned_clone_into`)
- #97565 (Add doc alias `memset` to `write_bytes`)
- #97569 (Remove `memset` alias from `fill_with`.)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Add doc alias `memset` to `write_bytes`
I were looking for `memset` in rust, but the docs only pointed me to `slice::fill`.
With only the old aliases, one might write code like this, which is incorrect if the memory is uninitialized.
``` Rust
core::slice::from_raw_parts(ptr, len).fill(0)
```
Document the current aliasing rules for `Box<T>`.
Currently, `Box<T>` gets `noalias`, meaning it has the same rules as `&mut T`. This is sparsely documented, even though it can have quite a big impact on unsafe code using box. Therefore, these rules are documented here, with a big warning that they are not normative and subject to change, since we have not yet committed to an aliasing model and the state of `Box<T>` is especially uncertain.
If you have any suggestions and improvements, make sure to leave them here. This is mostly intended to inform people about what is currently going on (to prevent misunderstandings such as [Jon Gjengset's Box aliasing](https://www.youtube.com/watch?v=EY7Wi9fV5bk)).
This is supposed to _only document current UB_ and not add any new guarantees or rules.
Implement [OsStr]::join
Implements join for `OsStr` and `OsString` slices:
```Rust
let strings = [OsStr::new("hello"), OsStr::new("dear"), OsStr::new("world")];
assert_eq!("hello dear world", strings.join(OsStr::new(" ")));
````
This saves one from converting to strings and back, or from implementing it manually.
This PR has been re-filed after #96744 was first accidentally merged and then reverted.
The change is instantly stable and thus:
r? rust-lang/libs-api `@rustbot` label +T-libs-api -T-libs
cc `@thomcc` `@m-ou-se` `@faptc`
refactor: VecDeques Iter fields to private
Made the fields of VecDeque's Iter private by creating a Iter::new(...) function to create a new instance of Iter and migrating usage to use Iter::new(...).
improve format impl for literals
The basic idea of this change can be seen here https://godbolt.org/z/MT37cWoe1.
Updates the format impl to have a fast path for string literals and the default path for regular format args.
This change will allow `format!("string literal")` to be used interchangably with `"string literal".to_owned()`.
This would be relevant in the case of `f!"string literal"` being legal (https://github.com/rust-lang/rfcs/pull/3267) in which case it would be the easiest way to create owned strings from literals, while also being just as efficient as any other impl
Remove "sys isn't exported yet" phrase
The oldest occurence is from 9e224c2bf1,
which is from the pre-1.0 days. In the years since then, std::sys still
hasn't been exported, and the last attempt was met with strong criticism:
https://github.com/rust-lang/rust/pull/97151
Thus, removing the "yet" part makes a lot of sense.
Use Box::new() instead of box syntax in library tests
The tests inside `library/*` have no reason to use `box` syntax as they have 0 performance relevance. Therefore, we can safely remove them (instead of having to use alternatives like the one in #97293).
The oldest occurence is from 9e224c2bf1,
which is from the pre-1.0 days. In the years since then, std::sys still
hasn't been exported, and the last attempt was met with strong criticism:
https://github.com/rust-lang/rust/pull/97151
Thus, removing the "yet" part makes a lot of sense.
Replace `#[default_method_body_is_const]` with `#[const_trait]`
pulled out of #96077
related issues: #67792 and #92158
cc `@fee1-dead`
This is groundwork to only allowing `impl const Trait` for traits that are marked with `#[const_trait]`. This is necessary to prevent adding a new default method from becoming a breaking change (as it could be a non-const fn).
Finish bumping stage0
It looks like the last time had left some remaining cfg's -- which made me think
that the stage0 bump was actually successful. This brings us to a released 1.62
beta though.
This now brings us to cfg-clean, with the exception of check-cfg-features in bootstrap;
I'd prefer to leave that for a separate PR at this time since it's likely to be more tricky.
cc https://github.com/rust-lang/rust/pull/97147#issuecomment-1132845061
r? `@pietroalbini`
ptr::invalid is not equivalent to a int2ptr cast
I just realized I forgot to update these docs when adding `from_exposed_addr`.
Right now the docs say `invalid` and `from_exposed_addr` are both equivalent to a cast, and that is clearly not what we want.
Cc ``@Gankra``
Previously, the `Number` part of the EBNF grammar had an option for `'.' Digit*`, which would include the string "." (a single decimal point). This is not valid, and does not return an Ok as stated. The corrected version removes this, and still allows for the `'.' Digit+` case with the already existing `Digit* '.' Digit+` case.
update libbacktrace
It seems like previously we were on a tag of the backtrace repo; not sure if there is a policy that it should always be a tag?
Cc https://github.com/rust-lang/backtrace-rs/pull/462 `@alexcrichton` `@DrMeepster`
proc_macro: don't pass a client-side function pointer through the server.
Before this PR, `proc_macro::bridge::Client<F>` contained both:
* the C ABI entry-point `run`, that the server can call to start the client
* some "payload" `f: F` passed to that entry-point
* in practice, this was always a (client-side Rust ABI) `fn` pointer to the actual function the proc macro author wrote, i.e. `#[proc_macro] fn foo(input: TokenStream) -> TokenStream`
In other words, the client was passing one of its (Rust) `fn` pointers to the server, which was passing it back to the client, for the client to call (see later below for why that was ever needed).
I was inspired by `@nnethercote's` attempt to remove the `get_handle_counters` field from `Client` (see https://github.com/rust-lang/rust/pull/97004#issuecomment-1139273301), which combined with removing the `f` ("payload") field, could theoretically allow for a `#[repr(transparent)]` `Client` that mostly just newtypes the C ABI entry-point `fn` pointer <sub>(and in the context of e.g. wasm isolation, that's *all* you want, since you can reason about it from outside the wasm VM, as just a 32-bit "function table index", that you can pass to the wasm VM to call that function)</sub>.
<hr/>
So this PR removes that "payload". But it's not a simple refactor: the reason the field existed in the first place is because monomorphizing over a function type doesn't let you call the function without having a value of that type, because function types don't implement anything like `Default`, i.e.:
```rust
extern "C" fn ffi_wrapper<A, R, F: Fn(A) -> R>(arg: A) -> R {
let f: F = ???; // no way to get a value of `F`
f(arg)
}
```
That could be solved with something like this, if it was allowed:
```rust
extern "C" fn ffi_wrapper<
A, R,
F: Fn(A) -> R,
const f: F // not allowed because the type is a generic param
>(arg: A) -> R {
f(arg)
}
```
Instead, this PR contains a workaround in `proc_macro::bridge::selfless_reify` (see its module-level comment for more details) that can provide something similar to the `ffi_wrapper` example above, but limited to `F` being `Copy` and ZST (and requiring an `F` value to prove the caller actually can create values of `F` and it's not uninhabited or some other unsound situation).
<hr/>
Hopefully this time we don't have a performance regression, and this has a chance to land.
cc `@mystor` `@bjorn3`
Partially stabilize `(const_)slice_ptr_len` feature by stabilizing `NonNull::len`
This PR partially stabilizes features `const_slice_ptr_len` and `slice_ptr_len` by only stabilizing `NonNull::len`. This partial stabilization is tracked under features `slice_ptr_len_nonnull` and `const_slice_ptr_len_nonnull`, for which this PR can serve as the tracking issue.
To summarize the discussion from #71146 leading up to this partial stabilization request:
It's currently a bit footgunny to obtain the length of a raw slice pointer, stabilization of `NonNull:len` will help with removing these footguns. Some example footguns are:
```rust
/// # Safety
/// The caller must ensure that `ptr`:
/// 1. does not point to memory that was previously allocated but is now deallocated;
/// 2. is within the bounds of a single allocated object;
/// 3. does not to point to a slice for which the length exceeds `isize::MAX` bytes;
/// 4. points to a properly aligned address;
/// 5. does not point to uninitialized memory;
/// 6. does not point to a mutably borrowed memory location.
pub unsafe fn ptr_len<T>(ptr: core::ptr::NonNull<[T]>) -> usize {
(&*ptr.as_ptr()).len()
}
```
A slightly less complicated version (but still more complicated than it needs to be):
```rust
/// # Safety
/// The caller must ensure that the start of `ptr`:
/// 1. does not point to memory that was previously allocated but is now deallocated;
/// 2. must be within the bounds of a single allocated object.
pub unsafe fn ptr_len<T>(ptr: NonNull<[T]>) -> usize {
(&*(ptr.as_ptr() as *const [()])).len()
}
```
This PR does not stabilize `<*const [T]>::len` and `<*mut [T]>::len` because the tracking issue #71146 list a potential blocker for these methods, but this blocker [does not apply](https://github.com/rust-lang/rust/issues/71146#issuecomment-808735714) to `NonNull::len`.
We should probably also ping the [Constant Evaluation WG](https://github.com/rust-lang/const-eval) since this PR includes a `#[rustc_allow_const_fn_unstable(const_slice_ptr_len)]`. My instinct here is that this will probably be okay because the pointer is not actually dereferenced and `len()` does not touch the address component of the pointer, but would be best to double check :)
One potential down-side was raised that stabilizing `NonNull::len` could lead to encouragement of coding patterns like:
```
pub fn ptr_len<T>(ptr: *mut [T]) -> usize {
NonNull::new(ptr).unwrap().len()
}
```
which unnecessarily assert non-nullness. However, these are much less of a footgun than the above examples and this should be resolved when `slice_ptr_len` fully stabilizes eventually.
Remove impossible panic note from `Vec::append`
Neither the number of elements in a vector can overflow a `usize`, nor
can the amount of elements in two vectors.
Normally, `Thread::spawn` takes care of setting the thread's name, if
one was provided, but since the main thread wasn't created by calling
`Thread::spawn`, we need to call that function in `std::rt::init`.
This is mainly useful for system tools like debuggers and profilers
which might show the thread name to a user. Prior to these changes, gdb
and WinDbg would show all thread names except the main thread's name to
a user. I've validated that this patch resolves the issue for both
debuggers.
It looks like the last time had left some remaining cfg's -- which made me think
that the stage0 bump was actually successful. This brings us to a released 1.62
beta though.
