Forbid freely casting lifetime bounds of dyn-types #136776
+634
−22
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1106,15 +1106,23 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
| self.prove_predicate( | ||
| ty::ClauseKind::WellFormed(src_ty.into()), | ||
| location.to_locations(), | ||
| ConstraintCategory::Cast { | ||
| is_raw_ptr_dyn_type_cast: false, | ||
| is_implicit_coercion, | ||
| unsize_to: None, | ||
| }, | ||
| ); | ||
|
|
||
| let src_ty = self.normalize(src_ty, location); | ||
| if let Err(terr) = self.sub_types( | ||
| src_ty, | ||
| *ty, | ||
| location.to_locations(), | ||
| ConstraintCategory::Cast { | ||
| is_raw_ptr_dyn_type_cast: false, | ||
| is_implicit_coercion, | ||
| unsize_to: None, | ||
| }, | ||
| ) { | ||
| span_mirbug!( | ||
| self, | ||
|
|
@@ -1135,7 +1143,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
| self.prove_predicate( | ||
| ty::ClauseKind::WellFormed(src_ty.into()), | ||
| location.to_locations(), | ||
| ConstraintCategory::Cast { | ||
| is_raw_ptr_dyn_type_cast: false, | ||
| is_implicit_coercion, | ||
| unsize_to: None, | ||
| }, | ||
| ); | ||
|
|
||
| // The type that we see in the fcx is like | ||
|
|
@@ -1148,7 +1160,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
| src_ty, | ||
| *ty, | ||
| location.to_locations(), | ||
| ConstraintCategory::Cast { | ||
| is_raw_ptr_dyn_type_cast: false, | ||
| is_implicit_coercion, | ||
| unsize_to: None, | ||
| }, | ||
| ) { | ||
| span_mirbug!( | ||
| self, | ||
|
|
@@ -1177,7 +1193,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
| ty_fn_ptr_from, | ||
| *ty, | ||
| location.to_locations(), | ||
| ConstraintCategory::Cast { | ||
| is_raw_ptr_dyn_type_cast: false, | ||
| is_implicit_coercion, | ||
| unsize_to: None, | ||
| }, | ||
| ) { | ||
| span_mirbug!( | ||
| self, | ||
|
|
@@ -1210,7 +1230,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
| ty_fn_ptr_from, | ||
| *ty, | ||
| location.to_locations(), | ||
| ConstraintCategory::Cast { | ||
| is_raw_ptr_dyn_type_cast: false, | ||
| is_implicit_coercion, | ||
| unsize_to: None, | ||
| }, | ||
| ) { | ||
| span_mirbug!( | ||
| self, | ||
|
|
@@ -1239,6 +1263,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
| trait_ref, | ||
| location.to_locations(), | ||
| ConstraintCategory::Cast { | ||
| is_raw_ptr_dyn_type_cast: false, | ||
| is_implicit_coercion, | ||
| unsize_to: Some(unsize_to), | ||
| }, | ||
|
|
@@ -1264,7 +1289,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
| *ty_from, | ||
| *ty_to, | ||
| location.to_locations(), | ||
| ConstraintCategory::Cast { | ||
| is_raw_ptr_dyn_type_cast: false, | ||
| is_implicit_coercion, | ||
| unsize_to: None, | ||
| }, | ||
| ) { | ||
| span_mirbug!( | ||
| self, | ||
|
|
@@ -1327,7 +1356,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
| *ty_elem, | ||
| *ty_to, | ||
| location.to_locations(), | ||
| ConstraintCategory::Cast { | ||
| is_raw_ptr_dyn_type_cast: false, | ||
| is_implicit_coercion, | ||
| unsize_to: None, | ||
| }, | ||
| ) { | ||
| span_mirbug!( | ||
| self, | ||
|
|
@@ -1484,11 +1517,12 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
| trait_ref, | ||
| location.to_locations(), | ||
| ConstraintCategory::Cast { | ||
| is_raw_ptr_dyn_type_cast: false, | ||
| is_implicit_coercion: true, | ||
| unsize_to: None, | ||
| }, | ||
| ); | ||
| } else if let ty::Dynamic(src_tty, src_lt) = | ||
| *self.struct_tail(src.ty, location).kind() | ||
| && let ty::Dynamic(dst_tty, dst_lt) = | ||
| *self.struct_tail(dst.ty, location).kind() | ||
|
|
@@ -1503,15 +1537,13 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
| // Debug`) are in `rustc_hir_typeck`. | ||
|
|
||
| // Remove auto traits. | ||
| // Auto trait checks are handled in `rustc_hir_typeck`. | ||
| let src_obj = Ty::new_dynamic( | ||
| tcx, | ||
| tcx.mk_poly_existential_predicates( | ||
| &src_tty.without_auto_traits().collect::<Vec<_>>(), | ||
| ), | ||
| src_lt, | ||
| ); | ||
| let dst_obj = Ty::new_dynamic( | ||
| tcx, | ||
|
|
@@ -1523,16 +1555,47 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
|
|
||
| debug!(?src_tty, ?dst_tty, ?src_obj, ?dst_obj); | ||
|
|
||
| // Trait parameters are Invariant, the only part that actually has | ||
| // subtyping here is the lifetime bound of the dyn-type. | ||
| // | ||
| // For example in `dyn Trait<'a> + 'b <: dyn Trait<'c> + 'd` we would | ||
| // require that `'a == 'c` but only that `'b: 'd`. | ||
| // | ||
| // We must not allow freely casting lifetime bounds of dyn-types as it | ||
| // may allow for inaccessible VTable methods being callable: #136702 | ||
| self.sub_types( | ||
| src_obj, | ||
| dst_obj, | ||
| location.to_locations(), | ||
| ConstraintCategory::Cast { | ||
| is_raw_ptr_dyn_type_cast: true, | ||
| is_implicit_coercion: false, | ||
| unsize_to: None, | ||
| }, | ||
| ) | ||
| .unwrap(); | ||
| } else if let ty::Dynamic(src_tty, src_lt) = | ||
| *self.struct_tail(src.ty, location).kind() | ||
| && let ty::Dynamic(dst_tty, dst_lt) = | ||
| *self.struct_tail(dst.ty, location).kind() | ||
| && src_tty.principal().is_none() | ||
| && dst_tty.principal().is_none() | ||
|
Comment on lines
+1581
to
+1582
Member
Author
There was a problem hiding this comment. @Darksonn I think this currently means that casting something like:
Contributor
There was a problem hiding this comment. Could you help come up with a test that catches this? I tried adding a few, but I got the results I expected.
Member
Author
There was a problem hiding this comment. gotta say I would have expected those tests to not be emitting errors right now and can't figure out just from skimming the code why they wouldn't be 🤔
Member
Author
There was a problem hiding this comment. ah, it just happens to work due to MIR lowering jank. doing this kind of cast seems to result in both an
Member
Author
There was a problem hiding this comment. fn unprincipled3_static<'a>(x: *mut (dyn Trait + Send + 'a)) -> *mut (dyn Send + 'static) {
x as _
}fn unprincipled3_static(_1: *mut dyn Trait + Send) -> *mut dyn Send {
debug x => _1; // in scope 0 at ./qux.rs:2:29: 2:30
let mut _0: *mut dyn std::marker::Send; // return place in scope 0 at ./qux.rs:2:65: 2:90
let mut _2: *mut dyn std::marker::Send; // in scope 0 at ./qux.rs:3:5: 3:11
let mut _3: *mut dyn std::marker::Send; // in scope 0 at ./qux.rs:3:5: 3:11
let mut _4: *mut dyn std::marker::Send; // in scope 0 at ./qux.rs:3:5: 3:11
let mut _5: *mut dyn Trait + std::marker::Send; // in scope 0 at ./qux.rs:3:5: 3:6
_5 = copy _1; // scope 0 at ./qux.rs:3:5: 3:6
_4 = move _5 as *mut dyn std::marker::Send (PointerCoercion(Unsize, AsCast)); // scope 0 at ./qux.rs:3:5: 3:11
StorageDead(_5); // scope 0 at ./qux.rs:3:5: 3:6
_3 = move _4 as *mut dyn std::marker::Send (PtrToPtr); // scope 0 at ./qux.rs:3:5: 3:11
AscribeUserType(_3, o, UserTypeProjection { base: UserType(0), projs: [] }); // scope 0 at ./qux.rs:3:10: 3:11
_2 = copy _3; // scope 0 at ./qux.rs:3:5: 3:11
StorageDead(_4); // scope 0 at ./qux.rs:3:10: 3:11
_0 = move _2 as *mut dyn std::marker::Send (PointerCoercion(Unsize, Implicit)); // scope 0 at ./qux.rs:3:5: 3:11this is the mir from that test case. note the 3 separate pointer casts, one of which is a PtrToPtr between two pointers to
Member
Author
There was a problem hiding this comment. Could ICE on encountering Alternatively we just write the code to handle (Some, None) properly and then hope it's correct without a test case
Contributor
There was a problem hiding this comment. Do you expect uncovered
Contributor
There was a problem hiding this comment. It makes sense that it happens in the coercion. After all, ptr-to-ptr casts are only possible in cases where the vtable is the same, but I would not expect |
||
| { | ||
| // The principalless (no non-auto traits) case: | ||
| // You can only cast `dyn Send + 'long` to `dyn Send + 'short`. | ||
| self.constraints.outlives_constraints.push(OutlivesConstraint { | ||
| sup: src_lt.as_var(), | ||
| sub: dst_lt.as_var(), | ||
| locations: location.to_locations(), | ||
| span: location.to_locations().span(self.body), | ||
| category: ConstraintCategory::Cast { | ||
| is_raw_ptr_dyn_type_cast: true, | ||
| is_implicit_coercion: false, | ||
| unsize_to: None, | ||
| }, | ||
| variance_info: ty::VarianceDiagInfo::default(), | ||
| from_closure: false, | ||
| }); | ||
| } | ||
| } | ||
| CastKind::Transmute => { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| //@ check-pass | ||
|
|
||
| trait Trait { | ||
| fn foo(&self) {} | ||
| } | ||
|
|
||
| fn bar<'a>(a: *mut *mut (dyn Trait + 'a)) -> *mut *mut (dyn Trait + 'static) { | ||
| a as _ | ||
| } | ||
|
|
||
| fn main() {} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| error: lifetime may not live long enough | ||
| --> $DIR/ptr-to-ptr-different-regions.rs:17:5 | ||
| | | ||
| LL | fn assert_static<'a>(ptr: *mut (dyn Trait + 'a)) -> *mut (dyn Trait + 'static) { | ||
| | -- lifetime `'a` defined here | ||
| LL | ptr as _ | ||
| | ^^^^^^^^ returning this value requires that `'a` must outlive `'static` | ||
| | | ||
| = note: requirement occurs because of a mutable pointer to `dyn Trait` | ||
| = note: mutable pointers are invariant over their type parameter | ||
| = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance | ||
| note: raw pointer casts of trait objects do not cast away lifetimes | ||
| --> $DIR/ptr-to-ptr-different-regions.rs:17:5 | ||
| | | ||
| LL | ptr as _ | ||
| | ^^^^^^^^ | ||
| = note: this was previously accepted by the compiler but was changed recently | ||
| = help: see <https://github.com/rust-lang/rust/issues/141402> for more information | ||
| help: consider changing the trait object's explicit `'static` bound to the lifetime of argument `ptr` | ||
| | | ||
| LL - fn assert_static<'a>(ptr: *mut (dyn Trait + 'a)) -> *mut (dyn Trait + 'static) { | ||
| LL + fn assert_static<'a>(ptr: *mut (dyn Trait + 'a)) -> *mut (dyn Trait + 'a) { | ||
| | | ||
| help: alternatively, add an explicit `'static` bound to this reference | ||
| | | ||
| LL - fn assert_static<'a>(ptr: *mut (dyn Trait + 'a)) -> *mut (dyn Trait + 'static) { | ||
| LL + fn assert_static<'a>(ptr: *mut (dyn Trait + 'static)) -> *mut (dyn Trait + 'static) { | ||
| | | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| trait Trait { | ||
| fn foo(&self) {} | ||
| } | ||
|
|
||
| struct MyWrap<T: ?Sized>(T); | ||
|
|
||
| fn bar<'a>(a: *mut MyWrap<(dyn Trait + 'a)>) -> *mut MyWrap<(dyn Trait + 'static)> { | ||
| a as _ | ||
| //~^ ERROR: lifetime may not live long enough | ||
| } | ||
|
|
||
| fn main() {} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| error: lifetime may not live long enough | ||
| --> $DIR/ptr-to-ptr-indirect-different-regions.rs:8:5 | ||
| | | ||
| LL | fn bar<'a>(a: *mut MyWrap<(dyn Trait + 'a)>) -> *mut MyWrap<(dyn Trait + 'static)> { | ||
| | -- lifetime `'a` defined here | ||
| LL | a as _ | ||
| | ^^^^^^ returning this value requires that `'a` must outlive `'static` | ||
| | | ||
| = note: requirement occurs because of a mutable pointer to `MyWrap<dyn Trait>` | ||
| = note: mutable pointers are invariant over their type parameter | ||
| = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance | ||
| note: raw pointer casts of trait objects do not cast away lifetimes | ||
| --> $DIR/ptr-to-ptr-indirect-different-regions.rs:8:5 | ||
| | | ||
| LL | a as _ | ||
| | ^^^^^^ | ||
| = note: this was previously accepted by the compiler but was changed recently | ||
| = help: see <https://github.com/rust-lang/rust/issues/141402> for more information | ||
| help: consider changing the trait object's explicit `'static` bound to the lifetime of argument `a` | ||
| | | ||
| LL - fn bar<'a>(a: *mut MyWrap<(dyn Trait + 'a)>) -> *mut MyWrap<(dyn Trait + 'static)> { | ||
| LL + fn bar<'a>(a: *mut MyWrap<(dyn Trait + 'a)>) -> *mut MyWrap<(dyn Trait + 'a)> { | ||
| | | ||
| help: alternatively, add an explicit `'static` bound to this reference | ||
| | | ||
| LL - fn bar<'a>(a: *mut MyWrap<(dyn Trait + 'a)>) -> *mut MyWrap<(dyn Trait + 'static)> { | ||
| LL + fn bar<'a>(a: *mut MyWrap<(dyn Trait + 'static)>) -> *mut MyWrap<(dyn Trait + 'static)> { | ||
| | | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
maybe? "cast away" sounds weird
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.
"must not" sounds like "you can do it but you shouldn't". How about "can't extend lifetimes"?
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.
I learned in school that "must not" means "is not allowed to". But maybe it doesn't universally have that meaning?
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.
Well... It does mean that it's not allowed to do it. But to me "not allowed to" brings forward the wrong connotations compared to "can't"/"not able to"/"not possible" in the specific case of a compiler error message.