| Age | Commit message (Collapse) | Author | Lines |
|---|
|
|
|
coverage: Defer part of counter-creation until codegen
Follow-up to #135481 and #135873.
One of the pleasant properties of the new counter-assignment algorithm is that we can stop partway through the process, store the intermediate state in MIR, and then resume the rest of the algorithm during codegen. This lets it take into account which parts of the control-flow graph were eliminated by MIR opts, resulting in fewer physical counters and simpler counter expressions.
Those improvements end up completely obsoleting much larger chunks of code that were previously responsible for cleaning up the coverage metadata after MIR opts, while also doing a more thorough cleanup job.
(That change also unlocks some further simplifications that I've kept out of this PR to limit its scope.)
|
|
|
|
|
|
|
|
|
|
|
|
Fix deduplication mismatches in vtables leading to upcasting unsoundness
We currently have two cases where subtleties in supertraits can trigger disagreements in the vtable layout, e.g. leading to a different vtable layout being accessed at a callsite compared to what was prepared during unsizing. Namely:
### #135315
In this example, we were not normalizing supertraits when preparing vtables. In the example,
```
trait Supertrait<T> {
fn _print_numbers(&self, mem: &[usize; 100]) {
println!("{mem:?}");
}
}
impl<T> Supertrait<T> for () {}
trait Identity {
type Selff;
}
impl<Selff> Identity for Selff {
type Selff = Selff;
}
trait Middle<T>: Supertrait<()> + Supertrait<T> {
fn say_hello(&self, _: &usize) {
println!("Hello!");
}
}
impl<T> Middle<T> for () {}
trait Trait: Middle<<() as Identity>::Selff> {}
impl Trait for () {}
fn main() {
(&() as &dyn Trait as &dyn Middle<()>).say_hello(&0);
}
```
When we prepare `dyn Trait`, we see a supertrait of `Middle<<() as Identity>::Selff>`, which itself has two supertraits `Supertrait<()>` and `Supertrait<<() as Identity>::Selff>`. These two supertraits are identical, but they are not duplicated because we were using structural equality and *not* considering normalization. This leads to a vtable layout with two trait pointers.
When we upcast to `dyn Middle<()>`, those two supertraits are now the same, leading to a vtable layout with only one trait pointer. This leads to an offset error, and we call the wrong method.
### #135316
This one is a bit more interesting, and is the bulk of the changes in this PR. It's a bit similar, except it uses binder equality instead of normalization to make the compiler get confused about two vtable layouts. In the example,
```
trait Supertrait<T> {
fn _print_numbers(&self, mem: &[usize; 100]) {
println!("{mem:?}");
}
}
impl<T> Supertrait<T> for () {}
trait Trait<T, U>: Supertrait<T> + Supertrait<U> {
fn say_hello(&self, _: &usize) {
println!("Hello!");
}
}
impl<T, U> Trait<T, U> for () {}
fn main() {
(&() as &'static dyn for<'a> Trait<&'static (), &'a ()>
as &'static dyn Trait<&'static (), &'static ()>)
.say_hello(&0);
}
```
When we prepare the vtable for `dyn for<'a> Trait<&'static (), &'a ()>`, we currently consider the PolyTraitRef of the vtable as the key for a supertrait. This leads two two supertraits -- `Supertrait<&'static ()>` and `for<'a> Supertrait<&'a ()>`.
However, we can upcast[^up] without offsetting the vtable from `dyn for<'a> Trait<&'static (), &'a ()>` to `dyn Trait<&'static (), &'static ()>`. This is just instantiating the principal trait ref for a specific `'a = 'static`. However, when considering those supertraits, we now have only one distinct supertrait -- `Supertrait<&'static ()>` (which is deduplicated since there are two supertraits with the same substitutions). This leads to similar offsetting issues, leading to the wrong method being called.
[^up]: I say upcast but this is a cast that is allowed on stable, since it's not changing the vtable at all, just instantiating the binder of the principal trait ref for some lifetime.
The solution here is to recognize that a vtable isn't really meaningfully higher ranked, and to just treat a vtable as corresponding to a `TraitRef` so we can do this deduplication more faithfully. That is to say, the vtable for `dyn for<'a> Tr<'a>` and `dyn Tr<'x>` are always identical, since they both would correspond to a set of free regions on an impl... Do note that `Tr<for<'a> fn(&'a ())>` and `Tr<fn(&'static ())>` are still distinct.
----
There's a bit more that can be cleaned up. In codegen, we can stop using `PolyExistentialTraitRef` basically everywhere. We can also fix SMIR to stop storing `PolyExistentialTraitRef` in its vtable allocations.
As for testing, it's difficult to actually turn this into something that can be tested with `rustc_dump_vtable`, since having multiple supertraits that are identical is a recipe for ambiguity errors. Maybe someone else is more creative with getting that attr to work, since the tests I added being run-pass tests is a bit unsatisfying. Miri also doesn't help here, since it doesn't really generate vtables that are offset by an index in the same way as codegen.
r? `@lcnr` for the vibe check? Or reassign, idk. Maybe let's talk about whether this makes sense.
<sup>(I guess an alternative would also be to not do any deduplication of vtable supertraits (or only a really conservative subset) rather than trying to normalize and deduplicate more faithfully here. Not sure if that works and is sufficient tho.)</sup>
cc `@steffahn` -- ty for the minimizations
cc `@WaffleLapkin` -- since you're overseeing the feature stabilization :3
Fixes #135315
Fixes #135316
|
|
Co-authored-by: FedericoBruzzone <federico.bruzzone.i@gmail.com>
|
|
|
|
|
|
|
|
This reflects the fact that we can't compute meaningful info for a function
that wasn't instrumented and therefore doesn't have `function_coverage_info`.
|
|
|
|
|
|
Exclude dependencies of `std` for diagnostics
Currently crates in the sysroot can show up in diagnostic suggestions, such as in https://github.com/rust-lang/rust/issues/135232. To prevent this, duplicate `all_traits` into `visible_traits` which only shows traits in non-private crates.
Setting `#![feature(rustc_private)]` overrides this and makes items in private crates visible as well, since `rustc_private` enables use of `std`'s private dependencies.
This may be reviewed per-commit.
Fixes: https://github.com/rust-lang/rust/issues/135232
|
|
Add an alternative to `tcx.all_traits()` that only shows traits that the
user might be able to use, for diagnostic purposes. With this available,
make use of it for diagnostics including associated type errors, which
is part of the problem with [1].
Includes a few comment updates for related API.
[1]: https://github.com/rust-lang/rust/issues/135232
|
|
Depth limit const eval query
Currently the const-eval query doesn't have a recursion limit or timeout, causing the complier to freeze in an infinite loop, see #125718. This PR depth limits the `eval_to_const_value_raw` query (with the [`recursion_limit`](https://doc.rust-lang.org/reference/attributes/limits.html) attribute) and improves the diagnostics for query overflow errors, so spans are reported for other dep kinds than `layout_of` (e.g. `eval_to_const_value_raw`).
fixes #125718
fixes #114192
|
|
This should make it clearer that this bitset is dense, with the
advantages and disadvantages that it entails.
|
|
|
|
|
|
Add a notion of "some ABIs require certain target features"
I think I finally found the right shape for the data and checks that I recently added in https://github.com/rust-lang/rust/pull/133099, https://github.com/rust-lang/rust/pull/133417, https://github.com/rust-lang/rust/pull/134337: we have a notion of "this ABI requires the following list of target features, and it is incompatible with the following list of target features". Both `-Ctarget-feature` and `#[target_feature]` are updated to ensure we follow the rules of the ABI. This removes all the "toggleability" stuff introduced before, though we do keep the notion of a fully "forbidden" target feature -- this is needed to deal with target features that are actual ABI switches, and hence are needed to even compute the list of required target features.
We always explicitly (un)set all required and in-conflict features, just to avoid potential trouble caused by the default features of whatever the base CPU is. We do this *before* applying `-Ctarget-feature` to maintain backward compatibility; this poses a slight risk of missing some implicit feature dependencies in LLVM but has the advantage of not breaking users that deliberately toggle ABI-relevant target features. They get a warning but the feature does get toggled the way they requested.
For now, our logic supports x86, ARM, and RISC-V (just like the previous logic did). Unsurprisingly, RISC-V is the nicest. ;)
As a side-effect this also (unstably) allows *enabling* `x87` when that is harmless. I used the opportunity to mark SSE2 as required on x86-64, to better match the actual logic in LLVM and because all x86-64 chips do have SSE2. This infrastructure also prepares us for requiring SSE on x86-32 when we want to use that for our ABI (and for float semantics sanity), see https://github.com/rust-lang/rust/issues/133611, but no such change is happening in this PR.
r? `@workingjubilee`
|
|
Project to `TyKind::Error` when there are unconstrained non-lifetime (ty/const) impl params
It splits the `enforce_impl_params_are_constrained` function into lifetime/non-lifetime, and queryfies the latter. We can then use the result of the latter query (`Result<(), ErrorGuaranteed>`) to intercept projection and constrain the projected type to `TyKind::Error`, which ensures that we leak no ty or const vars to places that don't expect them, like `normalize_erasing_regions`.
The reason we split `enforce_impl_params_are_constrained` into two parts is because we only error for *lifetimes* if the lifetime ends up showing up in any of the associated types of the impl (e.g. we allow `impl<'a> Foo { type Assoc = (); }`). However, in order to compute the `type_of` query for the anonymous associated type of an RPITIT, we need to do trait solving (in `query collect_return_position_impl_trait_in_trait_tys`). That would induce cycles. Luckily, it turns out for lifetimes we don't even care about if they're unconstrained, since they're erased in all contexts that we are trying to fix ICEs. So it's sufficient to keep this check separated out of the query.
I think this is a bit less invasive of an approach compared to #127973. The major difference between this PR and that PR is that we queryify the check instead of merging it into the `explicit_predicates_of` query, and we use the result to taint just projection goals, rather than trait goals too. This doesn't require a lot of new tracking in `ItemCtxt` and `GenericPredicates`, and it also seems to not require any other changes to typeck like that PR did.
Fixes #123141
Fixes #125874
Fixes #126942
Fixes #127804
Fixes #130967
r? oli-obk
|
|
|
|
|
|
the ABI target feature check
|
|
const eval
|
|
`rustc_span::symbol` defines some things that are re-exported from
`rustc_span`, such as `Symbol` and `sym`. But it doesn't re-export some
closely related things such as `Ident` and `kw`. So you can do `use
rustc_span::{Symbol, sym}` but you have to do `use
rustc_span::symbol::{Ident, kw}`, which is inconsistent for no good
reason.
This commit re-exports `Ident`, `kw`, and `MacroRulesNormalizedIdent`,
and changes many `rustc_span::symbol::` qualifiers in `compiler/` to
`rustc_span::`. This is a 200+ net line of code reduction, mostly
because many files with two `use rustc_span` items can be reduced to
one.
|
|
|
|
|
|
forbid toggling x87 and fpregs on hard-float targets
Part of https://github.com/rust-lang/rust/issues/116344, follow-up to https://github.com/rust-lang/rust/pull/129884:
The `x87` target feature on x86 and the `fpregs` target feature on ARM must not be disabled on a hardfloat target, as that would change the float ABI. However, *enabling* `fpregs` on ARM is [explicitly requested](https://github.com/rust-lang/rust/issues/130988) as it seems to be useful. Therefore, we need to refine the distinction of "forbidden" target features and "allowed" target features: all (un)stable target features can determine on a per-target basis whether they should be allowed to be toggled or not. `fpregs` then checks whether the current target has the `soft-float` feature, and if yes, `fpregs` is permitted -- otherwise, it is not. (Same for `x87` on x86).
Also fixes https://github.com/rust-lang/rust/issues/132351. Since `fpregs` and `x87` can be enabled on some builds and disabled on others, it would make sense that one can query it via `cfg`. Therefore, I made them behave in `cfg` like any other unstable target feature.
The first commit prepares the infrastructure, but does not change behavior. The second commit then wires up `fpregs` and `x87` with that new infrastructure.
r? `@workingjubilee`
|
|
|
|
be invalid to toggle
Also rename some things for extra clarity
|
|
constness
|
|
coverage: Use a query to find counters/expressions that must be zero
As of #133446, this query (`coverage_ids_info`) determines which counter/expression IDs are unused. So with only a little extra work, we can take the code that was using that information to determine which coverage counters/expressions must be zero, and move that inside the query as well.
There should be no change in compiler output.
|
|
A bunch of cleanups
These are all extracted from a branch I have to get rid of driver queries. Most of the commits are not directly necessary for this, but were found in the process of implementing the removal of driver queries.
Previous PR: https://github.com/rust-lang/rust/pull/132410
|
|
|
|
|
|
It was inconsistently done (sometimes even within a single function) and
most of the rest of the compiler uses fatal errors instead, which need
to be caught using catch_with_exit_code anyway. Using fatal errors
instead of ErrorGuaranteed everywhere in the driver simplifies things a
bit.
|
|
implement checks for tail calls
Quoting the [RFC draft](https://github.com/phi-go/rfcs/blob/guaranteed-tco/text/0000-explicit-tail-calls.md):
> The argument to become is a function (or method) call, that exactly matches the function signature and calling convention of the callee. The intent is to ensure a matching ABI. Note that lifetimes may differ as long as they pass borrow checking, see [below](https://github.com/phi-go/rfcs/blob/guaranteed-tco/text/0000-explicit-tail-calls.md#return-type-coercion) for specifics on the return type.
> Tail calling closures and tail calling from closures is not allowed. This is due to the high implementation effort, see below, this restriction can be lifted by a future RFC.
> Invocations of operators were considered as valid targets but were rejected on grounds of being too error-prone. In any case, these can still be called as methods.
> Tail calling [variadic functions](https://doc.rust-lang.org/beta/unstable-book/language-features/c-variadic.html) and tail calling from variadic functions is not allowed. As support for variadic function is stabilized on a per target level, support for tail-calls regarding variadic functions would need to follow a similar approach. To avoid this complexity and to minimize implementation effort for backends, this interaction is currently not allowed but support can be added with a future RFC.
-----
The checks are implemented as a query, similarly to `check_unsafety`.
The code is cherry-picked straight out of #112657 which was written more than a year ago, so I expect we might need to change some things ^^"
|
|
improve TagEncoding::Niche docs, sanity check, and UB checks
Turns out the `niche_variants` range can actually contain the `untagged_variant`. We should report this as UB in Miri, so this PR implements that.
Also rename `partially_check_layout` to `layout_sanity_check` for better consistency with how similar functions are called in other parts of the compiler.
Turns out my adjustments to the transmutation logic also fix https://github.com/rust-lang/rust/issues/126267.
|
|
Get rid of HIR const checker
As far as I can tell, the HIR const checker was implemented in https://github.com/rust-lang/rust/pull/66170 because we were not able to issue useful const error messages in the MIR const checker.
This seems to have changed in the last 5 years, probably due to work like #90532. I've tweaked the diagnostics slightly and think the error messages have gotten *better* in fact.
Thus I think the HIR const checker has reached the end of its usefulness, and we can retire it.
cc `@RalfJung`
|
|
|
|
|
|
this implements checks necessary to guarantee that we can actually
perform a tail call. while extremely restrictive, this is what is
documented in the RFC, and all these checks are needed for one reason or
another.
|
|
|
|
|
|
|
|
|
|
|
