Skip to content

Don't suggest the use of impl Trait in closure parameter #138998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 11, 2025
Merged

Don't suggest the use of impl Trait in closure parameter #138998

merged 1 commit into from
Apr 11, 2025

Conversation

rperier
Copy link
Contributor

@rperier rperier commented Mar 26, 2025 •
edited
Loading

Fixes #138932

@rustbot
Copy link
Collaborator

rustbot commented Mar 26, 2025

r? @Noratrieb

rustbot has assigned @Noratrieb.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 26, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rperier rperier force-pushed the donot_suggest_to_use_impl_trait_in_closure_params branch from 133718c to d280652 Compare March 27, 2025 17:46
@rustbot rustbot added A-tidy Area: The tidy tool T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Mar 27, 2025
@rperier
Copy link
Contributor Author

rperier commented Apr 4, 2025

r? rust-lang/diagnostics

@rustbot rustbot assigned estebank and unassigned Noratrieb Apr 4, 2025
mejrs
mejrs reviewed Apr 8, 2025
View reviewed changes
Copy link
Contributor

@mejrs mejrs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Regarding the naming of the file, please don't name it issue-### , but something descriptive like dont-suggest-impl-as-closure-arg.rs.

@rperier
Suggest the use of impl Trait in function parameter only
8b6ff4a 
Currently in case of a Trait object in closure parameter, the compiler
suggests either to use a reference, which is correct or to use an
`impl Trait` which is not. Do not emit this suggestion when the parameter
is part of a closure.
@rperier rperier force-pushed the donot_suggest_to_use_impl_trait_in_closure_params branch from d280652 to 8b6ff4a Compare April 9, 2025 07:23
@rperier
Copy link
Contributor Author

rperier commented Apr 9, 2025

Fixed and rebased onto master. Do I need to split into two commits ? One for the tests and one for the fix ? Regarding the git history some people do it some other don't. The maintainer has the last word :) . What do you prefer ?

@mejrs
Copy link
Contributor

mejrs commented Apr 9, 2025

Do I need to split into two commits ? One for the tests and one for the fix ?

that's just my personal preference, it is not necessary

@Noratrieb
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 10, 2025

📌 Commit 8b6ff4a has been approved by Noratrieb

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 10, 2025
@Zalathar Zalathar mentioned this pull request Apr 11, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 11, 2025
@bors
Auto merge of rust-lang#139657 - Zalathar:rollup-6oh6f9q, r=Zalathar
18a029c 
Rollup of 12 pull requests

Successful merges:

 - rust-lang#137447 (add `core::intrinsics::simd::{simd_extract_dyn, simd_insert_dyn}`)
 - rust-lang#138182 (rustc_target: update x86_win64 to match the documented calling convention for f128)
 - rust-lang#138682 (Allow drivers to supply a list of extra symbols to intern)
 - rust-lang#138904 (Test linking and running `no_std` binaries)
 - rust-lang#138998 (Don't suggest the use of  `impl Trait` in closure parameter)
 - rust-lang#139447 (doc changes: debug assertions -> overflow checks)
 - rust-lang#139469 (Introduce a `//@ needs-crate-type` compiletest directive)
 - rust-lang#139564 (Deeply normalize obligations in `BestObligation` folder)
 - rust-lang#139574 (bootstrap: improve `channel` handling)
 - rust-lang#139600 (Update `compiler-builtins` to 0.1.153)
 - rust-lang#139641 (Allow parenthesis around inferred array lengths)
 - rust-lang#139654 (Improve `AssocItem::descr`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 573ebf0 into rust-lang:master Apr 11, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 11, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 11, 2025
@rust-timer
Unrolled build for rust-lang#138998
fc8a2f2 
Rollup merge of rust-lang#138998 - rperier:donot_suggest_to_use_impl_trait_in_closure_params, r=Noratrieb

Don't suggest the use of  `impl Trait` in closure parameter

Fixes rust-lang#138932
@rperier rperier deleted the donot_suggest_to_use_impl_trait_in_closure_params branch April 12, 2025 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@mejrs mejrs mejrs left review comments

Reviewers whose approvals may not affect merge requirements
Assignees

@estebank estebank

Labels
A-tidy Area: The tidy tool S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.88.0
Development

Successfully merging this pull request may close these issues.

rustc suggests to use impl trait in closure parameter which doesn't work
7 participants
@rperier @rustbot @rust-log-analyzer @mejrs @Noratrieb @bors @estebank