Skip to content

Commit

Permalink
Improve the cases in which c_enum! will detect multiple declarations
Browse files Browse the repository at this point in the history
The problem with the previous version here was that it failed to detect
the most common error case:

c_enum! {
    #[derive(SomeTrait)]
    pub enum A: u32 { X = 1 }

    #[derive(SomeTrait)]
    pub enum B: u32 { Y = 1 }
}

In that case you would get an error message saying "no rules expected
the token `pub`" and the custom error message would not be emitted at
all.

The new version here splits the macro into two cases:
- the normal one with the new impl block syntax, and,
- the error case that has a struct definition followed by some extra
  stuff that is definitely not an impl block.

We emit a custom error in the second case and pass the extra input to a
helper macro so we also emit the regular "no rules expected the token
`...`" error message that we were supposed to emit.

This works well enough in practice. If you end up with some extra stuff
after an impl block you will likely end up getting some spurious
warnings or missing attributes but you're already getting an error in
that case so things should mostly work out OK.
  • Loading branch information
Phantomical committed Oct 24, 2023
1 parent d78cb0d commit f794902
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 20 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased
### Changed
- `c_enum!` will now emit the error message about multiple declarations in a
block in most cases matching needing to be migrated to v0.2.0.

## 0.2.1 - 2023-10-23
### Changed
Expand Down
65 changes: 45 additions & 20 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,6 @@ macro_rules! c_enum {
$( #[$iattr:meta] )*
impl {}
)?

// Capture part of a potential next entry in order to emit a better error.
$( $evis:vis enum $($error:tt)+ )?
} => {
$crate::__c_enum_no_debug! {
$( #[$attr] )*
Expand Down Expand Up @@ -296,17 +293,38 @@ macro_rules! c_enum {
}
}
}
};
// Catch cases where there are multiple enums declared in the same block.
//
// This was valid up until version 0.2.0 so providing a good error message
// is useful.
{
$( #[$attr:meta] )*
$vis:vis enum $name:ident : $inner:ty {
$(
$( #[ $field_attr:meta ] )*
$field:ident $( = $value:expr )?
),* $(,)?
}

$(
$crate::__expects_no_input! {
$evis enum $($error)+
$( $error:tt )+
} => {
$crate::c_enum! {
$( #[$attr] )*
$vis enum $name : $inner {
$(
$( #[$field_attr] )*
$field $( = $value )?,
)*
}
}

compile_error!(
"Declaring multiple enums using a single c_enum! macro block is no longer supported",
);
)?
};
$crate::__c_enum_expects_impl_or_nothing! { $( $error )+ }

compile_error!(
"Declaring multiple enums using a single c_enum! macro block is no longer supported",
);
}
}

// TODO: not sure if this is worth adding to the public API.
Expand Down Expand Up @@ -380,6 +398,22 @@ macro_rules! __c_enum_no_debug {
};
}

/// Helper macro to emit a "no rules expected the token `...`" error message.
///
/// The input spec here matches the one in the `c_enum!` macro after the end of
/// the enum declaration. That way we give the appropriate error message in case
/// it's due to a typo and not multiple declarations.
#[doc(hidden)]
#[macro_export]
macro_rules! __c_enum_expects_impl_or_nothing {
{
$(
$( #[$iattr:meta] )*
impl {}
)?
} => {};
}

/// Helper macro for defining stuff in c_enum.
///
/// These could be a bunch of different macros but those would clutter up the
Expand Down Expand Up @@ -415,15 +449,6 @@ macro_rules! __c_enum_impl {
}
}

/// Helper macro to emit a "no rules expected the token `...`" error message.
///
/// We use this mainly to emit proper error messages when the user provides
#[doc(hidden)]
#[macro_export]
macro_rules! __expects_no_input {
{} => {};
}

// This needs to be after all the macro definitions.
/// This module shows an example of code generated by the macro.
///
Expand Down

0 comments on commit f794902

Please sign in to comment.