Tracking issue for future-incompatibility lint `patterns_in_fns_without_body`

Opened by Vadim Petrochenkov at 2024-09-14 11:59:40

Code using patterns in parameters of foreign functions, function pointers or trait methods without body is not accepted by the compiler.

extern {
    fn f((a, b): (u8, u16)); // ERROR
}

type T = fn((a, b): (u8, u16)); // ERROR

trait Tr {
    fn f((a, b): (u8, u16)); // ERROR
}

Previously some simple patterns like &ident, &&ident or mut ident were allowed in these positions, now they aren't. Such patterns weren't properly type checked, e.g. type A = fn(&arg: u8); compiled despite u8 not being a reference.

What are these errors for?

Full patterns don't make sense in functions without bodies, but simple identifiers may be useful for documenting argument purposes even if they aren't actually used - type Callback = fn(useful_name: u8). By restricting patterns in body-less function signatures to ident: TYPE we can make argument names optional and accept simply a TYPE in argument position (type T = fn(u8)) without introducing parsing ambiguities.

How to fix this warning/error

Remove & or mut from the pattern and make the function parameter a single identifier or _.

Current status

[x] https://github.com/rust-lang/rust/pull/35015 made patterns in foreign functions and function pointers hard errors
[x] https://github.com/rust-lang/rust/pull/37378 made patterns in trait methods without body warn-by-default lints
[x] https://github.com/rust-lang/rust/pull/45775 made patterns in trait methods without body hard errors, except for mut IDENT
[x] https://github.com/rust-lang/rust/pull/65785 made patterns in trait methods without body deny-by-default lints
[ ] ? made patterns in trait methods without body hard errors

Code using patterns in parameters of foreign functions, function pointers or trait methods without body is not accepted by the compiler.
```
extern {
    fn f((a, b): (u8, u16)); // ERROR
}

type T = fn((a, b): (u8, u16)); // ERROR

trait Tr {
    fn f((a, b): (u8, u16)); // ERROR
}
```
Previously some simple patterns like &ident, &&ident or mut ident were allowed in these positions, now they aren't. Such patterns weren't properly type checked, e.g. type A = fn(&arg: u8); compiled despite u8 not being a reference.

What are these errors for?

Full patterns don't make sense in functions without bodies, but simple identifiers may be useful for documenting argument purposes even if they aren't actually used - type Callback = fn(useful_name: u8). By restricting patterns in body-less function signatures to ident: TYPE we can make argument names optional and accept simply a TYPE in argument position (type T = fn(u8)) without introducing parsing ambiguities.

How to fix this warning/error

Remove & or mut from the pattern and make the function parameter a single identifier or _.

Current status
- [x] https://github.com/rust-lang/rust/pull/35015 made patterns in foreign functions and function pointers hard errors
- [x] https://github.com/rust-lang/rust/pull/37378 made patterns in trait methods without body warn-by-default lints
- [x] https://github.com/rust-lang/rust/pull/45775 made patterns in trait methods without body hard errors, except for mut IDENT
- [x] https://github.com/rust-lang/rust/pull/65785 made patterns in trait methods without body deny-by-default lints
- [ ] ? made patterns in trait methods without body hard errors
- [x] https://github.com/rust-lang/rust/issues/78927 provide structured suggestions
Esteban Kuber at 2020-11-10 18:45:56
@sanxiyn You have closed this issue accidentally by updating your cargo clone (!?).
Vadim Petrochenkov at 2016-11-15 12:58:36
https://github.com/xfix/extension-trait uses patterns in functions without body to use the same signature for a trait and its implementation. Due to pat macro pattern not being able to able to be followed with :, this is tricky to fix.
Kamila Borowska at 2017-11-04 23:23:27
@xfix This is a largely orthogonal parsing error, fixed in https://github.com/rust-lang/rust/pull/45775.
Vadim Petrochenkov at 2017-11-05 01:29:28
I would like to propose that we continue to support mut x patterns forever. I think we should just lint on them as a "unnecessary mut error" (like any other mut that has no effect). They don't seem "logically" harmful in any way, and there are some number of crates using them. I don't see a lot of value to making the deprecating harder.
Niko Matsakis at 2017-11-14 20:03:39
@nikomatsakis

I would like to propose that we continue to support mut x patterns forever.

So, why do we disallow patterns in these positions in the first place?
1. They are ignored and useless. If some pattern is written in the trait, or foreign fn, or fn pointer position it's either a mistake/misunderstanding (all examples found by crater) or some macro expanding into both trait definition and implementations (this wasn't possible before https://github.com/rust-lang/rust/pull/45775).
2. Technical reason - we don't want to check these patterns. Patterns are checked in bodies and fn pointers etc don't have bodies and we don't want to create bodies for them only to check something as useless as patterns that are ignored otherwise.
Is there a middle ground? Some patterns don't need checking because they don't do any destructuring. Legal non-destructuring patterns are _, binding, mut binding, ref binding, ref mut binding, binding @ _, mut binding @ _, ref binding @ _, ref mut binding @ _. (From these _ and binding are currently permitted in all positions, and mut binding is permitted only in trait methods with a warning.) These patterns can be easily permitted without any checks or creating any additional bodies. All of these patterns are still useless and mean that some misunderstanding is happening, so we should keep at least warning on them.

