Skip to content

Commit 4b23fc3

Browse files
authored
Prevent top level nested attributes
* Add remove_repr macro for testing: This macro removes `#[repr(_)]` from an enum, this is useful for testing the bug described in #1. `#[remove_repr]` is only available behind the feature flag test-utils. * Add a fake `repr` attribute in test-utils: This patch adds a fake `repr` and add tests to try to confuse the compiler into thinking that it is the real repr. If possible, that would cause problems with safe-discriminant. That is because we assume that there is always a repr attributed. * Reject enums with attribute macros: This patch checks if there is any top level attribute macro expansion, and if there it, we report it as an error. * test marcro ordering fixes: #1 --------- Signed-off-by: Ahmed Abdelraoof <ahmed.abdelraoof@huawei.com>
1 parent 6be62f8 commit 4b23fc3

17 files changed

+187
-0
lines changed

safe-discriminant-derive/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,5 @@ proc-macro2 = { workspace = true }
1919
quote = { workspace = true }
2020
syn = { workspace = true }
2121

22+
[features]
23+
test-utils = []

safe-discriminant-derive/src/lib.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,13 +83,28 @@ fn validate_all_variants(variants: impl Iterator<Item = Variant>) -> Result<()>
8383
.unwrap_or(Ok(()))
8484
}
8585

86+
/// Returns true if there is any #[x] where x is not `repr`
87+
/// returns false otherwise.
88+
fn contains_attribute_macros(attrs: &[Attribute]) -> bool {
89+
attrs.iter().any(|attr| !attr.path().is_ident("repr"))
90+
}
8691
/// Constructs Discriminant trait implementation for given enum.
8792
/// Returns error in one of two cases:
8893
/// 1- No valid `#[repr(x)]` is found.
8994
/// 2- Any of the enum variants is missing discriminant.
95+
/// 3- contains any additional top level attribute macros.
9096
fn derive_discriminant_inner(tagged_enum: ItemEnum) -> Result<TokenStream> {
9197
let prim = get_enum_repr_prim(&tagged_enum.attrs, tagged_enum.ident.span())?;
9298
validate_all_variants(tagged_enum.variants.into_iter())?;
99+
if contains_attribute_macros(&tagged_enum.attrs) {
100+
return Err(Error::new(
101+
tagged_enum.ident.span(),
102+
concat!(
103+
"Discriminant is not compatiable with any top ",
104+
"level `#[attr]` except `#[repr(_)]`."
105+
),
106+
));
107+
}
93108
let name = tagged_enum.ident;
94109
let generics = tagged_enum.generics;
95110
let derive = quote! {
@@ -110,3 +125,33 @@ pub fn derive_discriminant(item: TokenStream) -> TokenStream {
110125
Ok(s) => s,
111126
}
112127
}
128+
129+
#[cfg(feature = "test-utils")]
130+
/// This macro will remove `#[repr(_)]` from any given enum.
131+
/// This is only used for testing.
132+
#[proc_macro_attribute]
133+
pub fn remove_repr(_: TokenStream, item: TokenStream) -> TokenStream {
134+
let mut tagged_enum = parse_macro_input!(item as ItemEnum);
135+
tagged_enum
136+
.attrs
137+
.retain(|attr| !attr.path().is_ident("repr"));
138+
quote! {
139+
#tagged_enum
140+
}
141+
.into()
142+
}
143+
144+
#[cfg(feature = "test-utils")]
145+
/// This macro is fake `#[repr(_)]` attribute, it will be a problem if we
146+
/// can trick the macro system into thinking this is the real #[repr(_)]
147+
#[proc_macro_attribute]
148+
pub fn repr(_: TokenStream, item: TokenStream) -> TokenStream {
149+
item
150+
}
151+
152+
#[cfg(feature = "test-utils")]
153+
/// exactly as the name suggests!
154+
#[proc_macro_attribute]
155+
pub fn do_nothing(_: TokenStream, item: TokenStream) -> TokenStream {
156+
item
157+
}

safe-discriminant/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@ safe-discriminant-derive = {workspace = true}
1515

1616
[dev-dependencies]
1717
trybuild = { workspace = true }
18+
safe-discriminant-derive = {workspace = true, features = ["test-utils"]}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
use safe_discriminant::Discriminant;
2+
use safe_discriminant_derive::do_nothing;
3+
4+
#[repr(u8)]
5+
#[derive(Discriminant)]
6+
#[do_nothing]
7+
pub enum Foo {
8+
A = 0,
9+
B = 1,
10+
}
11+
12+
fn main() {}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
error: Discriminant is not compatiable with any top level `#[attr]` except `#[repr(_)]`.
2+
--> tests/fail/do_nothing_last.rs:7:10
3+
|
4+
7 | pub enum Foo {
5+
| ^^^
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// attempting to confuse Discriminant into thinking
2+
// the fake repr is the real repr
3+
use safe_discriminant::Discriminant;
4+
use safe_discriminant_derive::repr;
5+
6+
#[derive(Discriminant)]
7+
#[repr(u8)]
8+
pub enum Foo {
9+
A = 0,
10+
B = 1,
11+
}
12+
13+
fn main() {}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error[E0659]: `repr` is ambiguous
2+
--> tests/fail/fake_repr1.rs:7:3
3+
|
4+
7 | #[repr(u8)]
5+
| ^^^^ ambiguous name
6+
|
7+
= note: ambiguous because of a name conflict with a builtin attribute
8+
= note: `repr` could refer to a built-in attribute
9+
note: `repr` could also refer to the attribute macro imported here
10+
--> tests/fail/fake_repr1.rs:4:5
11+
|
12+
4 | use safe_discriminant_derive::repr;
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
= help: use `crate::repr` to refer to this attribute macro unambiguously
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// attempting to confuse Discriminant into thinking
2+
// the fake repr is the real repr
3+
use safe_discriminant::Discriminant;
4+
use safe_discriminant_derive::repr;
5+
6+
#[derive(Discriminant)]
7+
#[crate::repr(u8)]
8+
pub enum Foo {
9+
A = 0,
10+
B = 1,
11+
}
12+
13+
fn main() {}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
error: Discriminant requires a `#[repr(x)] where x is one of u8, i8, u16, i16, u32, i32, u64, i64, u128, i128.
2+
--> tests/fail/fake_repr2.rs:8:10
3+
|
4+
8 | pub enum Foo {
5+
| ^^^
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// attempting to confuse Discriminant into thinking
2+
// the fake repr is the real repr
3+
use safe_discriminant::Discriminant;
4+
5+
#[derive(Discriminant)]
6+
#[safe_discriminant_derive::repr(u8)]
7+
pub enum Foo {
8+
A = 0,
9+
B = 1,
10+
}
11+
12+
fn main() {}

0 commit comments

Comments
 (0)