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

Use #[non_exhaustive] where appropriate #86592

Merged
merged 1 commit into from
Jun 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 3 additions & 4 deletions compiler/rustc_metadata/src/dynamic_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,12 @@ mod dl {
use std::sync::{Mutex, MutexGuard};

pub fn lock() -> MutexGuard<'static, Guard> {
static LOCK: SyncLazy<Mutex<Guard>> = SyncLazy::new(|| Mutex::new(Guard { _priv: () }));
static LOCK: SyncLazy<Mutex<Guard>> = SyncLazy::new(|| Mutex::new(Guard));
LOCK.lock().unwrap()
}

pub struct Guard {
_priv: (),
}
#[non_exhaustive]
pub struct Guard;
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this permit construction of Guard by users who can't access Guard's fields? Like this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Inside of a crate, #[non_exhaustive] has no effect. To external users, you can effectively pretend there's a hidden private field. There's an RFC and presumably a fair amount of documentation about what this feature is.

Copy link
Member

Choose a reason for hiding this comment

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

That's true. My concern still applies equally much. rustc_metadata has a bunch of stuff in it, not just the dynamic loading utilities, and so there's still value in not (easily) allowing other code in rustc_metadata to accidentally shoot their foot off.

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 suppose, but I don't particularly see that being an issue given the code reviews done on any PR.


