Remove unstable --pretty flag

[jyn514]

It doesn't do anything --unpretty doesn't, and due to a bug, also
didn't show up in --help. I don't think there's any reason to keep it
around, I haven't seen anyone using it.

Closes #36473.

[rust-highfive]

r? @varkor

(rust-highfive has picked a reviewer for you, use r? to override)

[jyn514]

Happy to go through the MCP process if you think it would be helpful, not sure what the procedure is for unstable flags.

[camelid]

I haven't seen anyone using it.

The Rust Playground uses it:

~/rust-playground master
❯ rg -- --pretty
ui/src/sandbox.rs
402:            "--pretty=expanded",

[jyn514]

@varkor I saw you marked this as waiting on author - do you want me to fix CI first? Or does that mean you think removing the flag is a bad idea?

[varkor]

@jyn514: it seemed the premise of the change was in question, as @camelid pointed out that the flag currently is used. Maybe it would be best to remove the use in the Playground before going ahead with removal altogether?

[varkor]

@rust-lang/compiler @rust-lang/compiler-contributors: does know of any reason --pretty is worth keeping around? It seems like this option is subsumed by --unpretty, but perhaps there's some subtlety.

[varkor]

@jyn514: if no-one objects, the changes look good to me.

[spastorino]

Meanwhile we are at this, couldn't (on a different PR of course) we also propose to rename unpretty to something more meaningful?. I've seen a bunch of people confused about it and I also was confused at some point.

[jyn514]

I mean, we could change --unpretty to pretty and remove unpretty if you like. Not sure if you think that's more clear.

[nikomatsakis]

(cc @shepmaster regarding play)

[nikomatsakis]

I don't really have a clear picture of the differences behind these flags, but I think that consolidating seems fine, or perhaps just make one an alias for the other. I think it's pretty amusing that Rust has "unpretty printing" myself :)

[spastorino]

Does it really work like a pretty printer in most cases? or is it more like a dump-* kind of thing?

[jyn514]

Does it really work like a pretty printer in most cases? or is it more like a dump-* kind of thing?

I think most of the formats are just for debugging, not pretty-printing.

    -Z                       unpretty=val -- present the input source, unstable (and less-pretty) variants;
        valid types are any of the types for `--pretty`, as well as:
        `expanded`, `expanded,identified`,
        `expanded,hygiene` (with internal representations),
        `everybody_loops` (all function bodies replaced with `loop {}`),
        `ast-tree` (raw AST before expansion),
        `ast-tree,expanded` (raw AST after expansion),
        `hir` (the HIR), `hir,identified`,
        `hir,typed` (HIR with types for each node),
        `hir-tree` (dump the raw HIR),
        `mir` (the MIR), or `mir-cfg` (graphviz formatted MIR)

[spastorino]

@jyn514 right, that's what I meant, then maybe dump or something else is more precise than pretty/unpretty.

[shepmaster]

remove the use in the Playground

I'm fine removing it, but the playground CI cannot run now because rustfmt is broken. When it unbreaks, I plan to merge the change.

[jyn514]

Rustfmt should have been fixed in #84886.

[pnkfelix]

1. So, in my opinion, the distinction between --pretty and --unpretty is that the options under --pretty are meant to be meaningful to Rust programmers, while the options under --unpretty are solely meaningful to rustc developers.

   • I don't know if that distinction currently holds, but it was what was in my head at the time that I added --unpretty.

2. I also had a vague thought that --pretty, or some subset of it, would be on the path to eventual stabilization. Is that a non-starter at this point? (Or is it an irrelevant question, in the sense that a subset of any flag could be stabilized, and the issue here is whether it makes sense to have two flags...)

I definitely would think it strange to keep a flag named --unpretty if we did not have a --pretty flag that the former is referencing. But it sounds like the conversation here is suggesting that we would rename --unpretty to --pretty, or to --dump, et cetera...

Anyway, I don't know. I think there's value in having two variants, but I suspect I'm in the minority here.

[apiraino]

I'll remove the S-waiting-on-team label to signal that this PR can move forward (discussion)

@rustbot label -S-waiting-on-team

[jyn514]

So, in my opinion, the distinction between --pretty and --unpretty is that the options under --pretty are meant to be meaningful to Rust programmers, while the options under --unpretty are solely meaningful to rustc developers.

The current --pretty options are:

• normal
• identified
• expanded
• expanded,identified
• expanded,hygiene

Here are some examples of those on a hello-world type program:

Expansions

$ rustc --pretty=normal src/main.rs -Zunstable-options
fn main() { println!("{}", std :: env :: var("LD_LIBRARY_PATH").unwrap()); }
$ rustc --pretty=identified src/main.rs -Zunstable-options
fn main() { println!("{}", std :: env :: var("LD_LIBRARY_PATH").unwrap()); }
/* block 4294967040 */ /* 4294967040 */
$ rustc --pretty=expanded src/main.rs -Zunstable-options
#![feature(prelude_import)]
#![no_std]
#[prelude_import]
use ::std::prelude::rust_2015::*;
#[macro_use]
extern crate std;
fn main() {
    {
        ::std::io::_print(::core::fmt::Arguments::new_v1(&["", "\n"],
                                                         &match (&std::env::var("LD_LIBRARY_PATH").unwrap(),)
                                                              {
                                                              (arg0,) =>
                                                              [::core::fmt::ArgumentV1::new(arg0,
                                                                                            ::core::fmt::Display::fmt)],
                                                          }));
    };
}
$ rustc --pretty=expanded,identified src/main.rs -Zunstable-options
#![feature(prelude_import)]
#![no_std]
#[prelude_import]
use ::std::prelude::rust_2015::*; /* 4 */
#[macro_use]
extern crate std; /* 10 */
fn main() {
    ({
         ((::std::io::_print /* 17
              */)(((::core::fmt::Arguments::new_v1 /* 24
                       */)((&([("" /* 25 */), ("\n" /* 26 */)] /* 27 */) /* 28
                               */),
                           (&(match (((&(((std::env::var /* 33
                                              */)(("LD_LIBRARY_PATH" /* 34
                                                      */)) /* 35 */).unwrap()
                                            /* 36 */) /* 37 */),) /* 38 */) {
                                  (arg0 /* pat 41 */,) /* pat 40 */ =>
                                  ([((::core::fmt::ArgumentV1::new /* 46
                                         */)((arg0 /* 48 */),
                                             (::core::fmt::Display::fmt /* 53
                                                 */)) /* 54 */)] /* 55 */),
                              } /* 56 */) /* 57 */)) /* 58 */)) /* 18 */);
     } /* block 13 */ /* 19 */);
} /* block 12 */ /* 11 */
(-bash@build-server) ~/test-rustdoc/hello [15:31:06]
$ rustc --pretty=expanded,hygiene src/main.rs -Zunstable-options
#![feature /* 491#0 */(prelude_import)]
#![no_std /* 747#0 */]
#[prelude_import /* 837#1 */]
use ::std /* 1108#1 */::prelude /* 836#1 */::rust_2015 /* 925#1 */::*;
#[macro_use /* 658#1 */]
extern crate std /* 1108#2 */;
fn main /* 661#0 */() {
    {
        ::std /* 2#3 */::io /* 1320#3 */::_print /* 1467#3
            */(::core /* 2#4 */::fmt /* 507#0 */::Arguments /* 67#0 */::new_v1
                   /* 1480#0
                   */(&["", "\n"],
                      &match (&std /* 1108#0 */::env /* 455#0 */::var /*
                                   1247#0 */("LD_LIBRARY_PATH").unwrap /*
                                   1234#0 */(),) {
                           (arg0 /* 1478#4 */,) =>
                           [::core /* 2#4 */::fmt /* 507#0 */::ArgumentV1 /*
                                66#0 */::new /* 727#0
                                */(arg0 /* 1478#4 */,
                                   ::core /* 2#4 */::fmt /* 507#0 */::Display
                                       /* 1479#0 */::fmt /* 507#0 */)],
                       }));
    };
}

/*
Expansions:
0: parent: ExpnId(0), call_site_ctxt: #0, def_site_ctxt: #0, kind: Root
1: parent: ExpnId(0), call_site_ctxt: #0, def_site_ctxt: #0, kind: AstPass(StdImports)
2: parent: ExpnId(0), call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro { kind: Bang, name: "println", proc_macro: false }
3: parent: ExpnId(2), call_site_ctxt: #3, def_site_ctxt: #0, kind: Macro { kind: Bang, name: "$crate::format_args_nl", proc_macro: false }

SyntaxContexts:
#0: parent: #0, outer_mark: (ExpnId(0), Opaque)
#1: parent: #0, outer_mark: (ExpnId(1), Opaque)
#2: parent: #0, outer_mark: (ExpnId(1), Transparent)
#3: parent: #0, outer_mark: (ExpnId(2), SemiTransparent)
#4: parent: #0, outer_mark: (ExpnId(3), Opaque)
*/

Of those, only rustc --pretty=expanded seems meaningful to users to me (and there's a cargo wrapper for it: https://github.com/dtolnay/cargo-expand).

I think if you want to keep --pretty as "user-facing" pretty-printing, we should move everything but --pretty=expanded to --unpretty.

I also had a vague thought that --pretty, or some subset of it, would be on the path to eventual stabilization. Is that a non-starter at this point? (Or is it an irrelevant question, in the sense that a subset of any flag could be stabilized, and the issue here is whether it makes sense to have two flags...)

I think this is relevant only in that it seems silly to stabilize a flag called --unpretty; if you want to stabilize something I think we should stabilize it as --pretty. Other than that I agree we can have either 1 or 2 flags without it impacting anything.

[jyn514]

r? @pnkfelix - I know you said it was fine to go ahead without you, but I don't want to merge this if you have concerns, and no one else seems to have strong opinions.

[jyn514]

Also, I kind of wonder what the point of --pretty=normal is - I kind of expect it to be the same as running rustfmt, but it is definitely not that.

[bjorn3]

Rust has it's own "pretty" printer. --pretty=normal runs the source through it without doing anything else. I put "pretty" in quotes as it is anything but pretty. It does match the old style of rust a bit though with visual indentation instead of block indentation.

[jyn514]

Another alternative @nikomatsakis / @Mark-Simulacrum thought of in the meantime is to actually use rustfmt as a library for pretty-printing; that should be feasible now that it's in-tree: https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/rustdoc.20is.20using.20rustc_ast_pretty.2C.20would.20.E2.80.A6.20compiler-team.23403/near/225171876

[pnkfelix]

@jyn514 I retract the objection I voiced above. I'll go ahead and take a look.

[pnkfelix]

@bors r+

[bors]

📌 Commit 23bbd65 has been approved by pnkfelix

[bors]

⌛ Testing commit 23bbd65 with merge 7d6bf86...

[bors]

☀️ Test successful - checks-actions
Approved by: pnkfelix
Pushing 7d6bf86 to master...