-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Don't consider generic args of supertrait in deref_into_dyn_supertrait
lint
#118026
Don't consider generic args of supertrait in deref_into_dyn_supertrait
lint
#118026
Conversation
I think matching super trait by |
bea815b
to
816d811
Compare
example breakage with #![deny(deref_into_dyn_supertrait)]
#![feature(trait_upcasting)] // remove this and the test compiles
use std::ops::Deref;
trait Bar<T> {}
impl<T, U> Bar<U> for T {}
trait Foo: Bar<i32> {
fn as_dyn_bar_u32<'a>(&self) -> &(dyn Bar<u32> + 'a);
}
impl Foo for () {
fn as_dyn_bar_u32<'a>(&self) -> &(dyn Bar<u32> + 'a) {
self
}
}
impl<'a> Deref for dyn Foo + 'a {
//~^ ERROR `dyn Foo` implements `Deref<Target = dyn Bar<u32>>` which conflicts with supertrait `Bar<i32>`
//~| WARN this was previously accepted by the compiler
type Target = dyn Bar<u32> + 'a;
fn deref(&self) -> &Self::Target {
self.as_dyn_bar_u32()
}
}
fn take_dyn<T>(x: &dyn Bar<T>) -> T {
todo!()
}
fn main() {
let x: &dyn Foo = &();
let y = take_dyn(x);
let z: u32 = y;
} |
@@ -9,6 +9,7 @@ LL | type Target = dyn A; | |||
| | |||
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should use FutureIncompatibilityReason::FutureReleaseSemanticsChange
instead of FutureReleaseErrorDontReportInDeps
for this lint. Can you change that in this PR?
r=me after adding the additional behavior test and dealing with the other suggestion |
816d811
to
e6ca8e1
Compare
@bors r=lcnr |
@bors rollup |
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#117327 (Add documentation for some queries) - rust-lang#117835 (Note about object lifetime defaults in does not live long enough error) - rust-lang#117851 (Uplift `InferConst` to `rustc_type_ir`) - rust-lang#117973 (test: Add test for async-move in 2015 Rust proc macro) - rust-lang#117992 (Don't require intercrate mode for negative coherence) - rust-lang#118010 (Typeck break expr even if break is illegal) - rust-lang#118026 (Don't consider regions in `deref_into_dyn_supertrait` lint) - rust-lang#118089 (intercrate_ambiguity_causes: handle self ty infer + reservation impls) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#117327 (Add documentation for some queries) - rust-lang#117835 (Note about object lifetime defaults in does not live long enough error) - rust-lang#117851 (Uplift `InferConst` to `rustc_type_ir`) - rust-lang#117973 (test: Add test for async-move in 2015 Rust proc macro) - rust-lang#117992 (Don't require intercrate mode for negative coherence) - rust-lang#118010 (Typeck break expr even if break is illegal) - rust-lang#118026 (Don't consider regions in `deref_into_dyn_supertrait` lint) - rust-lang#118089 (intercrate_ambiguity_causes: handle self ty infer + reservation impls) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#117327 (Add documentation for some queries) - rust-lang#117835 (Note about object lifetime defaults in does not live long enough error) - rust-lang#117851 (Uplift `InferConst` to `rustc_type_ir`) - rust-lang#117973 (test: Add test for async-move in 2015 Rust proc macro) - rust-lang#117992 (Don't require intercrate mode for negative coherence) - rust-lang#118010 (Typeck break expr even if break is illegal) - rust-lang#118026 (Don't consider regions in `deref_into_dyn_supertrait` lint) - rust-lang#118089 (intercrate_ambiguity_causes: handle self ty infer + reservation impls) r? `@ghost` `@rustbot` modify labels: rollup
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
Holy shit lol, I definitely pushed the wrong commits here. I'm dumb. |
…ions, r=lcnr Rework supertrait lint once again I accidentally pushed the wrong commits because I totally didn't check I was on the right computer when updating rust-lang#118026. Sorry, this should address all the nits in rust-lang#118026. r? lcnr
Rollup merge of rust-lang#118146 - compiler-errors:deref-into-dyn-regions, r=lcnr Rework supertrait lint once again I accidentally pushed the wrong commits because I totally didn't check I was on the right computer when updating rust-lang#118026. Sorry, this should address all the nits in rust-lang#118026. r? lcnr
deref_into_dyn_supertrait
lintderef_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 #89460
r? types