Skip to content

Commit d1d5608

Browse files
peter-lyons-kehlpeter-lyons-kehl
committed
Cleanup
1 parent 1ddb6a0 commit d1d5608

File tree

2 files changed

+40
-112
lines changed

2 files changed

+40
-112
lines changed

‎README.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,13 @@ stable version (available for your Rust) automatically.
458458
This is special only to `0.*` - it is **not** possible to have a wildcard matching various **major**
459459
versions `1.0` or higher.
460460

461+
## Procedural macros with side effects
462+
Several `prudent` macros duplicate their expression "parameter". In the generated Rust code the parameter expression is evaluated only once, but it's present in the code twice - once in an inactive `if false {...}` branch for verification, and once in the following active `else {...}` branch.
463+
464+
That is OK with macros by example (defined with `macro_rules!`), and OK with any well-behaving
465+
procedural macros. However, if you pass in an expression that invokes a procedural macro that has
466+
side effects or state, it's your problem. Such a macro contradicts Rust guidelines.
467+
461468
# Quality assurance
462469

463470
Checks and tests are run by [GitHub Actions]. See

‎src/lib.rs‎

Lines changed: 33 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,4 @@
11
#![doc = include_str!("../README.md")]
2-
// @TODO nightly?
3-
//
4-
// https://github.com/rust-lang/rust/issues/143874
5-
//
6-
// #![feature(const_trait_impl)]
72
#![cfg_attr(not(any(doc, test)), no_std)]
83
// This only refuses unsafe code in functions/expressions in this crate, but it ignores any unsafe
94
// code generated by this crate's macros. (https://github.com/rust-lang/nomicon/issues/506)
@@ -53,91 +48,16 @@
5348
#[cfg(doc)]
5449
extern crate alloc;
5550

