check `FnDef` return type for WF
better version of #106807, fixes#84533 (mostly). It's not perfect given that we still ignore WF requirements involving bound regions but I wasn't able to quickly write an example, so even if theoretically exploitable, it should be far harder to trigger.
This is strictly more restrictive than checking the return type for WF as part of the builtin `FnDef: FnOnce` impl (#106807) and moving to this approach in the future will not break any code.
~~It also agrees with my theoretical view of how this should behave~~
r? types
CFI: Support function pointers for trait methods
Adds support for both CFI and KCFI for function pointers to trait methods by attaching both concrete and abstract types to functions.
KCFI does this through generation of a `ReifyShim` on any function pointer for a method that could go into a vtable, and keeping this separate from `ReifyShim`s that are *intended* for vtable us by setting a `ReifyReason` on them.
CFI does this by setting both the concrete and abstract type on every instance.
This should land after #123024 or a similar PR, as it diverges the implementation of CFI vs KCFI.
r? `@compiler-errors`
instantiate higher ranked goals outside of candidate selection
This PR modifies `evaluate` to more eagerly instantiate higher-ranked goals, preventing the `leak_check` during candidate selection from detecting placeholder errors involving that binder.
For a general background regarding higher-ranked region solving and the leak check, see https://hackmd.io/qd9Wp03cQVy06yOLnro2Kg.
> The first is something called the **leak check**. You can think of it as a "quick and dirty" approximation for the region check, which will come later. The leak check detects some kinds of errors early, essentially deciding between "this set of outlives constraints are guaranteed to result in an error eventually" or "this set of outlives constraints may be solvable".
## The ideal future
We would like to end up with the following idealized design to handle universal binders:
```rust
fn enter_forall<'tcx, T, R>(
forall: Binder<'tcx, T>,
f: impl FnOnce(T) -> R,
) -> R {
let new_universe = infcx.increment_universe_index();
let value = instantiate_binder_with_placeholders_in(new_universe, forall);
let result = f(value);
eagerly_handle_higher_ranked_region_constraints_in(new_universe);
infcx.decrement_universe_index();
assert!(!result.has_placeholders_in_or_above(new_universe));
result
}
```
That is, when universally instantiating a binder, anything using the placeholders has to happen inside of a limited scope (the closure `f`). After this closure has completed, all constraints involving placeholders are known.
We then handle any *external constraints* which name these placeholders. We destructure `TypeOutlives` constraints involving placeholders and eagerly handle any region constraints involving these placeholders. We do not return anything mentioning the placeholders created inside of this function to the caller.
Being able to eagerly handle *all* region constraints involving placeholders will be difficult due to complex `TypeOutlives` constraints, involving inference variables or alias types, and higher ranked implied bounds. The exact issues and possible solutions are out of scope of this FCP.
#### How does the leak check fit into this
The `leak_check` is an underapproximation of `eagerly_handle_higher_ranked_region_constraints_in`. It detects some kinds of errors involving placeholders from `new_universe`, but not all of them.
It only looks at region outlives constraints, ignoring `TypeOutlives`, and checks whether one of the following two conditions are met for **placeholders in or above `new_universe`**, in which case it results in an error:
- `'!p1: '!p2` a placeholder `'!p2` outlives a different placeholder `'!p1`
- `'!p1: '?2` an inference variable `'?2` outlives a placeholder `'!p1` *which it cannot name*
It does not handle all higher ranked region constraints, so we still return constraints involving placeholders from `new_universe` which are then (re)checked by `lexical_region_resolve` or MIR borrowck.
As we check higher ranked constraints in the full regionck anyways, the `leak_check` is not soundness critical. It's current only purpose is to move some higher ranked region errors earlier, enabling it to guide type inference and trait solving. Adding additional uses of the `leak_check` in the future would only strengthen inference and is therefore not breaking.
## Where do we use currently use the leak check
The `leak_check` is currently used in two places:
Coherence does not use a proper regionck, only relying on the `leak_check` called [at the end of the implicit negative overlap check](8b94152af6/compiler/rustc_trait_selection/src/traits/coherence.rs (L235-L238)). During coherence all parameters are instantiated with inference variables, so the only possible region errors are higher-ranked. We currently also sometimes make guesses when destructuring `TypeOutlives` constraints which can theoretically result in incorrect errors. This could result in overlapping impls.
We also use the `leak_check` [at the end of `fn evaluation_probe`](8b94152af6/compiler/rustc_trait_selection/src/traits/select/mod.rs (L607-L610)). This function is used during candidate assembly for `Trait` goals. Most notably we use [inside of `evaluate_candidate` during winnowing](0e4243538b/compiler/rustc_trait_selection/src/traits/select/mod.rs (L491-L502)). Conceptionally, it is as if we compute each candidate in a separate `enter_forall`.
## The current use in `fn evaluation_probe` is undesirable
Because we only instantiate a higher-ranked goal once inside of `fn evaluation_probe`, errors involving placeholders from that binder can impact selection. This results in inconsistent behavior ([playground](
*[playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=dac60ebdd517201788899ffa77364831)*)):
```rust
trait Leak<'a> {}
impl Leak<'_> for Box<u32> {}
impl Leak<'static> for Box<u16> {}
fn impls_leak<T: for<'a> Leak<'a>>() {}
trait IndirectLeak<'a> {}
impl<'a, T: Leak<'a>> IndirectLeak<'a> for T {}
fn impls_indirect_leak<T: for<'a> IndirectLeak<'a>>() {}
fn main() {
// ok
//
// The `Box<u16>` impls fails the leak check,
// meaning that we apply the `Box<u32>` impl.
impls_leak::<Box<_>>();
// error: type annotations needed
//
// While the `Box<u16>` impl would fail the leak check
// we have already instantiated the binder while applying
// the generic `IndirectLeak` impl, so during candidate
// selection of `Leak` we do not detect the placeholder error.
// Evaluation of `Box<_>: Leak<'!a>` is therefore ambiguous,
// resulting in `for<'a> Box<_>: Leak<'a>` also being ambiguous.
impls_indirect_leak::<Box<_>>();
}
```
We generally prefer `where`-bounds over implementations during candidate selection, both for [trait goals](11f32b73e0/compiler/rustc_trait_selection/src/traits/select/mod.rs (L1863-L1887)) and during [normalization](11f32b73e0/compiler/rustc_trait_selection/src/traits/project.rs (L184-L198)). However, we currently **do not** use the `leak_check` during candidate assembly in normalizing. This can result in inconsistent behavior:
```rust
trait Trait<'a> {
type Assoc;
}
impl<'a, T> Trait<'a> for T {
type Assoc = usize;
}
fn trait_bound<T: for<'a> Trait<'a>>() {}
fn projection_bound<T: for<'a> Trait<'a, Assoc = usize>>() {}
// A function with a trivial where-bound which is more
// restrictive than the impl.
fn function<T: Trait<'static, Assoc = usize>>() {
// ok
//
// Proving `for<'a> T: Trait<'a>` using the where-bound results
// in a leak check failure, so we use the more general impl,
// causing this to succeed.
trait_bound::<T>();
// error
//
// Proving the `Projection` goal `for<'a> T: Trait<'a, Assoc = usize>`
// does not use the leak check when trying the where-bound, causing us
// to prefer it over the impl, resulting in a placeholder error.
projection_bound::<T>();
// error
//
// Trying to normalize the type `for<'a> fn(<T as Trait<'a>>::Assoc)`
// only gets to `<T as Trait<'a>>::Assoc` once `'a` has been already
// instantiated, causing us to prefer the where-bound over the impl
// resulting in a placeholder error. Even if were were to also use the
// leak check during candidate selection for normalization, this
// case would still not compile.
let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
}
```
This is also likely to be more performant. It enables more caching in the new trait solver by simply [recursively calling the canonical query][new solver] after instantiating the higher-ranked goal.
It is also unclear how to add the leak check to normalization in the new solver. To handle https://github.com/rust-lang/trait-system-refactor-initiative/issues/1 `Projection` goals are implemented via `AliasRelate`. This again means that we instantiate the binder before ever normalizing any alias. Even if we were to avoid this, we lose the ability to [cache normalization by itself, ignoring the expected `term`](5bd5d214ef/compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs (L34-L49)). We cannot replace the `term` with an inference variable before instantiating the binder, as otherwise `for<'a> T: Trait<Assoc<'a> = &'a ()>` breaks. If we only replace the term after instantiating the binder, we cannot easily evaluate the goal in a separate context, as [we'd then lose the information necessary for the leak check](11f32b73e0/compiler/rustc_next_trait_solver/src/canonicalizer.rs (L230-L232)). Adding this information to the canonical input also seems non-trivial.
## Proposed solution
I propose to instantiate the binder outside of candidate assembly, causing placeholders from higher-ranked goals to get ignored while selecting their candidate. This mostly[^1] matches the [current behavior of the new solver][new solver]. The impact of this change is therefore as follows:
```rust
trait Leak<'a> {}
impl Leak<'_> for Box<u32> {}
impl Leak<'static> for Box<u16> {}
fn impls_leak<T: for<'a> Leak<'a>>() {}
trait IndirectLeak<'a> {}
impl<'a, T: Leak<'a>> IndirectLeak<'a> for T {}
fn impls_indirect_leak<T: for<'a> IndirectLeak<'a>>() {}
fn guide_selection() {
// ok -> ambiguous
impls_leak::<Box<_>>();
// ambiguous
impls_indirect_leak::<Box<_>>();
}
trait Trait<'a> {
type Assoc;
}
impl<'a, T> Trait<'a> for T {
type Assoc = usize;
}
fn trait_bound<T: for<'a> Trait<'a>>() {}
fn projection_bound<T: for<'a> Trait<'a, Assoc = usize>>() {}
// A function which a trivial where-bound which is more
// restrictive than the impl.
fn function<T: Trait<'static, Assoc = usize>>() {
// ok -> error
trait_bound::<T>();
// error
projection_bound::<T>();
// error
let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
}
```
This does not change the behavior if candidates have higher ranked nested goals, as in this case the `leak_check` causes the nested goal to result in an error ([playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=a74c25300b23db9022226de99d8a2fa6)):
```rust
trait LeakCheckFailure<'a> {}
impl LeakCheckFailure<'static> for () {}
trait Trait<T> {}
impl Trait<u32> for () where for<'a> (): LeakCheckFailure<'a> {}
impl Trait<u16> for () {}
fn impls_trait<T: Trait<U>, U>() {}
fn main() {
// ok
//
// It does not matter whether candidate assembly
// considers the placeholders from higher-ranked goal.
//
// Either `for<'a> (): LeakCheckFailure<'a>` has no
// applicable candidate or it has a single applicable candidate
// when then later results in an error. This allows us to
// infer `U` to `u16`.
impls_trait::<(), _>()
}
```
## Impact on existing crates
This is a **breaking change**. [A crater run](https://github.com/rust-lang/rust/pull/119820#issuecomment-1926862174) found 17 regressed crates with 7 root causes.
For a full analysis of all affected crates, see https://gist.github.com/lcnr/7c1c652f30567048ea240554a36ed95c.
---
I believe this breakage to be acceptable and would merge this change. I am confident that the new position of the leak check matches our idealized future and cannot envision any other consistent alternative. Where possible, I intend to open PRs fixing/avoiding the regressions before landing this PR.
I originally intended to remove the `coherence_leak_check` lint in the same PR. However, while I am confident in the *position* of the leak check, deciding on its exact behavior is left as future work, cc #112999. This PR therefore only moves the leak check while keeping the lint when relying on it in coherence.
[new solver]: https://github.com/rust-lang/rust/blob/master/compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs#L479-L484
[^1]: the new solver has a separate cause of inconsistent behavior rn https://github.com/rust-lang/trait-system-refactor-initiative/issues/53#issuecomment-1914310171
r? `@nikomatsakis`
Assert that args are actually compatible with their generics, rather than just their count
Right now we just check that the number of args is right, rather than actually checking the kinds. Uplift a helper fn that I wrote from trait selection to do just that. Found a couple bugs along the way.
r? `@lcnr` or `@fmease` (or anyone really lol)
pattern analysis: fix union handling
Little known fact: rust supports union patterns. Exhaustiveness handles them soundly but reports nonsensical missing patterns. This PR fixes the reported patterns and documents what we're doing.
r? `@compiler-errors`
Rename `expose_addr` to `expose_provenance`
`expose_addr` is a bad name, an address is just a number and cannot be exposed. The operation is actually about the provenance of the pointer.
This PR thus changes the name of the method to `expose_provenance` without changing its return type. There is sufficient precedence for returning a useful value from an operation that does something else without the name indicating such, e.g. [`Option::insert`](https://doc.rust-lang.org/nightly/std/option/enum.Option.html#method.insert) and [`MaybeUninit::write`](https://doc.rust-lang.org/nightly/std/mem/union.MaybeUninit.html#method.write).
Returning the address is merely convenient, not a fundamental part of the operation. This is implied by the fact that integers do not have provenance since
```rust
let addr = ptr.addr();
ptr.expose_provenance();
let new = ptr::with_exposed_provenance(addr);
```
must behave exactly like
```rust
let addr = ptr.expose_provenance();
let new = ptr::with_exposed_provenance(addr);
```
as the result of `ptr.expose_provenance()` and `ptr.addr()` is the same integer. Therefore, this PR removes the `#[must_use]` annotation on the function and updates the documentation to reflect the important part.
~~An alternative name would be `expose_provenance`. I'm not at all opposed to that, but it makes a stronger implication than we might want that the provenance of the pointer returned by `ptr::with_exposed_provenance`[^1] is the same as that what was exposed, which is not yet specified as such IIUC. IMHO `expose` does not make that connection.~~
A previous version of this PR suggested `expose` as name, libs-api [decided on](https://github.com/rust-lang/rust/pull/122964#issuecomment-2033194319) `expose_provenance` to keep the symmetry with `with_exposed_provenance`.
CC `@RalfJung`
r? libs-api
[^1]: I'm using the new name for `from_exposed_addr` suggested by #122935 here.
Better reporting on generic argument mismatchs
This allows better reporting as per issue #116615 .
If you have a function:
```
fn foo(a: T, b: T) {}
```
and call it like so:
```
foo(1, 2.)
```
it'll give improved error reported similar to the following:
```
error[E0308]: mismatched types
--> generic-mismatch-reporting-issue-116615.rs:6:12
|
6 | foo(1, 2.);
| --- - ^^ expected integer, found floating-point number
| | |
| | expected argument `b` to be an integer because that argument needs to match the type of this parameter
| arguments to this function are incorrect
|
note: function defined here
--> generic-mismatch-reporting-issue-116615.rs:1:4
|
1 | fn foo<T>(a: T, b: T) {}
| ^^^ - ---- ----
| | | |
| | | this parameter needs to match the integer type of `a`
| | `b` needs to match the type of this parameter
| `a` and `b` all reference this parameter T
```
Open question, do we need to worry about error message translation into other languages? Not sure what the status of that is in Rust.
NB: Needs some checking over and some tests have altered that need sanity checking, but overall this is starting to get somewhere now. Will take out of draft PR status when this has been done, raising now to allow feedback at this stage, probably 90% ready.
Reproduce the bug from <https://github.com/rust-lang/rust/issues/123282>
that indicates this feature gate hits edition-dependent resolution paths.
Resolution changed in edition 2018, so test that as well.
Remove MIR unsafe check
Now that THIR unsafeck is enabled by default in stable I think we can remove MIR unsafeck entirely. This PR also removes safety information from MIR.
Fix capture analysis for by-move closure bodies
The check we were doing to figure out if a coroutine was borrowing from its parent coroutine-closure was flat-out wrong -- a misunderstanding of mine of the way that `tcx.closure_captures` represents its captures.
Fixes#123251 (the miri/ui test I added should more than cover that issue)
r? `@oli-obk` -- I recognize that this PR may be underdocumented, so please ask me what I should explain further.
Add `Context::ext`
This change enables `Context` to carry arbitrary extension data via a single `&mut dyn Any` field.
```rust
#![feature(context_ext)]
impl Context {
fn ext(&mut self) -> &mut dyn Any;
}
impl ContextBuilder {
fn ext(self, data: &'a mut dyn Any) -> Self;
fn from(cx: &'a mut Context<'_>) -> Self;
fn waker(self, waker: &'a Waker) -> Self;
}
```
Basic usage:
```rust
struct MyExtensionData {
executor_name: String,
}
let mut ext = MyExtensionData {
executor_name: "foo".to_string(),
};
let mut cx = ContextBuilder::from_waker(&waker).ext(&mut ext).build();
if let Some(ext) = cx.ext().downcast_mut::<MyExtensionData>() {
println!("{}", ext.executor_name);
}
```
Currently, `Context` only carries a `Waker`, but there is interest in having it carry other kinds of data. Examples include [LocalWaker](https://github.com/rust-lang/rust/issues/118959), [a reactor interface](https://github.com/rust-lang/libs-team/issues/347), and [multiple arbitrary values by type](https://docs.rs/context-rs/latest/context_rs/). There is also a general practice in the ecosystem of sharing data between executors and futures via thread-locals or globals that would arguably be better shared via `Context`, if it were possible.
The `ext` field would provide a low friction (to stabilization) solution to enable experimentation. It would enable experimenting with what kinds of data we want to carry as well as with what data structures we may want to use to carry such data.
Dedicated fields for specific kinds of data could still be added directly on `Context` when we have sufficient experience or understanding about the problem they are solving, such as with `LocalWaker`. The `ext` field would be for data for which we don't have such experience or understanding, and that could be graduated to dedicated fields once proven.
Both the provider and consumer of the extension data must be aware of the concrete type behind the `Any`. This means it is not possible for the field to carry an abstract interface. However, the field can carry a concrete type which in turn carries an interface. There are different ways one can imagine an interface-carrying concrete type to work, hence the benefit of being able to experiment with such data structures.
## Passing interfaces
Interfaces can be placed in a concrete type, such as a struct, and then that type can be casted to `Any`. However, one gotcha is `Any` cannot contain non-static references. This means one cannot simply do:
```rust
struct Extensions<'a> {
interface1: &'a mut dyn Trait1,
interface2: &'a mut dyn Trait2,
}
let mut ext = Extensions {
interface1: &mut impl1,
interface2: &mut impl2,
};
let ext: &mut dyn Any = &mut ext;
```
To work around this without boxing, unsafe code can be used to create a safe projection using accessors. For example:
```rust
pub struct Extensions {
interface1: *mut dyn Trait1,
interface2: *mut dyn Trait2,
}
impl Extensions {
pub fn new<'a>(
interface1: &'a mut (dyn Trait1 + 'static),
interface2: &'a mut (dyn Trait2 + 'static),
scratch: &'a mut MaybeUninit<Self>,
) -> &'a mut Self {
scratch.write(Self {
interface1,
interface2,
})
}
pub fn interface1(&mut self) -> &mut dyn Trait1 {
unsafe { self.interface1.as_mut().unwrap() }
}
pub fn interface2(&mut self) -> &mut dyn Trait2 {
unsafe { self.interface2.as_mut().unwrap() }
}
}
let mut scratch = MaybeUninit::uninit();
let ext: &mut Extensions = Extensions::new(&mut impl1, &mut impl2, &mut scratch);
// ext can now be casted to `&mut dyn Any` and back, and used safely
let ext: &mut dyn Any = ext;
```
## Context inheritance
Sometimes when futures poll other futures they want to provide their own `Waker` which requires creating their own `Context`. Unfortunately, polling sub-futures with a fresh `Context` means any properties on the original `Context` won't get propagated along to the sub-futures. To help with this, some additional methods are added to `ContextBuilder`.
Here's how to derive a new `Context` from another, overriding only the `Waker`:
```rust
let mut cx = ContextBuilder::from(parent_cx).waker(&new_waker).build();
```
Avoid expanding to unstable internal method
Fixes#123156
Rather than expanding to `std::rt::begin_panic`, the expansion is now to `unreachable!()`. The resulting behavior is identical. A test that previously triggered the same error as #123156 has been added to ensure it does not regress.
r? compiler
rename ptr::from_exposed_addr -> ptr::with_exposed_provenance
As discussed on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/To.20expose.20or.20not.20to.20expose/near/427757066).
The old name, `from_exposed_addr`, makes little sense as it's not the address that is exposed, it's the provenance. (`ptr.expose_addr()` stays unchanged as we haven't found a better option yet. The intended interpretation is "expose the provenance and return the address".)
The new name nicely matches `ptr::without_provenance`.
Make inductive cycles always ambiguous
This makes inductive cycles always result in ambiguity rather than be treated like a stack-dependent error.
This has some interactions with specialization, and so breaks a few UI tests that I don't agree should've ever worked in the first place, and also breaks a handful of crates in a way that I don't believe is a problem.
On the bright side, it puts us in a better spot when it comes to eventually enabling coinduction everywhere.
## Results
This was cratered in https://github.com/rust-lang/rust/pull/116494#issuecomment-2008657494, which boils down to two regressions:
* `lu_packets` - This code should have never compiled in the first place. More below.
* **ALL** other regressions are due to `commit_verify@0.11.0-beta.1` (edit: and `commit_verify@0.10.x`) - This actually seems to be fixed in version `0.11.0-beta.5`, which is the *most* up to date version, but it's still prerelease on crates.io so I don't think cargo ends up picking `beta.5` when building dependent crates.
### `lu_packets`
Firstly, this crate uses specialization, so I think it's automatically worth breaking. However, I've minimized [the regression](https://crater-reports.s3.amazonaws.com/pr-116494-3/try%23d614ed876e31a5f3ad1d0fbf848fcdab3a29d1d8/gh/lcdr.lu_packets/log.txt) to:
```rust
// Upstream crate
pub trait Serialize {}
impl Serialize for &() {}
impl<S> Serialize for &[S] where for<'a> &'a S: Serialize {}
// ----------------------------------------------------------------------- //
// Downstream crate
#![feature(specialization)]
#![allow(incomplete_features, unused)]
use upstream::Serialize;
trait Replica {
fn serialize();
}
impl<T> Replica for T {
default fn serialize() {}
}
impl<T> Replica for Option<T>
where
for<'a> &'a T: Serialize,
{
fn serialize() {}
}
```
Specifically this fails when computing the specialization graph for the `downstream` crate.
The code ends up cycling on `&[?0]: Serialize` when we equate `&?0 = &[?1]` during impl matching, which ends up needing to prove `&[?1]: Serialize`, which since cycles are treated like ambiguity, ends up in a **fatal overflow**. For some reason this requires two crates, squashing them into one crate doesn't work.
Side-note: This code is subtly order dependent. When minimizing, I ended up having the code start failing on `nightly` very easily after removing and reordering impls. This seems to me all the more reason to remove this behavior altogether.
## Side-note: Item Bounds (edit: this was fixed independently in #121123)
Due to the changes in #120584 where we now consider an alias's item bounds *and* all the item bounds of the alias's nested self type aliases, I've had to add e6b64c6194 which is a hack to make sure we're not eagerly normalizing bounds that have nothing to do with the predicate we're trying to solve, and which result in.
This is fixed in a more principled way in #121123.
---
r? lcnr for an initial review
CFI: Support non-general coroutines
Previously, we assumed all `ty::Coroutine` were general coroutines and attempted to generalize them through the `Coroutine` trait. Select appropriate traits for each kind of coroutine.
I have this marked as a draft because it currently only fixes async coroutines, and I think it make sense to try to fix gen/async gen coroutines before this is merged.
If the issue [mentioned](https://github.com/rust-lang/rust/pull/123106#issuecomment-2030794213) in the original PR is actually affecting someone, we can land this as is to remedy it.
Check that nested statics in thread locals are duplicated per thread.
follow-up to #123310
cc ``@compiler-errors`` ``@RalfJung``
fwiw: I have no idea how thread local statics make that work under LLVM, and miri fails on this example, which I would have expected to be the correct behaviour.
Since the `#[thread_local]` attribute is just an internal implementation detail, I'm just going to start hard erroring on nested mutable statics in thread locals.
Make sure to insert `Sized` bound first into clauses list
#120323 made it so that we don't insert an implicit `Sized` bound whenever we see an *explicit* `Sized` bound. However, since the code that inserts implicit sized bounds puts the bound as the *first* in the list, that means that it had the **side-effect** of possibly meaning we check `Sized` *after* checking other trait bounds.
If those trait bounds result in ambiguity or overflow or something, it may change how we winnow candidates. (**edit: SEE** #123303) This is likely the cause for the regression in https://github.com/rust-lang/rust/issues/123279#issuecomment-2028899598, since the impl...
```rust
impl<T: Job + Sized> AsJob for T { // <----- changing this to `Sized + Job` or just `Job` (which turns into `Sized + Job`) will FIX the issue.
}
```
...looks incredibly suspicious.
Fixes [after beta-backport] #123279.
Alternative is to revert #120323. I don't have a strong opinion about this, but think it may be nice to keep the diagnostic changes around.
De-LLVM the unchecked shifts [MCP#693]
This is just one part of the MCP (https://github.com/rust-lang/compiler-team/issues/693), but it's the one that IMHO removes the most noise from the standard library code.
Seems net simpler this way, since MIR already supported heterogeneous shifts anyway, and thus it's not more work for backends than before.
r? WaffleLapkin
Adds support for both CFI and KCFI for attaching concrete and abstract
types to functions. KCFI does this through generation of `ReifyShim` on
any function pointer that could go in a vtable, and checking the
`ReifyReason` when emitting the instance. CFI does this by attaching
both the concrete and abstract type to every instance.
TypeID codegen tests are switched to be anchored on the left rather than
the right in order to allow emission of additional type attachments.
Fixes#115953
Previously, we assumed all `ty::Coroutine` were general coroutines and
attempted to generalize them through the `Coroutine` trait. Select
appropriate traits for each kind of coroutine.
Refactor stack overflow handling
Currently, every platform must implement a `Guard` that protects a thread from stack overflow. However, UNIX is the only platform that actually does so. Windows has a different mechanism for detecting stack overflow, while the other platforms don't detect it at all. Also, the UNIX stack overflow handling is split between `sys::pal::unix::stack_overflow`, which implements the signal handler, and `sys::pal::unix::thread`, which detects/installs guard pages.
This PR cleans this by getting rid of `Guard` and unifying UNIX stack overflow handling inside `stack_overflow` (commit 1). Therefore we can get rid of `sys_common::thread_info`, which stores `Guard` and the current `Thread` handle and move the `thread::current` TLS variable into `thread` (commit 2).
The second commit is not strictly speaking necessary. To keep the implementation clean, I've included it here, but if it causes too much noise, I can split it out without any trouble.
Don't inherit codegen attrs from parent static
Putting this up partly for discussion and partly for review. Specifically, in #121644, `@oli-obk` designed a system that creates new static items for representing nested allocations in statics. However, in that PR, oli made it so that these statics inherited the codegen attrs from the parent.
This causes problems such as colliding symbols with `#[export_name]` and ICEs with `#[no_mangle]` since these synthetic statics have no `tcx.item_name(..)`.
So the question is, is there any case where we *do* want to inherit codegen attrs from the parent? The only one that seems a bit suspicious is the thread-local attribute. And there may be some interesting interactions with the coverage attributes as well...
Fixes (after backport) #123274. Fixes#123243. cc #121644.
r? `@oli-obk` cc `@nnethercote` `@RalfJung` (reviewers on that pr)
Use the `Align` type when parsing alignment attributes
Use the `Align` type in `rustc_attr::parse_alignment`, removing the need to call `Align::from_bytes(...).unwrap()` later in the compilation process.
The original proposal allows reference patterns
with "compatible" mutability, however it's not clear
what that means so for now we require an exact match.
I don't know the type system code well, so if something
seems to not make sense it's probably because I made a
mistake
CFI: Abstract Closures and Coroutines
This will abstract coroutines in a moment, it's just abstracting closures for now to show `@rcvalle`
This uses the same principal as the methods on traits - figure out the `dyn` type representing the fn trait, instantiate it, and attach that alias set. We're essentially just computing how we would be called in a dynamic context, and attaching that.
Similar to methods on a trait object, the most common way to indirectly
call a closure or coroutine is through the vtable on the appropriate
trait. This uses the same approach as we use for trait methods, after
backing out the trait arguments from the type.
Add support for `NonNull`s in the `ambiguous_wide_ptr_comparisions` lint
This PR add support for `NonNull` pointers in the `ambiguous_wide_ptr_comparisions` lint.
Fixes https://github.com/rust-lang/rust/issues/121264
r? `@Nadrieril` (since you just reviewed #121268, feel free to reassign)
KCFI: Require -C panic=abort
While the KCFI scheme is not incompatible with unwinding, LLVM's `invoke` instruction does not currently support KCFI bundles. While it likely will in the near future, we won't be able to assume that in Rust for a while.
We encountered this problem while [turning on closure support](https://github.com/rust-lang/rust/pull/123106#issuecomment-2027436640).
r? ``@workingjubilee``
Replace regions in const canonical vars' types with `'static` in next-solver canonicalizer
We shouldn't ever have non-static regions in consts on stable (or really any regions at all, lol).
The test I committed is less minimal than, e.g., https://github.com/rust-lang/rust/issues/123155?notification_referrer_id=NT_kwDOADgQyrMxMDAzNDU4MDI0OTozNjc0MzE0#issuecomment-2025472029 -- however, I believe that it actually portrays the underlying issue here a bit better than that one.
In the linked issue, we end up emitting a normalizes-to predicate for a const placeholder because we don't actually unify `false` and `""`. In the test I committed, we emit a normalizes-to predicate as a part of actually solving a negative coherence goal.
Fixes#123155Fixes#118783
r? lcnr
This is just one part of the MCP, but it's the one that IMHO removes the most noise from the standard library code.
Seems net simpler this way, since MIR already supported heterogeneous shifts anyway, and thus it's not more work for backends than before.
CFI: Support calling methods on supertraits
Automatically adjust `Virtual` calls to supertrait functions to use the supertrait's trait object type as the receiver rather than the child trait.
cc `@compiler-errors` - this is the next usage of `trait_object_ty` I intend to have, so I thought it might be relevant while reviewing the existing one.
Stabilize `unchecked_{add,sub,mul}`
Tracking issue: #85122
I think we might as well just stabilize these basic three. They're the ones that have `nuw`/`nsw` flags in LLVM.
Notably, this doesn't include the potentially-more-complex or -more-situational things like `unchecked_neg` or `unchecked_shr` that are under different feature flags.
To quote Ralf https://github.com/rust-lang/rust/issues/85122#issuecomment-1681669646,
> Are there any objections to stabilizing at least `unchecked_{add,sub,mul}`? For those there shouldn't be any surprises about what their safety requirements are.
*Semantially* these are [already available on stable, even in `const`, via](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=bdb1ff889b61950897f1e9f56d0c9a36) `checked_*`+`unreachable_unchecked`. So IMHO we might as well just let people write them directly, rather than try to go through a `let Some(x) = x.checked_add(y) else { unsafe { hint::unreachable_unchecked() }};` dance.
I added additional text to each method to attempt to better describe the behaviour and encourage `wrapping_*` instead.
r? rust-lang/libs-api
Add detection of [Partial]Ord methods in the `ambiguous_wide_pointer_comparisons` lint
Partially addresses https://github.com/rust-lang/rust/issues/121264 by adding diagnostics items for PartialOrd and Ord methods, detecting such diagnostics items as "binary operation" and suggesting the correct replacement.
I also took the opportunity to change the suggestion to use new methods `.cast()` on `*mut T` an d `*const T`.
While the KCFI scheme is not incompatible with unwinding, LLVM's
`invoke` instruction does not currently support KCFI bundles. While it
likely will in the near future, we won't be able to assume that in Rust
for a while.
Match ergonomics 2024: implement mutable by-reference bindings
Implements the mutable by-reference bindings portion of match ergonomics 2024 (#123076), with the `mut ref`/`mut ref mut` syntax, under feature gate `mut_ref`.
r? `@Nadrieril`
`@rustbot` label A-patterns A-edition-2024
CFI: Fix methods as function pointer cast
Fix casting between methods and function pointers by assigning a secondary type id to methods with their concrete self so they can be used as function pointers.
This was split off from #116404.
cc `@compiler-errors` `@workingjubilee`
Don't check match scrutinee of postfix match for unused parens
We only check the scrutinees of block-like constructs and a few others (return/index/assign/method calls). Just don't do it for postfix match at all.
Fixes#123064
r? fmease
Suggest correct path in include_bytes!
`include_bytes!` paths are relative, and I'm often not sure how nested is the `.rs` file that I'm editing, so I have to guess the number of `"../.."`. This change searches `..` and `../..` for the given file and offers corrected path as a suggestion.
I wasn't sure how to get the right span, and how to properly escape it.
```text
error: couldn't read src/file.txt: No such file or directory (os error 2)
--> src/main.rs:2:13
|
2 | let x = include_bytes!("file.txt");
| ^^^^^^^^^^^^^^^----------^
| |
| help: it's in a parent directory: `"../../file.txt"`
```
Fix casting between methods and function pointers by assigning a
secondary type id to methods with their concrete self so they can be
used as function pointers.
Delegation: fix ICE on wrong `Self` instantiation
fixes https://github.com/rust-lang/rust/issues/119921
fixes https://github.com/rust-lang/rust/issues/119919
There is no way to instantiate `Self` param for caller in delegation item if
1. callee is a trait method && callee contains `Self` param
2. delegation item isn't an associative item
In general, we can consider `Self` param as independent type param in these cases:
```rust
trait Trait {
fn foo(_: Option<&Self>) {...}
}
reuse Trait::foo;
// will be desugared to:
fn foo<T: Trait>(x: Option<&T>) { Trait::foo(x) }
```
But this requires early bound parameters support. For now, I suggest banning such cases to avoid ICE's.
r? ``@petrochenkov``
CFI: Fix drop and drop_in_place
Fix drop and drop_in_place by transforming self of drop and drop_in_place methods into a Drop trait objects.
This was split off from https://github.com/rust-lang/rust/pull/116404.
cc `@compiler-errors` `@workingjubilee`
CFI: Enable KCFI testing of run-pass tests
This enables KCFI-based testing for all the CFI run-pass tests in the suite today. We can add the test header on top of in-flight CFI tests once they land. This is becoming more important as we get closer to leveraging CFI's multiple type attachment feature, as that is where the implementations will have a divergence.
It also enables KCFI as a sanitizer for x86_64 and aarch64 Linux to make this possible. The sanitizer should likely be available for all aarch64, x86_64, and riscv targets, but that isn't critical for initial testing.
Soft-destabilize `RustcEncodable` & `RustcDecodable`, remove from prelude in next edition
cc rust-lang/libs-team#272
Any use of `RustcEncodable` and `RustcDecodable` now triggers a deny-by-default lint. The derives have been removed from the 2024 prelude. I specifically chose **not** to document this in the module-level documentation, as the presence in existing preludes is not documented (which I presume is intentional).
This does not implement the proposed change for `rustfix`, which I will be looking into shortly.
With regard to the items in the preludes being stable, this should not be an issue because #15702 has been resolved.
r? libs-api
Rollup of 9 pull requests
Successful merges:
- #108675 (Document `adt_const_params` feature in Unstable Book)
- #122120 (Suggest associated type bounds on problematic associated equality bounds)
- #122589 (Fix diagnostics for async block cloning)
- #122835 (Require `DerefMut` and `DerefPure` on `deref!()` patterns when appropriate)
- #123049 (In `ConstructCoroutineInClosureShim`, pass receiver by mut ref, not mut pointer)
- #123055 (enable cargo miri test doctests)
- #123057 (unix fs: Make hurd using explicit new rather than From)
- #123087 (Change `f16` and `f128` clippy stubs to be nonpanicking)
- #123103 (Rename `Inherited` -> `TypeckRootCtxt`)
r? `@ghost`
`@rustbot` modify labels: rollup
Suggest associated type bounds on problematic associated equality bounds
Fixes#105056. TL;DR: Suggest `Trait<Ty: Bound>` on `Trait<Ty = Bound>` in Rust >=2021.
~~Blocked on #122055 (stabilization of `associated_type_bounds`), I'd say.~~ (merged)
Delegation: fix ICE on wrong `self` resolution
fixes https://github.com/rust-lang/rust/issues/122874
Delegation item should be wrapped in a `rib` to behave like a regular function during name resolution.
r? `@petrochenkov`
CFI: (actually) check that methods are object-safe before projecting their receivers to `dyn Trait` in CFI
`trait_object_ty` assumed that associated types would be fully determined by the trait. This is *almost* true - const parameters and type parameters are no longer allowed, but lifetime parameters are. Since we erase all lifetime parameters anyways, instantiate it with as many erased regions as it needs.
Fixes: #123053
r? `@compiler-errors`
Avoid some unnecessary query invocations.
Specifically this inlines `const_eval_poly` and avoids computing the generic params, the param env, normalizing the param env and erasing lifetimes on everything.
should fix the perf regression from https://github.com/rust-lang/rust/pull/121087
This enables KCFI-based testing for all the CFI run-pass tests in the
suite today. We can add the test header on top of in-flight CFI tests
once they land.
It also enables KCFI as a sanitizer for x86_64 and aarch64 Linux to make
this possible. The sanitizer should likely be available for all aarch64,
x86_64, and riscv targets, but that isn't critical for initial testing.
`trait_object_ty` assumed that associated types would be fully
determined by the trait. This is *almost* true - const parameters and
type parameters are no longer allowed, but lifetime parameters are.
Since we erase all lifetime parameters anyways, instantiate it with as
many erased regions as it needs.
Fixes: #123053
In `pretty_print_type()`, print `async fn` futures' paths instead of spans.
This makes `-Zprint-type-sizes`'s output easier to read, because the name of an `async fn` is more immediately recognizable than its span. This change will also synergize with my other `-Zprint-type-sizes` PR #122922 which prints the type of child futures being awaited.
I also deleted the comment "FIXME(eddyb) should use `def_span`." because it appears to have already been fixed by commit 67727aa7c3.