My conclusion is that I'd rather keep mut binding a deprecation warning for indefinite amount of time (maybe until one of the next epochs or something) than permit more useless patterns, or more special diverging rules permitting these patterns e.g. in traits but not in foreign functions.
Vadim Petrochenkov at 2017-11-18 13:57:16
@petrochenkov What's the status of this right now? It would be nice to see a crater run near term to bring this to a close (possibly including mut x cause it would be nice with a cleaner grammar).
Mazdak Farrokhzad at 2019-01-13 14:04:13
@Centril My plan was to run crater for all those old deprecation lints from 2016 after the edition release. It's actually pretty close to the top in my work queue now.
Vadim Petrochenkov at 2019-01-13 14:11:57
@petrochenkov Awesome!
Mazdak Farrokhzad at 2019-01-13 14:13:06
Hi there, today I found that define something like fn into_blah(mut self) in a trait trigger this error. I wonder if this is intended behavior.
yangzhe1990 at 2019-09-09 13:39:08
@yangzhe1990 Yep; that's intended behavior, at least for now.
Mazdak Farrokhzad at 2019-09-09 15:12:36

error: patterns aren't allowed in functions without bodies
  --> src/lib.rs:11:45
   |
11 |     fn partition_dedup_by_new<F>(&mut self, mut same_bucket: F) -> (&mut [T], &mut [T])
   |                                             ^^^^^^^^^^^^^^^
   |
   = note: `#[deny(patterns_in_fns_without_body)]` on by default
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #35203 <https://github.com/rust-lang/rust/issues/35203>

error: aborting due to previous error

error: could not compile `alt-dedup-rs`

To learn more, run the command again with --verbose.

Looks rust is giving me this error without suggestion, I didn't know the fix was just simply removing mut which does not seemed related to this issue. CC @estebank

use std::mem;

pub trait NewDedup<T> {
    fn partition_dedup_new(&mut self) -> (&mut [T], &mut [T])
    where
        T: PartialEq<T>,
    {
        self.partition_dedup_by_new(|a, b| a == b)
    }

    fn partition_dedup_by_new<F>(&mut self, mut same_bucket: F) -> (&mut [T], &mut [T])
    where
        F: FnMut(&mut T, &mut T) -> bool;
}

impl<T> NewDedup<T> for &mut [T] {
    fn partition_dedup_by_new<F>(&mut self, mut same_bucket: F) -> (&mut [T], &mut [T])
    where
        F: FnMut(&mut T, &mut T) -> bool,
    {
        let len = self.len();
        if len <= 1 {
            return (self, &mut []);
        }

        let ptr = self.as_mut_ptr();
        let mut next_read: usize = 1;
        let mut next_write: usize = 1;

        // SAFETY: the `while` condition guarantees `next_read` and `next_write`
        // are less than `len`, thus are inside `self`. `prev_ptr_write` points to
        // one element before `ptr_write`, but `next_write` starts at 1, so
        // `prev_ptr_write` is never less than 0 and is inside the slice.
        // This fulfils the requirements for dereferencing `ptr_read`, `prev_ptr_write`
        // and `ptr_write`, and for using `ptr.add(next_read)`, `ptr.add(next_write - 1)`
        // and `prev_ptr_write.offset(1)`.
        //
        // `next_write` is also incremented at most once per loop at most meaning
        // no element is skipped when it may need to be swapped.
        //
        // `ptr_read` and `prev_ptr_write` never point to the same element. This
        // is required for `&mut *ptr_read`, `&mut *prev_ptr_write` to be safe.
        // The explanation is simply that `next_read >= next_write` is always true,
        // thus `next_read > next_write - 1` is too.
        unsafe {
            // Avoid bounds checks by using raw pointers.
            while next_read < len {
                let ptr_read = ptr.add(next_read);
                let prev_ptr_write = ptr.add(next_write - 1);
                if !same_bucket(&mut *ptr_read, &mut *prev_ptr_write) {
                    next_write += 1;
                    next_read += 1;
                    // Avoid checking next_write != next_read once it is not in the same bucket,
                    // always swap memory if it is not in the same bucket.
                    while next_read < len {
                        let ptr_read = ptr.add(next_read);
                        let prev_ptr_write = ptr.add(next_write - 1);
                        if !same_bucket(&mut *ptr_read, &mut *prev_ptr_write) {
                            let ptr_write = prev_ptr_write.offset(1);
                            mem::swap(&mut *ptr_read, &mut *ptr_write);
                            next_write += 1;
                        }
                        next_read += 1;
                    }
                    return self.split_at_mut(next_write);
                }
                next_read += 1;
            }
        }

        self.split_at_mut(next_write)
    }
}

Ivan Tham at 2020-11-08 09:47:03

@pickfire filed https://github.com/rust-lang/rust/issues/78927 to track only that change.
Esteban Kuber at 2020-11-10 18:45:18
Possible reason for this to stay around: macros. While I never write code like this by hand, I just happened to generate code that hit this warning by auto-generating a trait definition based on a provided implementation. Without any way to cleanly extract a function header, it's stupidly difficult to avoid this warning in a situation like this.
Jacob Pratt at 2021-03-28 01:42:04
Another use case: hinting to auto-completion tools like rust-analyzer, which will copy parameter names, patterns, etc. when auto-implementing a trait. Just tried to make a parameter default to mut and failed.
MaulingMonkey at 2023-09-20 23:34:27