Switch `time` to `jiff` for time formatting in ICE dumps
Due to https://github.com/jhpratt/deranged/issues/21, Clippy, R-A and Miri currently fail to build if we bump to 0.4.1, pulled in via `time`. ~~Add some specific type annotations so we don't have to just pin it.~~
~~I can open 3 PRs to the tool repos if preferred, but I thought it might be easier to do this than to pin the transitive dep and go back and remove it once the changes are synced back.~~
Avoid a reverse map that is only used in diagnostics paths
r? `@petrochenkov`
iterating a map until a value matches and returning the key is bad obviously, but it happens very rarely and only on diagnostics paths. It would also be a lot cheaper with https://github.com/rust-lang/rust/pull/138995. Which is actually why I'm trying this out, that PR adds a new entry in `create_def`, which makes `create_def` show up in cachegrind. So I'm trying out if removing adding an entry in `create_def` is a perf improvement
Prepend temp files with per-invocation random string to avoid temp filename conflicts
https://github.com/rust-lang/rust/issues/139407 uncovered a very subtle unsoundness with incremental codegen, failing compilation sessions (due to assembler errors), and the "prefer hard linking over copying files" strategy we use in the compiler for file management.
Specifically, imagine we're building a single file 3 times, all with `-Csave-temps -Cincremental=...`. Let's call the object file we're building for the codegen unit for `main` "`XXX.o`" just for clarity since it's probably some gigantic hash name:
```
#[inline(never)]
#[cfg(any(rpass1, rpass3))]
fn a() -> i32 {
0
}
#[cfg(any(cfail2))]
fn a() -> i32 {
1
}
fn main() {
evil::evil();
assert_eq!(a(), 0);
}
mod evil {
#[cfg(any(rpass1, rpass3))]
pub fn evil() {
unsafe {
std::arch::asm!("/* */");
}
}
#[cfg(any(cfail2))]
pub fn evil() {
unsafe {
std::arch::asm!("missing");
}
}
}
```
Session 1 (`rpass1`):
* Type-check, borrow-check, etc.
* Serialize the dep graph to the incremental working directory `.../s-...-working/`.
* Codegen object file to a temp file `XXX.rcgu.o` which is spit out in the cwd.
* Hard-link[^1] `XXX.rcgu.o` to the incremental working directory `.../s-...-working/XXX.o`.
* Save-temps option means we don't delete `XXX.rgcu.o`.
* Link the binary and stuff.
* Finalize[^2] the working incremental session by renaming `.../s-...-working` to ` s-...-asjkdhsjakd` (some other finalized incr comp session dir name).
Session 2 (`cfail2`):
* Load artifacts from the previous *finalized* incremental session, namely the dep graph.
* Type-check, borrow-check, etc. since the file has changed, so most dep graph nodes are red.
* Serialize the dep graph to the incremental working directory `.../s-...-working/`.
* Codegen object file to a temp file `XXX.rcgu.o`. **HERE IS THE PROBLEM**: The hard-link is still set up to point to the inode from `XXX.o` from the first session, so this also modifies the `XXX.o` in the previous finalized session directory.
* Codegen emits an error b/c `missing` is not an instruction, so we abort before finalizing the incremental session. Specifically, this means that the *previous* session is the last finalized session.
Session 3 (`rpass3`):
* Load artifacts from the previous *finalized* incremental session, namely the dep graph. NOTE that this is from session 1.
* All the dep graph nodes are green since we are basically replaying session 1.
* codegen object file `XXX.o`, which is detected as *reused* from session 1 since dep nodes were green. That means we **reuse** `XXX.o` which had been dirtied from session 2.
* Link the binary and stuff.
This results in a binary which reuses some of the build artifacts from session 2, but thinks it's from session 1.
At this point, I hope it's clear to see that the incremental results from session 1 were dirtied from session 2, but we reuse them as if session 1 was the previous (finalized) incremental session we ran. This is at best really buggy, and at worst **unsound**.
This isn't limited to `-C save-temps`, since there are other combinations of flags that may keep around temporary files (hard linked) in the working directory (like `-C debuginfo=1 -C split-debuginfo=unpacked` on darwin, for example).
---
This PR implements a fix which is to prepend temp filenames with a random string that is generated per invocation of rustc. This string is not *deterministic*, but temporary files are transient anyways, so I don't believe this is a problem.
That means that temp files are now something like... `{crate-name}.{cgu}.{invocation_temp}.rcgu.o`, where `{invocation_temp}` is the new temporary string we generate per invocation of rustc.
Fixes https://github.com/rust-lang/rust/issues/139407
[^1]: 175dcc7773/compiler/rustc_fs_util/src/lib.rs (L60)
[^2]: 175dcc7773/compiler/rustc_incremental/src/persist/fs.rs (L1-L40)
PR #137977 changed `DefPathData::TypeNs` to contain `Option<Symbol>` to
account for RPITIT assoc types being anonymous. This commit changes it
back to `Symbol` and gives anonymous assoc types their own variant. It
makes things a bit nicer overall.
This previously incorrectly returned `true` for parent functions whose
first statement was `let local = <coroutine>;`. While that didn't cause
any bugs as we only ever access `movable_coroutine` for `yield`
terminators. It was still wrong.
Improve `AssocItem::descr`.
The commit adds "associated" to the description of associated types and associated consts, to match the description of associated functions. This increases error message precision and consistency with `AssocKind::fmt`.
The commit also notes an imperfection in `AssocKind::fmt`; fixing this imperfection is possible but beyond the scope of this PR.
r? `@estebank`
Allow parenthesis around inferred array lengths
In #135272 it was noticed that we weren't handling `Vec<(((((_)))))>` correctly under the new desugaring for `generic_arg_infer`, this had to be fixed in order to not regress stable code for types that should continue working. This has the side effect of *also* allowing the following to work:
```rust
#![feature(generic_arg_infer)]
struct Bar<const N: usize>;
fn main() {
let a: Bar<((_))> = Bar::<10>;
}
```
However I did not make the same change for array lengths resulting in the following not compiling:
```rust
#![feature(generic_arg_infer)]
fn main() {
let a: [u8; (((_)))] = [2; 2];
let a: [u8; 2] = [2; (((((_)))))];
}
```
This is rather inconsistent as parenthesis around `_` *are* supported for const args to non-arrays, and type args. This PR fixes this allowing the above example to compile. No stable impact.
r? compiler-errors
Deeply normalize obligations in `BestObligation` folder
Built on #139513.
This establishes a somewhat rough invariant that the `Obligation`'s predicate is always deeply normalized in the folder; when we construct a new obligation we normalize it.
Putting this up for discussion since it does affect some goals.
r? lcnr
Allow drivers to supply a list of extra symbols to intern
Allows adding new symbols as `const`s in external drivers, desirable in Clippy so we can use them in patterns to replace code like 75530e9f72/src/tools/clippy/clippy_lints/src/casts/cast_ptr_alignment.rs (L66)
The Clippy change adds a couple symbols as a demo, the exact `clippy_utils` API and replacing other usages can be done on the Clippy side to minimise sync conflicts
---
try-job: aarch64-gnu
add `core::intrinsics::simd::{simd_extract_dyn, simd_insert_dyn}`
fixes https://github.com/rust-lang/rust/issues/137372
adds `core::intrinsics::simd::{simd_extract_dyn, simd_insert_dyn}`, which contrary to their non-dyn counterparts allow a non-const index. Many platforms (but notably not x86_64 or aarch64) have dedicated instructions for this operation, which stdarch can emit with this change.
Future work is to also make the `Index` operation on the `Simd` type emit this operation, but the intrinsic can't be used directly. We'll need some MIR shenanigans for that.
r? `@ghost`
The commit adds "associated" to the description of associated types and
associated consts, to match the description of associated functions.
This increases error message precision and consistency with
`AssocKind::fmt`.
The commit also notes an imperfection in `AssocKind::fmt`; fixing this
imperfection is possible but beyond the scope of this PR.
Reuse the index from promoted nodes when coloring executed tasks
https://github.com/rust-lang/rust/pull/138824 did not correctly handle the case where a dep node was promoted green, but later or concurrently executed. It resulted in multiple dep nodes being allocated to it. This fixes that by checking that the node was not previously green in the encoder lock.
This also fixes a race when forcing diagnostic nodes introduced in https://github.com/rust-lang/rust/pull/138824.
https://github.com/rust-lang/rust/pull/138824 should get reverted on beta.
This should fix#139110.
r? `@oli-obk`
Rename some `name` variables as `ident`.
It bugs me when variables of type `Ident` are called `name`. It leads to silly things like `name.name`. `Ident` variables should be called `ident`, and `name` should be used for variables of type `Symbol`.
This commit improves things by by doing `s/name/ident/` on a bunch of `Ident` variables. Not all of them, but a decent chunk.
r? `@fee1-dead`
fix "still mutable" ice while metrics are enabled
Resolves "still mutable" ICE discovered by `@matthiaskrgr` here: [#t-docs-rs > metrics intitiative @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/356853-t-docs-rs/topic/metrics.20intitiative/near/510490790)
This was caused by invoking `crate_hash` before the `definitions` struct was frozen here: e643f59f6d/compiler/rustc_interface/src/passes.rs (L951)
resolved by moving metrics dumping to occur after `analysis` freezes the definitions
I'm guessing we didn't discover this in CI because the problem only occurs when you try to calculate the crash hash with incremental compilation enabled when it tries to freeze the definitions here: e643f59f6d/compiler/rustc_middle/src/hir/map.rs (L1172)
my understanding is that this causes us to freeze the definitions too early in compilation, then we subsequently try to mutate them, likely during `analysis`, and this causes the ICE.
r? `@bjorn3`
Rollup of 13 pull requests
Successful merges:
- #138167 (Small code improvement in rustdoc hidden stripper)
- #138605 (Clean up librustdoc::html::render to be better encapsulated)
- #139423 (Suppress missing field error when autoderef bottoms out in infer)
- #139449 (match ergonomics: replace `peel_off_references` with a recursive call)
- #139507 (compiletest: Trim whitespace from environment variable names)
- #139530 (Remove some dead or leftover code related to rustc-intrinsic abi removal)
- #139560 (fix title of offset_of_enum feature)
- #139563 (emit a better error message for using the macro incorrectly)
- #139568 (Don't use empty trait names)
- #139580 (Temporarily leave the review rotation)
- #139589 (saethlin is back from vacation)
- #139592 (rustdoc: Enable Markdown extensions when looking for doctests)
- #139599 (Tracking issue template: fine-grained information on style update status)
r? `@ghost`
`@rustbot` modify labels: rollup
emit a better error message for using the macro incorrectly
fixing: https://github.com/EnzymeAD/rust/issues/185
I feel like it's not a perfect message either, so I'm open to suggestions.
But at the end of the day users will need to read the docs anyway, and emitting
multi-line errors each time this gets triggered can probably become annoying?
r? ``@jieyouxu`` since you've reviewed my frontend work back in the days.
Tracking:
- https://github.com/rust-lang/rust/issues/124509
match ergonomics: replace `peel_off_references` with a recursive call
This makes it imo quite a bit easier to follow how the binding mode gets calculated.
cc ```@dianne```
Suppress missing field error when autoderef bottoms out in infer
I see this error repeatedly when doing refactorings, and it's pretty misleading b/c it's not the source of the error.
Rigidly project missing item due to guaranteed impossible sized predicate
This is a somewhat involved change, but it amounts to treating missing impl items due to guaranteed impossible where clauses (dyn/str/slice sized, cc #135480) as *rigid projections* rather than projecting to an error term, since that was preventing either reporting a proper error (in an empty param env) *or* successfully type checking the code (in the presence of trivially false where clauses).
Fixes https://github.com/rust-lang/rust/issues/138970
r? `@lcnr` `@oli-obk`
It bugs me when variables of type `Ident` are called `name`. It leads to
silly things like `name.name`. `Ident` variables should be called
`ident`, and `name` should be used for variables of type `Symbol`.
This commit improves things by by doing `s/name/ident/` on a bunch of
`Ident` variables. Not all of them, but a decent chunk.
report call site of inlined scopes for large assignment lints
Addressed issue: #121672
Tracking issue: #83518
r? `@oli-obk`
I tried to follow your comment about what to do [here](https://github.com/rust-lang/rust/issues/121672#issuecomment-1972783675). However, I'm totally unfamiliar with the code so far (this is my first contribution touching compiler code), so I apologize in advance if I did something stupid 😅
In particular, I'm not sure I use the _correct_ source scope to look for inline data, as there is a whole `IndexVec` of them. My changes definitely did something, as can be seen by the added ui test. However, the result is not as anticipated in the issue:
```
LL | let cell = std::cell::UnsafeCell::new(data);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ value moved from here
```
instead of
```
LL | let cell = std::cell::UnsafeCell::new(data);
| ^^^^ value moved from here
```
raising my suspicion that maybe I got the wrong source scope.
Report higher-ranked trait error when higher-ranked projection goal fails in new solver
~~See HACK comment inline. Not actually sure if it should be marked as a *HACK*, b/c~~ it's kinda a legitimate case we want to care about unless we're going to make the proof tree visitor *smarter* about the leak check than the actual trait solver itself.
Encountered this while battling with `NiceRegionError`s in the old solver b/c I wondered what this code ended up giving us in the *new* solver as a comparison:
```rust
trait Foo {}
impl<T: FnOnce(&())> Foo for T {}
fn baz<T: Foo>() {}
fn main() {
baz::<fn(&'static ())>();
}
```
On master it's pretty bad:
```
error[E0271]: type mismatch resolving `<fn(&()) as FnOnce<(&(),)>>::Output == ()`
--> <source>:8:11
|
8 | baz::<fn(&'static ())>();
| ^^^^^^^^^^^^^^^ types differ
|
note: required for `fn(&'static ())` to implement `Foo`
--> <source>:3:22
|
3 | impl<T: FnOnce(&())> Foo for T {}
| ----------- ^^^ ^
| |
| unsatisfied trait bound introduced here
```
After this PR it's much better:
```
error[E0277]: the trait bound `fn(&'static ()): Foo` is not satisfied
--> /home/mgx/test.rs:8:11
|
8 | baz::<fn(&'static ())>();
| ^^^^^^^^^^^^^^^ the trait `for<'a> FnOnce(&'a ())` is not implemented for `fn(&'static ())`
|
= note: expected a closure with arguments `(&'static (),)`
found a closure with arguments `(&(),)`
note: required for `fn(&'static ())` to implement `Foo`
--> /home/mgx/test.rs:3:22
|
3 | impl<T: FnOnce(&())> Foo for T {}
| ----------- ^^^ ^
| |
| unsatisfied trait bound introduced here
note: required by a bound in `baz`
--> /home/mgx/test.rs:5:11
|
5 | fn baz<T: Foo>() {}
| ^^^ required by this bound in `baz`
```
r? lcnr
Don't call `Span::with_parent` on the good path in `has_stashed_diagnostic`
More unnecessary incurred span tracking avoided by not calling `span.with_parent(None)`. This is useless on its own but makes it much easier to fix other "span tracking on the good path" issues in the future.
r? oli-obk
Make the compiler suggest actual paths instead of visible paths if the visible paths are through any doc hidden path.
close#127011
Currently, when emitting a diagnostic about a valid trait, the compiler suggestes using visible paths of the trait even if they are through a doc hidden path. This PR updates the compiler to suggest actual paths in these cases.
There's an existing fast path for the `type_op_prove_predicate`
predicate, checking for trivially `Sized` types, which can be re-used
when evaluating obligations within queries. This should improve
performance, particularly in anticipation of new sizedness traits being
added which can take advantage of this.
Allow GVN to produce places and not just locals.
That may be too big of a hammer, as we may introduce new deref projections (possible UB footgun + probably not good for perf).
The second commit opts out of introducing projections that don't have a stable offset, which is probably what we want. Hence no new Deref and no new Index projections.
Fixes https://github.com/rust-lang/rust/issues/138936
cc `@scottmcm` `@dianqk`
Currently in case of a Trait object in closure parameter, the compiler
suggests either to use a reference, which is correct or to use an
`impl Trait` which is not. Do not emit this suggestion when the parameter
is part of a closure.
`resolve_ident_in_lexical_scope` checks for an empty name. Why is this
necessary? Because `parse_item_impl` can produce an `impl` block with an
empty trait name in some cases. This is pretty gross and very
non-obvious.
This commit avoids the use of the empty trait name. In one case the
trait name is instead pulled from `TyKind::ImplTrait`, which prevents
the output for `tests/ui/impl-trait/extra-impl-in-trait-impl.rs` from
changing. In the other case we just fail the parse and don't try to
recover. I think losing error recovery in this obscure case is worth
the code cleanup.
This change affects `tests/ui/parser/impl-parsing.rs`, which is split in
two, and the obsolete `..` syntax cases are removed (they are tested
elsewhere).
Instantiate higher-ranked transmute goal w/ placeholders before emitting sub-obligations
This avoids an ICE where we weren't keeping track of bound variables correctly in the `Freeze` obligations we emit for transmute goals. We could use `rebind` instead on that goal, but I think it's better just to instantiate the binder.
Fixes#139538
r? `@lcnr` or reassign
Improve presentation of closure signature mismatch from `Fn` trait goal
Flip the order of "expected" and "found" since that wasn't correct.
Don't present the arguments as a tuple, since it leaves a trailing comma. Instead, just use `fn(arg, arg)`.
Finally, be better with binders since we were just skipping binders.
r? oli-obk or reassign
Invert the order that we pass the arguments to the
`contract_check_ensures` function to avoid the warning when the tail
of the function is unreachable.
Note that the call itself is also unreachable, but we have already
handled that case by ignoring unreachable call for contract calls.
Fix stack overflow in exhaustiveness due to recursive HIR opaque hidden types
This fixes several spicy non-trivial recursive opaque definitions inferred from HIR typeck, ensuring that they don't cause stack overflows in exhaustiveness code, which currently reveals opaques manually in a way that is not overflow aware (as opposed to something like the normalizer folders).
These should eventually be outright rejected, but today (some) non-trivial recursive opaque definitions are accepted, and changing that requires an FCP, so for now just make sure we don't stack overflow :^)
Fixes https://github.com/rust-lang/rust/issues/139402
r? lcnr
borrowck typeck children together with their root
This introduces new cycle errors, even with `feature(inline_const_pat)` removed, see the `non-structural-match-types-cycle-err.rs` test.
The new cycle error happens as the layout of `async`-blocks relies on their `optimized_mir`. As that now depends on `mir_borrowck` of its typeck parent, computing the layout of an `async`-block during MIR building, e.g. when evaluating a named `const` pattern. I think there's currently no way to have a named const pattern whose type references an async block while being allowed? cc `@oli-obk` `@RalfJung`
I cannot think of other cases where we currently rely on the MIR of a typeck children while borrowchecking their parent. The crater run came back without any breakage. My work here will prevent any future features which rely on this as we'll get locked into borrowchecking them together as I continue to work on https://github.com/rust-lang/types-team/issues/129, cc `@rust-lang/types.`
r? compiler-errors
Fix trait upcasting to dyn type with no principal when there are projections
#126660 (which I had originally authored, lol) had a subtle bug that is the moral equivalent of #114036, which is that when upcasting from `dyn Principal<Projection = Ty> + AutoTrait` to `dyn AutoTrait`, we were dropping the trait ref for `Principal` but not its projections (if there were any).
With debug assertions enabled, this triggers the assertion I luckily added in a2a0cfe825, but even without debug assertions this is a logical bug since we had a dyn type with just a projection bound but no principal, so it caused a type mismatch.
This does not need an FCP because this should've been covered by the FCP in #126660, but we just weren't testing a case when casting from a `dyn` type with projections 😸Fixes#139418
r? ````@oli-obk```` (or anyone)
compiler: report error when trait object type param reference self
Fixes#139082.
Emits an error when `Self` is found in the projection bounds of a trait
object. In type aliases, `Self` has no meaning, so `type A = &'static
dyn B` where `trait B = Fn() -> Self` will expands to `type A = &'static
Fn() -> Self` which is illegal, causing the region solver to bail out
when hitting the uninferred Self.
r? ````@compiler-errors```` ````@fee1-dead````
Tell LLVM about impossible niche tags
I was trying to find a better way of emitting discriminant calculations, but sadly had no luck.
So here's a fairly small PR with the bits that did seem worth bothering:
1. As the [`TagEncoding::Niche` docs](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/enum.TagEncoding.html#variant.Niche) describe, it's possible to end up with a dead value in the input that's not already communicated via the range parameter attribute nor the range load metadata attribute. So this adds an `llvm.assume` in non-debug mode to tell LLVM about that. (That way it can tell that the sides of the `select` have disjoint possible values.)
2. I'd written a bunch more tests, or at least made them parameterized, in the process of trying things out, so this checks in those tests to hopefully help future people not trip on the same weird edge cases, like when the tag type is `i8` but yet there's still a variant index and discriminant of `258` which doesn't fit in that tag type because the enum is really weird.
Make error message for missing fields with `..` and without `..` more consistent
When `..` is not present, we say "missing field `bar` in initializer", but when it is present we say "missing mandatory field `bar`". I don't see why the primary error message should change, b/c the root cause is the same.
Let's harmonize these error messages and instead use a label to explain that `..` is required b/c it's not defaulted.
r? estebank
Fixes#139445.
The additional errors aren't great but the first one is still good and
it's the most important, and imperfect errors are better than ICEing.
This can happen when invalid syntax is passed to a declarative macro. We
shouldn't be too strict about the token stream position once the parser
has rejected the invalid syntax.
Fixes#139248.
Do not visit whole crate to compute `lints_that_dont_need_to_run`.
This allows to reuse the computed lint levels instead of re-visiting the whole crate.
Use `const_eval_select!()` macro to enable contract checking only at
runtime. The existing contract logic relies on closures,
which are not supported in constant functions.
This commit also removes one level of indirection for ensures clauses,
however, it currently has a spurious warning message when the bottom
of the function is unreachable.
