Skip to content

Add From and TryFrom impls in libc_enum macro #1088

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.

Sign up for GitHub

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

Closed
wants to merge 11 commits into from
Closed

Add From and TryFrom impls in libc_enum macro #1088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a0dd24c
Replace Signal::from_c_int with TryFrom<c_int>
frangio Jun 15, 2019
aedd467
Add From and TryFrom impls in libc_enum macro
frangio Jun 15, 2019
367a4d6
Change synax of libc_enum macro
frangio Jun 15, 2019
1a14711
Use speed_t type for BaudRate
frangio Jun 15, 2019
14de743
Use full path to libc crate
frangio Jun 16, 2019
f9b344d
Improve documentation for libc_enum
frangio Jun 16, 2019
84a6620
Implement try_from for rust < 1.34.0
frangio Jun 16, 2019
c225ee6
Add changelog entries
frangio Jun 16, 2019
e4b4e91
Add changelog PR link
frangio Jun 17, 2019
fa5641f
Add support for pub(crate), pub(super), etc.
frangio Jun 17, 2019
b2dcb49
Add missing backticks in documentation
frangio Jun 17, 2019
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
([#1069](https://github.com/nix-rust/nix/pull/1069))
- Add `mkdirat`.
([#1084](https://github.com/nix-rust/nix/pull/1084))
- `From` and `TryFrom` impls for many enums that correspond to libc constants.
([#1088](https://github.com/nix-rust/nix/pull/1088))

### Changed
- Support for `ifaddrs` now present when building for Android.
Expand All @@ -22,6 +24,10 @@ This project adheres to [Semantic Versioning](http://semver.org/).

### Fixed
### Removed
- `Signal::from_c_int`. Use `Signal::try_from` instead.
([#1088](https://github.com/nix-rust/nix/pull/1088))
- `From<speed_t>` impl for `BaudRate`. Use `BaudRate::try_from` instead.
([#1088](https://github.com/nix-rust/nix/pull/1088))

## [0.14.1] - 2019-06-06
### Added
Expand Down
3 changes: 3 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ bitflags = "1.0"
cfg-if = "0.1.0"
void = "1.0.2"

[build-dependencies]
version_check = "0.9.1"

[target.'cfg(target_os = "dragonfly")'.build-dependencies]
cc = "1"

Expand Down
10 changes: 6 additions & 4 deletions build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
#[cfg(target_os = "dragonfly")]
extern crate cc;
extern crate version_check as rustc;

#[cfg(target_os = "dragonfly")]
fn main() {
#[cfg(target_os = "dragonfly")]
cc::Build::new()
.file("src/errno_dragonfly.c")
.compile("liberrno_dragonfly.a");
}

#[cfg(not(target_os = "dragonfly"))]
fn main() {}
if rustc::is_min_version("1.34.0").unwrap_or(false) {
println!("cargo:rustc-cfg=try_from");
}
}
231 changes: 73 additions & 158 deletions src/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
/// with values from the libc crate. It is used the same way as the `bitflags!` macro, except
/// that only the name of the flag value has to be given.
///
/// The `libc` crate must be in scope with the name `libc`.
///
/// # Example
/// ```
/// libc_bitflags!{
Expand Down Expand Up @@ -53,204 +51,121 @@ macro_rules! libc_bitflags {
pub struct $BitFlags: $T {
$(
$(#[$inner $($args)*])*
const $Flag = libc::$Flag $(as $cast)*;
const $Flag = ::libc::$Flag $(as $cast)*;
)+
}
}
};
}

/// The `libc_enum!` macro helps with a common use case of defining an enum exclusively using
/// values from the `libc` crate. This macro supports both `pub` and private `enum`s.
/// values from the `libc` crate. The type after the enum name specifies the type of the constants
/// in `libc`. The macro will generate impls of `From` and `TryFrom` to convert between numeric and
/// enum values.
///
/// `TryFrom` is only implemented for Rust >= 1.34.0, where the trait is stable. An equivalent
/// `try_from` inherent method is made available regardless of the Rust version. `TryInto` should
/// not be used as long as the MSRV for nix is less than 1.34.0.
///
///
/// The `libc` crate must be in scope with the name `libc`.
/// Documentation for each variant must be provided before any cfg attributes.
///
/// # Example
/// ```
/// libc_enum!{
/// pub enum ProtFlags {
/// libc_enum! {
/// pub enum ProtFlags: c_int {
/// PROT_NONE,
/// PROT_READ,
/// PROT_WRITE,
/// PROT_EXEC,
/// /// Documentation before cfg attribute.
/// #[cfg(any(target_os = "linux", target_os = "android"))]
/// PROT_GROWSDOWN,
/// #[cfg(any(target_os = "linux", target_os = "android"))]
/// PROT_GROWSUP,
/// }
/// }
///
/// let flag: c_int = ProtFlags::PROT_NONE.into();
/// let flag: ProtFlags = ProtFlags::try_from(::libc::PROT_NONE).unwrap();
/// ```
macro_rules! libc_enum {
// (non-pub) Exit rule.
(@make_enum
{
name: $BitFlags:ident,
attrs: [$($attrs:tt)*],
entries: [$($entries:tt)*],
}
) => {
$($attrs)*
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
enum $BitFlags {
$($entries)*
}
};

// (pub) Exit rule.
(@make_enum
{
pub,
name: $BitFlags:ident,
attrs: [$($attrs:tt)*],
entries: [$($entries:tt)*],
}
) => {
$($attrs)*
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub enum $BitFlags {
$($entries)*
}
};

// (non-pub) Done accumulating.
(@accumulate_entries
{
name: $BitFlags:ident,
attrs: $attrs:tt,
},
$entries:tt;
// pub
(
$(#[$enum_attr:meta])*
pub $(($($scope:tt)*))* enum $($def:tt)*
) => {
libc_enum! {
@make_enum
{
name: $BitFlags,
attrs: $attrs,
entries: $entries,
}
@(pub $(($($scope)*))*)
$(#[$enum_attr])*
enum $($def)*
}
};

// (pub) Done accumulating.
(@accumulate_entries
{
pub,
name: $BitFlags:ident,
attrs: $attrs:tt,
},
$entries:tt;
// non-pub
(
$(#[$enum_attr:meta])*
enum $($def:tt)*
) => {
libc_enum! {
@make_enum
{
pub,
name: $BitFlags,
attrs: $attrs,
entries: $entries,
}
@()
$(#[$enum_attr])*
enum $($def)*
}
};

// Munch an attr.
(@accumulate_entries
$prefix:tt,
[$($entries:tt)*];
#[$attr:meta] $($tail:tt)*
) => {
libc_enum! {
@accumulate_entries
$prefix,
[
$($entries)*
#[$attr]
];
$($tail)*
(
@($($vis:tt)*)
$(#[$enum_attr:meta])*
enum $enum:ident : $prim:ty {
$(
$(#[doc = $var_doc:tt])*
Copy link
Member

Choose a reason for hiding this comment

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

Doc strings and cfg attributes are both different kinds of attributes. Could you simplify the macro by combining these two lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The compiler emits a warning for "unused doc comment" when it sees a doc comment annotating a match arm, so I match them separately here so that I can use the cfg attributes on their own later.

$(#[cfg($var_cfg:meta)])*
$entry:ident
),* $(,)*
}
};

// Munch last ident if not followed by a comma.
(@accumulate_entries
$prefix:tt,
[$($entries:tt)*];
$entry:ident
) => {
libc_enum! {
@accumulate_entries
$prefix,
[
$($entries)*
$entry = libc::$entry,
];
$(#[$enum_attr])*
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
$($vis)* enum $enum {
$(
$(#[doc = $var_doc])*
$(#[cfg($var_cfg)])*
$entry = ::libc::$entry as isize
Copy link
Member

Choose a reason for hiding this comment

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

why cast to isize? It seems like $prim would be more appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isize is the type that rust expects here. I tried using $prim, but that requires adding a #[repr($prim)] attribute to the enum, which I couldn't get to work for some reason I unfortunately can't remember.

Copy link
Member

Choose a reason for hiding this comment

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

Given the way that these types will be used, I think that #[repr($prim)] would be better. Could you try again to make it work? Show me the error if you have trouble.

Copy link
Contributor Author

@frangio frangio Jun 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

The problem is that in some places we're using a custom type as $prim, and it turns out the repr attribute only works with simple unsigned or signed integer types u* and i* (see the Rustonomicon).

The reason we need to use a custom type is that in some cases the underlying type depends on the target machine. I'm not sure how we could get around that with the enum Signal : c_int syntax I set up here.

Given the way that these types will be used

Can you share an example where the repr makes a difference?

),*
}
};

// Munch an ident; covers terminating comma case.
(@accumulate_entries
$prefix:tt,
[$($entries:tt)*];
$entry:ident, $($tail:tt)*
) => {
libc_enum! {
@accumulate_entries
$prefix,
[
$($entries)*
$entry = libc::$entry,
];
$($tail)*
impl $enum {
pub fn try_from(value: $prim) -> std::result::Result<$enum, ::Error> {
match value {
$(
$(#[cfg($var_cfg)])*
::libc::$entry => Ok($enum::$entry),
)*
// don't think this Error is the correct one
Copy link
Member

Choose a reason for hiding this comment

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

I think that's a fine choice of error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I doubt is that Error::Sys(Errno::EINVAL) probably makes one think this is a "system error" that was picked up from errno.

How about adding an Error::InvalidConversion?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I understand. Error::InvalidConversion would work, too.

_ => Err(::Error::invalid_argument())
}
}
}
};

// Munch an ident and cast it to the given type; covers terminating comma.
(@accumulate_entries
$prefix:tt,
[$($entries:tt)*];
$entry:ident as $ty:ty, $($tail:tt)*
) => {
libc_enum! {
@accumulate_entries
$prefix,
[
$($entries)*
$entry = libc::$entry as $ty,
];
$($tail)*
impl std::convert::From<$enum> for $prim {
fn from(value: $enum) -> $prim {
match value {
$(
$(#[cfg($var_cfg)])*
$enum::$entry => ::libc::$entry,
)*
}
}
}
};

// (non-pub) Entry rule.
(
$(#[$attr:meta])*
enum $BitFlags:ident {
$($vals:tt)*
}
) => {
libc_enum! {
@accumulate_entries
{
name: $BitFlags,
attrs: [$(#[$attr])*],
},
[];
$($vals)*
}
};
#[cfg(try_from)]
impl std::convert::TryFrom<$prim> for $enum {
type Error = ::Error;

// (pub) Entry rule.
(
$(#[$attr:meta])*
pub enum $BitFlags:ident {
$($vals:tt)*
}
) => {
libc_enum! {
@accumulate_entries
{
pub,
name: $BitFlags,
attrs: [$(#[$attr])*],
},
[];
$($vals)*
fn try_from(value: $prim) -> std::result::Result<$enum, Self::Error> {
$enum::try_from(value)
}
}
};
}
Expand Down
9 changes: 3 additions & 6 deletions src/sys/aio.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ use sys::time::TimeSpec;
libc_enum! {
/// Mode for `AioCb::fsync`. Controls whether only data or both data and
/// metadata are synced.
#[repr(i32)]
pub enum AioFsyncMode {
pub enum AioFsyncMode: i32 {
/// do it like `fsync`
O_SYNC,
/// on supported operating systems only, do it like `fdatasync`
Expand All @@ -57,8 +56,7 @@ libc_enum! {
/// When used with [`lio_listio`](fn.lio_listio.html), determines whether a
/// given `aiocb` should be used for a read operation, a write operation, or
/// ignored. Has no effect for any other aio functions.
#[repr(i32)]
pub enum LioOpcode {
pub enum LioOpcode: i32 {
LIO_NOP,
LIO_WRITE,
LIO_READ,
Expand All @@ -67,8 +65,7 @@ libc_enum! {

libc_enum! {
/// Mode for [`lio_listio`](fn.lio_listio.html)
#[repr(i32)]
pub enum LioMode {
pub enum LioMode: i32 {
/// Requests that [`lio_listio`](fn.lio_listio.html) block until all
/// requested operations have been completed
LIO_WAIT,
Expand Down
4 changes: 1 addition & 3 deletions src/sys/event.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ type type_of_event_filter = u32;
#[cfg(not(target_os = "netbsd"))]
type type_of_event_filter = i16;
libc_enum! {
#[cfg_attr(target_os = "netbsd", repr(u32))]
#[cfg_attr(not(target_os = "netbsd"), repr(i16))]
pub enum EventFilter {
pub enum EventFilter: type_of_event_filter {
EVFILT_AIO,
/// Returns whenever there is no remaining data in the write buffer
#[cfg(target_os = "freebsd")]
Expand Down
5 changes: 2 additions & 3 deletions src/sys/mman.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,11 @@ libc_bitflags!{
}
}

libc_enum!{
libc_enum! {
/// Usage information for a range of memory to allow for performance optimizations by the kernel.
///
/// Used by [`madvise`](./fn.madvise.html).
#[repr(i32)]
pub enum MmapAdvise {
pub enum MmapAdvise: i32 {
/// No further special treatment. This is the default.
MADV_NORMAL,
/// Expect random page references.
Expand Down
Loading