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

Implement Debug for MaybeUninit #65013

Merged
merged 1 commit into from Nov 28, 2019
Merged

Conversation

petertodd
Copy link
Contributor

@petertodd petertodd commented Oct 2, 2019

Precedent: UnsafeCell implements Debug even though it can't actually display the value. I noticed this omission while writing the following:

#[derive(Debug)]
 pub struct SliceInitializer<'a, T> {
    marker: PhantomData<&'a mut T>, 
    uninit: &'a mut [MaybeUninit<T>],
    written: usize,
}

...which currently unergonomically fails to compile.

UnsafeCell does require T: Debug. Because of things like the above I think it'd be better to leave that requirement off. In fact, I'd also suggest removing that requirement for UnsafeCell too, which again I noticed in some low-level real world code.

@rust-highfive
Copy link
Collaborator

@rust-highfive rust-highfive commented Oct 2, 2019

r? @withoutboats

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review label Oct 2, 2019
@petertodd petertodd force-pushed the 2019-maybeuninit-debug branch from 9aab81b to 5fefbd1 Compare Oct 2, 2019
@Centril
Copy link
Contributor

@Centril Centril commented Oct 2, 2019

cc @RalfJung

src/libcore/mem/maybe_uninit.rs Outdated Show resolved Hide resolved
@RalfJung
Copy link
Member

@RalfJung RalfJung commented Oct 3, 2019

No objections from my side; this is mostly a T-libs question IMO.

@Centril Centril added T-libs-api needs-fcp labels Oct 3, 2019
@Centril Centril added this to the 1.40 milestone Oct 3, 2019
@Centril Centril added the relnotes label Oct 3, 2019
@Centril
Copy link
Contributor

@Centril Centril commented Oct 3, 2019

Updated labels accordingly :)

@JohnTitor
Copy link
Member

@JohnTitor JohnTitor commented Oct 27, 2019

Ping from triage: @withoutboats could you review this?

@Centril Centril removed this from the 1.40 milestone Nov 7, 2019
@Centril Centril added this to the 1.41 milestone Nov 7, 2019
@sfackler
Copy link
Member

@sfackler sfackler commented Nov 7, 2019

@rfcbot fcp merge

I think it makes sense to me that this wouldn't need a T: Debug bound since there's no way we could ever actually display the T, but let's go through an FCP round to make sure people agree.

@rfcbot
Copy link

@rfcbot rfcbot commented Nov 7, 2019

Team member @sfackler has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

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.

@rfcbot rfcbot added proposed-final-comment-period disposition-merge labels Nov 7, 2019
src/libcore/mem/maybe_uninit.rs Outdated Show resolved Hide resolved
Precedent: UnsafeCell implements Debug even though it can't actually
display the value.
@petertodd petertodd force-pushed the 2019-maybeuninit-debug branch from edfd0fa to 8fad66b Compare Nov 7, 2019
@rfcbot rfcbot added final-comment-period and removed proposed-final-comment-period labels Nov 21, 2019
@rfcbot
Copy link

@rfcbot rfcbot commented Nov 21, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@programmerjake
Copy link
Member

@programmerjake programmerjake commented Nov 27, 2019

Is this intended to be insta-stable, or was that an oversight?

@dtolnay
Copy link
Member

@dtolnay dtolnay commented Nov 27, 2019

@programmerjake there is no such thing as an unstable trait impl. The libs team reviewed this and decided to stabilize.

@sfackler
Copy link
Member

@sfackler sfackler commented Nov 28, 2019

@bors r+

@bors
Copy link
Contributor

@bors bors commented Nov 28, 2019

📌 Commit 8fad66b has been approved by sfackler

@bors bors removed the S-waiting-on-review label Nov 28, 2019
@bors bors added the S-waiting-on-bors label Nov 28, 2019
@bors
Copy link
Contributor

@bors bors commented Nov 28, 2019

Testing commit 8fad66b with merge 96ad8e5...

bors added a commit that referenced this issue Nov 28, 2019
Implement Debug for MaybeUninit

Precedent: `UnsafeCell` implements `Debug` even though it can't actually display the value. I noticed this omission while writing the following:

```
#[derive(Debug)]
 pub struct SliceInitializer<'a, T> {
    marker: PhantomData<&'a mut T>,
    uninit: &'a mut [MaybeUninit<T>],
    written: usize,
}
```

...which currently unergonomically fails to compile.

`UnsafeCell` does require `T: Debug`. Because of things like the above I think it'd be better to leave that requirement off. In fact, I'd also suggest removing that requirement for `UnsafeCell` too, which again I noticed in some low-level real world code.
@bors
Copy link
Contributor

@bors bors commented Nov 28, 2019

☀️ Test successful - checks-azure
Approved by: sfackler
Pushing 96ad8e5 to master...

@bors bors added the merged-by-bors label Nov 28, 2019
@bors bors merged commit 8fad66b into rust-lang:master Nov 28, 2019
5 checks passed
@petertodd petertodd deleted the 2019-maybeuninit-debug branch Nov 28, 2019
@rfcbot rfcbot added finished-final-comment-period and removed final-comment-period labels Dec 1, 2019
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Feb 17, 2020
Version 1.41.0 (2020-01-30)
===========================

Language
--------

- [You can now pass type parameters to foreign items when implementing
  traits.][65879] E.g. You can now write `impl<T> From<Foo> for Vec<T> {}`.