impl Guard {
pub fn get(&mut self) -> Result<(), String> {
Expand Down
19 changes: 9 additions & 10 deletions library/core/src/alloc/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl Layout {
#[inline]
pub const fn from_size_align(size: usize, align: usize) -> Result<Self, LayoutError> {
if !align.is_power_of_two() {
return Err(LayoutError { private: () });
return Err(LayoutError);
}

// (power-of-two implies align != 0.)
Expand All @@ -78,7 +78,7 @@ impl Layout {
// Above implies that checking for summation overflow is both
// necessary and sufficient.
if size > usize::MAX - (align - 1) {
return Err(LayoutError { private: () });
return Err(LayoutError);
}

// SAFETY: the conditions for `from_size_align_unchecked` have been
Expand Down Expand Up @@ -288,7 +288,7 @@ impl Layout {
// > must not overflow (i.e., the rounded value must be less than
// > `usize::MAX`)
let padded_size = self.size() + self.padding_needed_for(self.align());
let alloc_size = padded_size.checked_mul(n).ok_or(LayoutError { private: () })?;
let alloc_size = padded_size.checked_mul(n).ok_or(LayoutError)?;

// SAFETY: self.align is already known to be valid and alloc_size has been
// padded already.
Expand Down Expand Up @@ -346,8 +346,8 @@ impl Layout {
let new_align = cmp::max(self.align(), next.align());
let pad = self.padding_needed_for(next.align());

let offset = self.size().checked_add(pad).ok_or(LayoutError { private: () })?;
let new_size = offset.checked_add(next.size()).ok_or(LayoutError { private: () })?;
let offset = self.size().checked_add(pad).ok_or(LayoutError)?;
let new_size = offset.checked_add(next.size()).ok_or(LayoutError)?;

let layout = Layout::from_size_align(new_size, new_align)?;
Ok((layout, offset))
Expand All @@ -368,7 +368,7 @@ impl Layout {
#[unstable(feature = "alloc_layout_extra", issue = "55724")]
#[inline]
pub fn repeat_packed(&self, n: usize) -> Result<Self, LayoutError> {
let size = self.size().checked_mul(n).ok_or(LayoutError { private: () })?;
let size = self.size().checked_mul(n).ok_or(LayoutError)?;
Layout::from_size_align(size, self.align())
}

Expand All @@ -381,7 +381,7 @@ impl Layout {
#[unstable(feature = "alloc_layout_extra", issue = "55724")]
#[inline]
pub fn extend_packed(&self, next: Self) -> Result<Self, LayoutError> {
let new_size = self.size().checked_add(next.size()).ok_or(LayoutError { private: () })?;
let new_size = self.size().checked_add(next.size()).ok_or(LayoutError)?;
Layout::from_size_align(new_size, self.align())
}

Expand Down Expand Up @@ -409,10 +409,9 @@ pub type LayoutErr = LayoutError;
/// or some other `Layout` constructor
/// do not satisfy its documented constraints.
#[stable(feature = "alloc_layout_error", since = "1.50.0")]
#[non_exhaustive]
#[derive(Clone, PartialEq, Eq, Debug)]
pub struct LayoutError {
private: (),
}
pub struct LayoutError;

// (we need this for downstream impl of trait Error)
#[stable(feature = "alloc_layout", since = "1.28.0")]
Expand Down
7 changes: 2 additions & 5 deletions library/core/src/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,8 +593,8 @@ pub struct RefCell<T: ?Sized> {

/// An error returned by [`RefCell::try_borrow`].
#[stable(feature = "try_borrow", since = "1.13.0")]
#[non_exhaustive]
pub struct BorrowError {
_private: (),
#[cfg(feature = "debug_refcell")]
location: &'static crate::panic::Location<'static>,
}
Expand All @@ -620,8 +620,8 @@ impl Display for BorrowError {

/// An error returned by [`RefCell::try_borrow_mut`].
#[stable(feature = "try_borrow", since = "1.13.0")]
#[non_exhaustive]
pub struct BorrowMutError {
_private: (),
#[cfg(feature = "debug_refcell")]
location: &'static crate::panic::Location<'static>,
}
Expand Down Expand Up @@ -872,7 +872,6 @@ impl<T: ?Sized> RefCell<T> {
Ok(Ref { value: unsafe { &*self.value.get() }, borrow: b })
}
None => Err(BorrowError {
_private: (),
// If a borrow occured, then we must already have an outstanding borrow,
// so `borrowed_at` will be `Some`
#[cfg(feature = "debug_refcell")]
Expand Down Expand Up @@ -958,7 +957,6 @@ impl<T: ?Sized> RefCell<T> {
Ok(RefMut { value: unsafe { &mut *self.value.get() }, borrow: b })
}
None => Err(BorrowMutError {
_private: (),
// If a borrow occured, then we must already have an outstanding borrow,
// so `borrowed_at` will be `Some`
#[cfg(feature = "debug_refcell")]
Expand Down Expand Up @@ -1080,7 +1078,6 @@ impl<T: ?Sized> RefCell<T> {
Ok(unsafe { &*self.value.get() })
} else {
Err(BorrowError {
_private: (),
// If a borrow occured, then we must already have an outstanding borrow,
// so `borrowed_at` will be `Some`
#[cfg(feature = "debug_refcell")]
Expand Down
5 changes: 2 additions & 3 deletions library/core/src/str/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,9 @@ impl fmt::Display for Utf8Error {
///
/// [`from_str`]: super::FromStr::from_str
#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
#[stable(feature = "rust1", since = "1.0.0")]
pub struct ParseBoolError {
pub(super) _priv: (),
}
pub struct ParseBoolError;

#[stable(feature = "rust1", since = "1.0.0")]
impl fmt::Display for ParseBoolError {
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/str/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ impl FromStr for bool {
match s {
"true" => Ok(true),
"false" => Ok(false),
_ => Err(ParseBoolError { _priv: () }),
_ => Err(ParseBoolError),
}
}
}
7 changes: 3 additions & 4 deletions library/proc_macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,13 @@ impl !Sync for TokenStream {}

/// Error returned from `TokenStream::from_str`.
#[stable(feature = "proc_macro_lib", since = "1.15.0")]
#[non_exhaustive]
#[derive(Debug)]
pub struct LexError {
_inner: (),
}
pub struct LexError;

impl LexError {
fn new() -> Self {
LexError { _inner: () }
LexError
}
}

Expand Down
14 changes: 6 additions & 8 deletions library/std/src/io/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ use crate::io::{
/// This struct is generally created by calling [`empty()`]. Please see
/// the documentation of [`empty()`] for more details.
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Empty {
_priv: (),
}
#[non_exhaustive]
pub struct Empty;

/// Constructs a new handle to an empty reader.
///
Expand All @@ -35,7 +34,7 @@ pub struct Empty {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_io_structs", issue = "78812")]
pub const fn empty() -> Empty {
Empty { _priv: () }
Empty
}

#[stable(feature = "rust1", since = "1.0.0")]
Expand Down Expand Up @@ -172,9 +171,8 @@ impl fmt::Debug for Repeat {
/// This struct is generally created by calling [`sink`]. Please
/// see the documentation of [`sink()`] for more details.
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Sink {
_priv: (),
}
#[non_exhaustive]
pub struct Sink;

/// Creates an instance of a writer which will successfully consume all data.
///
Expand All @@ -195,7 +193,7 @@ pub struct Sink {
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_io_structs", issue = "78812")]
pub const fn sink() -> Sink {
Sink { _priv: () }
Sink
}

#[stable(feature = "rust1", since = "1.0.0")]
Expand Down
7 changes: 3 additions & 4 deletions library/std/src/thread/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,10 +324,9 @@ macro_rules! __thread_local_inner {

/// An error returned by [`LocalKey::try_with`](struct.LocalKey.html#method.try_with).
#[stable(feature = "thread_local_try_with", since = "1.26.0")]
#[non_exhaustive]
#[derive(Clone, Copy, Eq, PartialEq)]
pub struct AccessError {
_private: (),
}
pub struct AccessError;

#[stable(feature = "thread_local_try_with", since = "1.26.0")]
impl fmt::Debug for AccessError {
Expand Down Expand Up @@ -396,7 +395,7 @@ impl<T: 'static> LocalKey<T> {
F: FnOnce(&T) -> R,
{
unsafe {
let thread_local = (self.inner)().ok_or(AccessError { _private: () })?;
let thread_local = (self.inner)().ok_or(AccessError)?;
Ok(f(thread_local))
}
}
Expand Down