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

Update partially initialized values in drop documentation. #648

Merged
merged 2 commits into from
Aug 7, 2019

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Aug 6, 2019

As part of rust-lang/rust#63230, it does not appear to be possible to create a partially initialized value.

cc @RalfJung @tmandry Please correct me if I'm wrong, I am unfamiliar with this. I was planning to do an update today, but can't with breakage.

@@ -2,9 +2,7 @@

When an [initialized] [variable] in Rust goes out of scope or a [temporary]
is no longer needed its _destructor_ is run. [Assignment] also runs the
destructor of its left-hand operand, unless it's an uninitialized variable. If a
[struct] variable has been partially initialized, only its initialized fields
are dropped.
Copy link
Contributor

Choose a reason for hiding this comment

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

We can still do:

struct ND(u8);

impl Drop for ND {
    fn drop(&mut self) {
        dbg!(self.0);
    }
}

struct Prod(ND, ND, ND);

fn main() {
    let mut prod = Prod(ND(0), ND(1), ND(2));
    core::mem::forget(prod.0);
    // prod.0 is not initialized here.
}

===>

[src/main.rs:5] self.0 = 1
[src/main.rs:5] self.0 = 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. So how about just updating the example to do that? I just pushed a change, with the wording from #514.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or is a "partial move" different from a partially initialized value?

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to me that "partial move" is something that happens and "partially initialized" is a state of being that can result from a "partial move".

cc @RalfJung @matthewjasper

Copy link
Member

Choose a reason for hiding this comment

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

The corresponding adjective would be "partially moved out of", I guess.

But as a starter, I don't see how removing struct here helps with anything. If we (re)spin "partially initialized" to mean "partially moved out of", this is still about structs (and tuples), like before.

However, I think it would be better to change the wording. or do we consistently call moved-out-of variables "uninitialized"?

Copy link
Contributor

Choose a reason for hiding this comment

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

However, I think it would be better to change the wording. or do we consistently call moved-out-of variables "uninitialized"?

I'm not sure. I'm not too keen on a particular wording as long as the content is there (which it is).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only removed the word "struct" to avoid confusion (and that's what #514 did), since it could be a struct, enum, tuple, or whatever. Since the example uses a tuple, it seemed better to not be so specific.

I think it would be good to be clearer about this, but I also think #514 would be the place to discuss and debate the specifics. I'd like to keep this PR focused on the minimum to get the tests to pass. Does the new example make sense? Should it use a different wording? Or should it just be removed?

Copy link
Member

Choose a reason for hiding this comment

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

I'd like to keep this PR focused on the minimum to get the tests to pass.

Then please don't make unrelated wording changes. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair, I removed it.

@ehuss ehuss force-pushed the partially-initialized-drop branch from e3966e5 to c85e5a7 Compare August 6, 2019 13:49
@ehuss ehuss changed the title Remove partially initialized values from drop documentation. Update partially initialized values in drop documentation. Aug 6, 2019
@ehuss ehuss force-pushed the partially-initialized-drop branch from c85e5a7 to 8eb4f09 Compare August 6, 2019 13:52
let mut partially_initialized: (ShowOnDrop, ShowOnDrop);
partially_initialized.0 = ShowOnDrop("Partial tuple first");
let mut partially_initialized = (ShowOnDrop("one"), ShowOnDrop("two"));
core::mem::forget(partially_initialized.1);
Copy link
Member

Choose a reason for hiding this comment

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

Just above there is a move example that actually drops early by moving, so maybe the same would make sense here?

// drops now, but second field (.1) is then uninitialized
partially_initialized.1;
// first field (.0) implicitly dropped.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah that's probably better stylistically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the explicitness of calling forget, since if you run the example you can actually see how it only drops the remaining fields. What about the following text to be more explicit (and to match the wording in rustc):

    // After a partial move, only the remaining fields are dropped.
    let mut partial_move = (ShowOnDrop("first"), ShowOnDrop("forgotten"));
    // Perform a partial move, leaving `partial_move.0` initialized.
    core::mem::forget(partial_move.1);
    // When `partial_move` scope ends, only the first field is dropped.

Copy link
Contributor

Choose a reason for hiding this comment

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

Some wording nits: "leaving" -> "leaving only"

"partial_move" -> "partial_move's"

Aside from that I think it is good.
Imo we can land this and iterate on style :)

@ehuss ehuss force-pushed the partially-initialized-drop branch from 8eb4f09 to 663b0fb Compare August 6, 2019 17:55
@Centril Centril merged commit b4b3536 into rust-lang:master Aug 7, 2019
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Aug 7, 2019
Update books

## nomicon

