-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Further improve space_between
#120227
Further improve space_between
#120227
Conversation
This gives better output for code produced by proc macros.
There are a number of cases where we erroneously omit the space between two tokens, all involving an exception to a more general case. The affected tokens are `$`, `!`, `.`, `,`, and `let` followed by a parenthesis. This fixes a lot of FIXME comments.
60d86ae
to
1fbabee
Compare
#117433 was an attempt to make It will need a crater run before being merged, but based on the crater run from #117433 I think it should be ok, because I haven't include the cases that caused crater failures in #117433, which were mostly about spacing around |
@bors try |
…tween, r=<try> Further improve `space_between` `space_between` is used by `print_tts` to decide when spaces should be put between tokens. This PR improves it in two ways: - avoid unnecessary spaces before semicolons, and - don't omit some necessary spaces before/after some punctuation symbols. r? `@petrochenkov`
☀️ Try build successful - checks-actions |
@craterbot check |
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🎉 Experiment
|
Only 2 regressions out of 6 are spurious. |
odbc-futures-0.2.2This line is the problem, it does substring matching with The crate has only 438 downloads ever and hasn't been updated in 5 years. KittyCAD.kcl-lsp.844488870375d9e36254825ab04a2cfe5d5fd107Due to kcl-lib/derive-docs, as described here. In short, this:
becomes this:
which breaks some string matching in a proc macro. kcl-lib and derive-docs have had both had updates in the past two months. carghai.contain.be6f4b896c7594bac76ae43a2027ed7dd602a433Due to const-random use, seen before here, so I'm surprised it's showing up here.
gen-nested-iter-yield-0.1.3Substring matching done here in the Crate has 3,376 total downloads ever, and was last updated almost 2 years ago. Malikazz.ProjectEuler.81ee5516c17a5425ef7b0b2dbf8ef010031deefc, tperdue321.rust_structs.c5244df34b978d5d014720c1fa42514cc89a7c37, Mallig.advent-of-code-2020.27889bb9ca6c8080172bef594543c49e52fc524c (fixed)
Seems unrelated to this PR, looks like temporary infra problems. lcdr.lu_packets.144e56d2c46043da7924ffa34705f763982676fc (fixed)
Old failure was due to hitting the recursion limit. Unclear why it worked now, but seems unrelated to this PR. |
This would make sense if crater is comparing this PR against release Rust, rather than against nightly Rust. I filed tkaitchuck/constrandom#33 to fix const-random, making the proc macro iterate over the token trees rather than using |
@petrochenkov: with tkaitchuck/constrandom#33 filed, I think the only other breakage that matters is the I suggest we merge this PR and then I'll file an issue on @rustbot reviewer |
Okay, seems reasonable. |
💥 Test timed out |
@bors retry |
☀️ Test successful - checks-actions |
Finished benchmarking commit (80deabd): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 660.046s -> 661.683s (0.25%) |
I filed an issue on derive-docs: KittyCAD/modeling-app#1339 |
@nnethercote https://perf.rust-lang.org/compare.html?start=cb4d9a1902b3ea17e93872dafb76d24aa6295c47&end=80deabd0987201e1b8d060400f50e03309a0105e&stat=max-rss doesn't look great to me... Is the noticable increase in RSS expected? |
(Discussion about that ongoing here). |
Tagging relnotes for future compat notice. See #120227 (comment) for crater. |
This is hitting my |
Looking at the code, I would guess these regexes are to blame. For the first regex, changing this:
to this:
should fix the problem caused by this particular PR. Changing it to this:
would be more robust against possible future pretty-printing changes. But the awfulness of that regex shows the limits of substring matching for this purpose. The fully robust way to do this kind of thing is at the token level, like I did in tkaitchuck/constrandom#33. |
Pkgsrc changes: * Adapt checksums and patches. Upstream chnages: Version 1.77.0 (2024-03-21) ========================== - [Reveal opaque types within the defining body for exhaustiveness checking.] (rust-lang/rust#116821) - [Stabilize C-string literals.] (rust-lang/rust#117472) - [Stabilize THIR unsafeck.] (rust-lang/rust#117673) - [Add lint `static_mut_refs` to warn on references to mutable statics.] (rust-lang/rust#117556) - [Support async recursive calls (as long as they have indirection).] (rust-lang/rust#117703) - [Undeprecate lint `unstable_features` and make use of it in the compiler.] (rust-lang/rust#118639) - [Make inductive cycles in coherence ambiguous always.] (rust-lang/rust#118649) - [Get rid of type-driven traversal in const-eval interning] (rust-lang/rust#119044), only as a [future compatiblity lint] (rust-lang/rust#122204) for now. - [Deny braced macro invocations in let-else.] (rust-lang/rust#119062) Compiler -------- - [Include lint `soft_unstable` in future breakage reports.] (rust-lang/rust#116274) - [Make `i128` and `u128` 16-byte aligned on x86-based targets.] (rust-lang/rust#116672) - [Use `--verbose` in diagnostic output.] (rust-lang/rust#119129) - [Improve spacing between printed tokens.] (rust-lang/rust#120227) - [Merge the `unused_tuple_struct_fields` lint into `dead_code`.] (rust-lang/rust#118297) - [Error on incorrect implied bounds in well-formedness check] (rust-lang/rust#118553), with a temporary exception for Bevy. - [Fix coverage instrumentation/reports for non-ASCII source code.] (rust-lang/rust#119033) - [Fix `fn`/`const` items implied bounds and well-formedness check.] (rust-lang/rust#120019) - [Promote `riscv32{im|imafc}-unknown-none-elf` targets to tier 2.] (rust-lang/rust#118704) - Add several new tier 3 targets: - [`aarch64-unknown-illumos`] (rust-lang/rust#112936) - [`hexagon-unknown-none-elf`] (rust-lang/rust#117601) - [`riscv32imafc-esp-espidf`] (rust-lang/rust#119738) - [`riscv32im-risc0-zkvm-elf`] (rust-lang/rust#117958) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Implement `From<&[T; N]>` for `Cow<[T]>`.] (rust-lang/rust#113489) - [Remove special-case handling of `vec.split_off (0)`.](rust-lang/rust#119917) Stabilized APIs --------------- - [`array::each_ref`] (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_ref) - [`array::each_mut`] (https://doc.rust-lang.org/stable/std/primitive.array.html#method.each_mut) - [`core::net`] (https://doc.rust-lang.org/stable/core/net/index.html) - [`f32::round_ties_even`] (https://doc.rust-lang.org/stable/std/primitive.f32.html#method.round_ties_even) - [`f64::round_ties_even`] (https://doc.rust-lang.org/stable/std/primitive.f64.html#method.round_ties_even) - [`mem::offset_of!`] (https://doc.rust-lang.org/stable/std/mem/macro.offset_of.html) - [`slice::first_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk) - [`slice::first_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.first_chunk_mut) - [`slice::split_first_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk) - [`slice::split_first_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_first_chunk_mut) - [`slice::last_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk) - [`slice::last_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.last_chunk_mut) - [`slice::split_last_chunk`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk) - [`slice::split_last_chunk_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.split_last_chunk_mut) - [`slice::chunk_by`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by) - [`slice::chunk_by_mut`] (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.chunk_by_mut) - [`Bound::map`] (https://doc.rust-lang.org/stable/std/ops/enum.Bound.html#method.map) - [`File::create_new`] (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.create_new) - [`Mutex::clear_poison`] (https://doc.rust-lang.org/stable/std/sync/struct.Mutex.html#method.clear_poison) - [`RwLock::clear_poison`] (https://doc.rust-lang.org/stable/std/sync/struct.RwLock.html#method.clear_poison) Cargo ----- - [Extend the build directive syntax with `cargo::`.] (rust-lang/cargo#12201) - [Stabilize metadata `id` format as `PackageIDSpec`.] (rust-lang/cargo#12914) - [Pull out as `cargo-util-schemas` as a crate.] (rust-lang/cargo#13178) - [Strip all debuginfo when debuginfo is not requested.] (rust-lang/cargo#13257) - [Inherit jobserver from env for all kinds of runners.] (rust-lang/cargo#12776) - [Deprecate rustc plugin support in cargo.] (rust-lang/cargo#13248) Rustdoc ----- - [Allows links in markdown headings.] (rust-lang/rust#117662) - [Search for tuples and unit by type with `()`.] (rust-lang/rust#118194) - [Clean up the source sidebar's hide button.] (rust-lang/rust#119066) - [Prevent JS injection from `localStorage`.] (rust-lang/rust#120250) Misc ---- - [Recommend version-sorting for all sorting in style guide.] (rust-lang/rust#115046) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Add more weirdness to `weird-exprs.rs`.] (rust-lang/rust#119028)
space_between
is used byprint_tts
to decide when spaces should be put between tokens. This PR improves it in two ways:r? @petrochenkov