completely deduplicate Visitor and MutVisitor

[fee1-dead]

r? oli-obk

This closes #127615.

Discussion

• Give every MutVisitor::visit_* method a corresponding flat_map_* method.

Not every AST node exists in a location where they can be mapped to multiple instances of themselves. Not every AST node exists in a location where they can be removed from existence (e.g. filter_map_expr). I don't think this is doable.

• Give every MutVisitor::visit_* method a corresponding Visitor method and vice versa

The only three remaining method-level asymmetries after this PR are visit_stmt and visit_nested_use_tree (only on Visitor) and visit_span (only on MutVisitor).

visit_stmt doesn't seem applicable to MutVisitor because walk_flat_map_stmt_kind will ask flat_map_item / filter_map_expr to potentially turn a single Stmt to multiple based on what a visitor wants. So only using flat_map_stmt seems appropriate.

visit_nested_use_tree is used for rustc_resolve to track stuff. Not useful for MutVisitor for now.

visit_span is currently not used for MutVisitor already, it was just kept in case we want to revive #127241. cc @cjgillot maybe we could remove for now and re-insert later if we find a use-case? It does involve some extra effort to maintain.

• Remaining FIXMEs

visit_lifetime has an extra param for Visitor that's not in MutVisitor. This is again something only used by rustc_resolve. I think we can keep that symmetry for now.

[fee1-dead]

@bors2 try @rust-timer queue

[rust-bors]

⌛ Trying commit ed397ad with merge a1b90e8…

To cancel the try build, run the command @bors2 try cancel.

[rust-bors]

☀️ Try build successful (CI)
Build commit: a1b90e8 (a1b90e8b552cbb8c94842e1b04cb49b07d4b9475, parent: 8a65ee08296b36342bf7c3cdc15312ccbc357227)

[rust-timer]

Finished benchmarking commit (a1b90e8): comparison URL.

Overall result: no relevant changes - no action needed

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.

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

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 0.9%, secondary -3.0%)

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

	mean	range	count
Regressions ❌ (primary)	0.9%	[0.9%, 0.9%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.0%	[-5.7%, -1.2%]	4
All ❌✅ (primary)	0.9%	[0.9%, 0.9%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 692.997s -> 691.913s (-0.16%)
Artifact size: 372.00 MiB -> 372.02 MiB (0.01%)

[oli-obk]

Not every AST node exists in a location where they can be mapped to multiple instances of themselves. Not every AST node exists in a location where they can be removed from existence (e.g. filter_map_expr). I don't think this is doable.

yea that makes sense. The scheme you have now that generates the flat map functions and similar resolves that point sufficiently imo

@bors r+

[bors]

📌 Commit 3da58e6 has been approved by oli-obk

It is now in the queue for this repository.

[bors]

⌛ Testing commit 3da58e6 with merge c2ec753...

[bors]

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing c2ec753 to master...

[github-actions]

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing a30f178 (parent) -> c2ec753 (this PR)

Test differences

Show 4 test diffs

4 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard c2ec7532eed172e79800d28f087727c4b048badd --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

1. x86_64-apple-2: 3390.8s -> 6193.4s (82.6%)
2. dist-apple-various: 8421.8s -> 5763.0s (-31.6%)
3. mingw-check-1: 1514.1s -> 1883.7s (24.4%)
4. x86_64-apple-1: 7441.0s -> 5895.0s (-20.8%)
5. x86_64-rust-for-linux: 2604.8s -> 2905.4s (11.5%)
6. i686-gnu-2: 5440.3s -> 6047.8s (11.2%)
7. dist-x86_64-apple: 7918.0s -> 8744.3s (10.4%)
8. x86_64-gnu-tools: 3365.4s -> 3715.7s (10.4%)
9. test-various: 4358.2s -> 4780.1s (9.7%)
10. aarch64-gnu-debug: 3765.2s -> 4126.7s (9.6%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

[rust-timer]

Finished benchmarking commit (c2ec753): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -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)	-	-	0
Improvements ✅ (primary)	-1.3%	[-1.3%, -1.3%]	1
Improvements ✅ (secondary)	-1.3%	[-1.3%, -1.3%]	1
All ❌✅ (primary)	-1.3%	[-1.3%, -1.3%]	1

Max RSS (memory usage)

Results (secondary -3.9%)

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)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.9%	[-3.9%, -3.9%]	1
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 689.082s -> 690.174s (0.16%)
Artifact size: 371.89 MiB -> 371.90 MiB (0.00%)