Rustfmt some code (WIP)

[killercup]

For https://users.rust-lang.org/t/please-help-test-rustfmt/5386

I divided the changes that rustfmt into 4 sets: Good, Ambiguous, Bad, and Errors. All but the last are pretty subjective, I guess.

I myself would only go with the "Good Changes" (and error fixes, of course), but I included all changes so this PR can also be used for discussing some rustfmt stuff.

[Stebalien]

IMO, this is actually a good change, for consistency. Attributes usually go above statements. You'd never write:

#[inline] fn test() {
}

[killercup]

Good point.

I would put empty lines around the #[macro_use] extern crate line though. And re-order the imports so that the #[macro_use] ones come first.

[Stebalien]

You mean like this?

#[macro_use]
extern crate rustc;

extern crate arena;
extern crate rustc_data_structures;
extern crate rustc_mir;
extern crate syntax;

I agree that putting a newline before attributes makes sense.

However, re-ordering statements isn't rustfmt can reliably do due to comments. That is, there's no way to determine if // From rustc. should stick to extern crate arena; or if it should stay where it is.

[solson]

I'm mostly ambivalent about this change. I used the same-line #[macro_use] after seeing it in other projects including rustc. However, checking now, it's not even consistent within rustc.

On the other hand, I want to keep the list of crates alphabetical.

[solson]

There are a bunch of good changes in here (a bunch of which I didn't comment on), but there's also a lot of stuff I consider harmful to readability and editability, like use of alignment for function arguments and multi-line method chains.

I say it's harmful to readability because it rapidly increases rightward drift which leads to yet more line-breaking and alignment when the things being pushed right are already long. Altogether that can lead to way more lines for an originally compact piece of code.

I say it's harmful to editability because alignment means you need to update a bunch of other lines when you make a change to something like the name of function that is called with aligned arguments.

For me, either rustfmt defaults need to change or I'd have to adopt a custom configuration, which seems unfortunate. I'm pro-uniform formatting across the community, but rustfmt is currently a bit crazy.

[solson]

Oh, I should note as well that the original code is definitely not fully consistent, especially not function signatures. I was trying out a few different styles at different times, and now I think I lean towards a style I got from @retep998.

For signatures that fit on line, nothing is done. For signatures that are too long, it becomes:

    fn assign_to_aggregate(
        &mut self,
        dest: Pointer,
        dest_repr: &Repr,
        variant: usize,
        discr: Option<u64>,
        operands: &[mir::Operand<'tcx>],
    ) -> EvalResult<()> {
        // ...
    }

(NB: As it happens, though, dest/dest_repr can probably be merged in some way, I just haven't gotten around with it.)

[killercup]

I left some comments as well, trying to link some rustfmt issues. In general, I don't want to add a rustfmt.toml because I want to have One True Rust Style.

I agree that 4-space indentation is better than visual alignment—although I have to admit that I like reading it (on a widescreen display). I tend to write chained method calls like this:

let long_variable_name: Result<Vec<String>, Box<Error>> =
    thingy // could also be directly after the =
    .iter().enumerate() // 'simple' method calls in one line
    .map(|(i, x)| {
        /* do stuff */
        /* possibly multiple lines, themselves indented */
    }).collect(); // no need to put this on a different line, also visually end the .map

(Well, I should've probably written that in a rustfmt issue instead.)

[solson]

I left some comments as well, trying to link some rustfmt issues. In general, I don't want to add a rustfmt.toml because I want to have One True Rust Style.

Thanks for working to improve rustfmt! I would also like to have one community style, and I'm generally willing to accept things I don't prefer to get there, within reason (including some, but not all of the things I complained about on this PR).

I agree that 4-space indentation is better than visual alignment—although I have to admit that I like reading it (on a widescreen display).

I use widescreen displays, but I use them so I can have multiple files side by side, or an editor and a terminal. So I have a relatively narrow effective display, generally enough for 100 chars wide in a given buffer, and rustfmt's alignment really pushes a lot of code to that breaking point.

I tend to write chained method calls like this:

This is basically exactly how I like them. These indentation rules seem simpler to reason about and implement to me, too. Especially considering that every editor needs to implement something approximating rustfmt style if we want programmers to actually use the style (it makes no sense to write in one style and then convert - you want your editor to handle the code as it should be).

[killercup]

I'll close this PR, it was only meant to gather some feedback for rustfmt.

When some changes to rustfmt are release (e.g. rust-lang/rustfmt#960), we can try again 😄

[solson]

Sounds good.