Expand `for_loops_over_fallibles` lint to lint on fallibles behind references.
Extends the scope of the (warn-by-default) lint `for_loops_over_fallibles` from just `for _ in x` where `x: Option<_>/Result<_, _>` to also cover `x: &(mut) Option<_>/Result<_>`
```rs
fn main() {
// Current lints
for _ in Some(42) {}
for _ in Ok::<_, i32>(42) {}
// New lints
for _ in &Some(42) {}
for _ in &mut Some(42) {}
for _ in &Ok::<_, i32>(42) {}
for _ in &mut Ok::<_, i32>(42) {}
// Should not lint
for _ in Some(42).into_iter() {}
for _ in Some(42).iter() {}
for _ in Some(42).iter_mut() {}
for _ in Ok::<_, i32>(42).into_iter() {}
for _ in Ok::<_, i32>(42).iter() {}
for _ in Ok::<_, i32>(42).iter_mut() {}
}
```
<details><summary><code>cargo build</code> diff</summary>
```diff
diff --git a/old.out b/new.out
index 84215aa..ca195a7 100644
--- a/old.out
+++ b/new.out
`@@` -1,33 +1,93 `@@`
warning: for loop over an `Option`. This is more readably written as an `if let` statement
--> src/main.rs:3:14
|
3 | for _ in Some(42) {}
| ^^^^^^^^
|
= note: `#[warn(for_loops_over_fallibles)]` on by default
help: to check pattern in a loop use `while let`
|
3 | while let Some(_) = Some(42) {}
| ~~~~~~~~~~~~~~~ ~~~
help: consider using `if let` to clear intent
|
3 | if let Some(_) = Some(42) {}
| ~~~~~~~~~~~~ ~~~
warning: for loop over a `Result`. This is more readably written as an `if let` statement
--> src/main.rs:4:14
|
4 | for _ in Ok::<_, i32>(42) {}
| ^^^^^^^^^^^^^^^^
|
help: to check pattern in a loop use `while let`
|
4 | while let Ok(_) = Ok::<_, i32>(42) {}
| ~~~~~~~~~~~~~ ~~~
help: consider using `if let` to clear intent
|
4 | if let Ok(_) = Ok::<_, i32>(42) {}
| ~~~~~~~~~~ ~~~
-warning: `for-loops-over-fallibles` (bin "for-loops-over-fallibles") generated 2 warnings
- Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
+warning: for loop over a `&Option`. This is more readably written as an `if let` statement
+ --> src/main.rs:7:14
+ |
+7 | for _ in &Some(42) {}
+ | ^^^^^^^^^
+ |
+help: to check pattern in a loop use `while let`
+ |
+7 | while let Some(_) = &Some(42) {}
+ | ~~~~~~~~~~~~~~~ ~~~
+help: consider using `if let` to clear intent
+ |
+7 | if let Some(_) = &Some(42) {}
+ | ~~~~~~~~~~~~ ~~~
+
+warning: for loop over a `&mut Option`. This is more readably written as an `if let` statement
+ --> src/main.rs:8:14
+ |
+8 | for _ in &mut Some(42) {}
+ | ^^^^^^^^^^^^^
+ |
+help: to check pattern in a loop use `while let`
+ |
+8 | while let Some(_) = &mut Some(42) {}
+ | ~~~~~~~~~~~~~~~ ~~~
+help: consider using `if let` to clear intent
+ |
+8 | if let Some(_) = &mut Some(42) {}
+ | ~~~~~~~~~~~~ ~~~
+
+warning: for loop over a `&Result`. This is more readably written as an `if let` statement
+ --> src/main.rs:9:14
+ |
+9 | for _ in &Ok::<_, i32>(42) {}
+ | ^^^^^^^^^^^^^^^^^
+ |
+help: to check pattern in a loop use `while let`
+ |
+9 | while let Ok(_) = &Ok::<_, i32>(42) {}
+ | ~~~~~~~~~~~~~ ~~~
+help: consider using `if let` to clear intent
+ |
+9 | if let Ok(_) = &Ok::<_, i32>(42) {}
+ | ~~~~~~~~~~ ~~~
+
+warning: for loop over a `&mut Result`. This is more readably written as an `if let` statement
+ --> src/main.rs:10:14
+ |
+10 | for _ in &mut Ok::<_, i32>(42) {}
+ | ^^^^^^^^^^^^^^^^^^^^^
+ |
+help: to check pattern in a loop use `while let`
+ |
+10 | while let Ok(_) = &mut Ok::<_, i32>(42) {}
+ | ~~~~~~~~~~~~~ ~~~
+help: consider using `if let` to clear intent
+ |
+10 | if let Ok(_) = &mut Ok::<_, i32>(42) {}
+ | ~~~~~~~~~~ ~~~
+
+warning: `for-loops-over-fallibles` (bin "for-loops-over-fallibles") generated 6 warnings
+ Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.02s
```
</details>
-----
Question:
* ~~Currently, the article `an` is used for `&Option`, and `&mut Option` in the lint diagnostic, since that's what `Option` uses. Is this okay or should it be changed? (likewise, `a` is used for `&Result` and `&mut Result`)~~ The article `a` is used for `&Option`, `&mut Option`, `&Result`, `&mut Result` and (as before) `Result`. Only `Option` uses `an` (as before).
`@rustbot` label +A-lint
This makes a small change as requested in code review, such that if there's
ambiguity in the self lifetime, we avoid lifetime elision entirely instead of
considering using lifetimes from any of the other parameters.
For example,
impl Something {
fn method(self: &Box<&Self>, something_else: &u32) -> &u32 { ... }
}
in standard Rust would have assumed the return lifetime was that of &Self;
with this PR prior to this commit would have chosen the lifetime of
'something_else', and after this commit would give an error message explaining
that the lifetime is ambiguous.
struct Concrete(u32);
impl Concrete {
fn m(self: &Box<Self>) -> &u32 {
&self.0
}
}
resulted in a confusing error.
impl Concrete {
fn n(self: &Box<&Self>) -> &u32 {
&self.0
}
}
resulted in no error or warning, despite apparent ambiguity over the elided
lifetime.
This commit changes two aspects of the behavior.
Previously, when examining the self type, we considered lifetimes only if they
were immediately adjacent to Self. We now consider lifetimes anywhere in the
self type.
Secondly, if more than one lifetime is discovered in the self type, we
disregard it as a possible lifetime elision candidate.
This is a compatibility break, and in fact has required some changes to tests
which assumed the earlier behavior.
Fixes https://github.com/rust-lang/rust/issues/117715
Move `#[do_not_recommend]` to the `#[diagnostic]` namespace
This commit moves the `#[do_not_recommend]` attribute to the `#[diagnostic]` namespace. It still requires
`#![feature(do_not_recommend)]` to work.
r? `@compiler-errors`
Translation of the lint message happens when the actual diagnostic is
created, not when the lint is buffered. Generating the message from
BuiltinLintDiag ensures that all required data to construct the message
is preserved in the LintBuffer, eventually allowing the messages to be
moved to fluent.
Remove the `msg` field from BufferedEarlyLint, it is either generated
from the data in the BuiltinLintDiag or stored inside
BuiltinLintDiag::Normal.
Some minor (English only) heroics are performed to print error messages
like "5th rule of macro `m` is never used". The form "rule #5 of macro
`m` is never used" is just as good and much simpler to implement.
Remove braces when fixing a nested use tree into a single item
[Back in 2019](https://github.com/rust-lang/rust/pull/56645) I added rustfix support for the `unused_imports` lint, to automatically remove them when running `cargo fix`. For the most part this worked great, but when removing all but one childs of a nested use tree it turned `use foo::{Unused, Used}` into `use foo::{Used}`. This is slightly annoying, because it then requires you to run `rustfmt` to get `use foo::Used`.
This PR automatically removes braces and the surrouding whitespace when all but one child of a nested use tree are unused. To get it done I had to add the span of the nested use tree to the AST, and refactor a bit the code I wrote back then.
A thing I noticed is, there doesn't seem to be any `//@ run-rustfix` test for fixing the `unused_imports` lint. I created a test in `tests/suggestions` (is that the right directory?) that for now tests just what I added in the PR. I can followup in a separate PR to add more tests for fixing `unused_lints`.
This PR is best reviewed commit-by-commit.
ast: Generalize item kind visiting
And avoid duplicating logic for visiting `Item`s with different kinds (regular, associated, foreign).
The diff is better viewed with whitespace ignored.
Macro calls are ephemeral, they should not add anything to the definition tree, even if their AST could contains something with identity.
Thankfully, macro call AST cannot contain anything like that, so these walks are just noops.
In majority of other places in def_collector / build_reduced_graph they are already not visited.
(Also, a minor match reformatting is included.)
Also allow `impl Trait` in delegated functions.
The delegation item will refer to the original opaque type from the callee, fresh opaque type won't be created.
weak lang items are not allowed to be #[track_caller]
For instance the panic handler will be called via this import
```rust
extern "Rust" {
#[lang = "panic_impl"]
fn panic_impl(pi: &PanicInfo<'_>) -> !;
}
```
A `#[track_caller]` would add an extra argument and thus make this the wrong signature.
The 2nd commit is a consistency rename; based on the docs [here](https://doc.rust-lang.org/unstable-book/language-features/lang-items.html) and [here](https://rustc-dev-guide.rust-lang.org/lang-items.html) I figured "lang item" is more widely used. (In the compiler output, "lang item" and "language item" seem to be pretty even.)
Delegation: fix ICE on wrong `self` resolution
fixes https://github.com/rust-lang/rust/issues/122874
Delegation item should be wrapped in a `rib` to behave like a regular function during name resolution.
r? `@petrochenkov`
Fix bad span for explicit lifetime suggestions
Fixes#121267
Current explicit lifetime suggestions are not showing correct spans for some lifetimes - e.g. elided lifetime generic parameters;
This should be done correctly regarding elided lifetime kind like the following code
43fdd4916d/compiler/rustc_resolve/src/late/diagnostics.rs (L3015-L3044)
`f16` and `f128` step 3: compiler support & feature gate
Continuation of https://github.com/rust-lang/rust/pull/121841, another portion of https://github.com/rust-lang/rust/pull/114607
This PR exposes the new types to the world and adds a feature gate. Marking this as a draft because I need some feedback on where I did the feature gate check. It also does not yet catch type via suffixed literals (so the feature gate test will fail, probably some others too because I haven't belssed).
If there is a better place to check all types after resolution, I can do that. If not, I figure maybe I can add a second gate location in AST when it checks numeric suffixes.
Unfortunately I still don't think there is much testing to be done for correctness (codegen tests or parsed value checks) until we have basic library support. I think that will be the next step.
Tracking issue: https://github.com/rust-lang/rust/issues/116909
r? `@compiler-errors`
cc `@Nilstrieb`
`@rustbot` label +F-f16_and_f128
Fix the conflict problem between the diagnostics fixes of lint `unnecessary_qualification` and `unused_imports`
fixes#121331
For an `item` that triggers lint unnecessary_qualification, if the `use item` which imports this item is also trigger unused import, fixing the two lints at the same time may lead to the problem that the `item` cannot be found.
This PR will avoid reporting lint unnecessary_qualification when conflict occurs.
r? ``@petrochenkov``
Includes related tests and documentation pages.
Michael Goulet: Don't issue feature error in resolver for f16/f128
unless finalize
Co-authored-by: Michael Goulet <michael@errs.io>
Add asm goto support to `asm!`
Tracking issue: #119364
This PR implements asm-goto support, using the syntax described in "future possibilities" section of [RFC2873](https://rust-lang.github.io/rfcs/2873-inline-asm.html#asm-goto).
Currently I have only implemented the `label` part, not the `fallthrough` part (i.e. fallthrough is implicit). This doesn't reduce the expressive though, since you can use label-break to get arbitrary control flow or simply set a value and rely on jump threading optimisation to get the desired control flow. I can add that later if deemed necessary.
r? ``@Amanieu``
cc ``@ojeda``
Remove `feed_local_def_id`
best reviewed commit by commit
Basically I returned `TyCtxtFeed` from `create_def` and then preserved that in the local caches
based on https://github.com/rust-lang/rust/pull/121084
r? ````@petrochenkov````
Rollup of 9 pull requests
Successful merges:
- #121958 (Fix redundant import errors for preload extern crate)
- #121976 (Add an option to have an external download/bootstrap cache)
- #122022 (loongarch: add frecipe and relax target feature)
- #122026 (Do not try to format removed files)
- #122027 (Uplift some feeding out of `associated_type_for_impl_trait_in_impl` and into queries)
- #122063 (Make the lowering of `thir::ExprKind::If` easier to follow)
- #122074 (Add missing PartialOrd trait implementation doc for array)
- #122082 (remove outdated fixme comment)
- #122091 (Note why we're using a new thread in `test_get_os_named_thread`)
r? `@ghost`
`@rustbot` modify labels: rollup
Fix linting paths with qself in `unused_qualifications`
Fixes#121999
`resolve_qpath` ends up being called again with `qself` set to `None` to check trait items from fully qualified paths. To avoid this the lint is moved to a place that accounts for this already
96561a8fd1/compiler/rustc_resolve/src/late.rs (L4074-L4088)
r? `````@petrochenkov`````
Rollup of 8 pull requests
Successful merges:
- #121202 (Limit the number of names and values in check-cfg diagnostics)
- #121301 (errors: share `SilentEmitter` between rustc and rustfmt)
- #121658 (Hint user to update nightly on ICEs produced from outdated nightly)
- #121846 (only compare ambiguity item that have hard error)
- #121961 (add test for #78894#71450)
- #121975 (hir_analysis: enums return `None` in `find_field`)
- #121978 (Fix duplicated path in the "not found dylib" error)
- #121991 (Merge impl_trait_in_assoc_types_defined_by query back into `opaque_types_defined_by`)
r? `@ghost`
`@rustbot` modify labels: rollup
Existing names for values of this type are `sess`, `parse_sess`,
`parse_session`, and `ps`. `sess` is particularly annoying because
that's also used for `Session` values, which are often co-located, and
it can be difficult to know which type a value named `sess` refers to.
(That annoyance is the main motivation for this change.) `psess` is nice
and short, which is good for a name used this much.
The commit also renames some `parse_sess_created` values as
`psess_created`.
Consider middle segments of paths in `unused_qualifications`
Currently `unused_qualifications` looks at the last segment of a path to see if it can be trimmed, this PR extends the check to the middle segments also
```rust
// currently linted
use std::env::args();
std::env::args(); // Removes `std::env::`
```
```rust
// newly linted
use std::env;
std::env::args(); // Removes `std::`
```
Paths with generics in them are now linted as long as the part being trimmed is before any generic args, e.g. it will now suggest trimming `std::vec::` from `std::vec::Vec<usize>`
Paths with any segments that are from an expansion are no longer linted
Fixes#100979Fixes#96698
Improve renaming suggestion when item starts with underscore
Fixes https://github.com/rust-lang/rust/issues/121776.
It goes from:
```terminal
error[E0433]: failed to resolve: use of undeclared type `Foo`
--> src/foo.rs:6:13
|
6 | let _ = Foo::Bar;
| ^^^ use of undeclared type `Foo`
|
help: an enum with a similar name exists, consider changing it
|
1 | enum Foo {
| ~~~
```
to:
```terminal
error[E0433]: failed to resolve: use of undeclared type `Foo`
--> foo.rs:6:13
|
6 | let _ = Foo::Bar;
| ^^^ use of undeclared type `Foo`
|
help: an enum with a similar name exists, consider renaming `_Foo` into `Foo`
|
1 | enum Foo {
| ~~~
error: aborting due to 1 previous error
```
Diagnostic renaming
Renaming various diagnostic types from `Diagnostic*` to `Diag*`. Part of https://github.com/rust-lang/compiler-team/issues/722. There are more to do but this is enough for one PR.
r? `@davidtwco`
Add newtypes for bool fields/params/return types
Fixed all the cases of this found with some simple searches for `*/ bool` and `bool /*`; probably many more
I have a suspicion that quite a few delayed bug paths are impossible to
reach, so I did an experiment.
I converted every `delayed_bug` to a `bug`, ran the full test suite,
then converted back every `bug` that was hit. A surprising number were
never hit.
The next commit will convert some more back, based on human judgment.
There are lots of functions that modify a diagnostic. This can be via a
`&mut Diagnostic` or a `&mut DiagnosticBuilder`, because the latter type
wraps the former and impls `DerefMut`.
This commit converts all the `&mut Diagnostic` occurrences to `&mut
DiagnosticBuilder`. This is a step towards greatly simplifying
`Diagnostic`. Some of the relevant function are made generic, because
they deal with both errors and warnings. No function bodies are changed,
because all the modifier methods are available on both `Diagnostic` and
`DiagnosticBuilder`.
fixes#117448
For example unnecessary imports in std::prelude that can be eliminated:
```rust
use std::option::Option::Some;//~ WARNING the item `Some` is imported redundantly
use std::option::Option::None; //~ WARNING the item `None` is imported redundantly
```
errors: only eagerly translate subdiagnostics
Subdiagnostics don't need to be lazily translated, they can always be eagerly translated. Eager translation is slightly more complex as we need to have a `DiagCtxt` available to perform the translation, which involves slightly more threading of that context.
This slight increase in complexity should enable later simplifications - like passing `DiagCtxt` into `AddToDiagnostic` and moving Fluent messages into the diagnostic structs rather than having them in separate files (working on that was what led to this change).
r? ```@nnethercote```
Subdiagnostics don't need to be lazily translated, they can always be
eagerly translated. Eager translation is slightly more complex as we need
to have a `DiagCtxt` available to perform the translation, which involves
slightly more threading of that context.
This slight increase in complexity should enable later simplifications -
like passing `DiagCtxt` into `AddToDiagnostic` and moving Fluent messages
into the diagnostic structs rather than having them in separate files
(working on that was what led to this change).
Signed-off-by: David Wood <david@davidtw.co>
Make sure `tcx.create_def` also depends on the forever red node, instead of just `tcx.at(span).create_def`
oversight from https://github.com/rust-lang/rust/pull/119136
Not actually an issue, because all uses of `tcx.create_def` were in the resolver, which is `eval_always`, but still good to harden against future uses of `create_def`
cc `@petrochenkov` `@WaffleLapkin`
- improve diagnostics of field uniqueness check and representation check
- simplify the implementation of field uniqueness check
- remove some useless codes and improvement neatness
Invert diagnostic lints.
That is, change `diagnostic_outside_of_impl` and `untranslatable_diagnostic` from `allow` to `deny`, because more than half of the compiler has been converted to use translated diagnostics.
This commit removes more `deny` attributes than it adds `allow` attributes, which proves that this change is warranted.
r? ````@davidtwco````
