Detect when user is trying to create a lending Iterator and give a custom explanation
#125407
Conversation
rustbot
added
S-waiting-on-review
| && def_id_matches_path( | ||
| self.r.tcx, | ||
| trait_id, | ||
| &["core", "iter", "traits", "iterator", "Iterator"], |
There was a problem hiding this comment.
Can this be extended to all non-generic associated types?
There was a problem hiding this comment.
Which part specifically?
There was a problem hiding this comment.
I think Jack is saying that this PR has been tailored to just Iterator (and Iterator::Item), and is then posing the thought exercise of whether the logic here (and the associated diagnostic output) could be generalized, so that it would cover all associated types lacking a parameter (i.e. "non-generic").
I.e., instead of specializing this check to trait Iterator { type Item; ... }, can it be extended to trait <AnyTraitNameHere> { type AnyAssociatedTypeHere; ... // but not GATs } ?
There was a problem hiding this comment.
Possibly, but I think this would be a separate diagnostic. I've seen people trying to implement lending iterators, I even tried to implement it myself years ago before I understood how it works :) So note about iterators is the main part and I want to keep it.
I can try to make the check more generic and add an error message about using non-GAT associated type.
There was a problem hiding this comment.
Rebased, added a better error message for general case of non-GAT associated type, left the error for iterators as is.
There was a problem hiding this comment.
And I'm open to any suggestions on wording/formatting/etc.
pacak
force-pushed
the
no-lending-iterators
branch
from
6651570
to
b70fb41
Compare
|
sorry for the delay. I agree that the proposed experiment, while potentially interesting, need not block landing this. @bors r+ rollup |
bors
added
S-waiting-on-bors
No worries. I did implemented more general case as well FWIW in addition to custom error for iterators. Are you okay with the wording/terminology? |
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#124746 (`rustc --explain E0582` additional example) - rust-lang#125407 (Detect when user is trying to create a lending `Iterator` and give a custom explanation) - rust-lang#125505 (Add intra-doc-links to rustc_middle crate-level docs.) - rust-lang#125792 (Don't drop `Unsize` candidate in intercrate mode) - rust-lang#125819 (Various `HirTyLowerer` cleanups) - rust-lang#125861 (rustc_codegen_ssa: fix `get_rpath_relative_to_output` panic when lib only contains file name) - rust-lang#125911 (delete bootstrap build before switching to bumped rustc) - rust-lang#125921 (coverage: Carve out hole spans in a separate early pass) - rust-lang#125940 (std::unix::fs::get_path: using fcntl codepath for netbsd instead.) - rust-lang#126022 (set `has_unconstrained_ty_var` when generalizing aliases in bivariant contexts) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#125407 - pacak:no-lending-iterators, r=pnkfelix Detect when user is trying to create a lending `Iterator` and give a custom explanation The scope for this diagnostic is to detect lending iterators specifically and it's main goal is to help beginners to understand that what they are trying to implement might not be possible for `Iterator` trait specifically. I ended up to changing the wording from originally proposed in the ticket because it might be misleading otherwise: `Data` might have a lifetime parameter but it can be unrelated to items user is planning to return. Fixes rust-lang#125337
The scope for this diagnostic is to detect lending iterators specifically and it's main goal is to help beginners to understand that what they are trying to implement might not be possible for
Iteratortrait specifically.I ended up to changing the wording from originally proposed in the ticket because it might be misleading otherwise:
Datamight have a lifetime parameter but it can be unrelated to items user is planning to return.Fixes #125337