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

Document how MaybeUninit<Struct> can be initialized. #81580

Merged
merged 4 commits into from
Feb 6, 2021

Conversation

rodrimati1992
Copy link
Contributor

No description provided.

@rust-highfive
Copy link
Collaborator

r? @dtolnay

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 31, 2021
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
configure: rust.channel         := nightly
configure: rust.debug-assertions := True
configure: llvm.assertions      := True
configure: dist.missing-tools   := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure: 
configure: run `python /checkout/x.py --help`
configure: 

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Nice!

@dtolnay
Copy link
Member

dtolnay commented Jan 31, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Jan 31, 2021

📌 Commit 0974026 has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 31, 2021
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
configure: rust.channel         := nightly
configure: rust.debug-assertions := True
configure: llvm.assertions      := True
configure: dist.missing-tools   := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure: 
configure: run `python /checkout/x.py --help`
configure: 
---
 Documenting core v0.0.0 (/checkout/library/core)
error: unresolved link to `std::ptr::addr_of_mut`
   --> library/core/src/mem/maybe_uninit.rs:175:44
    |
175 | /// You can use `MaybeUninit<T>`, and the [`std::ptr::addr_of_mut`] macro, to initialize structs field by field:
    |                                            ^^^^^^^^^^^^^^^^^^^^^^^ no item named `std` in scope
    |
    = note: `-D broken-intra-doc-links` implied by `-D warnings`
error: unresolved link to `std::ptr::addr_of_mut`
   --> library/core/src/mem/maybe_uninit.rs:175:44
    |
    |
175 | /// You can use `MaybeUninit<T>`, and the [`std::ptr::addr_of_mut`] macro, to initialize structs field by field:
    |                                            ^^^^^^^^^^^^^^^^^^^^^^^ no item named `std` in scope
error: aborting due to 2 previous errors

error: could not document `core`


Caused by:
  process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustdoc --edition=2018 --crate-type lib --crate-name core library/core/src/lib.rs --target x86_64-unknown-linux-gnu -o /checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/doc --error-format=json --json=diagnostic-rendered-ansi --markdown-css rust.css --markdown-no-toc -Z unstable-options --resource-suffix 1.51.0 --index-page /checkout/src/doc/index.md -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/release/deps --cfg=bootstrap -Dwarnings -Winvalid_codeblock_attributes --crate-version '1.51.0-nightly
  (c25232a55
  2021-01-31)'` (exit code: 1)


command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "rustdoc" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "16" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "/checkout/library/test/Cargo.toml" "-p" "core" "--" "--markdown-css" "rust.css" "--markdown-no-toc" "-Z" "unstable-options" "--resource-suffix" "1.51.0" "--index-page" "/checkout/src/doc/index.md"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap doc --stage 0 library/std
Build completed unsuccessfully in 0:00:09

@dtolnay
Copy link
Member

dtolnay commented Jan 31, 2021

^

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 31, 2021
/// unsafe { addr_of_mut!((*ptr).name).write("Bob".to_string()); }
///
/// // Initializing the `list` field
/// // If there was a panic here, then the `String` in the `name` field would be leaked.
Copy link
Contributor

Choose a reason for hiding this comment

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

Language nitpick: would be leaked --> leaks

Copy link
Member

Choose a reason for hiding this comment

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

Maybe the leak can be avoided by reordering the code a bit?

let name = ...
let list = ...

unsafe {
   addr_of_mut!((*ptr).name).write(name);
   addr_of_mut!((*ptr).list).write(list);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@the8472 In this case yes, but I wanted this to translate to how one would initialize a large struct field by field.
If the struct had been a heap allocated struct with a large array field instead of a Vec, it wouldn't be possible to construct the array on the stack before moving it to the field in debug builds, since it would overflow the stack.

Copy link
Member

@the8472 the8472 Jan 31, 2021

Choose a reason for hiding this comment

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

Well, afaik currently there is no guarantee that passing a large array directly to function arguments gets optimized any better than first assigning it to a named variable. You're at the mercy of the optimizer in either case since there's no placement API. rust-lang/rfcs#2884 is still under discussion.

I think to initialize a large array without blowing the stack in debug mode you would have to loop over each item.

@dtolnay
Copy link
Member

dtolnay commented Feb 5, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Feb 5, 2021

📌 Commit 21c2343 has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 5, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 6, 2021
Rollup of 7 pull requests

Successful merges:

 - rust-lang#80011 (Stabilize `peekable_next_if`)
 - rust-lang#81580 (Document how `MaybeUninit<Struct>` can be initialized.)
 - rust-lang#81610 (BTreeMap: make Ord bound explicit, compile-test its absence)
 - rust-lang#81664 (Avoid a hir access inside get_static)
 - rust-lang#81675 (Make rustdoc respect `--error-format short` in doctests)
 - rust-lang#81753 (Never MIR inline functions with a different instruction set)
 - rust-lang#81795 (Small refactor with Iterator::reduce)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 43b3adb into rust-lang:master Feb 6, 2021
@rustbot rustbot added this to the 1.51.0 milestone Feb 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants