Stream: t-compiler

Topic: rustfmt landing


simulacrum (Dec 21 2019 at 15:59, on Zulip):

We should be landing rustfmt checking and a mass formatting some time in the next week (basically, as soon as one lands, we'll be posting the other).

This will mean that developers will want to enable rustfmt in editors or run ./x.py fmt before pushing up. We have an ignore file -- it's likely we'll need some manual editing to pacify tidy around line lengths in some edge cases, but that should hopefully be quite rare.

oli (Dec 21 2019 at 16:20, on Zulip):

It is happening! May the rustfmt finally be with us

Santiago Pastorino (Dec 22 2019 at 00:34, on Zulip):

this is so cool, hope that happens after #67000 is in :smile:

oli (Dec 22 2019 at 00:40, on Zulip):

unlikely, but rebasing should not be a problem, just force your own changes and rustfmt them, the diff should be just your changes after that

RalfJ (Dec 22 2019 at 16:37, on Zulip):

I guess this is the default rustfmt setting, not whatever @oli used in Miri to make rustfmt not waste quite that much vertical space? Does that mean I need a new screen because rustc code will take twice as much vertical space as before?^^ (rustfmt seems to be quite willing to waste tons of vertical space in complex code, in my experience.)

Matthew Jasper (Dec 22 2019 at 16:56, on Zulip):

It's not the default settings.

anp (Dec 22 2019 at 18:11, on Zulip):

iirc the settings were agreed upon in a compiler meeting that i wasn’t even aware of when i started the PR that @simulacrum has picked up and overhauled to land

anp (Dec 22 2019 at 18:11, on Zulip):

ie it happened a while ago

simulacrum (Dec 22 2019 at 18:11, on Zulip):

We can adjust them as we land the pr in theory too

simulacrum (Dec 22 2019 at 18:12, on Zulip):

(that actually formats code)

simulacrum (Dec 22 2019 at 18:12, on Zulip):

I'm working on that and should have it up tonight

Matthew Jasper (Dec 22 2019 at 18:12, on Zulip):

How much are you formatting? Everything?

anp (Dec 22 2019 at 18:14, on Zulip):

i missed the conversation discussing how to do follow up formatting

anp (Dec 22 2019 at 18:14, on Zulip):

i imagine we don’t want to have unformatted parts of the tree for long

anp (Dec 22 2019 at 18:14, on Zulip):

i had this fever dream for a bot that could do a directory at a time and land them only when no approved prs would conflict

simulacrum (Dec 22 2019 at 21:37, on Zulip):

I don't think it's too worth it

oli (Dec 22 2019 at 21:59, on Zulip):

+76,400 −85,063

Zoxc (Dec 23 2019 at 05:52, on Zulip):

I think @simulacrum code contribution ranking is going to be halved =P

simulacrum (Dec 23 2019 at 12:45, on Zulip):

I recommend git rebase -i origin/master --exec './x.py fmt && git add -u && git commit --no-edit --amend' -Xtheirs

simulacrum (Dec 23 2019 at 12:45, on Zulip):

that should auto rebase past formatting

simulacrum (Dec 23 2019 at 12:46, on Zulip):

(but note: only formatting, if you conflicted with some other PR this will just eat that change)

Santiago Pastorino (Dec 23 2019 at 13:18, on Zulip):

I recommend git rebase -i origin/master --exec './x.py fmt && git add -u && git commit --no-edit --amend' -Xtheirs

I was going to say, would be nice to document the way to rebase PRs against this massive change

Santiago Pastorino (Dec 23 2019 at 13:19, on Zulip):

unsure if a post in IRLO, a blog post or where should we explain what happened and what contributors can do

Santiago Pastorino (Dec 23 2019 at 13:19, on Zulip):

mainly for the people that are not around in general ...

Santiago Pastorino (Dec 23 2019 at 13:20, on Zulip):

... just thinking a bit loudly :)

Santiago Pastorino (Dec 23 2019 at 13:20, on Zulip):

@simulacrum pretty sure that you have considered this stuff but just wondering also what's the way forward

nikomatsakis (Dec 23 2019 at 13:23, on Zulip):

This feels worth of an Inside Rust blog post once it lands

centril (Dec 23 2019 at 13:23, on Zulip):

It landed :P

simulacrum (Dec 23 2019 at 13:24, on Zulip):

I can try to write something up

pnkfelix (Dec 23 2019 at 13:24, on Zulip):

I wonder whether this would have qualified as a major change

centril (Dec 23 2019 at 13:24, on Zulip):

I think it would have

simulacrum (Dec 23 2019 at 13:24, on Zulip):

we did approve and discuss it a few times, so I think it did to some extent

simulacrum (Dec 23 2019 at 13:24, on Zulip):

(and certainly if we did it again, it would go through that process)

simulacrum (Dec 23 2019 at 13:25, on Zulip):

I will work on a inside rust post

centril (Dec 23 2019 at 13:25, on Zulip):

I'm changing origin/master to upstream/master in the above and then running ./x.py test -i src/test/ui --stage 1 --pass check --bless at the end just to check it's all OK

centril (Dec 23 2019 at 13:25, on Zulip):

sadly, some of the PRs no longer build after every commit

centril (Dec 23 2019 at 13:25, on Zulip):

maybe ./x.py check in --exec to verify that it does

simulacrum (Dec 23 2019 at 13:25, on Zulip):

I'm actually going to write up the command such that it always works (even if there's other issues)

simulacrum (Dec 23 2019 at 13:26, on Zulip):

i.e., you have to resolve rebase conflicts if necessary

simulacrum (Dec 23 2019 at 13:26, on Zulip):

but if you have syntax errors then this isn't going to work (of course)

centril (Dec 23 2019 at 13:27, on Zulip):

I had to rescue a fairly large PR because I used -Xours instead of -Xtheirs :D

simulacrum (Dec 23 2019 at 13:42, on Zulip):

@nikomatsakis @pnkfelix https://github.com/rust-lang/blog.rust-lang.org/pull/485

Maybe would be good to get that in sooner rather than later

davidtwco (Dec 23 2019 at 15:20, on Zulip):

I’m really excited about this. Perhaps a post on internals would be good too?

simulacrum (Dec 23 2019 at 15:33, on Zulip):

I'm going to cross-post the inside rust blogpost once @nikomatsakis takes a look (and hopefully merges it!)

nikomatsakis (Dec 23 2019 at 15:34, on Zulip):

I'll look now :)

nikomatsakis (Dec 23 2019 at 15:34, on Zulip):

also, I'm super excited too

nikomatsakis (Dec 23 2019 at 15:34, on Zulip):

I guess I can enable "rustfmt on save" basically all the time now

nikomatsakis (Dec 23 2019 at 15:35, on Zulip):

shall I merge @simulacrum ?

nikomatsakis (Dec 23 2019 at 15:35, on Zulip):

sounds like yes

simulacrum (Dec 23 2019 at 15:35, on Zulip):

sure yes

nikomatsakis (Dec 23 2019 at 15:36, on Zulip):

done

simulacrum (Dec 23 2019 at 15:36, on Zulip):

I currently have to change rustfmt to point to the "local" copy in our repo to get precisely the same formatting (usually at e.g. build/x86_64-apple-darwin/stage0/bin/rustfmt) but that's only true of 3 files across all of rust

simulacrum (Dec 23 2019 at 15:37, on Zulip):

and plausibly spurious? I saw it what felt like only sometimes, so not sure what was up

nikomatsakis (Dec 23 2019 at 15:37, on Zulip):

yeah I have no idea what binary emcs runs

nikomatsakis (Dec 23 2019 at 15:37, on Zulip):

but I guess it's unlikely to be an issue

simulacrum (Dec 23 2019 at 15:37, on Zulip):

in theory though yes auto formatting everywhere should be fine :)

simulacrum (Dec 23 2019 at 15:37, on Zulip):

I'll cross post a link to internals

nikomatsakis (Dec 23 2019 at 15:37, on Zulip):

I really enjoy now writing really horrible rust syntax

nikomatsakis (Dec 23 2019 at 15:38, on Zulip):

and then hitting save and watching it get cleaned up

simulacrum (Dec 23 2019 at 15:39, on Zulip):

my favorite is being able to freely change function arguments without rejiggering the whole function

simulacrum (Dec 23 2019 at 15:41, on Zulip):

https://internals.rust-lang.org/t/formatting-the-compiler-tree/11529

simulacrum (Dec 23 2019 at 15:41, on Zulip):

https://blog.rust-lang.org/inside-rust/2019/12/23/formatting-the-compiler.html

RalfJ (Dec 24 2019 at 10:16, on Zulip):

I saw the use_small_heuristics = "Max", awesome :)

RalfJ (Dec 24 2019 at 10:16, on Zulip):

and then hitting save and watching it get cleaned up

what magic is that?

RalfJ (Dec 24 2019 at 10:17, on Zulip):

are there docs for how to set up IDEs and stuff in a way that actually works for the compiler? reading this I now feel like I am still in the stone age, just using a plain editor with no Rust IDE features whatsoever for rustc hacking^^

simulacrum (Dec 24 2019 at 10:54, on Zulip):

@RalfJ What editor do you use?

RalfJ (Dec 24 2019 at 11:01, on Zulip):

Kate (the KDE editor)

RalfJ (Dec 24 2019 at 11:01, on Zulip):

I'm using vscode for miri hacking though. but even there I have no IDE features due to a lack of RLS.

simulacrum (Dec 24 2019 at 11:02, on Zulip):

I personally use vim -- I imagine vscode though, even without RLS, supports some sort of "run command on save"

RalfJ (Dec 24 2019 at 11:03, on Zulip):

I guess it does. though the blog post said running fmt on the rustc code base takes a while, so taht doesnt sound like something one wants to do on every save?

simulacrum (Dec 24 2019 at 11:03, on Zulip):

well, that's every file

simulacrum (Dec 24 2019 at 11:04, on Zulip):

if you just want to run it on the current file that'll be pretty instant

simulacrum (Dec 24 2019 at 11:04, on Zulip):

you can mostly just run rustfmt --edition=2018 $CURRENT_FILE

RalfJ (Dec 24 2019 at 11:25, on Zulip):

ah makes sense

oli (Dec 24 2019 at 12:14, on Zulip):

in vscode I set

    "editor.formatOnSave": true,
    "editor.formatOnPaste": true,
    "editor.formatOnType": true,

for the projects that are rustfmtable
and


oli (Dec 24 2019 at 12:16, on Zulip):

we could add a .vscode folder to the rustc repo doing the editor.format* settings, but not sure whether everyone is happy with that

Wesley Wiser (Dec 24 2019 at 13:03, on Zulip):

I personally think it would be really great to provide recommended editor settings for the common editors/IDEs in the repo. We could do something similarly to how config.toml works and have one editors or .editors folder in the root with subfolders for the different editors. We'd also setup .gitignore in the project root to ignore the correct locations for the files/folders. Interested users would just need to cp .editors/$EDITOR . to get the recommended default settings.

centril (Dec 24 2019 at 15:53, on Zulip):

@RalfJ oh you need to install rust-analyzer, it's so nice and works really well with rustc

anp (Dec 25 2019 at 00:38, on Zulip):

fwiw there are multiple places in libstd and libcore where rust-analyzer autoformatting results in breakage :confused: and pinned rustfmt is fine

anp (Dec 25 2019 at 00:40, on Zulip):

i’ve started running watchexec — ./x.py fmt in a separate terminal instead

simulacrum (Dec 25 2019 at 01:00, on Zulip):

@anp you might need a different rustfmt in environment

simulacrum (Dec 25 2019 at 01:00, on Zulip):

I'm not sure what rust-analyzer uses by default

oli (Dec 25 2019 at 01:03, on Zulip):

current nightly works well

oli (Dec 25 2019 at 01:03, on Zulip):

but we need a better scheme XD

simulacrum (Dec 25 2019 at 01:06, on Zulip):

well, rustfmt is supposed to be stable

simulacrum (Dec 25 2019 at 01:06, on Zulip):

I'm to be honest not sure why we're seeing this given that

oli (Dec 25 2019 at 01:07, on Zulip):

maybe @anp has a really old one :D

simulacrum (Dec 25 2019 at 01:07, on Zulip):

eh, I see it vs. stable

anp (Dec 25 2019 at 01:12, on Zulip):

i think it might be picking up my stable rustfmt as the default in my environment

anp (Dec 25 2019 at 01:12, on Zulip):

thinking more now, it might be possible to teach ra-lsp to use the bootstrap-managed rustfmt