56-
// Implementation notes ARE a part of the documentation:
57-
// - Otherwise it's a pain to edit them.
58-
// - Users deserve documentation of how a macro works, because
59-
// - macros are much more difficult to read than Rust non-macro code, and
60-
// - macros inject code evaluated as if written by the user, so potential for a security problem,
61-
// even without any `unsafe` code.
62-
//
63-
/// Invoke an `unsafe` function.
51+
/// Invoke an `unsafe` function, but isolate `unsafe {...}` only for the function invocation itself.
52+
/// - `If $fn`, that is, the function itself, is NOT given as an identifier/qualified path, but it's
53+
/// given as an expression, then this expression is treated as if evaluated **outside** `unsafe
54+
/// {...}`.
55+
/// - Any arguments passed in as expressions are treated as if evaluated **outside** `unsafe {...}`.
6456
///
6557
/// There is **no** extra enclosing pair of parenthesis `(...)` around the list of arguments (if
6658
/// any). If there was such a pair, it could be confused for a tuple. It would also be less readable
6759
/// when some parameters were tuples/complex expressions.
6860
///
69-
/// Implementation notes
70-
///
71-
/// We want to accept any valid expressions for function parameters. But we don't want them to
72-
/// evaluate inside `unsafe {...}` block that calls the given `unsafe` function. That could "hide"
73-
/// other `unsafe` code (in expressions passed as parameters). So, we assign the given expressions
74-
/// (for parameters) to local variables first, before `unsafe{...}`.
75-
///
76-
/// However, that is a challenge with `macro_rules!`:
77-
/// - variable number of parameters => we store them in a tuple; but
78-
/// - storing them in an ordinary "flat" tuple would involve index numbers 0, 1, 2... That is
79-
/// difficult to generate with `macro_rules!` so that the generated numbers are literals:
80-
/// - It is possible for `macro_rules!` to generate integer **expressions**, but even that is
81-
/// [tricky](https://lukaswirth.dev/tlborm/decl-macros/building-blocks/counting.html).
82-
/// - Even if we generated integer expressions, we can't use them to access tuple fields. Instead,
83-
/// tuple access works with integer literals only.
84-
///
85-
/// Solution: A "tuple tree". It's an unbalanced binary tree. See it expanded:
86-
/// - [simple_examples/fn_add_three/src/main.rs](https://github.com/peter-lyons-kehl/prudent/blob/main/simple_examples/fn_add_three/src/main.rs)
87-
/// - `cd simple_examples/fn_add_three`
88-
/// - `cargo expand`
89-
///
90-
/// // @TODO update/remove - replace with links to the stored output of `cargo expand`
91-
/// ```rust
92-
/// # use prudent::unsafe_fn;
93-
/// unsafe fn add_three(left: u64, middle: u64, right: u64) -> u64 {
94-
/// left + middle + right
95-
/// }
96-
/// unsafe_fn!(add_three, 1, 2, 3);
97-
/// ```
98-
/// expands to:
99-
/// ```rust
100-
/// # unsafe fn add_three(left: u64, middle: u64, right: u64) -> u64 {
101-
/// # left + middle + right
102-
/// # }
103-
/// // ...
104-
/// {
105-
/// let (tuple, fun) = ( (1, (2, (3,))), add_three );
106-
/// #[allow(unsafe_code)]
107-
/// unsafe {
108-
/// fun(tuple.0, tuple.1 .0, tuple.1 .1 .0)
109-
/// }
110-
/// };
111-
/// ```
112-
/// and
113-
/// [simple_examples/fn_add_four/src/main.rs](https://github.com/peter-lyons-kehl/prudent/blob/main/simple_examples/fn_add_four/src/main.rs):
114-
/// ```rust
115-
/// # use prudent::unsafe_fn;
116-
/// unsafe fn add_four(left: u64, middle_left:u64, middle_right:u64, right: u64) -> u64 {
117-
/// left + middle_left + middle_right + right
118-
/// }
119-
/// unsafe_fn!(add_four, 1, 2, 3, 4);
120-
/// ```
121-
/// expands to:
122-
/// ```rust
123-
/// # unsafe fn add_four(left: u64, middle_left: u64, middle_right: u64, right: u64) -> u64 {
124-
/// # left + middle_left + middle_right + right
125-
/// # }
126-
/// // ...
127-
/// {
128-
/// let (tuple_tree, fun) = ( (1, (2, (3, (4,)))), add_four );
129-
/// #[allow(unsafe_code)]
130-
/// unsafe {
131-
/// fun(
132-
/// tuple_tree.0,
133-
/// tuple_tree.1 .0,
134-
/// tuple_tree.1 .1 .0,
135-
/// tuple_tree.1 .1 .1 .0,
136-
/// )
137-
/// }
138-
/// };
139-
/// ```
140-
///
14161
/// @TODO replace with examples/ :
14262
/// ```
14363
#[doc = include_str!("../simple_examples/fn_add_three/src/main.rs")]
@@ -164,7 +84,7 @@ macro_rules! unsafe_fn {
16484

16585
/// Invoke an `unsafe` method. Like [unsafe_fn], but
16686
/// - This accepts a receiver `&self`, `&mut self` and `self`. TODO Box/Rc/Arc, dyn?
167-
/// - This treats `self` as if in an `unsafe {...}` block.
87+
/// - This treats `self` as if it were evaluated **outside** the `unsafe {...}` block.
16888
/// - $fn can **NOT** be an expression or a qualified path (which doesn't work in standard methods
16989
/// calls anyways), but only an identifier.
17090
#[macro_export]
@@ -186,7 +106,8 @@ macro_rules! unsafe_method {
186106
}
187107
//-------------
188108

189-
/// Set a value of a `static mut` variable or its (sub...-)field.
109+
/// Set a value of a `static mut` variable or its (sub...-)field, but isolate `unsafe {...}` only to
110+
/// that assignment.
190111
///
191112
/// To minimize unintended `unsafe`, calculate any indexes etc. beforehand, store them in local
192113
/// variables and pass them in.
@@ -196,29 +117,31 @@ macro_rules! unsafe_method {
196117
#[macro_export]
197118
macro_rules! unsafe_static_set {
198119
($static:expr, $val:expr) => {{
199-
let val = $val;
200-
#[allow(unsafe_code)]
201-
unsafe {
202-
$static = val;
120+
if false {
121+
let _ = $val;
122+
unreachable!()
123+
} else {
124+
#[allow(unsafe_code)]
125+
unsafe {
126+
$static = $val;
127+
}
203128
}
204129
}};
205130
}
206131

207132
/// Deref a pointer (either `const` or `mut`) and yield a read-only reference.
208133
///
209-
/// If `$type` is given, it's expected to be the referenced type (NOT the given pointer, NOT the
210-
/// target reference type) and the given pointer is cast to `* const $type`. `$type` may start with
211-
/// `dyn`. `$type` may be a slice `[...]`.
134+
/// If `$type` is given, it's expected to be the referenced type (NOT the given pointer, NOT a
135+
/// reference based on the given pointer), and the given pointer is cast to `* const $type`. `$type`
136+
/// may start with `dyn`. `$type` may be a slice `[...]`.
212137
#[macro_export]
213138
macro_rules! unsafe_ref {
214139
($ptr:expr) => {{
215-
let ptr = $ptr;
216-
let _: *const _ = ptr; // Partial type check that $ptr yields a const pointer
140+
let ptr: *const _ = $ptr;
217141
unsafe { &*ptr }
218142
}};
219143
($ptr:expr, $lifetime:lifetime) => {{
220-
let ptr = $ptr;
221-
let _: *const _ = ptr; // Partial type check that $ptr yields a const pointer
144+
let ptr: *const _ = $ptr;
222145
unsafe { &*ptr as &$lifetime _ }
223146
}};
224147
($ptr:expr, $type:ty) => {{
@@ -241,25 +164,21 @@ macro_rules! unsafe_ref {
241164
#[macro_export]
242165
macro_rules! unsafe_mut {
243166
($ptr:expr) => {{
244-
let ptr = $ptr;
245-
let _: *mut _ = ptr; // Partial type check that $ptr yields a mut pointer
167+
let ptr: *mut _ = $ptr;
246168
unsafe { &mut *ptr }
247169
}};
248170
($ptr:expr, $lifetime:lifetime) => {{
249-
let ptr = $ptr;
250-
let _: *mut _ = ptr; // Partial type check that $ptr yields a mut pointer
171+
let ptr: *mut _ = $ptr;
251172
unsafe { &mut *ptr as &$lifetime mut _}
252173
}};
253174
($ptr:expr, $ptr_type:ty) => {{
254175
let ptr = $ptr;
255176
let ptr = ptr as *mut $ptr_type;
256-
// let _: *mut _ = ptr; // Partial type check that $ptr yields a mut pointer
257177
unsafe { &mut *ptr}
258178
}};
259179
($ptr:expr, $ptr_type:ty, $lifetime:lifetime) => {{
260180
let ptr = $ptr;
261181
let ptr = ptr as *mut $ptr_type;
262-
//let _: *mut _ = ptr; // Partial type check that $ptr yields a mut pointer
263182
unsafe { &mut *ptr as &$lifetime mut _}
264183
}};
265184
}
@@ -274,15 +193,13 @@ pub const fn expect_copy_ptr<T: Copy>(_: *const T) {}
274193
#[macro_export]
275194
macro_rules! unsafe_val {
276195
($ptr:expr) => {{
277-
let ptr = $ptr;
278-
let _: *const _ = ptr; // Partial type check that $ptr yields a const pointer
196+
let ptr: *const _ = $ptr;
279197
$crate::expect_copy_ptr(ptr);
280198
unsafe { *ptr }
281199
}};
282200
($ptr:expr, $ptr_type:ty) => {{
283201
let ptr = $ptr;
284202
let ptr = ptr as *const $ptr_type;
285-
let _: *const _ = ptr; // Partial type check that $ptr yields a const pointer
286203
$crate::expect_copy_ptr(ptr);
287204
unsafe { *ptr }
288205
}};
@@ -319,11 +236,15 @@ macro_rules! unsafe_use {
319236
#[macro_export]
320237
macro_rules! unsafe_set {
321238
($ptr:expr, $value:expr) => {{
322-
let (ptr, value) = ($ptr, $value);
323-
let _: *mut _ = ptr; // Partial type check that $ptr yields a mut pointer
324-
#[allow(unsafe_code)]
325-
unsafe {
326-
*ptr = value;
239+
if false {
240+
let _: *mut _ = $ptr;
241+
let _ = $value;
242+
unreachable!()
243+
} else {
244+
#[allow(unsafe_code)]
245+
unsafe {
246+
*$ptr = $value;
247+
}
327248
}
328249
}};
329250
}

0 commit comments

Comments
 (0)