coverage: Memoize and simplify counter expressions
When creating coverage counter expressions as part of coverage instrumentation, we often end up creating obviously-redundant expressions like `c1 + (c0 - c1)`, which is equivalent to just `c0`.
To avoid doing so, this PR checks when we would create an expression matching one of 5 patterns, and uses the simplified form instead:
- `(a - b) + b` → `a`.
- `(a + b) - b` → `a`.
- `(a + b) - a` → `b`.
- `a + (b - a)` → `b`.
- `a - (a - b)` → `b`.
Of all the different ways to combine 3 operands and 2 operators, these are the patterns that allow simplification.
(Some of those patterns currently don't occur in practice, but are included anyway for completeness, to avoid having to add them later as branch coverage and MC/DC coverage support expands.)
---
This PR also adds memoization for newly-created (or newly-simplified) counter expressions, to avoid creating duplicates.
This currently makes no difference to the final mappings, but is expected to be useful for MC/DC coverage of match expressions, as proposed by https://github.com/rust-lang/rust/pull/124278#issuecomment-2106754753.
Suggest setting lifetime in borrowck error involving types with elided lifetimes
```
error: lifetime may not live long enough
--> $DIR/ex3-both-anon-regions-both-are-structs-2.rs:7:5
|
LL | fn foo(mut x: Ref, y: Ref) {
| ----- - has type `Ref<'_, '1>`
| |
| has type `Ref<'_, '2>`
LL | x.b = y.b;
| ^^^^^^^^^ assignment requires that `'1` must outlive `'2`
|
help: consider introducing a named lifetime parameter
|
LL | fn foo<'a>(mut x: Ref<'a, 'a>, y: Ref<'a, 'a>) {
| ++++ ++++++++ ++++++++
```
As can be seen above, it currently doesn't try to compare the `ty::Ty` lifetimes that diverged vs the `hir::Ty` to correctly suggest the following
```
help: consider introducing a named lifetime parameter
|
LL | fn foo<'a>(mut x: Ref<'_, 'a>, y: Ref<'_, 'a>) {
| ++++ ++++++++ ++++++++
```
but I believe this to still be an improvement over the status quo.
Fix#40990.
Make `EvalCtxt` generic over `InferCtxtLike`
...but don't change any of the impls, yet! These can get uplifted as we add more methods to `InferCtxtLike`/`Interner` :3
This is built on top of #125230.
r? lcnr
defrost `RUST_MIN_STACK=ice rustc hello.rs`
I didn't think too hard about testing my previous PR rust-lang/rust#122847 which makes our stack overflow handler assist people in discovering the `RUST_MIN_STACK` variable (which apparently is surprisingly useful for Really Big codebases). After it was merged, some useful comments left in a drive-by review led me to discover I had added an ICE. This reworks the code a bit to explain the rationale, remove the ICE that I introduced, and properly test one of the diagnostics.
fix suggestion in E0373 for !Unpin coroutines
Coroutines can be prefixed with the `static` keyword to make them
`!Unpin`.
However, given the following function:
```rust
fn check() -> impl Sized {
let x = 0;
#[coroutine]
static || {
yield;
x
}
}
```
We currently suggest prefixing `move` before `static`, which is
syntactically incorrect:
```
error[E0373]: coroutine may outlive the current function, but it borrows
...
--> src/main.rs:6:5
|
6 | static || {
| ^^^^^^^^^ may outlive borrowed value `x`
7 | yield;
8 | x
| - `x` is borrowed here
|
note: coroutine is returned here
--> src/main.rs:6:5
|
6 | / static || {
7 | | yield;
8 | | x
9 | | }
| |_____^
help: to force the coroutine to take ownership of `x` (and any other
referenced variables), use the `move` keyword
| // this is syntactically incorrect, it should be `static move ||`
6 | move static || {
| ++++
```
This PR suggests adding `move` after `static` for these coroutines.
I also added a UI test for this case.
Never type unsafe lint improvements
- Move linting code to a separate method
- Remove mentions of `core::convert::absurd` (#124311 was rejected)
- Make the lint into FCW
The last thing is a bit weird though. On one hand it should be `EditionSemanticsChange(2024)`, but on the other hand it shouldn't, because we also plan to break it on all editions some time later. _Also_, it's weird that we don't have `FutureReleaseSemanticsChangeReportInDeps`, IMO "this might cause UB in a future release" is important enough to be reported in deps...
IMO we ought to have three enums instead of [`FutureIncompatibilityReason`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint_defs/enum.FutureIncompatibilityReason.html#):
```rust
enum IncompatibilityWhen {
FutureRelease,
Edition(Edition),
}
enum IncompatibilyWhat {
Error,
SemanticChange,
}
enum IncompatibilityReportInDeps {
No,
Yes,
}
```
Tracking:
- https://github.com/rust-lang/rust/issues/123748
An earlier commit included the change for a suggestion here.
Unfortunately, it also used unwrap instead of dying properly.
Roll out the ~~rice paper~~ EarlyDiagCtxt before we do anything that
might leave a mess.
Coroutines can be prefixed with the `static` keyword to make them
`!Unpin`.
However, given the following function:
```rust
fn check() -> impl Sized {
let x = 0;
#[coroutine]
static || {
yield;
x
}
}
```
We currently suggest prefixing `move` before `static`, which is
syntactically incorrect:
```
error[E0373]: coroutine may outlive the current function, but it borrows
...
--> src/main.rs:6:5
|
6 | static || {
| ^^^^^^^^^ may outlive borrowed value `x`
7 | yield;
8 | x
| - `x` is borrowed here
|
note: coroutine is returned here
--> src/main.rs:6:5
|
6 | / static || {
7 | | yield;
8 | | x
9 | | }
| |_____^
help: to force the coroutine to take ownership of `x` (and any other
referenced variables), use the `move` keyword
| // this is syntactically incorrect, it should be `static move ||`
6 | move static || {
| ++++
```
This PR suggests adding `move` after `static` for these coroutines.
chore: Remove repeated words (extension of #124924)
When I saw #124924 I thought "Hey, I'm sure that there are far more than just two typos of this nature in the codebase". So here's some more typo-fixing.
Some found with regex, some found with a spellchecker. Every single one manually reviewed by me (along with hundreds of false negatives by the tools)
This change tweaks the error message generation to actually use the
`#[do_not_recommend]` attribute if present by just skipping the marked
trait impl in favour of the parent impl. It also adds a compile test for
this behaviour. Without this change the test would output the following
error:
```
error[E0277]: the trait bound `&str: Expression` is not satisfied
--> /home/weiznich/Documents/rust/rust/tests/ui/diagnostic_namespace/do_not_recommend.rs:53:15
|
LL | SelectInt.check("bar");
| ^^^^^ the trait `Expression` is not implemented for `&str`, which is required by `&str: AsExpression<Integer>`
|
= help: the following other types implement trait `Expression`:
Bound<T>
SelectInt
note: required for `&str` to implement `AsExpression<Integer>`
--> /home/weiznich/Documents/rust/rust/tests/ui/diagnostic_namespace/do_not_recommend.rs:26:13
|
LL | impl<T, ST> AsExpression<ST> for T
| ^^^^^^^^^^^^^^^^ ^
LL | where
LL | T: Expression<SqlType = ST>,
| ------------------------ unsatisfied trait bound introduced here
```
Note how that mentions `&str: Expression` before and now mentions `&str:
AsExpression<Integer>` instead which is much more helpful for users.
Open points for further changes before stabilization:
* We likely want to move the attribute to the `#[diagnostic]` namespace
to relax the guarantees given?
* How does it interact with the new trait solver?
Uplift more query stuff
- Uplift various query input/response internals
- Uplift the `ProofTree` structures and make the `ProofTreeBuilder` stuff (mostly) generic over `Interner`
- Stop using `TyCtxt::def_kind` in favor of `AliasTerm::kind`
r? lcnr
Only make GAT ambiguous in `match_projection_projections` considering shallow resolvability
In #123537, I tweaked the hack from #93892 to use `resolve_vars_if_possible` instead of `shallow_resolve`. This considers more inference guidance ambiguous. This resulted in crater regressions in #125196.
I've effectively reverted the change to the old behavior. That being said, I don't *like* this behavior, but I'd rather keep it for now since #123537 was not meant to make any behavioral changes. See the attached example.
This also affects the new solver, for the record, which doesn't have any rules about not guiding inference from param-env candidates which may constrain GAT args as a side-effect.
r? `@lcnr` or `@jackh726`
Rename Unsafe to Safety
Alternative to #124455, which is to just have one Safety enum to use everywhere, this opens the posibility of adding `ast::Safety::Safe` that's useful for unsafe extern blocks.
This leaves us today with:
```rust
enum ast::Safety {
Unsafe(Span),
Default,
// Safe (going to be added for unsafe extern blocks)
}
enum hir::Safety {
Unsafe,
Safe,
}
```
We would convert from `ast::Safety::Default` into the right Safety level according the context.
