Repository navigation
stop adding dropck outlives requirements for [T; 0] #110288
Description
[T; 0] currently doesn't need drop glue by itself but still adds liveness requirements for T when used in a more complex value which already needs drop.
This behavior is identitical to PhantomData, which also does not need drop glue by itself but adds liveness requirements when used in values who do. I do not propose to change anything about PhantomData with this issue.
Example
struct PrintOnDrop<'s>(&'s str);
impl<'s> Drop for PrintOnDrop<'s> {
fn drop(&mut self) {
println!("{}", self.0);
}
}
fn to_array_zero<T>(_: T) -> [T; 0] {
[]
}
pub fn array_zero_by_itself() {
let mut x = [];
{
let s = String::from("temporary");
let p = PrintOnDrop(&s);
x = to_array_zero(p)
}
// implicitly dropping `x` here, no drop glue needed,
// so ignoring outlives requirements. NO ERROR
}
pub fn array_zero_in_tuple() {
let mut x = ([], String::new());
{
let s = String::from("temporary");
let p = PrintOnDrop(&s);
x.0 = to_array_zero(p)
}
// implicitly dropping `x` here, drop glue needed to drop the `String`,
// adding outlives requirements for the zero array. ERROR
}This behavior is confusing. I propose that we don't add any dropck outlives requirements for arrays of length zero. This allows strictly more code to compile, it may however result in unsoundness if people rely on this behavior to guarantee errors.
Activity
lcnr commentedon Apr 13, 2023
cc #103413 for more details about dropck
@rfcbot fcp merge
rfcbot commentedon Apr 13, 2023
Team member @lcnr has proposed to merge this. The next step is review by the rest of the tagged team members:
Concerns:
rushed-fcpresolved by stop adding dropck outlives requirements for[T; 0]#110288 (comment)we could instead stop specialcasingresolved by stop adding dropck outlives requirements for[T; 0]inneeds_drop[T; 0]#110288 (comment)Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
RalfJung commentedon Apr 13, 2023
How (if at all) does this interact with const generics? What if the length is unknown but could be zero? Seems surprising that behavior depends so heavily on whether the compiler can "figure out" the length.
lcnr commentedon Apr 14, 2023
This happens after type inference, so we should only fail to evaluate for generic constants. In this case they are always considered to be potentially non-zero.
yes, I considered to propose changing the behavior for arrays to always require drop glue, even for
N == 0because relying on the value of constants in this way is always a bit sketchy. The problem is that this is actually quite useful.I am a bit confused by why exactly this errors, but the following snippet would fail if we remove the special case for zero length arrays in
needs_dropopened #110322 if you want to take a deeper look at this. Afaict we never actually drop
[T; 0]here so this is the const check being too pessimistic (iirc using dataflow for that was too involved so we didn't end up stabilizing precise const drop).@rfcbot concern we could instead stop specialcasing
[T; 0]inneeds_dropRalfJung commentedon Apr 14, 2023
What probably happens is that
[]cannot get promoted any more because it looks like that would skip some drop glue. Presumably this also fails with your patch, saying the value does not live long enough?lcnr commentedon Apr 14, 2023
no, this still works 😅 apparently promotion does work. Had the same idea at first when seeing the test failures
RalfJung commentedon Apr 14, 2023
Hm, maybe promotion has its own knowledge that
[]never needs to run any drop glue...steffahn commentedon Apr 16, 2023
Promotion for arrays (unlike any other type) even supports mutable references, i.e.
&mut []promotes, so I wouldn’t be surprised if&[]hits some special case as well.lcnr commentedon Apr 18, 2023
@rfcbot resolve we could instead stop specialcasing
[T; 0]inneeds_dropdoing so results in too many regressions, see #110322
[-]`[T; 0]` adding outlives requirements to dropck[/-][+]drop adding outlives requirements to dropck for `[T; 0]`[/+]19 remaining items
rfcbot commentedon Aug 29, 2023
The final comment period, with a disposition to merge, as per the review above, is now complete.
As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.
This will be merged soon.
Rollup merge of rust-lang#128438 - Bryanskiy:empty-array-dropck, r=lcnr
Unrolled build for rust-lang#128438
lcnr commentedon Aug 1, 2024
implemented in #128438, we still need to update the documentation however, cc @Bryanskiy
SoniEx2 commentedon Aug 1, 2024
Rollup merge of rust-lang#128497 - Bryanskiy:fix-dropck-doc, r=lcnr
Rollup merge of rust-lang#128497 - Bryanskiy:fix-dropck-doc, r=lcnr
Unrolled build for rust-lang#128497
Rollup merge of #128497 - Bryanskiy:fix-dropck-doc, r=lcnr