Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking issue for patterns in functions without body #35203

Open
4 of 6 tasks
petrochenkov opened this issue Aug 2, 2016 · 14 comments
Open
4 of 6 tasks

Tracking issue for patterns in functions without body #35203

petrochenkov opened this issue Aug 2, 2016 · 14 comments
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-future-compatibility Category: Future-compatibility lints T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@petrochenkov
Copy link
Contributor

petrochenkov commented Aug 2, 2016

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

bors added a commit that referenced this issue Aug 4, 2016
Properly enforce the "patterns aren't allowed in foreign functions" rule

Cases like `arg @ PATTERN` or `mut arg` were missing.
Apply the same rule to function pointer types.

Closes #35203
[breaking-change], no breakage in sane code is expected though
r? @nikomatsakis

This is somewhat related to rust-lang/rfcs#1685 (cc @matklad).
The goal is to eventually support full pattern syntax where it makes sense (function body may present) and to support *only* the following forms - `TYPE`, `ident: TYPE`, `_: TYPE` - where patterns don't make sense (function body doesn't present), i.e. in foreign functions and function pointer types.
@petrochenkov petrochenkov changed the title Explanatory issue for patterns in foreign functions and function pointers Tracking issue for patterns in functions without body Oct 25, 2016
@eddyb eddyb reopened this Oct 25, 2016
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 26, 2016
Prohibit patterns in trait methods without bodies

They are not properly type checked
```rust
trait Tr {
    fn f(&a: u8); // <- This compiles
}
```
, mostly rejected by the parser already and generally don't make much sense.
This PR is kind of a missing part of rust-lang#35015.

Needs crater run.
cc rust-lang#35078 (comment) rust-lang#35015 rust-lang/rfcs#1685 rust-lang#35203
r? @eddyb
@TimNN TimNN reopened this Oct 26, 2016
bors added a commit that referenced this issue Oct 29, 2016
Prohibit patterns in trait methods without bodies

They are not properly type checked
```rust
trait Tr {
    fn f(&a: u8); // <- This compiles
}
```
, mostly rejected by the parser already and generally don't make much sense.
This PR is kind of a missing part of #35015.

Given the [statistics from crater](#37378 (comment)), the effect of this PR is mostly equivalent to improving `unused_mut` lint.

cc #35078 (comment) #35015 rust-lang/rfcs#1685 #35203
r? @eddyb
@sanxiyn sanxiyn closed this as completed Nov 15, 2016
@petrochenkov
Copy link
Contributor Author

@sanxiyn
You have closed this issue accidentally by updating your cargo clone (!?).

@TimNN TimNN reopened this Nov 15, 2016
gandro pushed a commit to gandro/timely-dataflow that referenced this issue Feb 13, 2017
These are deprecated and will become a hard error in the future,
see rust-lang/rust#35203
@petrochenkov petrochenkov added the A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. label Feb 19, 2017
@brson brson added the B-unstable Blocker: Implemented in the nightly compiler and unstable. label Mar 1, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Jul 22, 2017
@KamilaBorowska
Copy link
Contributor

KamilaBorowska commented Nov 4, 2017

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.

@petrochenkov
Copy link
Contributor Author

@xfix
This is a largely orthogonal parsing error, fixed in #45775.

bors added a commit that referenced this issue Nov 11, 2017
Accept interpolated patterns in trait method parameters

Permit this, basically
```rust
macro_rules! m {
    ($pat: pat) => {
        trait Tr {
            fn f($pat: u8) {}
        }
    }
}
```
it previously caused a parsing error during expansion because trait methods accept only very restricted set of patterns during parsing due to ambiguities caused by [anonymous parameters](#41686), and this set didn't include interpolated patterns.

Some outdated messages from "no patterns allowed" errors are also removed.

Addresses #35203 (comment)
@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Dec 23, 2018
@Centril Centril added C-future-compatibility Category: Future-compatibility lints and removed C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Jan 13, 2019
@Centril
Copy link
Contributor

Centril commented Jan 13, 2019

@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).

@petrochenkov
Copy link
Contributor Author

@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.

@Centril
Copy link
Contributor

Centril commented Jan 13, 2019

@petrochenkov Awesome!

bors added a commit that referenced this issue Aug 3, 2019
Transition some C-future-compatibility lints to {ERROR, DENY}

Closes #40107 (ERROR).
Closes #39207 (ERROR).
Closes #37872 (ERROR).
Closes #36887 (ERROR).
Closes #36247 (ERROR.
Closes #42238 (ERROR).
Transitions #59014 (DENY).
Transitions #57571 (DENY).
Closes #60210 (ERROR).
Transitions #35203 (DENY).

r? @petrochenkov
@yangzhe1990
Copy link

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.

@Centril
Copy link
Contributor

Centril commented Sep 9, 2019

@yangzhe1990 Yep; that's intended behavior, at least for now.

Centril added a commit to Centril/rust that referenced this issue Nov 8, 2019
Transition future compat lints to {ERROR, DENY} - Take 2

Follow up to rust-lang#63247 implementing rust-lang#63247 (comment).

- `legacy_ctor_visibility` (ERROR) -- closes rust-lang#39207
- `legacy_directory_ownership` (ERROR) -- closes rust-lang#37872
- `safe_extern_static` (ERROR) -- closes rust-lang#36247
- `parenthesized_params_in_types_and_modules` (ERROR) -- closes rust-lang#42238
- `duplicate_macro_exports` (ERROR)
- `nested_impl_trait` (ERROR) -- closes rust-lang#59014
- `ill_formed_attribute_input` (DENY) -- transitions rust-lang#57571
- `patterns_in_fns_without_body` (DENY) -- transitions rust-lang#35203

r? @varkor
cc @petrochenkov
Centril added a commit to Centril/rust that referenced this issue Nov 8, 2019
Transition future compat lints to {ERROR, DENY} - Take 2

Follow up to rust-lang#63247 implementing rust-lang#63247 (comment).

- `legacy_ctor_visibility` (ERROR) -- closes rust-lang#39207
- `legacy_directory_ownership` (ERROR) -- closes rust-lang#37872
- `safe_extern_static` (ERROR) -- closes rust-lang#36247
- `parenthesized_params_in_types_and_modules` (ERROR) -- closes rust-lang#42238
- `duplicate_macro_exports` (ERROR)
- `nested_impl_trait` (ERROR) -- closes rust-lang#59014
- `ill_formed_attribute_input` (DENY) -- transitions rust-lang#57571
- `patterns_in_fns_without_body` (DENY) -- transitions rust-lang#35203

r? @varkor
cc @petrochenkov
Mark-Simulacrum added a commit to rust-lang/rustc-perf that referenced this issue Nov 9, 2019
arcnmx added a commit to arcnmx/specialize-rs that referenced this issue Jan 30, 2020
richardfarr-okta added a commit to azuqua/fred.rs that referenced this issue Jan 31, 2020
alecembke-okta pushed a commit to azuqua/fred.rs that referenced this issue Jan 31, 2020
@pickfire
Copy link
Contributor

pickfire commented Nov 8, 2020

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

code
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)
    }
}

@estebank
Copy link
Contributor

@pickfire filed #78927 to track only that change.

@jhpratt
Copy link
Member

jhpratt commented Mar 28, 2021

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.

@MaulingMonkey
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-future-compatibility Category: Future-compatibility lints T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
Archived in project
Development

No branches or pull requests