[orlp]

This PR allows the function in kernel! to have lifetime and const generic arguments.

Originally I also planned to add generic types and where clauses but these required more complicated TT-muncher macros to correctly parse and reject all invalid syntax. We can always extend with that later.

To my knowledge the macro I've (re)written now only accepts legal Rust constructs, and maps those faithfully.

---

An earlier prototype also had type generics (without bounds) but ran into the following issue. It would need to be parsed as such:

$($($param_or_const:ident $($const_param:ident : $const_ty:ty)?),+ $(,)?)?

with a later

__fearless_simd_kernel::<$($($(
    // Because ident can match const we earlier had to parse (possibly const)
    // generic parameters as simply "ident ident?". This validates it as either
    // "name" or "const name" and extracts just the name.
    $crate::__fearless_simd_generic_arg!($param_or_const $($const_param)?)
),+)?)?>($token $(, $arg)*)

#[doc(hidden)]
#[macro_export]
macro_rules! __fearless_simd_generic_arg {
    (const $param:ident) => {
        $param
    };

    ($param:ident) => {
        $param
    };
}

but there's a problem with this. To truly get const params to forward correctly you need to expand them as { N } instead of N, otherwise a type N takes precedence in the case of a collision.

[RunDevelopment]

Is there an issue with "accepting" invalid code? The compiler will reject it anyway, so is there any point in us validating Rust syntax as well?

For example, if I give the macro an invalid Rust function, will I get a better error message from the macro or from the compiler? E.g.

kernel! {
fn foo<What am I?>(_: Avx2, a: f32) { }
}

[Shnatsel]

I admit this goes a bit over my head; I am having trouble imagining all possible substitutions and all possible ways this could break.

I had Astra 6 take a crack at breaking it; it didn't find any soundness issues. It did flag two issues but those probably aren't worth addressing, these are non-soundness edge cases on deliberately malformed inputs.

This was true even before this PR, but this code would really benefit from safety comments, explaining why this is actually safe. Something like "we never cast or transmute lifetimes so if we mess something up the code will simply fail to build in the worse case" (if that's true) or "the only unsafe block is for calling a #[target_feature] safe function, any unsafe ops inside the safe function will fail to compile"

[orlp]

@RunDevelopment Without parsing the generics properly we aren't sure what we're expanding in the unsafe { } block. At the very least we need to make sure that the expansion consists only of identifiers that are bound.

And it's a lot harder than you might think to accept arbitrary blobs. < $($generics:tt)* > doesn't work for example, > may not follow a tt.

[orlp]

@Shnatsel I have fixed the first concern of Astra, the second concern I think is fine to leave as is, it only causes a compilation error if you try to name your kernel __fearless_simd_kernel.

[Shnatsel]

We're inserting generics under the unsafe block so we need to validate them. Could you elaborate on how this validation happens right now and why it is robust?

[orlp]

@Shnatsel As the safety comment says, we only expand $const_param, $token and $arg, all of which are idents. This is already very safe, the only way it could possibly mess things up is if those referred to static muts or global variables with a hypothetical UnsafeDeref (which I don't think exists yet?).

However we also made sure that these identifiers are bound in that scope. We are inside a function:

$vis fn $name $(<
    $($lifetime $(: $lt_bound $(+ $lt_bounds)*)?,)*
    $($(const $const_param: $const_ty),+)?
>)? (
    $token: $crate::$level $(, $arg: $arg_ty)*
) $(-> $ret)? {

Once again here all the parameters involved are plain :vis, :lifetime, :ident, or :ty fragments. So it's not possible for malicious code to insert some arbitrary nonsense messing up the definition.

[Shnatsel]

Astra proposed a way to simplify the expression under unsafe, does that look sound to you?

simplify-pr-436.patch

Here's its commentary: https://gist.github.com/Shnatsel/23797a13a6093b78b5dd3cf1894301a1

[orlp]

@Shnatsel It's just an internal macro helper syntax change, doesn't affect soundness in any way. I think it's fine either way (applied or not).

[Shnatsel]

Thank you!

Looks good to me, but I am not very proficient with macros so I'd appreciate if someone else could take a look.

[Shnatsel]

Doesn't seem to noticeably impact build time either