`NoAnn` and `IdentifiedAnnotation` impl both `pprust_ast::PpAnn` and
`pprust_hir::PpAnn`, which is a bit confusing, because the optional
`tcx` is only needed for the HIR cases. (Currently the `tcx` is
unnecessarily provided in the `expanded` AST cases.)
This commit splits each one into `Ast` and `Hir` versions, which makes
things clear about where the `tcx` is needed. The commit also renames
all the traits so they consistently end with `Ann`.
`call_with_pp_support_ast` and `call_with_pp_support_hir` how each have
a single call site. This commit inlines and removes them, which also
removes the need for all the supporting traits: `Sess`,
`AstPrinterSupport`, and `HirPrinterSupport`. The `sess` member is also
removed from several structs.
The handling of the `PpMode` variants is currently spread across three
functions: `print_after_parsing`, `print_after_hir_lowering`, and
`print_with_analysis`. Each one handles some of the variants. This split
is primarily because `print_after_parsing` has slightly different
arguments to the other two.
This commit changes the structure. It merges the three functions into a
single `print` function, and encapsulates the different arguments in a
new enum `PrintExtra`.
Benefits:
- The code is a little shorter.
- All the `PpMode` variants are handled in a single `match`, with no
need for `unreachable!` arms.
- It enables the trait removal in the subsequent commit by reducing
the number of `call_with_pp_support_ast` call sites from two to one.
First, both `AstPrinterSupport` and `HirPrinterSupport` have a `sess`
method. This commit introduces a `Sess` trait and makes the support
traits be subtraits of `Sess`, to avoid some duplication.
Second, both support traits have a `pp_ann` method that isn't needed if
we enable `trait_upcasting`. This commit removes those methods.
(Both of these traits will be removed in a subsequent commit, as will
the `trait_upcasting` use.)
`phase_3_run_analysis_passes` no longer exists, and AFAICT this code has
been refactored so much since this comment was written that it no longer
has any useful meaning.
Copy 1-element arrays as scalars, not vectors
For `[T; 1]` it's silly to copy as `<1 x T>` when we can just copy as `T`.
Inspired by https://github.com/rust-lang/rust/issues/101210#issuecomment-1732470941, which pointed out that `Option<[u8; 1]>` was codegenning worse than `Option<u8>`.
(I'm not sure *why* LLVM doesn't optimize out `<1 x u8>`, but might as well just not emit it in the first place in this codepath.)
---
I think I bit off too much in #116479; let me try just the scalar case first.
r? `@ghost`
coverage: Clarify loop-edge detection and graph traversal
This is a collection of improvements to two semi-related pieces of code:
- The code in `counters` that detects which graph edges don't exit a loop, and would therefore be good candidates to have their coverage computed as an expression rather than having a physical counter.
- The code in `graph` that traverses the coverage BCB graph in a particular order, and tracks loops and loop edges along the way (which is relevant to the above).
I was originally only planning to make the `graph` changes, but there was going to be a lot of indentation churn in `counters` anyway, and once I started looking I noticed a lot of opportunities for simplification.
---
`@rustbot` label +A-code-coverage
Handle several `#[diagnostic::on_unimplemented]` attributes correctly
This PR fixes an issues where rustc would ignore subsequent `#[diagnostic::on_unimplemented]` attributes. The [corresponding RFC](https://rust-lang.github.io/rfcs/3368-diagnostic-attribute-namespace.html) specifies that the first matching instance of each option is used. Invalid attributes are linted and otherwise ignored.
Having to keep passing in a graph reference was a holdover from when the graph
was partly mutated during traversal. As of #114354 that is no longer necessary,
so we can simplify the traversal code by storing a graph reference as a field
in `TraverseCoverageGraphWithLoops`.
The previous code was storing the worklist in a vector, and then selectively
adding items to the start or end of the vector. That's a perfect use-case for a
double-ended queue.
This change also reveals that the existing code was a bit confused about which
end of the worklist is the front or back. For now, items are always removed
from the front of the queue (instead of the back), and code that adds items to
the queue has been flipped, to preserve the existing behaviour.
Fix duplicate note on internal feature gates with associated issues
Fixes#116293
Note sure if I should add tests because the issue occurs only for feature gates having associated issues and that set of feature gates will change unpredictably leading to an unnecessary churn in tests.
Use structured suggestion for #113174
When encountering a for loop that is rejected by the borrow checker because it is being advanced within its body, provide a structured suggestion for `while let Some(pat) = iter.next()`.
The BuiltinInternalFeatures gate already has a struct level #[note]
attribute. The additional note field in it caused a duplicate to be
displayed when it was set to Some(...) which happened when the
feature had an associated issue
This PR fixes an issues where rustc would ignore subsequent
`#[diagnostic::on_unimplemented]` attributes. The [corresponding
RFC](https://rust-lang.github.io/rfcs/3368-diagnostic-attribute-namespace.html)
specifies that the first matching instance of each option is used.
Invalid attributes are linted and otherwise ignored.
Rollup of 5 pull requests
Successful merges:
- #116219 (Relate alias ty with variance)
- #116315 (Do not check for impossible predicates in const-prop lint.)
- #116436 (Structurally normalize for closure)
- #116597 (Prevent showing methods from blanket impls of not available foreign traits to show up in the search results)
- #116627 (small cleanup)
r? `@ghost`
`@rustbot` modify labels: rollup
When encountering a for loop that is rejected by the borrow checker
because it is being advanced within its body, provide a structured
suggestion for `while let Some(pat) = iter.next()`.
Do not check for impossible predicates in const-prop lint.
The enclosing query already checks for them, and replaces the body with a single `unreachable` if they are indeed impossible.
Relate alias ty with variance
In the new solver, turns out that the subst-relate branch of the alias-relate predicate was relating args invariantly even for opaques, which have variance 💀.
This change is a bit more invasive, but I'd rather not special-case it [here](aeaa5c30e5/compiler/rustc_trait_selection/src/solve/alias_relate.rs (L171-L190)) and then have it break elsewhere. I'm doing a perf run to see if the extra call to `def_kind` is that expensive, if it is, I'll reconsider.
r? ``@lcnr``
Compute NLL loan scopes using the polonius model
For a *location-insensitive* analysis (that is, without expressiveness improvements for users yet), this PR implements loans going out of scope using reachability and liveness, rather than checking if the issuing region's values contain a given CFG point. This is equivalent to NLL scopes and computes the same data.
r? `@matthewjasper`
A couple of notes:
- there are some assumptions about SCC representatives, placeholders, free regions, and member constraints that I believe hold, and they're documented in the code
- this passes all the UI tests with `-Zpolonius=next` -- the perf is [not terrible](https://github.com/rust-lang/rust/pull/112432#issuecomment-1749685862) and there are a bunch of ways to improve it in the future.
- there's a fixme left, hopefully Matthew you know a clean way to get the information it mentions.
Implement `-Clink-self-contained=-linker` opt out
This implements the `-Clink-self-contained` opt out necessary to switch to lld by changing rustc's defaults instead of cargo's.
Components that are enabled and disabled on the CLI are recorded, for the purpose of being merged with the ones which the target spec will declare (I'll open another PR for that tomorrow, for easier review).
For MCP510, we now check whether using the self-contained linker is disabled on the CLI. Right now it would only be sensible to with `-Zgcc-ld=lld` (and I'll add some checks that we don't both enable and disable a component on the CLI in a future PR), but the goal is to simplify adding the check of the target's enabled components here in the follow-up PRs.
r? `@petrochenkov`
Fix overflow checking in range patterns
When a range pattern contains an overflowing literal, if we're not careful we might not notice the overflow and use the wrapped value. This makes for confusing error messages because linting against overflowing literals is only done in a later pass. So when a range is invalid we check for overflows to provide a better error.
This check didn't use to handle negative types; this PR fixes that. First commit adds tests, second cleans up without changing behavior, third does the fix.
EDIT: while I was at it, I fixed a small annoyance about the span of the overflow lint on negated literals.
Fixes https://github.com/rust-lang/rust/issues/94239
Also consider call and yield as MIR SSA.
The SSA analysis on MIR only considered `Assign` statements as defining a SSA local.
This PR adds assignments as part of a `Call` or `Yield` terminator in that category.
This mainly allows to perform CopyProp on a call return place.
The only subtlety is in the dominance property: the assignment is only complete at the beginning of the target block.
In smir use `FxIndexMap` to store indexed ids
Previously we used `vec` for storing indexed types, which is fine for small cases but will lead to huge performance issues when we use `smir` for real world cases.
Addresses https://github.com/rust-lang/project-stable-mir/issues/35
r? ``@oli-obk``
On type error of closure call argument, point at earlier calls that affected inference
Mitigate part of https://github.com/rust-lang/rust/issues/71209.
When we encounter a type error on a specific argument of a closure call argument, where the closure's definition doesn't have a type specified, look for other calls of the closure to try and find the specific call that cased that argument to be inferred of the expected type.
```
error[E0308]: mismatched types
--> $DIR/unboxed-closures-type-mismatch.rs:30:18
|
LL | identity(1u16);
| -------- ^^^^ expected `u8`, found `u16`
| |
| arguments to this function are incorrect
|
note: expected because the closure was earlier called with an argument of type `u8`
--> $DIR/unboxed-closures-type-mismatch.rs:29:18
|
LL | identity(1u8);
| -------- ^^^ expected because this argument is of type `u8`
| |
| in this closure call
note: closure parameter defined here
--> $DIR/unboxed-closures-type-mismatch.rs:28:25
|
LL | let identity = |x| x;
| ^
help: change the type of the numeric literal from `u16` to `u8`
|
LL | identity(1u8);
| ~~
```
coverage: Unbox and simplify `bcb_filtered_successors`
This is a small cleanup in the coverage instrumentor's graph-building code.
---
This function already has access to the MIR body, so instead of taking a reference to a terminator, it's simpler and easier to pass in a basic block index.
There is no need to box the returned iterator if we instead add appropriate lifetime captures, and make `short_circuit_preorder` generic over the type of iterator it expects.
We can also greatly simplify the function's implementation by observing that the only difference between its two cases is whether we take all of a BB's successors, or just the first one.
---
`@rustbot` label +A-code-coverage
use env variable to control thread ids in rustc_log
Currently, when parallel rustc is enabled, even if the number of threads is 1, the thread ID will be included before all the logs.
E.g.
`WARN rustc_mir_build::thir::pattern::const_to_pat ...`
=>
`2:rustcWARN rustc_mir_build::thir::pattern::const_to_pat ...`
This makes the logs confusing and results in inconsistent UI test results for serial and parallel rustc. Therefore I think we should let users decide whether thread id information is needed through explicit control.
miri: make NaN generation non-deterministic
This implements the [LLVM semantics for NaN generation](https://llvm.org/docs/LangRef.html#behavior-of-floating-point-nan-values). I will soon submit an RFC to make this also officially the Rust semantics, but it has been our de-facto semantics for a long time so there's no reason Miri has to wait for that RFC. This PR just better aligns Miri with codegen.
This PR does that just for the operations that have MIR primitives; a future PR will adjust the intrinsics.
coverage: Separate initial span extraction from span processing
One of the main subtasks of coverage instrumentation is looking through MIR to determine a list of source code spans that require coverage counters.
That task is in turn subdivided into a few main steps:
- Getting the initial spans from MIR statements/terminators
- Processing the list of spans to merge or truncate nearby spans as necessary
- Grouping the processed spans by their corresponding coverage graph node
---
This PR enforces a firmer separation between the first two steps (span extraction and span processing), which ends up slightly simplifying both steps, since they don't need to deal with state that is only meaningful for the other step.
---
`@rustbot` label +A-code-coverage
Improve handling of assertion failures with very long conditions
It's not perfectly clear what the best behaviour is here, but I think this is an improvement.
r? `@matthewjasper`
cc `@m-ou-se`
This function already has access to the MIR body, so instead of taking a
reference to a terminator, it's simpler and easier to pass in a basic block
index.
There is no need to box the returned iterator if we instead add appropriate
lifetime captures, since `short_circuit_preorder` is now generic over the type
of iterator it expects.
We can also greatly simplify the function's implementation by observing that
the only difference between its two cases is whether we take all of a BB's
successors, or just the first one.
There are several that are unused and can be removed.
And there are some calls to `to_string`, which can be expressed more
nicely as a `foo_to_string` call, and then `to_string` need not be
`pub`. (This requires adding `pat_to_string`).
This enum was mainly needed to track the precise origin of a span in MIR, for
debug printing purposes. Since the old debug code was removed in #115962, we
can replace it with just the span itself.
The assertion in `assert-long-condition.rs` used to be fail like this, all on
one line:
```
thread 'main' panicked at 'assertion failed: 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 + 11 + 12 + 13 + 14 + 15 + 16 + 17 + 18\n + 19 + 20 + 21 + 22 + 23 + 24 + 25 == 0', tests/ui/macros/assert-long-condition.rs:7:5
```
The `\n` and subsequent indent is because the condition is pretty-printed, and
the pretty-printer inserts a newline. Printing the newline in this way is
arguably reasonable given that the message appears within single quotes, which
is very similar to a string literal.
However, after the assertion printing improvements that were released in 1.73,
the assertion now fails like this:
```
thread 'main' panicked at tests/ui/macros/assert-long-condition.rs:7:5:
assertion failed: 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 + 11 + 12 + 13 + 14 + 15 + 16 + 17 + 18\n + 19 + 20 + 21 + 22 + 23 + 24 + 25 == 0
```
Now that there are no single quotes around the pretty-printed condition, the
`\n` is quite strange.
This commit gets rid of the `\n`, by removing the `escape_debug` done on the
pretty-printed message. This results in the following:
```
thread 'main' panicked at tests/ui/macros/assert-long-condition.rs:7:5:
assertion failed: 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 + 11 + 12 + 13 + 14 + 15 + 16 + 17 + 18
+ 19 + 20 + 21 + 22 + 23 + 24 + 25 == 0
```
The overly-large indent is still strange, but that's a separate pretty-printing issue.
This change helps with #108341.
Extend `impl`'s `def_span` to include its where clauses
Typically, we highlight the def-span of an impl in a diagnostic due to either:
1. coherence error
2. trait evaluation cycle
3. invalid implementation of built-in trait
I find that an impl's where clauses are very often required to understanding why these errors come about, which is unfortunate since where clauses may be located on different lines and don't show up in the error. This PR expands the def-span of impls to include these where clauses.
r? cjgillot since you've touched this code a while back to make some spans shorter, but you can also reassign to wg-diagnostics or compiler if you're busy or have no strong opinions.
Sync rustc_codegen_cranelift
The highlights this time are improved simd and inline asm support, `is_x86_feature_detected!()` returning the actual cpu features when inline asm support is enabled and a couple of bug fixes.
r? ```@ghost```
```@rustbot``` label +A-codegen +A-cranelift +T-compiler +subtree-sync
Fix suggestion span involving wrongly placed generic arg on variant
Fixes#116473
The span computation was wrong. It went from the end of the variant to the end of the (wrongly placed) args. However, the variant lived in a different expansion and this resulted in a nonsensical span that overlaps with another and thereby leads to the ICE.
In the fix I've changed span computation to not be based on the location of the variant, but purely on the location of the args. I simply extend the start of the args span 2 positions to the left and that includes the `::` and that's all we need apparently.
This approach produces a correct span regardless of which macro/expansion the args reside in and where the variant is.
improve the suggestion of `generic_bound_failure`
- Fixes#115375
- suggest the bound in the correct scope: trait or impl header vs assoc item. See `tests/ui/suggestions/lifetimes/type-param-bound-scope.rs`
- don't suggest a lifetime name that conflicts with the other late-bound regions of the function:
```rust
type Inv<'a> = *mut &'a ();
fn check_bound<'a, T: 'a>(_: T, _: Inv<'a>) {}
fn test<'a, T>(_: &'a str, t: T, lt: Inv<'_>) { // suggests a new name `'a`
check_bound(t, lt); //~ ERROR
}
```
When the variant and the (wrongly placed) args are at separate
source locations such as being in different macos or one in a macro and
the other somwhere outside of it, the arg spans we computed spanned
the entire distance between such locations and were hence invalid.
.
Generalize small dominators optimization
* Use small dominators optimization from 640ede7b0a more generally.
* Merge `DefLocation` and `LocationExtended` since they serve the same purpose.
Always preserve DebugInfo in DeadStoreElimination.
This is a version of #106852 that does not check the current crate's debuginfo flag, and always attempts to preserve debuginfo.
I haven't figured out how to handle mixing debuginfo levels for std, the one for the test, and the one for the CI target just right to merge #106852, so this can at least fix the debuginfo issue.
Fixes https://github.com/rust-lang/rust/issues/103655
Fix to register analysis passes with -Zllvm-plugins at link-time
This PR fixes an unexpected behavior of the `-Zllvm-plugins` flag. It allows to run an out-of-tree pass as part of LTO.
However, analysis passes are registered before the plugin is loaded. As a result an analysis pass, which is passed as a plugin, is not registered. This causes the LLVM PassManager to fail when the analysis pass is queried from a transformation pass [(here)](https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/IR/PassManager.h#L776).
This fix mimics the bahavior in [LLVM LTOBackend.cpp](https://github.com/llvm/llvm-project/blob/main/llvm/lib/LTO/LTOBackend.cpp#L273) by loading the plugin before the analysis passes are registered.
Tested with rustc 1.60 and 1.65 and LLVM-13.0.1.
remove Key impls for types that involve an AllocId
I don't understand how but somehow that leads to issues like https://github.com/rust-lang/rust/issues/83085? Anyway removing unused impls doesn't seem like a bad idea. The concerning part is that of course nothing will stop us from having such impls again in the future, alongside re-introducing bugs like #83085.
r? `@oli-obk`
Detect missing `=>` after match guard during parsing
```
error: expected one of `,`, `:`, or `}`, found `.`
--> $DIR/missing-fat-arrow.rs:25:14
|
LL | Some(a) if a.value == b {
| - while parsing this struct
LL | a.value = 1;
| -^ expected one of `,`, `:`, or `}`
| |
| while parsing this struct field
|
help: try naming a field
|
LL | a: a.value = 1;
| ++
help: you might have meant to start a match arm after the match guard
|
LL | Some(a) if a.value == b => {
| ++
```
Fix#78585.
Show more information when multiple `impl`s apply
- When there are `impl`s without type params, show only those (to avoid showing overly generic `impl`s).
```
error[E0283]: type annotations needed
--> $DIR/multiple-impl-apply.rs:34:9
|
LL | let y = x.into();
| ^ ---- type must be known at this point
|
note: multiple `impl`s satisfying `_: From<Baz>` found
--> $DIR/multiple-impl-apply.rs:14:1
|
LL | impl From<Baz> for Bar {
| ^^^^^^^^^^^^^^^^^^^^^^
...
LL | impl From<Baz> for Foo {
| ^^^^^^^^^^^^^^^^^^^^^^
= note: required for `Baz` to implement `Into<_>`
help: consider giving `y` an explicit type
|
LL | let y: /* Type */ = x.into();
| ++++++++++++
```
- Lower the importance of `T: Sized`, `T: WellFormed` and coercion errors, to prioritize more relevant errors. The pre-existing deduplication logic deals with hiding redundant errors better that way, and we show errors with more metadata that is useful to the user.
- Show `<SelfTy as Trait>::assoc_fn` suggestion in more cases.
```
error[E0790]: cannot call associated function on trait without specifying the corresponding `impl` type
--> $DIR/cross-return-site-inference.rs:38:16
|
LL | return Err(From::from("foo"));
| ^^^^^^^^^^ cannot call associated function of trait
|
help: use a fully-qualified path to a specific available implementation
|
LL | return Err(</* self type */ as From>::from("foo"));
| +++++++++++++++++++ +
```
Fix#88284.
Use `std::path::PathBuf` rather than `String`; use `std::env::var_os`
rather than `std::env::var`. These changes avoid a number of error paths
which can arise in the presence of non-UTF-8 paths.
Clarify `invalid_reference_casting` lint around interior mutable types
This is PR intends to clarify the `invalid_reference_casting` lint around interior mutable types by adding a note for them saying that they should go through `UnsafeCell::get`.
So for this code:
```rust
let cell = &std::cell::UnsafeCell::new(0);
let _num = &mut *(cell as *const _ as *mut i32);
```
the following note will be added to the lint output:
```diff
error: casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell`
--> $DIR/reference_casting.rs:68:16
|
LL | let _num = &mut *(cell as *const _ as *mut i32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: for more information, visit <https://doc.rust-lang.org/book/ch15-05-interior-mutability.html>
+ = note: even for types with interior mutability, the only legal way to obtain a mutable pointer from a shared reference is through `UnsafeCell::get`
```
Suggestion are welcome around the note contents.
Fixes https://github.com/rust-lang/rust/issues/116410
cc `@RalfJung`
Bring back generic parameters for indices in rustc_abi and make it compile on stable
This effectively reverses https://github.com/rust-lang/rust/pull/107163, allowing rust-analyzer to depend on this crate again,
It also moves some glob imports / expands them in the first commit because they made it more difficult for me to reason about things.
`serialize.rs` has the `Encodable`/`Decodable` impls for lots of basic
types, including `Vec`. `collection_impls` has it for lots of collection
types. The distinction isn't really meaningful, and it's simpler to have
them all in a single file.
The `rustc_span::FileName` enum already differentiates between real
files and "fake" files such as `<anon>`. We do not need to artificially
forbid real file names from ending in `>`.
Do not assert that hidden types don't have erased regions.
cc https://github.com/rust-lang/rust/issues/116306
`args` can have erased regions.
In the linked issue, this is reached by computing whether a large type is `Freeze` to compute its ABI.
I do not have a minimized test to include.
Add a note to duplicate diagnostics
Helps explain why there may be a difference between manual testing and the test suite output and highlights them as something to potentially look into
For existing duplicate diagnostics I just blessed them other than a few files that had other `NOTE` annotations in
