arduano / simdeez

easy simd
MIT License
332 stars 25 forks source link

cargo fmt breaks simdeez macro sometimes #40

Closed BillyDM closed 1 year ago

BillyDM commented 4 years ago

Running cargo fmt in a project will sometimes break the simd_compiletime_generate! and simd_runtime_generate! macros. When it does, rust says it cannot find any of the functions generated with the macro. For example,cargo fmt will break up long functions line by line for readability like this:

simd_compiletime_generate!(
fn smoothed_gain_4_simd(buf_1: &mut [f32], buf_2: &mut [f32], buf_3: &mut [f32], buf_4: &mut [f32], start_amp: f32, end_amp: f32) {
    // ...
});

becomes

simd_compiletime_generate!(
    fn smoothed_gain_4_simd(
        buf_1: &mut [f32],
        buf_2: &mut [f32],
        buf_3: &mut [f32],
        buf_4: &mut [f32],
        start_amp: f32,
        end_amp: f32,
    ) {
        // ...
    }
);

I found if I remove the comma after end_amp: f32, then it works again. However, running cargo fmt again adds the comma back.

jackmott commented 4 years ago

maybe we need to put the attribute that tells cargo fmt to leave it alone on there

cisaacson commented 3 years ago

I tried adding this attribute:

#[rustfmt::skip::macros(simd_runtime_generate)]

And it does work. However, it seems that since the trailing comma is legal Rust syntax as shown in the example above, can the macro be fixed to allow for this case? It would be much nicer to have it work well with the formatter.

BillyDM commented 3 years ago

I'll ask the Rust Community Discord to see if someone who is an expert on macros can help.

BillyDM commented 3 years ago

I also found this that may be useful. https://danielkeep.github.io/tlborm/book/pat-trailing-separators.html

BillyDM commented 3 years ago

The recent pull request fixed the issue for me. When can I expect the crates.io version to be updated with the fix?

verpeteren commented 1 year ago

I checked the commit logs and it seems that this PR fixed the trailing problem on 2020-08-29, just as @BillyDM mentioned on 2020-08-30.

I suggest that we close this issue.