1 commits in b7f0aba2f88a8feade70595efcfa3438e54e96c0..8a7d05615e5bc0a7fb961b4919c44f5221ee54da
2019-07-11 15:11:36 -0400 to 2019-08-07 07:46:59 -0500
- s/railguard/guardrail/ (rust-lang/nomicon#156)

## reference

5 commits in 8e7d614..b4b3536
2019-07-16 21:02:33 +0100 to 2019-08-07 02:29:50 +0200
- Update partially initialized values in drop documentation. (rust-lang/reference#648)
- Define sound and unsound (rust-lang/reference#647)
- Fix a type in the modules section: functions => modules (rust-lang/reference#645)
- Fix some links. (rust-lang/reference#642)
- Update recursion_limit default limit (rust-lang/reference#633)

## rust-by-example

14 commits in e3679e214d8db44586aca9b20aa27517007d1923..f2c15ba5ee89ae9469a2cf60494977749901d764
2019-07-15 11:13:44 -0300 to 2019-08-07 10:14:25 -0300
- Remove redundant semicolons (rust-lang/rust-by-example#1239)
- Rename "Read Lines" chapter title (rust-lang/rust-by-example#1230)
- Added space between word and inline code block in unit_testing.md (rust-lang/rust-by-example#1237)
- [typo] fix unit_testing wrong output (rust-lang/rust-by-example#1210)
- flow_control/match/binding.md: `...' -> `..=' (rust-lang/rust-by-example#1233)
- generics/impl.md: follow rustfmt style (rust-lang/rust-by-example#1236)
- freeze.md: Incorrect example (rust-lang/rust-by-example#1226)
- Make `point` consistent (rust-lang/rust-by-example#1229)
- Fix typo at error -> panic (rust-lang/rust-by-example#1227)
- Snake didn't deserve to die 🐍 (rust-lang/rust-by-example#1228)
- Reorder links in destructuring.md (rust-lang/rust-by-example#1225)
- Rename variable names for consistent in iter_result.md (rust-lang/rust-by-example#1224)
- Fix several shell output and code highlights. (rust-lang/rust-by-example#1222)
- Add new example for Rc. (rust-lang/rust-by-example#1223)

## edition-guide

5 commits in f6c8b92d4e63edd28e862be952f33861f35956f8..e58bc4ca104e890ac56af846877c874c432a64b5
2019-07-06 22:10:32 +0200 to 2019-07-31 20:14:12 +0200
- Hide extraneous `use` in anonymous lifetime example. (rust-lang/edition-guide#190)
- Attempt to clarify "no more mod.rs". (rust-lang/edition-guide#187)
- Remove -preview for rustup components. (rust-lang/edition-guide#188)
- rust-lang/edition-guide#184 More clear explanation and Title. (rust-lang/edition-guide#185)
- More clear table headers (rust-lang/edition-guide#186)

## embedded-book

2 commits in ff334e74fdb9f197e621efa6d7c3105be892e888..c5da1e11915d3f28266168baaf55822f7e3fe999
2019-07-16 13:47:34 +0000 to 2019-08-05 23:02:10 +0000
- install/verify: fix next section link  (rust-embedded/book#202)
- Syst small fix  (rust-embedded/book#198)
Centril added a commit to Centril/rust that referenced this pull request Aug 8, 2019
Update books

## nomicon

1 commits in b7f0aba2f88a8feade70595efcfa3438e54e96c0..8a7d05615e5bc0a7fb961b4919c44f5221ee54da
2019-07-11 15:11:36 -0400 to 2019-08-07 07:46:59 -0500
- s/railguard/guardrail/ (rust-lang/nomicon#156)

## reference

5 commits in 8e7d614..b4b3536
2019-07-16 21:02:33 +0100 to 2019-08-07 02:29:50 +0200
- Update partially initialized values in drop documentation. (rust-lang/reference#648)
- Define sound and unsound (rust-lang/reference#647)
- Fix a type in the modules section: functions => modules (rust-lang/reference#645)
- Fix some links. (rust-lang/reference#642)
- Update recursion_limit default limit (rust-lang/reference#633)

## rust-by-example

14 commits in e3679e214d8db44586aca9b20aa27517007d1923..f2c15ba5ee89ae9469a2cf60494977749901d764
2019-07-15 11:13:44 -0300 to 2019-08-07 10:14:25 -0300
- Remove redundant semicolons (rust-lang/rust-by-example#1239)
- Rename "Read Lines" chapter title (rust-lang/rust-by-example#1230)
- Added space between word and inline code block in unit_testing.md (rust-lang/rust-by-example#1237)
- [typo] fix unit_testing wrong output (rust-lang/rust-by-example#1210)
- flow_control/match/binding.md: `...' -> `..=' (rust-lang/rust-by-example#1233)
- generics/impl.md: follow rustfmt style (rust-lang/rust-by-example#1236)
- freeze.md: Incorrect example (rust-lang/rust-by-example#1226)
- Make `point` consistent (rust-lang/rust-by-example#1229)
- Fix typo at error -> panic (rust-lang/rust-by-example#1227)
- Snake didn't deserve to die 🐍 (rust-lang/rust-by-example#1228)
- Reorder links in destructuring.md (rust-lang/rust-by-example#1225)
- Rename variable names for consistent in iter_result.md (rust-lang/rust-by-example#1224)
- Fix several shell output and code highlights. (rust-lang/rust-by-example#1222)
- Add new example for Rc. (rust-lang/rust-by-example#1223)

## edition-guide

5 commits in f6c8b92d4e63edd28e862be952f33861f35956f8..e58bc4ca104e890ac56af846877c874c432a64b5
2019-07-06 22:10:32 +0200 to 2019-07-31 20:14:12 +0200
- Hide extraneous `use` in anonymous lifetime example. (rust-lang/edition-guide#190)
- Attempt to clarify "no more mod.rs". (rust-lang/edition-guide#187)
- Remove -preview for rustup components. (rust-lang/edition-guide#188)
- rust-lang/edition-guide#184 More clear explanation and Title. (rust-lang/edition-guide#185)
- More clear table headers (rust-lang/edition-guide#186)

## embedded-book

2 commits in ff334e74fdb9f197e621efa6d7c3105be892e888..c5da1e11915d3f28266168baaf55822f7e3fe999
2019-07-16 13:47:34 +0000 to 2019-08-05 23:02:10 +0000
- install/verify: fix next section link  (rust-embedded/book#202)
- Syst small fix  (rust-embedded/book#198)
Centril added a commit to Centril/rust that referenced this pull request Aug 8, 2019
Update books

## nomicon

1 commits in b7f0aba2f88a8feade70595efcfa3438e54e96c0..8a7d05615e5bc0a7fb961b4919c44f5221ee54da
2019-07-11 15:11:36 -0400 to 2019-08-07 07:46:59 -0500
- s/railguard/guardrail/ (rust-lang/nomicon#156)

## reference

5 commits in 8e7d614..b4b3536
2019-07-16 21:02:33 +0100 to 2019-08-07 02:29:50 +0200
- Update partially initialized values in drop documentation. (rust-lang/reference#648)
- Define sound and unsound (rust-lang/reference#647)
- Fix a type in the modules section: functions => modules (rust-lang/reference#645)
- Fix some links. (rust-lang/reference#642)
- Update recursion_limit default limit (rust-lang/reference#633)

## rust-by-example

14 commits in e3679e214d8db44586aca9b20aa27517007d1923..f2c15ba5ee89ae9469a2cf60494977749901d764
2019-07-15 11:13:44 -0300 to 2019-08-07 10:14:25 -0300
- Remove redundant semicolons (rust-lang/rust-by-example#1239)
- Rename "Read Lines" chapter title (rust-lang/rust-by-example#1230)
- Added space between word and inline code block in unit_testing.md (rust-lang/rust-by-example#1237)
- [typo] fix unit_testing wrong output (rust-lang/rust-by-example#1210)
- flow_control/match/binding.md: `...' -> `..=' (rust-lang/rust-by-example#1233)
- generics/impl.md: follow rustfmt style (rust-lang/rust-by-example#1236)
- freeze.md: Incorrect example (rust-lang/rust-by-example#1226)
- Make `point` consistent (rust-lang/rust-by-example#1229)
- Fix typo at error -> panic (rust-lang/rust-by-example#1227)
- Snake didn't deserve to die 🐍 (rust-lang/rust-by-example#1228)
- Reorder links in destructuring.md (rust-lang/rust-by-example#1225)
- Rename variable names for consistent in iter_result.md (rust-lang/rust-by-example#1224)
- Fix several shell output and code highlights. (rust-lang/rust-by-example#1222)
- Add new example for Rc. (rust-lang/rust-by-example#1223)

## edition-guide

5 commits in f6c8b92d4e63edd28e862be952f33861f35956f8..e58bc4ca104e890ac56af846877c874c432a64b5
2019-07-06 22:10:32 +0200 to 2019-07-31 20:14:12 +0200
- Hide extraneous `use` in anonymous lifetime example. (rust-lang/edition-guide#190)
- Attempt to clarify "no more mod.rs". (rust-lang/edition-guide#187)
- Remove -preview for rustup components. (rust-lang/edition-guide#188)
- rust-lang/edition-guide#184 More clear explanation and Title. (rust-lang/edition-guide#185)
- More clear table headers (rust-lang/edition-guide#186)

## embedded-book

2 commits in ff334e74fdb9f197e621efa6d7c3105be892e888..c5da1e11915d3f28266168baaf55822f7e3fe999
2019-07-16 13:47:34 +0000 to 2019-08-05 23:02:10 +0000
- install/verify: fix next section link  (rust-embedded/book#202)
- Syst small fix  (rust-embedded/book#198)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants