Skip to content
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

Tracking issue for future-incompatibility lint deref_into_dyn_supertrait #89460

Open
crlf0710 opened this issue Oct 2, 2021 · 5 comments
Open
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-trait-objects Area: trait objects, vtable layout C-future-incompatibility Category: Future-incompatibility lints C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@crlf0710
Copy link
Member

crlf0710 commented Oct 2, 2021

What is this lint about

We're planning to add the dyn upcasting coercion language feature (see https://github.com/rust-lang/dyn-upcasting-coercion-initiative). Unfortunately this new coercion rule will take priority over certain other coercion rules, which will mean some behavior change. See #89190 for initial bug report.

How to fix this warning/error

Ensure your own Deref implementation is consistent with the trait_upcasting feature brings and allow this lint. Or avoid this code pattern altogether.

Current status

Implementation: #89461

@crlf0710 crlf0710 added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Oct 2, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Oct 7, 2021
…matsakis

Add `deref_into_dyn_supertrait` lint.

Initial implementation of rust-lang#89460. Resolves rust-lang#89190.
Maybe also worth a beta backport if necessary.

r? `@nikomatsakis`
@joshtriplett joshtriplett added the S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. label Jul 27, 2022
@joshtriplett
Copy link
Member

Could we get a more detailed summary of the case this lint is addressing? dyn upcasting / coercion seems to have made a lot of progress, but this aspect of it doesn't seem to have gotten much discussion that I recall.

@crlf0710
Copy link
Member Author

crlf0710 commented Jul 28, 2022

@joshtriplett Sure.

It is known that from user's perspective, coercions has "priorities", and dyn upcasting coercion has a higher priority than user defined Deref-based coercions. In the end, this coercion "shadows" a rare certain setup of user Deref implementation, which mimicks dyn upcasting coercion manually(see #89190 for an example).

For now we don't want to affect user's code, since dyn upcasting coercion might take a little while before it's stablized. Though, if user provided code decide to do other things in the Deref/DerefMut implementation, (even rarer, but possible, like running a hook or something), there will be a silent behavior change, when dyn upcasting gets stablized in the future. This lint emits a warning for this matter, to make sure that user doesn't write such code in their Deref/DerefMut implementations, then the future migration to dyn upcasting coercion will be smooth.

@crlf0710 crlf0710 added the C-future-incompatibility Category: Future-incompatibility lints label Sep 12, 2022
@compiler-errors
Copy link
Member

I'm curious -- why wasn't this implemented as a lint on the impl?

@crlf0710
Copy link
Member Author

@compiler-errors It could be implemented that way, and thinking about it from current view, that would be a better way (No removal of the lint is needed with that approach)

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Nov 24, 2022
…, r=compiler-errors

Make `deref_into_dyn_supertrait` lint the impl and not the usage

Proposed by `@compiler-errors` in rust-lang#89460 (comment)
r? `@crlf0710`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 24, 2022
…, r=compiler-errors

Make `deref_into_dyn_supertrait` lint the impl and not the usage

Proposed by ``@compiler-errors`` in rust-lang#89460 (comment)
r? ``@crlf0710``
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Dec 1, 2022
…ler-errors

Make `deref_into_dyn_supertrait` lint the impl and not the usage

Proposed by ``@compiler-errors`` in rust-lang/rust#89460 (comment)
r? ``@crlf0710``
@WaffleLapkin
Copy link
Member

For future reference: the lint change suggested by @compiler-errors was implemented in #104742 (I just tried to do that again because I forgor). The lint now lints against Deref impls for dyn Trait with type Target = dyn SupertraitOfTrait (instead of linting against the use of such Deref impl like it used to).

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 20, 2023
…ions, r=lcnr

Don't consider regions in `deref_into_dyn_supertrait` lint

I actually wonder if we should just warn on *any* deref impl with a target type that matches a supertrait by *def-id*.

cc rust-lang#89460

r? types
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 21, 2023
Rollup merge of rust-lang#118026 - compiler-errors:deref-into-dyn-regions, r=lcnr

Don't consider regions in `deref_into_dyn_supertrait` lint

I actually wonder if we should just warn on *any* deref impl with a target type that matches a supertrait by *def-id*.

cc rust-lang#89460

r? types
@fmease fmease changed the title Tracking Issue for deref_into_dyn_supertrait compatibility lint Tracking issue for future-incompatibility lint deref_into_dyn_supertrait Sep 14, 2024
@fmease fmease added the A-trait-objects Area: trait objects, vtable layout label Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-trait-objects Area: trait objects, vtable layout C-future-incompatibility Category: Future-incompatibility lints C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants