This commit intends to fill out some of the remaining pieces of the
C-unwind ABI. This has a number of other changes with it though to move
this design space forward a bit. Notably contained within here is:
* On `panic=unwind`, the `extern "C"` ABI is now considered as "may
unwind". This fixes a longstanding soundness issue where if you
`panic!()` in an `extern "C"` function defined in Rust that's actually
UB because the LLVM representation for the function has the `nounwind`
attribute, but then you unwind.
* Whether or not a function unwinds now mainly considers the ABI of the
function instead of first checking the panic strategy. This fixes a
miscompile of `extern "C-unwind"` with `panic=abort` because that ABI
can still unwind.
* The aborting stub for non-unwinding ABIs with `panic=unwind` has been
reimplemented. Previously this was done as a small tweak during MIR
generation, but this has been moved to a separate and dedicated MIR
pass. This new pass will, for appropriate functions and function
calls, insert a `cleanup` landing pad for any function call that may
unwind within a function that is itself not allowed to unwind. Note
that this subtly changes some behavior from before where previously on
an unwind which was caught-to-abort it would run active destructors in
the function, and now it simply immediately aborts the process.
* The `#[unwind]` attribute has been removed and all users in tests and
such are now using `C-unwind` and `#![feature(c_unwind)]`.
I think this is largely the last piece of the RFC to implement.
Unfortunately I believe this is still not stabilizable as-is because
activating the feature gate changes the behavior of the existing `extern
"C"` ABI in a way that has no replacement. My thinking for how to enable
this is that we add support for the `C-unwind` ABI on stable Rust first,
and then after it hits stable we change the behavior of the `C` ABI.
That way anyone straddling stable/beta/nightly can switch to `C-unwind`
safely.
Add convenience method for handling ipv4-mapped addresses by canonicalizing them
This simplifies checking common properties in an address-family-agnostic
way since #86335 commits to not checking IPv4 semantics
of IPv4-mapped addresses in the `Ipv6Addr` property methods.
Commit to not supporting IPv4-in-IPv6 addresses
Stabilization of the `ip` feature has for a long time been blocked on the question of whether Rust should support handling "IPv4-in-IPv6" addresses: should the various `Ipv6Address` property methods take IPv4-mapped or IPv4-compatible addresses into account. See also the IPv4-in-IPv6 Address Support issue #85609 and #69772 which originally asked the question.
# Overview
In the recent PR #85655 I proposed changing `is_loopback` to take IPv4-mapped addresses into account, so `::ffff:127.0.0.1` would be recognized as a looback address. However, due to the points that came up in that PR, I alternatively propose the following: Keeping the current behaviour and commit to not assigning any special meaning for IPv4-in-IPv6 addresses, other than what the standards prescribe. This would apply to the stable method `is_loopback`, but also to currently unstable methods like `is_global` and `is_documentation` and any future methods. This is implemented in this PR as a change in documentation, specifically the following section:
> Both types of addresses are not assigned any special meaning by this implementation, other than what the relevant standards prescribe. This means that an address like `::ffff:127.0.0.1`, while representing an IPv4 loopback address, is not itself an IPv6 loopback address; only `::1` is. To handle these so called "IPv4-in-IPv6" addresses, they have to first be converted to their canonical IPv4 address.
# Discussion
In the discussion for or against supporting IPv4-in-IPv6 addresses the question what would be least surprising for users of other languages has come up several times. At first it seemed most big other languages supported IPv4-in-IPv6 addresses (or at least considered `::ffff:127.0.0.1` a loopback address). However after further investigation it appears that supporting IPv4-in-IPv6 addresses comes down to how a language represents addresses. .Net and Go do not have a separate type for IPv4 or IPv6 addresses, and do consider `::ffff:127.0.0.1` a loopback address. Java and Python, which do have separate types, do not consider `::ffff:127.0.0.1` a loopback address. Seeing as Rust has the separate `Ipv6Addr` type, it would make sense to also not support IPv4-in-IPv6 addresses. Note that this focuses on IPv4-mapped addresses, no other language handles IPv4-compatible addresses.
Another issue that was raised is how useful supporting these IPv4-in-IPv6 addresses would be in practice. Again with the example of `::ffff:127.0.0.1`, considering it a loopback address isn't too useful as to use it with most of the socket APIs it has to be converted to an IPv4 address anyway. From that perspective it would be better to instead provide better ways for doing this conversion like stabilizing `to_ipv4_mapped` or introducing a `to_canonical` method.
A point in favour of not supporting IPv4-in-IPv6 addresses is that that is the behaviour Rust has always had, and that supporting it would require changing already stable functions like `is_loopback`. This also keeps the documentation of these functions simpler, as we only have to refer to the relevant definitions in the IPv6 specification.
# Decision
To make progress on the `ip` feature, a decision needs to be made on whether or not to support IPv4-in-IPv6 addresses.
There are several options:
- Keep the current implementation and commit to never supporting IPv4-in-IPv6 addresses (accept this PR).
- Support IPv4-in-IPv6 addresses in some/all `IPv6Addr` methods (accept PR #85655).
- Keep the current implementation and but not commit to anything yet (reject both this PR and PR #85655), this entire issue will however come up again in the stabilization of several methods under the `ip` feature.
There are more options, like supporting IPv4-in-IPv6 addresses in `IpAddr` methods instead, but to my knowledge those haven't been seriously argued for by anyone.
There is currently an FCP ongoing on PR #85655. I would ask the libs team for an alternative FCP on this PR as well, which if completed means the rejection of PR #85655, and the decision to commit to not supporting IPv4-in-IPv6 addresses.
If anyone feels there is not enough evidence yet to make the decision for or against supporting IPv4-in-IPv6 addresses, let me know and I'll do whatever I can to resolve it.
This simplifies checking common properties in an address-family-agnostic
way since since #86335 commits to not checking IPv4 semantics
of IPv4-mapped addresses in the `Ipv6Addr` property methods.
Add missing "allocated object" doc link to `<*mut T>::add`
The portion of the documentation expecting the link was already there, but it was rendered as "[allocated object]". The added reference is just copied from the documentation for `<*const T>::add`.
rfc3052 followup: Remove authors field from Cargo manifests
Since RFC 3052 soft deprecated the authors field, hiding it from
crates.io, docs.rs, and making Cargo not add it by default, and it is
not generally up to date/useful information for contributors, we may as well
remove it from crates in this repo.
Recommend `swap_remove` in `Vec::remove` docs
I was able to increase the performance (by 20%!) of my project by changing a `Vec::remove` call to `Vec::swap_remove` in a hot function. I think we should explicitly put a note in the Vec::remove docs to guide people in the right direction so they don't make a similar oversight.
Add documentation for `Ipv6MulticastScope`
Adds basic documentation to the unstable `Ipv6MulticastScope`, as well as marking it `#[non_exhaustive]` because future IETF RFCs may introduce additional scopes. The documentation mentions this in a section "Stability Guarantees":
> /// Not all possible values for a multicast scope have been assigned.
/// Future RFCs may introduce new scopes, which will be added as variants to this enum;
/// because of this the enum is marked as `#[non_exhaustive]`.
Partially stabilize `const_slice_first_last`
This stabilizes the non-`mut` methods of `const_slice_first_last` as `const`. These methods are trivial to implement and have no blockers that I am aware of.
`@rustbot` label +A-const-fn +S-waiting-on-review +T-libs-api
Move `os_str_bytes` to `sys::unix`
Followup to #84967, with `OsStrExt` and `OsStringExt` moved out of `sys_common`, there is no reason anymore for `os_str_bytes` to live in `sys_common` and not in sys. This pr moves it to the location `sys::unix::os_str` and reuses the code on other platforms via `#[path]` (as is common in `sys`) instead of importing.
Remove `Ipv4Addr::is_ietf_protocol_assignment`
This PR removes the unstable method `Ipv4Addr::is_ietf_protocol_assignment`, as I suggested in https://github.com/rust-lang/rust/issues/85612#issuecomment-847863404. The method was added in #60145, as far as I can tell primarily for the implementation of `Ipv4Addr::is_global` (addresses reserved for IETF protocol assignment are not globally reachable unless otherwise specified).
The method was added in 2019, but I haven't been able to find any open-source code using this method so far. I'm also having a hard time coming up with a usecase for specifically this method; knowing that an address is reserved for future protocols doesn't allow you to do much with it, especially since now some of those addresses are indeed assigned to a protocol and have their own behaviour (and might even be defined to be globally reachable, so if that is what you care about it is always more accurate to call `!is_global()`, instead of `is_ietf_protocol_assignment()`).
Because of these reasons, I propose removing the method (or alternatively make it a private helper for `is_global`) and also not introduce `Ipv6Addr::is_ietf_protocol_assignment` and `IpAddr::is_ietf_protocol_assignment` in the future.
Change environment variable getters to error recoverably
This PR changes the standard library environment variable getter functions to error recoverably (i.e. not panic) when given an invalid value.
On some platforms, it is invalid for environment variable names to contain `'\0'` or `'='`, or for their values to contain `'\0'`. Currently, the standard library panics when manipulating environment variables with names or values that violate these invariants. However, this behavior doesn't make a lot of sense, at least in the case of getters. If the environment variable is missing, the standard library just returns an error value, rather than panicking. It doesn't make sense to treat the case where the variable is invalid any differently from that. See the [internals thread](https://internals.rust-lang.org/t/why-should-std-var-panic/14847) for discussion. Thus, this PR changes the functions to error recoverably in this case as well.
If desired, I could change the functions that manipulate environment variables in other ways as well. I didn't do that here because it wasn't entirely clear what to change them to. Should they error silently or do something else? If someone tells me how to change them, I'm happy to implement the changes.
This fixes#86082, an ICE that arises from the current behavior. It also adds a regression test to make sure the ICE does not occur again in the future.
`@rustbot` label +T-libs
r? `@joshtriplett`
BTree: lazily locate leaves in rangeless iterators
BTree iterators always locate both the first and last leaf edge and often only need either one, i.e., whenever they are traversed in a single direction, like in for-loops and in the common use of `iter().next()` or `iter().next_back()` to retrieve the first or last key/value-pair (#62924). It's fairly easy to avoid because the iterators with this disadvantage already are quite separate from other iterators.
r? `@Mark-Simulacrum`
Bump bootstrap compiler to 1.55
Changing the cfgs for stdarch is missing, but my understanding is that we don't need to do it as part of this PR?
r? `@Mark-Simulacrum`
Add Linux-specific pidfd process extensions (take 2)
Continuation of #77168.
I addressed the following concerns from the original PR:
- make `CommandExt` and `ChildExt` sealed traits
- wrap file descriptors in `PidFd` struct representing ownership over the fd
- add `take_pidfd` to take the fd out of `Child`
- close fd when dropped
Tracking Issue: #82971
Move UnwindSafe, RefUnwindSafe, AssertUnwindSafe to core
They were previously only available in std::panic, not core::panic.
- https://doc.rust-lang.org/1.51.0/std/panic/trait.UnwindSafe.html
- https://doc.rust-lang.org/1.51.0/std/panic/trait.RefUnwindSafe.html
- https://doc.rust-lang.org/1.51.0/std/panic/struct.AssertUnwindSafe.html
Where this is relevant: trait objects! Inside a `#![no_std]` library it's otherwise impossible to have a struct holding a trait object, and at the same time can be used from downstream std crates in a way that doesn't interfere with catch_unwind.
```rust
// common library
#![no_std]
pub struct Thing {
pub(crate) x: &'static (dyn SomeTrait + Send + Sync),
}
pub(crate) trait SomeTrait {...}
```
```rust
// downstream application
fn main() {
let thing: library::Thing = ...;
let _ = std::panic::catch_unwind(|| { let _ = thing; }); // does not work :(
}
```
See a4131708e2/src/gradient.rs (L7-L15) for a real life example of needing to work around this problem. In particular that workaround would not even be viable if implementors of the trait were provided externally by a caller, as the `feature = "std"` would become non-additive in that case.
What happens without the UnwindSafe constraints:
```rust
fn main() {
let gradient = colorous::VIRIDIS;
let _ = std::panic::catch_unwind(|| { let _ = gradient; });
}
```
```console
error[E0277]: the type `(dyn colorous::gradient::EvalGradient + Send + Sync + 'static)` may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
--> src/main.rs:3:13
|
3 | let _ = std::panic::catch_unwind(|| { let _ = gradient; });
| ^^^^^^^^^^^^^^^^^^^^^^^^ `(dyn colorous::gradient::EvalGradient + Send + Sync + 'static)` may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
|
::: .rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:430:40
|
430 | pub fn catch_unwind<F: FnOnce() -> R + UnwindSafe, R>(f: F) -> Result<R> {
| ---------- required by this bound in `catch_unwind`
|
= help: within `Gradient`, the trait `RefUnwindSafe` is not implemented for `(dyn colorous::gradient::EvalGradient + Send + Sync + 'static)`
= note: required because it appears within the type `&'static (dyn colorous::gradient::EvalGradient + Send + Sync + 'static)`
= note: required because it appears within the type `Gradient`
= note: required because of the requirements on the impl of `UnwindSafe` for `&Gradient`
= note: required because it appears within the type `[closure@src/main.rs:3:38: 3:62]`
```
Implement advance_by, advance_back_by for slice::{Iter, IterMut}
Part of #77404.
Picking up where #77633 was closed.
I have addressed https://github.com/rust-lang/rust/pull/77633#issuecomment-771842599 by restoring `nth` and `nth_back`. So according to that comment this should already be r=m-ou-se, but it has been sitting for a while.
Track caller of Vec::remove()
`vec.remove(invalid)` doesn't print a helpful source position:
> thread 'main' panicked at 'removal index (is 99) should be < len (is 1)', **library/alloc/src/vec/mod.rs:1379:13**
Add docs about performance and `Iterator::map` to `[T; N]::map`
This suboptimal code gen for some usages of array::map got a bit of
attention by multiple people throughout the community. Some cases:
- https://github.com/rust-lang/rust/issues/75243#issuecomment-866051086
- https://github.com/rust-lang/rust/issues/75243#issuecomment-874732134
- https://www.reddit.com/r/rust/comments/oeqqf7/unexpected_high_stack_usage/
My *guess* is that this gets the attention it gets because in JavaScript
(and potentially other languages), a `map` function on arrays is very
commonly used since in those languages, arrays basically take the role
of Rust's iterator. I considered explicitly naming JavaScript in the
first paragraph I added, but I couldn't find precedence of mentioning
other languages in standard library doc, so I didn't add it.
When array::map was stabilized, we still wanted to add docs, but that
somehow did not happen in time. So here we are. Not sure if this sounds
crazy but maybe it is worth considering beta backporting this? Only if
it's not a lot of work, of course! But yeah, stabilized array::map is
already in beta and if this problem is really as big as it sometimes seems,
might be worth having the docs in place when 1.55 is released.
CC ``@CryZe``
r? ``@m-ou-se`` (since you were involved in that discussion and the stabilization)
Make `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` warn by default
This PR makes the `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` lint warn by default.
To avoid showing a large number of un-actionable warnings to users, we only enable the lint for macros defined in the same crate. This ensures that users will be able to fix the warning by simply removing a semicolon.
In the future, I'd like to enable this lint unconditionally, and eventually make it into a hard error in a future edition. This PR is a step towards that goal.
