regression: proc-macro to_string change for "invisible" delimiters

[Mark-Simulacrum]

https://crater-reports.s3.amazonaws.com/beta-1.62-1/beta-2022-05-20/gh/Patryk27.lxd-snapper/log.txt

Seems to be due to an older(?) version of indoc, possibly already fixed, but wanted to file in case there was an unintentional change on our end.

cc @dtolnay (indoc-impl author, I believe)

dtolnay/indoc@29e8c96 may have been related to this as well.

[dtolnay]

That repo's Cargo.lock is set to indoc 1.0.3. This error is fixed in indoc 1.0.5 and a better fix (which you linked) in 1.0.6.

It's not clear to me that @nnethercote's #96682 was intended to apply to proc_macro's Display impls, as opposed to only rustc_ast_pretty and various places that tokens get shown in diagnostics, and whether that should've had a team FCP.

[nnethercote]

cc @petrochenkov, who knows more about all this stuff than I do.

[dtolnay]

@rust-lang/libs-api, @rust-lang/compiler:
#96682 has the effect (among other things) that proc_macro::TokenTree's Display impl is going to put /*«*/ /*»*/ around tokens of type TokenTree::Group with delimiter=Delimiter::None. #97076 has a minimal example.

This broke a small number of macros which involved manipulating the Display representation of tokens. One example is this code in scale-info which is a primitive way of turning a type into a pleasantly-spaced type name string, like ( bool , bool ) -> "(bool, bool)":

• https://github.com/paritytech/scale-info/blame/7140f90858eaa2b10c65e8c647a44e3fdf80f2e0/derive/src/lib.rs#L176
• https://github.com/paritytech/scale-info/blame/7140f90858eaa2b10c65e8c647a44e3fdf80f2e0/derive/src/lib.rs#L312

Another example is this code in indoc for categorizing kinds of literals:

• https://github.com/dtolnay/indoc/blame/63669087df5c3319766edf56ed5bdd344faeea10/src/lib.rs#L325-L328

I don't necessarily object to the change but it's worth being aware that this has made the strings /*«*/ and /*»*/ part of the API of proc_macro whether we like it or not and crates are going to start parsing exactly those strings, so any future change to how Delimiter::None appears in Display output is going to become even more disruptive.

If someone feels inclined to run a post-hoc FCP we may have time for one before 1.62 (4 weeks from now) or we can revert and take longer.

[nnethercote]

My inclination is to back out #96682 and move on, but @petrochenkov may have a better idea.

[petrochenkov]

Yes, reverting #96682 should be fine.

[apiraino]

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-medium