docs: Use String in Rc::into_raw examples #61421
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
r? @RalfJung |
|
Despite what the title says, this is just for the |
|
Ups, yes, the title is wrong. I don't think the original examples had this problem of claiming something is UB and maybe not being UB in the first place, so I've changed only the examples I've added in the linked merge request. Do you think it makes sense to go through the other examples too and unify it (for no other reason than just the consistency)? |
Yes I think that would make sense. But if you don't want to do that that's fine too; this PR is okay as-is (if you fix the title). |
|
I'll have a look and go through the other examples, that's fine (it'll probably take few days before I get around to it, though). |
|
OK, fixup with more example updates (I'll squash after full review, separating incremental changes for now). I've done it only with the *_raw methods, not on the |
Yes, that sounds great. I thought somewhere in these changed examples we'd also get the "but not anymore this, accessing the memory now would be UB", but I guess that can only happen with |
There's this one example that has a dangling pointer at the end (because the Arc was already freed), but that is dangling with an int inside it too. This specific UB can happen only with Weak, yes. Should I rebase? I've looked at the failing CI, but my feeling is this is problem in the build itself? |
|
Ignore Azure CI, that is just being tested currently. |
Fair. All right then, let's land this. @bors r+ |
|
📌 Commit e5af0e5 has been approved by |
bors
added
S-waiting-on-bors
docs: Use String in Rc::into_raw examples It is unclear if accessing an integer after `drop_in_place` has been called on it is undefined behaviour or not, as demonstrated by the discussion in #60766 (review). Avoid these uncertainties by using String which frees memory in its `drop_in_place` to make sure this is undefined behaviour. The message in the docs should be to watch out and not access the data after that, not discussing when one maybe could get away with it O:-).
It is unclear if accessing an integer after `drop_in_place` has been called on it is undefined behaviour or not, as demonstrated by the discussion in #60766 (review). Avoid these uncertainties by using String which frees memory in its `drop_in_place` to make sure this is undefined behaviour. The message in the docs should be to watch out and not access the data after that, not discussing when one maybe could get away with it O:-).
|
Um, wait? I was talking about rebasing because of the fixup commit that needs to be squashed before the merge (I've pushed the rebase now). Or does bors handle that automatically? It would be stupid if the fixup commit ended up in master :-(. |
|
Sorry, I didn't notice the fixup commit. @bors retry |
|
@bors r- |
bors
added
S-waiting-on-author
|
@pietroalbini why that? |
|
Based on @vorner's comment this wasn't supposed to land until the fixup was removed, but ...I now see the commit was already pushed auto-unapproving it... |
|
Oh that's what happened. I did the retry to cancel the build that was already ongoing. I forgot about auto-unapproval. (Is that state change visible anywhere from within the issue?) @bors r+ |
|
📌 Commit 79e5839 has been approved by |
bors
added
S-waiting-on-bors
docs: Use String in Rc::into_raw examples It is unclear if accessing an integer after `drop_in_place` has been called on it is undefined behaviour or not, as demonstrated by the discussion in #60766 (review). Avoid these uncertainties by using String which frees memory in its `drop_in_place` to make sure this is undefined behaviour. The message in the docs should be to watch out and not access the data after that, not discussing when one maybe could get away with it O:-).
|
☀️ Test successful - checks-travis, status-appveyor |
It is unclear if accessing an integer after
drop_in_placehas beencalled on it is undefined behaviour or not, as demonstrated by the
discussion in
#60766 (review).
Avoid these uncertainties by using String which frees memory in its
drop_in_placeto make sure this is undefined behaviour. The message inthe docs should be to watch out and not access the data after that, not
discussing when one maybe could get away with it O:-).