Rollup of 8 pull requests
Successful merges:
- #116905 (refactor(compiler/resolve): simplify some code)
- #117095 (Add way to differentiate argument locals from other locals in Stable MIR)
- #117143 (Avoid unbounded O(n^2) when parsing nested type args)
- #117194 (Minor improvements to `rustc_incremental`)
- #117202 (Revert "Remove TaKO8Ki from reviewers")
- #117207 (The value of `-Cinstrument-coverage=` doesn't need to be `Option`)
- #117214 (Quietly fail if an error has already occurred)
- #117221 (Rename type flag `HAS_TY_GENERATOR` to `HAS_TY_COROUTINE`)
r? `@ghost`
`@rustbot` modify labels: rollup
The value of `-Cinstrument-coverage=` doesn't need to be `Option`
(Extracted from #117199, since this is a purely internal cleanup that can land independently.)
Not using this flag is identical to passing `-Cinstrument-coverage=off`, so there's no need to distinguish between `None` and `Some(Off)`.
Revert "Remove TaKO8Ki from reviewers"
ref #116061
It's been a month since this pull request, and I now have some available time for reviews. Would it be okay to revisit it as a reviewer?
This reverts commit 8e06b25e39.
r? `@Nilstrieb`
Add way to differentiate argument locals from other locals in Stable MIR
This PR resolvesrust-lang/project-stable-mir#47 which request a way to differentiate argument locals in a SMIR `Body` from other locals.
Specifically, this PR exposes the `arg_count` field from the MIR `Body`. However, I'm opening this as a draft PR because I think there are a few outstanding questions on how this information should be exposed and described. Namely:
- Is exposing `arg_count` the best way to surface this information to SMIR users? Would it be better to leave `arg_count` as a private field and add public methods (e.g. `fn arguments(&self) -> Iter<'_, LocalDecls>`) that may use the underlying `arg_count` info from the MIR body, but expose this information to users in a more convenient form? Or is it best to stick close to the current MIR convention?
- If the answer to the above point is to stick with the current MIR convention (`arg_count`), is it reasonable to also commit to sticking to the current MIR convention that the first local is always the return local, while the next `arg_count` locals are always the (in-order) argument locals?
- Should `Body` in SMIR only represent function bodies (as implied by the comment I added)? That seems to be the current case in MIR, but should this restriction always be the case for SMIR?
r? `@celinval`
r? `@oli-obk`
Never consider raw pointer casts to be trival
HIR typeck tries to figure out which casts are trivial by doing them as
coercions and seeing whether this works. Since HIR typeck is oblivious
of lifetimes, this doesn't work for pointer casts that only change the
lifetime of the pointee, which are, as borrowck will tell you, not
trivial.
This change makes it so that raw pointer casts are never considered
trivial.
This also incidentally fixes the "trivial cast" lint false positive on
the same code. Unfortunately, "trivial cast" lints are now never emitted
on raw pointer casts, even if they truly are trivial. This could be
fixed by also doing the lint in borrowck for raw pointers specifically.
fixes#113257
Rework negative coherence to properly consider impls that only partly overlap
This PR implements a modified negative coherence that handles impls that only have partial overlap.
It does this by:
1. taking both impl trait refs, instantiating them with infer vars
2. equating both trait refs
3. taking the equated trait ref (which represents the two impls' intersection), and resolving any vars
4. plugging all remaining infer vars with placeholder types
these placeholder-plugged trait refs can then be used normally with the new trait solver, since we no longer have to worry about the issue with infer vars in param-envs.
We use the **new trait solver** to reason correctly about unnormalized trait refs (due to deferred projection equality), since this avoid having to normalize anything under param-envs with infer vars in them.
This PR then additionally:
* removes the `FnPtr` knowable hack by implementing proper negative `FnPtr` trait bounds for rigid types.
---
An example:
Consider these two partially overlapping impls:
```
impl<T, U> PartialEq<&U> for &T where T: PartialEq<U> {}
impl<F> PartialEq<F> for F where F: FnPtr {}
```
Under the old algorithm, we would take one of these impls and replace it with infer vars, then try unifying it with the other impl under identity substitutions. This is not possible in either direction, since it either sets `T = U`, or tries to equate `F = &?0`.
Under the new algorithm, we try to unify `?0: PartialEq<?0>` with `&?1: PartialEq<&?2>`. This gives us `?0 = &?1 = &?2` and thus `?1 = ?2`. The intersection of these two trait refs therefore looks like: `&?1: PartialEq<&?1>`. After plugging this with placeholders, we get a trait ref that looks like `&!0: PartialEq<&!0>`, with the first impl having substs `?T = ?U = !0` and the second having substs `?F = &!0`[^1].
Then we can take the param-env from the first impl, and try to prove the negated where clause of the second.
We know that `&!0: !FnPtr` never holds, since it's a rigid type that is also not a fn ptr, we successfully detect that these impls may never overlap.
[^1]: For the purposes of this example, I just ignored lifetimes, since it doesn't really matter.
Prepare the `bootstrap` tool for the new check-cfg syntax
This PR prepare the `bootstrap` tool for the [new check-cfg syntax](https://github.com/rust-lang/rust/pull/111072) as well as the according [changes to Cargo](https://github.com/rust-lang/cargo/pull/12845).
~~Note that while the new syntax can technically available on stage > 2, we actually cannot use it since we need a cargo version that supports the new syntax which won't happen until the next beta bump (if I understand everything correctly).~~
r? bootstrap
Store #[stable] attribute's `since` value in structured form
Followup to https://github.com/rust-lang/rust/pull/116773#pullrequestreview-1680913901.
Prior to this PR, if you wrote an improper `since` version in a `stable` attribute, such as `#[stable(feature = "foo", since = "wat.0")]`, rustc would emit a diagnostic saying **_'since' must be a Rust version number, such as "1.31.0"_** and then throw out the whole `stable` attribute as if it weren't there. This strategy had 2 problems, both fixed in this PR:
1. If there was also a `#[deprecated]` attribute on the same item, rustc would want to enforce that the stabilization version is older than the deprecation version. This involved reparsing the `stable` attribute's `since` version, with a diagnostic **_invalid stability version found_** if it failed to parse. Of course this diagnostic was unreachable because an invalid `since` version would have already caused the `stable` attribute to be thrown out. This PR deletes that unreachable diagnostic.
2. By throwing out the `stable` attribute when `since` is invalid, you'd end up with a second diagnostic saying **_function has missing stability attribute_** even though your function is not missing a stability attribute. This PR preserves the `stable` attribute even when `since` cannot be parsed, avoiding the misleading second diagnostic.
Followups I plan to try next:
- Do the same for the `since` value of `#[deprecated]`.
- See whether it makes sense to also preserve `stable` and/or `unstable` attributes when they contain an invalid `feature`. What redundant/misleading diagnostics can this eliminate? What problems arise from not having a usable feature name for some API, in the situation that we're already failing compilation, so not concerned about anything that happens in downstream code?
Mark .rmeta files as /SAFESEH on x86 Windows.
Chrome links .rlibs with /WHOLEARCHIVE or -Wl,--whole-archive to prevent the linker from discarding static initializers. This works well, except on Windows x86, where lld complains:
error: /safeseh: lib.rmeta is not compatible with SEH
The fix is simply to mark the .rmeta as SAFESEH aware. This is trivially true, since the metadata file does not contain any executable code.
Stop telling people to submit bugs for internal feature ICEs
This keeps track of usage of internal features, and changes the message to instead tell them that using internal features is not supported.
I thought about several ways to do this but now used the explicit threading of an `Arc<AtomicBool>` through `Session`. This is not exactly incremental-safe, but this is fine, as this is set during macro expansion, which is pre-incremental, and also only affects the output of ICEs, at which point incremental correctness doesn't matter much anyways.
See [MCP 620.](https://github.com/rust-lang/compiler-team/issues/596)
Remap Cargo dependencies to /rust/deps
⚠️ **This doesn't affect user-compiled programs, it only affects building the Rust compiler itself.** ⚠️
Right now, `rust.remap-debuginfo = true` doesn't completely remap all paths: while LLVM and rustc sources are properly remapped (respectively to `/rust/llvm` and `/rust/$commit`), Cargo dependencies still use absolute paths from the Cargo home.
This never affected builds from CI much, because `CARGO_HOME=/cargo` in CI, so users see paths like this included in the precompiled binaries and libraries:
```
/cargo/registry/src/index.crates.io-6f17d22bba15001f/gimli-0.26.2/src/read/line.rs
```
Builds outside CI don't have remapping though, and it's confusing that the config flag doesn't fully do what it advertises.
This PR fixes it by adding remapping for dependencies too. *All registries's* source directory are remapped to `/rust/deps`, to account for multiple registries being able to contain crates.io crates (sparse index vs git, and source replacement mirrors). This results in paths like this being included:
```
/rust/deps/gimli-0.26.2/src/read/line.rs
```
The latest locals() method in stable MIR returns slices instead of vecs.
This commit also includes fixes to the existing tests that previously
referenced the private locals field.
The word internal has connotations about information that's not exposed.
It's more accurate to say that the remaining locals apply only to the
inner part of the function, so I'm renaming them to inner locals.
Rollup of 6 pull requests
Successful merges:
- #116401 (Return multiple object-safety violation errors and code improvements to the object-safety check)
- #116553 (Do not suggest 'Trait<Assoc=arg>' when in trait impl)
- #116931 (Improve the warning messages for the `#[diagnostic::on_unimplemented]`)
- #117008 (Uplift `Canonical` to `rustc_type_ir`)
- #117009 (On unresolved imports, suggest a disambiguated path if necessary to avoid collision with local items)
- #117175 (Rename AsyncCoroutineKind to CoroutineSource)
r? `@ghost`
`@rustbot` modify labels: rollup
Rename AsyncCoroutineKind to CoroutineSource
pulled out of https://github.com/rust-lang/rust/pull/116447
Also refactors the printing infra of `CoroutineSource` to be ready for easily extending it with a `Gen` variant for `gen` blocks
Uplift `Canonical` to `rustc_type_ir`
I plan on moving the new trait solver's canonicalizer into either `rustc_type_ir` or a child crate. One dependency on this is lifting `Canonical<V>` to `rustc_type_ir` so we can actually name the canonicalized values.
I may also later lift `CanonicalVarInfo` into the new trait solver. I can't really tell what other changes need to be done, but I'm just putting this up sooner than later since I'm almost certain it'll need to be done regardless of other design choices.
There are a couple of warts introduced by this PR, since we no longer can define inherent `Canonical` impls in `rustc_middle` -- see the changes to:
* `compiler/rustc_trait_selection/src/traits/query/normalize.rs`
* `compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs`
r? lcnr
Improve the warning messages for the `#[diagnostic::on_unimplemented]`
This commit improves warnings emitted for malformed on unimplemented attributes by:
* Improving the span of the warnings
* Adding a label message to them
* Separating the messages for missing and unexpected options
* Adding a help message that says which options are supported
r? `@compiler-errors`
I'm happy to work on further improvements, so feel free to make suggestions.
Return multiple object-safety violation errors and code improvements to the object-safety check
See individual commits for more information. Split off of #114260, since it turned out that the main intent of that PR was wrong.
r? oli-obk
This keeps track of usage of internal features, and changes the message
to instead tell them that using internal features is not supported.
See MCP 620.
