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.

Sign up for GitHub

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

Jump to bottom

drop_in_place: weaken the claim of equivalence with drop(ptr.read()) #125739

Merged
merged 2 commits into from
May 30, 2024
Merged

drop_in_place: weaken the claim of equivalence with drop(ptr.read()) #125739

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c497cb
drop_in_place: weaken the claim of equivalence with drop(ptr.read())
RalfJung May 29, 2024
5c68a15
explain what the open questions are, and add a Miri test for that
RalfJung May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion library/core/src/ptr/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ mod mut_ptr;

/// Executes the destructor (if any) of the pointed-to value.
///
/// This is semantically equivalent to calling [`ptr::read`] and discarding
/// This is almost the same as calling [`ptr::read`] and discarding
/// the result, but has the following advantages:
Comment on lines +453 to 454
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be something like

Suggested change
/// This is almost the same as calling [`ptr::read`] and discarding
/// the result, but has the following advantages:
/// For most cases, this is equivalent to calling [`ptr::read`] and discarding
/// the result, but it is different in the following ways:

or is there a reason we should not prefer that more precise statement?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the discussion in #112015 -- it's not fully clear whether we really want it to be equivalent, e.g. when calling read on a *mut &T we definitely require the reference to be dereferenceable, but when calling drop_in_place maybe we don't.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm.

Can we include an internal comment here that line 453 is a bluff, then, and there are additional unresolved operational semantics questions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

somewhere around line 540, I think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a FIXME and a Miri test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Digging deep into histories is inevitable if someone wants full context, but it's nice to be able to see a plain-text hint in the source.

///
/// * It is *required* to use `drop_in_place` to drop unsized types like
Expand Down
Loading