`Module::getOrInsertGlobal` returns a `Constant*`, which is a super
class of `GlobalVariable`, but if the given type doesn't match an
existing declaration, it returns a bitcast of that global instead.
This causes UB when we pass that to `LLVMGetVisibility` which
unconditionally casts the opaque argument to a `GlobalValue*`.
Instead, we can do our own get-or-insert without worrying whether
existing types match exactly. It's not relevant when we're just trying
to get/set the linkage and visibility, and if types are needed we can
bitcast or error nicely from `rustc_codegen_llvm` instead.
Don't abort compilation after giving a lint error
The only reason to use `abort_if_errors` is when the program is so broken that either:
1. later passes get confused and ICE
2. any diagnostics from later passes would be noise
This is never the case for lints, because the compiler has to be able to deal with `allow`-ed lints.
So it can continue to lint and compile even if there are lint errors.
Closes https://github.com/rust-lang/rust/issues/82761. This is a WIP because I have a feeling it will exit with 0 even if there were lint errors; I don't have a computer that can build rustc locally at the moment.
The only reason to use `abort_if_errors` is when the program is so broken that either:
1. later passes get confused and ICE
2. any diagnostics from later passes would be noise
This is never the case for lints, because the compiler has to be able to deal with `allow`-ed lints.
So it can continue to lint and compile even if there are lint errors.
Add #[must_use] to mem/ptr functions
There's a lot of low-level / unsafe stuff here. Are there legit use cases for ignoring any of these return values?
* No regressions in `./x.py test --stage 1 library/std src/tools/clippy`.
* One regression in `./x.py test --stage 1 src/test/ui`. Fixed.
* I am unable to run `./x.py doc` on my machine so I'll need to wait for the CI to verify doctests pass. I eyeballed all the adjacent tests and they all look okay.
Parent issue: #89692
r? ```@joshtriplett```
Suggest a case insensitive match name regardless of levenshtein distance
Fixes#86170
Currently, `find_best_match_for_name` only returns a case insensitive match name depending on a Levenshtein distance. It's a bit unfortunate that that hides some suggestions for typos like `Bar` -> `BAR`. That idea is from https://github.com/rust-lang/rust/pull/46347#discussion_r153701834, but I think it still makes some sense to show a candidate when we find a case insensitive match name as it's more like a typo.
Skipped the `candidate != lookup` check because the current (i.e, `levenshtein_match`) returns the exact same `Symbol` anyway but it doesn't seem to confuse anything on UI tests.
r? ``@estebank``
Remove trailing semicolon from macro call span
Macro call site spans are now less surprising/more consistent since they no longer contain a semicolon after the macro call.
The downside is that we need to do a little guesswork to get the semicolon in diagnostics. But this should not be noticeable since it is rare for the semicolon to not immediately follow the macro call.
move implicit `Sized` predicate to end of list
In `Bounds::predicates()`, move the implicit `Sized` predicate to the
end of the generated list. This means that if there is an explicit
`Sized` bound, it will be checked first, and any resulting
diagnostics will have a more useful span.
Fixes#85998, at least partially. ~~Based on #85979, but only the last 2 commits are new for this pull request.~~ (edit: rebased) A full fix would need to deal with where-clauses, and that seems difficult. Basically, predicates are being collected in multiple stages, and there are two places where implicit `Sized` predicates can be inserted: once for generic parameters, and once for where-clauses. I think this insertion is happening too early, and we should actually do it only at points where we collect all of the relevant trait bounds for a type parameter.
I could use some help interpreting the changes to the stderr output. It looks like reordering the predicates changed some diagnostics that don't obviously have anything to do with `Sized` bounds. Possibly some error reporting code is making assumptions about ordering of predicates? The diagnostics for src/test/ui/derives/derives-span-Hash-*.rs seem to have improved, no longer pointing at the type parameter identifier, but src/test/ui/type-alias-impl-trait/generic_duplicate_param_use9.rs became less verbose for some reason.
I also ran into an instance of #84970 while working on this, but I kind of expected that could happen, because I'm reordering predicates. I can open a separate issue on that if it would be helpful.
``@estebank`` this seems likely to conflict (slightly?) with your work on #85947; how would you like to resolve that?
Show detailed expected/found types in error message when trait paths are the same
Fixes#65230.
### Issue solved by this PR
```rust
trait T {
type U;
fn f(&self) -> Self::U;
}
struct X<'a>(&'a mut i32);
impl<'a> T for X<'a> {
type U = &'a i32;
fn f(&self) -> Self::U {
self.0
}
}
fn main() {}
```
Compiler generates the following note:
```
note: ...so that the types are compatible
--> test.rs:10:28
|
10 | fn f(&self) -> Self::U {
| ____________________________^
11 | | self.0
12 | | }
| |_____^
= note: expected `T`
found `T`
```
This note is not useful since the expected type and the found type are the same.
### How this PR solve the issue
When the expected type and the found type are exactly the same in string representation, the note falls back to the detailed string representation of trait ref:
```
note: ...so that the types are compatible
--> test.rs:10:28
|
10 | fn f(&self) -> Self::U {
| ____________________________^
11 | | self.0
12 | | }
| |_____^
= note: expected `<X<'a> as T>`
found `<X<'_> as T>`
```
So that a user can notice what was different between the expected one and the found one.
Note specific regions involved in 'borrowed data escapes' error
Fixes#67007
Currently, a 'borrowed data escapes' error does not mention
the specific lifetime involved (except indirectly through a suggestion
about adding a lifetime bound). We now explain the specific lifetime
relationship that failed to hold, which improves otherwise vague
error messages.
When encountering a binop where the types would have been accepted, if
all the predicates had been fulfilled, include information about the
predicates and suggest appropriate `#[derive]`s if possible.
Point at trait(s) that needs to be `impl`emented.
Fixes#67007
Currently, a 'borrowed data escapes' error does not mention
the specific lifetime involved (except indirectly through a suggestion
about adding a lifetime bound). We now explain the specific lifetime
relationship that failed to hold, which improves otherwise vague
error messages.
Don't suggest replacing region with 'static in NLL
Fixes#73159
This is similar to #69350 - if the user didn't initially
write out a 'static lifetime, adding 'static in response to
a lifetime error is usually the wrong thing to do.
Fixes#73159
This is similar to #69350 - if the user didn't initially
write out a 'static lifetime, adding 'static in response to
a lifetime error is usually the wrong thing to do.
A subset of places referred to 'super-trait', so this changes them
to all use 'supertrait'. This matches 'supertype' and some other
usages. An exception is 'auto-trait' which is consistently used
in that manner.
Use larger span for adjustment THIR expressions
Currently, we use a relatively 'small' span for THIR
expressions generated by an 'adjustment' (e.g. an autoderef,
autoborrow, unsizing). As a result, if a borrow generated
by an adustment ends up causing a borrowcheck error, for example:
```rust
let mut my_var = String::new();
let my_ref = &my_var
my_var.push('a');
my_ref;
```
then the span for the mutable borrow may end up referring
to only the base expression (e.g. `my_var`), rather than
the method call which triggered the mutable borrow
(e.g. `my_var.push('a')`)
Due to a quirk of the MIR borrowck implementation,
this doesn't always get exposed in migration mode,
but it does in many cases.
This commit makes THIR building consistently use 'larger'
spans for adjustment expressions. These spans are recoded
when we first create the adjustment during typecheck. For
example, an autoref adjustment triggered by a method call
will record the span of the entire method call.
The intent of this change it make it clearer to users
when it's the specific way in which a variable is
used (for example, in a method call) that produdes
a borrowcheck error. For example, an error message
claiming that a 'mutable borrow occurs here' might
be confusing if it just points at a usage of a variable
(e.g. `my_var`), when no `&mut` is in sight. Pointing
at the entire expression should help to emphasize
that the method call itself is responsible for
the mutable borrow.
In several cases, this makes the `#![feature(nll)]` diagnostic
output match up exactly with the default (migration mode) output.
As a result, several `.nll.stderr` files end up getting removed
entirely.
Currently, we use a relatively 'small' span for THIR
expressions generated by an 'adjustment' (e.g. an autoderef,
autoborrow, unsizing). As a result, if a borrow generated
by an adustment ends up causing a borrowcheck error, for example:
```rust
let mut my_var = String::new();
let my_ref = &my_var
my_var.push('a');
my_ref;
```
then the span for the mutable borrow may end up referring
to only the base expression (e.g. `my_var`), rather than
the method call which triggered the mutable borrow
(e.g. `my_var.push('a')`)
Due to a quirk of the MIR borrowck implementation,
this doesn't always get exposed in migration mode,
but it does in many cases.
This commit makes THIR building consistently use 'larger'
spans for adjustment expressions
The intent of this change it make it clearer to users
when it's the specific way in which a variable is
used (for example, in a method call) that produdes
a borrowcheck error. For example, an error message
claiming that a 'mutable borrow occurs here' might
be confusing if it just points at a usage of a variable
(e.g. `my_var`), when no `&mut` is in sight. Pointing
at the entire expression should help to emphasize
that the method call itself is responsible for
the mutable borrow.
In several cases, this makes the `#![feature(nll)]` diagnostic
output match up exactly with the default (migration mode) output.
As a result, several `.nll.stderr` files end up getting removed
entirely.
Add `ConstraintCategory::Usage` for handling aggregate construction
In some cases, we emit borrowcheck diagnostics pointing
at a particular field expression in a struct expression
(e.g. `MyStruct { field: my_expr }`). However, this
behavior currently relies on us choosing the
`ConstraintCategory::Boring` with the 'correct' span.
When adding additional variants to `ConstraintCategory`,
(or changing existing usages away from `ConstraintCategory::Boring`),
the current behavior can easily get broken, since a non-boring
constraint will get chosen over a boring one.
To make the diagnostic output less fragile, this commit
adds a `ConstraintCategory::Usage` variant. We use this variant
for the temporary assignments created for each field of
an aggregate we are constructing.
Using this new variant, we can emit a message mentioning
"this usage", emphasizing the fact that the error message
is related to the specific use site (in the struct expression).
This is preparation for additional work on improving NLL error messages
(see #57374)
Suggest replacing an inexisting field for an unmentioned field
Fix#87938
This PR adds a suggestion to replace an inexisting field for an
unmentioned field. Given the following code:
```rust
enum Foo {
Bar { alpha: u8, bravo: u8, charlie: u8 },
}
fn foo(foo: Foo) {
match foo {
Foo::Bar {
alpha,
beta, // `bravo` miswritten as `beta` here.
charlie,
} => todo!(),
}
}
```
the compiler now emits the error messages below.
```text
error[E0026]: variant `Foo::Bar` does not have a field named `beta`
--> src/lib.rs:9:13
|
9 | beta, // `bravo` miswritten as `beta` here.
| ^^^^
| |
| variant `Foo::Bar` does not have this field
| help: `Foo::Bar` has a field named `bravo`: `bravo`
```
Note that this suggestion is available iff the number of inexisting
fields and unmentioned fields are both 1.
Propagate coercion cause into `try_coerce`
Currently, `coerce_inner` discards its `ObligationCause`
when calling `try_coerce`. This interfers with other
diagnostc improvements I'm working on, since we will lose
the original span by the time the actual coercion occurs.
Additionally, we now use the span of the trailing expression
(rather than the span of the entire function) when performing
a coercion in `check_return_expr`. This currently has no visible
effect on any of the unit tests, but will unblock future
diagnostic improvements.
Point to closure when emitting 'cannot move out' for captured variable
Attempts to fix#87456. The error message now points to the capturing closure, but I was not able to explain _why_ the closure implements `Fn` or `FnMut` (`TypeckResults::closure_kind_origins` did not contain anything for the closure in question).
cc `@Aaron1011`