simulacrum (Dec 25 2019 at 01:13, on Zulip):

it should be

simulacrum (Dec 25 2019 at 01:13, on Zulip):

I gave up myself and just used rust.vim's rustfmt functionality which seemed to work a little better anyway

simulacrum (Dec 25 2019 at 01:14, on Zulip):

(otoh, I'm doing some fairly ... sketchy stuff with piping the LSP protocol across ssh so that I can run ra-lsp-server on my more powerful machine and conserve battery life)

davidtwco (Dec 25 2019 at 01:17, on Zulip):

I'm using the current stable rustfmt through neovim+ALE and I've not had any issues yet.

simulacrum (Dec 25 2019 at 01:18, on Zulip):

@davidtwco I think it's pretty context-dependent

simulacrum (Dec 25 2019 at 01:18, on Zulip):

I observed it on src/librustc_resolve/lib.rs fwiw

simulacrum (Dec 25 2019 at 01:18, on Zulip):

(among others)

simulacrum (Dec 25 2019 at 01:18, on Zulip):

one thing might be that the old rust format won't undo modern rustfmt

oli (Dec 25 2019 at 01:19, on Zulip):

I wish we just had a proper sysroot for stage 0 that we could point a rustup against

simulacrum (Dec 25 2019 at 01:19, on Zulip):

we do?

simulacrum (Dec 25 2019 at 01:19, on Zulip):

build/x86_64-apple-darwin/stage0/ should be rustup linkable I think

simulacrum (Dec 25 2019 at 01:19, on Zulip):

(or whatever triple you have)

simulacrum (Dec 25 2019 at 01:20, on Zulip):

and that's a stable-ish place (obviously we can change it but it won't change often)

oli (Dec 25 2019 at 01:20, on Zulip):

I tried that before I think... let me recheck

simulacrum (Dec 25 2019 at 01:21, on Zulip):

seems to work for me

oli (Dec 25 2019 at 01:21, on Zulip):

hmm... curious, I'm sure I tried this before, but yea, I'm seeing everything in order

simulacrum (Dec 25 2019 at 01:21, on Zulip):

(it's literally just a beta toolchain)

oli (Dec 25 2019 at 01:21, on Zulip):

ok, this makes a few things much simpler

oli (Dec 25 2019 at 01:22, on Zulip):

O_o: can we just use a rustup toolchain file instead of hand-rolling the downloading logic and use rustup for stage 0?

oli (Dec 25 2019 at 01:22, on Zulip):

I mean we can still manually download rustup into build/triple

oli (Dec 25 2019 at 01:22, on Zulip):

ok, maybe that's bad for some environments

simulacrum (Dec 25 2019 at 01:23, on Zulip):

we might be able to get away with it

simulacrum (Dec 25 2019 at 01:23, on Zulip):

I think distros reconfigure the rustc anyway to point at their own binary

oli (Dec 25 2019 at 01:23, on Zulip):

oh, but we could use a rustup toolchain file and make that the canonical location for bootstrap to figure out the stage 0 compiler

simulacrum (Dec 25 2019 at 01:23, on Zulip):

kinda

simulacrum (Dec 25 2019 at 01:23, on Zulip):

we're actually not using a beta rustfmt today

simulacrum (Dec 25 2019 at 01:23, on Zulip):

(so we'd have two toolchains)

oli (Dec 25 2019 at 01:23, on Zulip):

oh ^^

simulacrum (Dec 25 2019 at 01:24, on Zulip):

because we need nightly rustfmt features and rustfmt does not have an equivalent of RUSTC_BOOTSTRAP

oli (Dec 25 2019 at 01:24, on Zulip):

because we need nightly rustfmt features and rustfmt does not have an equivalent of RUSTC_BOOTSTRAP

that seems surmountable

simulacrum (Dec 25 2019 at 01:24, on Zulip):

beta/stable branches will have the rustfmt-checking disabled

simulacrum (Dec 25 2019 at 01:24, on Zulip):

well, we/I asked if rustfmt could add it, and they were against it

simulacrum (Dec 25 2019 at 01:25, on Zulip):

I didn't push too hard though

oli (Dec 25 2019 at 01:31, on Zulip):

ah ok

Last update: May 26 2020 at 11:10UTC