- [You can now arbitrarily nest receiver types in the `self` position.][64325] E.g. you can
  now write `fn foo(self: Box<Box<Self>>) {}`. Previously only `Self`, `&Self`,
  `&mut Self`, `Arc<Self>`, `Rc<Self>`, and `Box<Self>` were allowed.
- [You can now use any valid identifier in a `format_args` macro.][66847]
  Previously identifiers starting with an underscore were not allowed.
- [Visibility modifiers (e.g. `pub`) are now syntactically allowed on trait items and
  enum variants.][66183] These are still rejected semantically, but
  can be seen and parsed by procedural macros and conditional compilation.

Compiler
--------

- [Rustc will now warn if you have unused loop `'label`s.][66325]
- [Removed support for the `i686-unknown-dragonfly` target.][67255]
- [Added tier 3 support\* for the `riscv64gc-unknown-linux-gnu` target.][66661]
- [You can now pass an arguments file passing the `@path` syntax
  to rustc.][66172] Note that the format differs somewhat from what is
  found in other tooling; please see [the documentation][argfile-docs] for
  more information.
- [You can now provide `--extern` flag without a path, indicating that it is
  available from the search path or specified with an `-L` flag.][64882]

\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.

[argfile-docs]: https://doc.rust-lang.org/nightly/rustc/command-line-arguments.html#path-load-command-line-flags-from-a-path

Libraries
---------

- [The `core::panic` module is now stable.][66771] It was already stable
  through `std`.
- [`NonZero*` numerics now implement `From<NonZero*>` if it's a smaller integer
  width.][66277] E.g. `NonZeroU16` now implements `From<NonZeroU8>`.
- [`MaybeUninit<T>` now implements `fmt::Debug`.][65013]

Stabilized APIs
---------------

- [`Result::map_or`]
- [`Result::map_or_else`]
- [`std::rc::Weak::weak_count`]
- [`std::rc::Weak::strong_count`]
- [`std::sync::Weak::weak_count`]
- [`std::sync::Weak::strong_count`]

Cargo
-----

- [Cargo will now document all the private items for binary crates
  by default.][cargo/7593]
- [`cargo-install` will now reinstall the package if it detects that it is out
  of date.][cargo/7560]
- [Cargo.lock now uses a more git friendly format that should help to reduce
  merge conflicts.][cargo/7579]
- [You can now override specific dependencies's build settings][cargo/7591] E.g.
  `[profile.dev.overrides.image] opt-level = 2` sets the `image` crate's
  optimisation level to `2` for debug builds. You can also use
  `[profile.<profile>.build_overrides]` to override build scripts and
  their dependencies.

Misc
----

- [You can now specify `edition` in documentation code blocks to compile the block
  for that edition.][66238] E.g. `edition2018` tells rustdoc that the code sample
  should be compiled the 2018 edition of Rust.
- [You can now provide custom themes to rustdoc with `--theme`, and check the
  current theme with `--check-theme`.][54733]
- [You can use `#[cfg(doc)]` to compile an item when building documentation.][61351]

Compatibility Notes
-------------------

- [As previously announced 1.41.0 will be the last tier 1 release for 32-bit
  Apple targets.][apple-32bit-drop] This means that the source code is still
  available to build, but the targets are no longer being tested and release
  binaries for those platforms will no longer be distributed by the Rust project.
  Please refer to the linked blog post for more information.

[54733]: rust-lang/rust#54733
[61351]: rust-lang/rust#61351
[67255]: rust-lang/rust#67255
[66661]: rust-lang/rust#66661
[66771]: rust-lang/rust#66771
[66847]: rust-lang/rust#66847
[66238]: rust-lang/rust#66238
[66277]: rust-lang/rust#66277
[66325]: rust-lang/rust#66325
[66172]: rust-lang/rust#66172
[66183]: rust-lang/rust#66183
[65879]: rust-lang/rust#65879
[65013]: rust-lang/rust#65013
[64882]: rust-lang/rust#64882
[64325]: rust-lang/rust#64325
[cargo/7560]: rust-lang/cargo#7560
[cargo/7579]: rust-lang/cargo#7579
[cargo/7591]: rust-lang/cargo#7591
[cargo/7593]: rust-lang/cargo#7593
[`Result::map_or_else`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.map_or_else
[`Result::map_or`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.map_or
[`std::rc::Weak::weak_count`]: https://doc.rust-lang.org/std/rc/struct.Weak.html#method.weak_count
[`std::rc::Weak::strong_count`]: https://doc.rust-lang.org/std/rc/struct.Weak.html#method.strong_count
[`std::sync::Weak::weak_count`]: https://doc.rust-lang.org/std/sync/struct.Weak.html#method.weak_count
[`std::sync::Weak::strong_count`]: https://doc.rust-lang.org/std/sync/struct.Weak.html#method.strong_count
[apple-32bit-drop]: https://blog.rust-lang.org/2020/01/03/reducing-support-for-32-bit-apple-targets.html
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 12, 2021
Remove `T: Debug` bound on UnsafeCell Debug impl

Prior art: rust-lang#65013
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 13, 2021
Remove `T: Debug` bound on UnsafeCell Debug impl

Prior art: rust-lang#65013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge finished-final-comment-period merged-by-bors needs-fcp relnotes S-waiting-on-bors T-libs-api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet