-
Notifications
You must be signed in to change notification settings - Fork 12.3k
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.
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
refining_impl_trait
only fires on public traits
#119535
Labels
A-lint
Area: Lints (warnings about flaws in source code) such as unused_mut.
A-traits
Area: Trait system
F-refine
`#![feature(refine)]`; RFC #3245
F-return_position_impl_trait_in_trait
`#![feature(return_position_impl_trait_in_trait)]`
T-lang
Relevant to the language team, which will review and decide on the PR/issue.
Comments
rustbot
added
the
needs-triage
This issue may need triage. Remove it if it has been sufficiently triaged.
label
Jan 3, 2024
tmandry
added
A-lint
Area: Lints (warnings about flaws in source code) such as unused_mut.
A-traits
Area: Trait system
T-lang
Relevant to the language team, which will review and decide on the PR/issue.
I-lang-nominated
The issue / PR has been nominated for discussion during a lang team meeting.
F-return_position_impl_trait_in_trait
`#![feature(return_position_impl_trait_in_trait)]`
F-refine
`#![feature(refine)]`; RFC #3245
and removed
needs-triage
This issue may need triage. Remove it if it has been sufficiently triaged.
labels
Jan 3, 2024
@rustbot labels -I-lang-nominated We discussed this today in triage and developed a consensus to:
|
rustbot
removed
the
I-lang-nominated
The issue / PR has been nominated for discussion during a lang team meeting.
label
Jan 25, 2024
fmease
added a commit
to fmease/rust
that referenced
this issue
Mar 16, 2024
…-errors Split refining_impl_trait lint into _reachable, _internal variants As discussed in rust-lang#119535 (comment): > We discussed this today in triage and developed a consensus to: > > * Add a separate lint against impls that refine a return type defined with RPITIT even when the trait is not crate public. > * Place that in a lint group along with the analogous crate public lint. > * Create an issue to solicit feedback on these lints (or perhaps two separate ones). > * Have the warnings displayed with each lint reference this issue in a similar manner to how we do that today with the required `Self: '0'` bound on GATs. > * Make a note to review this feedback on 2-3 release cycles. This points users to rust-lang#121718 to leave feedback.
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this issue
Mar 17, 2024
Rollup merge of rust-lang#121720 - tmandry:split-refining, r=compiler-errors Split refining_impl_trait lint into _reachable, _internal variants As discussed in rust-lang#119535 (comment): > We discussed this today in triage and developed a consensus to: > > * Add a separate lint against impls that refine a return type defined with RPITIT even when the trait is not crate public. > * Place that in a lint group along with the analogous crate public lint. > * Create an issue to solicit feedback on these lints (or perhaps two separate ones). > * Have the warnings displayed with each lint reference this issue in a similar manner to how we do that today with the required `Self: '0'` bound on GATs. > * Make a note to review this feedback on 2-3 release cycles. This points users to rust-lang#121718 to leave feedback.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-lint
Area: Lints (warnings about flaws in source code) such as unused_mut.
A-traits
Area: Trait system
F-refine
`#![feature(refine)]`; RFC #3245
F-return_position_impl_trait_in_trait
`#![feature(return_position_impl_trait_in_trait)]`
T-lang
Relevant to the language team, which will review and decide on the PR/issue.
The
refining_impl_trait
lint only fires for public traits. It does not fire without thepub
keyword in the code sample below:Difference from
async_fn_in_trait
lintApparently I was part of the discussion of this at one point (see also the zulip topic on this). I think it got lumped together with the discussion about the lint for
async fn
in traits, though, when there are some important distinctions:async fn
lint is only temporary to help avoid footguns created by missing language features, and we want to make non-footgunny uses more convenient.The second point is important, because as a user I would expect such a fundamental mechanism to behave independently of whether the trait happens to be crate-public or not. This can lead to false expectations being created about the behavior in the other case.
Violating abstraction boundaries within a crate
As an example of the last point, let's say I as a user want to define a trait that my type implements ahead of actually generalizing my code:
Later on, I want to write a test for
all_windows
. But in order to do that, I have to change it to acceptimpl Application
, which requires changing the output type toimpl Window '_
, and possibly changing all the users ofall_windows
as well. This can get unwieldy quick.We can say that the user should have used
impl Trait
from the beginning, but that might be inconvenient when prototyping. If they are leaning on traits to provide the outlines of an abstraction boundary, we should let them opt in before punching through said boundary, IMO.cc @compiler-errors
The text was updated successfully, but these errors were encountered: