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

Single-line where. #74

Closed
ticki opened this issue Mar 24, 2017 · 13 comments
Closed

Single-line where. #74

ticki opened this issue Mar 24, 2017 · 13 comments

Comments

@ticki
Copy link

ticki commented Mar 24, 2017

https://users.rust-lang.org/t/new-style-guidelines/10037/46?u=ticki

ticki added a commit to ticki/fmt-rfcs that referenced this issue Mar 24, 2017
@ticki
Copy link
Author

ticki commented Mar 24, 2017

master...ticki:patch-1

ticki added a commit to redox-os/tfs that referenced this issue Mar 24, 2017
@killercup
Copy link
Member

Am I right to assume that for simple functions the where clause will be inline? So that with this proposal we have another intermediate step between "short fn" and "complex fn"?

I can see where you are coming from (too many lines for the where clauses), but I would prefer not to do this, as it adds another special case and also the potential for diff churn each time we add a new clause. (Please note that having the where clause at the end of the return type's line could also make this nicer.)

@ticki
Copy link
Author

ticki commented Mar 25, 2017

@killercup

I guess I didn't make clear that I'm only talking about single-bound wheres, so something like

fn blah()
where T: Abc, U: Abc, F: Abc, // long line

is not allowed

@ticki
Copy link
Author

ticki commented Mar 25, 2017

See redox-os/tfs@a929e98 for examples.

@killercup
Copy link
Member

Ah, okay. I'd say

fn key_matches(&self, key: &K) -> bool
    where K: PartialEq {

can be put on one line, like

fn key_matches(&self, key: &K) -> bool where K: PartialEq {

The next fn,

fn scan_mut<F>(&self, key: &K, matches: F) -> RwLockWriteGuard<Bucket<K, V>>
where F: Fn(&Bucket<K, V>) -> bool {
    let hash = self.hash(key);
    // ....

is too long for one line and has a 'complex' constraint (as do many fns taking closures – is this common case we should optimize for?).

I think your suggestion is a good suggestion for that case. I's a bit unfortunate that adding another constraint will result in 4 changed lines (+4 -1), though:

fn scan_mut<F>(&self, key: &K, matches: F) -> RwLockWriteGuard<Bucket<K, V>>
where
    F: Fn(&Bucket<K, V>) -> bool,
    K: Hash,
{
    let hash = self.hash(key);
    // ....

Because of that, and because I think the currently suggested version (basically the previous example without the second constraint) still looks okay, I'm wary of introducing an in-between version.

@ticki
Copy link
Author

ticki commented Mar 25, 2017

fn key_matches(&self, key: &K) -> bool where K: PartialEq {

I dislike that, because it's harder to scan for the return type.

@ticki
Copy link
Author

ticki commented Mar 25, 2017

Also, it clogs the diff.

@joshtriplett
Copy link
Member

Yeah, I really don't want to have where or its clauses on the same line as a return type.

@nrc
Copy link
Member

nrc commented May 9, 2017

I think we discussed this option in the where clause discussions and we decided we did not want to support it. In particular, if the clause is really this simple, then it should probably be an inline bound.

@rust-lang-nursery/style What does the rest of the team think?

@nrc
Copy link
Member

nrc commented May 15, 2017

OK, so this came up in the style meeting while discussing other issues. While we still agree that this should not be the default, we think that Rustfmt should support it as an option, in particular for people who prefer to use where clauses all the time, rather than putting simple bounds inline. Furthermore, it seems that it is a common enough option (and complicated enough) that we should specify how it works, rather than just hacking it in Rustfmt.

I see the following constraints:

  • there must be exactly one clause of the where clause
  • the whole signature of the item including the where clause must fit on one line (e.g., multi-line args also force a multi-line where clause)
  • do we want to enforce a simplicity constraint on the where clause? I.e., to prevent something like where T: Copy + Clone + 'a + 'b + ?Sized to be on one line?

In terms of formatting I think the only non-obvious point is that there should be no trailing comma.

@joshtriplett
Copy link
Member

joshtriplett commented May 17, 2017 via email

@mathstuf
Copy link

For where clauses I have been doing:

fn foo<T, U>(in: U) -> T
    where T: Default,
          U: MyTrait,
{
}

I like the trailing comma because I like the consistency (the only place I drop it are for single-line [] or () expressions). Is there a reason to not allow this (I'm fine with dropping the first clause down off the where line, but the rest is something I personally prefer over the other candidates shown here.

The hoisted { looks weird to me after the comma, but I prefer trailing commas over the hoisted brace.

@nrc
Copy link
Member

nrc commented Jun 11, 2017

Entering FCP that this will be an option supported by Rustfmt, but off by default. When switched on it has the following constraints:

  • there must be exactly one clause of the where clause
  • the whole signature of the item including the where clause must fit on one line (e.g., multi-line args also force a multi-line where clause)

The formatting would be that the where clause starts at the same indent as fn, has no linebreaks, no trailing comma and the opening { of the body is on the same line, e.g.,

fn foo<X>() -> Bar
where X: SomeBound {
    ...
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants