Skip to content

More Printer cleanups#144949

Merged
bors merged 7 commits intorust-lang:masterfrom
nnethercote:more-Printer-cleanups
Aug 13, 2025
Merged

More Printer cleanups#144949
bors merged 7 commits intorust-lang:masterfrom
nnethercote:more-Printer-cleanups

Conversation

@nnethercote
Copy link
Contributor

@nnethercote nnethercote commented Aug 5, 2025 •
edited by rustbot
Loading

A sequel to #144776.

r? @davidtwco

@nnethercote
Clarify value_path_str_with_args.
566cdab 
The use of `print_value_path` means the value namespace is always used
and the `guess_def_namespace` call is unnecessary. This commit removes
the `guess_def_namespace` call and hard-codes `ValueNS`. It also changes
the `print_value_path` to `print_def_path` for consistency with
`def_path_str_with_args`.
@nnethercote
Rename most of the printers.
b0c36dd 
Three of them are named `AbsolutePathPrinter`, which is confusing, so
give those names that better indicate how they are used. And then there
is `SymbolPrinter` and `SymbolMangler`, which are renamed as
`LegacySymbolMangler` and `V0SymbolMangler`, better indicating their
similarity.
@nnethercote
Simplify multiple things.
d76eef4 
- `same_path` can just be a `bool`.
- `expected` and `found` are only needed inside the block.
- Neaten a comment.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 5, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 5, 2025

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

@nnethercote nnethercote changed the title More printer cleanups More Printer cleanups Aug 5, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@nnethercote
Add comments to Printer.
dfd38f0 
These details took me some time to work out.
@nnethercote
Move should_truncate from trait Printer to sub-trait `PrettyPrint…
69bcd79 
…er`.

It's not used in `Printer`.
@nnethercote
Rename some Printer methods.
b8adda6 
I find these name clearer, and starting them all with `print_` makes
things more consistent.
@nnethercote
Rename some PrettyPrinter methods.
42a1042 
More consistency.
@nnethercote nnethercote force-pushed the more-Printer-cleanups branch from dd90af9 to 42a1042 Compare August 6, 2025 03:06
@davidtwco
Copy link
Member

davidtwco commented Aug 13, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 13, 2025

📌 Commit 42a1042 has been approved by davidtwco

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 13, 2025
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Aug 13, 2025
@GuillaumeGomez
Rollup merge of rust-lang#144949 - nnethercote:more-Printer-cleanups,…
e8261fb 
… r=davidtwco

More `Printer` cleanups

A sequel to rust-lang#144776.

r? `@davidtwco`
@GuillaumeGomez GuillaumeGomez mentioned this pull request Aug 13, 2025
Closed
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Aug 13, 2025
@GuillaumeGomez
Rollup merge of rust-lang#144949 - nnethercote:more-Printer-cleanups,…
8fa0870 
… r=davidtwco

More `Printer` cleanups

A sequel to rust-lang#144776.

r? ``@davidtwco``
This was referenced Aug 13, 2025
Closed
Merged
bors added a commit that referenced this pull request Aug 13, 2025
@bors
Auto merge of #145366 - GuillaumeGomez:rollup-v0a6v3u, r=GuillaumeGomez
3672a55 
Rollup of 9 pull requests

Successful merges:

 - #144761 (aarch64: Make `outline-atomics` a known target feature)
 - #144949 (More `Printer` cleanups)
 - #144955 (search graph: lazily update parent goals)
 - #144962 (Add aarch64_be-unknown-none-softfloat target)
 - #145153 (Handle macros with multiple kinds, and improve errors)
 - #145241 ([AVR] Changed data_layout)
 - #145341 (Install libgccjit into the compiler's sysroot when cg_gcc is enabled)
 - #145349 (Correctly handle when there are no unstable items in the documented crate)
 - #145356 (Add another example for escaped `#` character in doctest in rustdoc book)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e7e3a37 into rust-lang:master Aug 13, 2025
10 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Aug 13, 2025
rust-timer added a commit that referenced this pull request Aug 13, 2025
@rust-timer
Unrolled build for #144949
fb2fec8 
Rollup merge of #144949 - nnethercote:more-Printer-cleanups, r=davidtwco

More `Printer` cleanups

A sequel to #144776.

r? ```@davidtwco```
@nnethercote nnethercote deleted the more-Printer-cleanups branch August 14, 2025 07:49
@Kobzol
Copy link
Member

Kobzol commented Aug 14, 2025

@rust-timer build fb2fec8

Testing for #145366.

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Collaborator

rust-timer commented Aug 14, 2025

Finished benchmarking commit (fb2fec8): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 1.9% [1.9%, 1.9%] 1
Improvements ✅
(primary)		 -1.0% [-1.0%, -1.0%] 1
Improvements ✅
(secondary)		 -0.3% [-0.4%, -0.2%] 6
All ❌✅ (primary) -1.0% [-1.0%, -1.0%] 1

Max RSS (memory usage)

Results (secondary 4.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 4.5% [4.5%, 4.5%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Cycles

Results (secondary 2.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 2.6% [1.5%, 3.6%] 2
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Binary size

Results (primary -0.0%, secondary -0.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 0.1% [0.1%, 0.1%] 3
Regressions ❌
(secondary)		 0.1% [0.1%, 0.1%] 1
Improvements ✅
(primary)		 -0.1% [-0.1%, -0.0%] 4
Improvements ✅
(secondary)		 -0.4% [-0.9%, -0.1%] 8
All ❌✅ (primary) -0.0% [-0.1%, 0.1%] 7

Bootstrap: 467.825s -> 467.816s (-0.00%)
Artifact size: 377.36 MiB -> 377.37 MiB (0.00%)

@rustbot rustbot added the perf-regression Performance regression. label Aug 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidtwco davidtwco davidtwco approved these changes

Assignees

@davidtwco davidtwco

Labels

perf-regression Performance regression. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.91.0

Development

Successfully merging this pull request may close these issues.

7 participants

@nnethercote @rustbot @rust-log-analyzer @davidtwco @bors @Kobzol @rust-timer