Skip to content

adjust how closure/generator types are printed #115696

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Sep 9, 2023

I saw &[closure@$DIR/issue-20862.rs:2:5] and I thought it is a slice type, because that's usually what &[_] is... it took me a while to realize that this is just a confusing printer and actually there's no slice. Let's use something that cannot be mistaken for a regular type.

@rustbot
Copy link
Collaborator

rustbot commented Sep 9, 2023

r? @petrochenkov

(rustbot has picked a reviewer for you, use r? to override)

@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 Sep 9, 2023
@RalfJung
Copy link
Member Author

RalfJung commented Sep 9, 2023

Hm, something somewhere is still printing [closure@...], I'm seeing it in some ui tests.

@RalfJung
Copy link
Member Author

RalfJung commented Sep 9, 2023

Ah, what I changed was actually the rvalue printing (which doesn't seem to be exercised by the test suite), not the type printing.

@rustbot
Copy link
Collaborator

rustbot commented Sep 9, 2023

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@oli-obk
Copy link
Contributor

oli-obk commented Sep 11, 2023

Seems fine to me, please create an MCP for it so all of the compiler team gets informed about it.

@oli-obk oli-obk self-assigned this Sep 11, 2023
@bors
Copy link
Collaborator

bors commented Sep 11, 2023

☔ The latest upstream changes (presumably #115661) made this pull request unmergeable. Please resolve the merge conflicts.

@RalfJung
Copy link
Member Author

@oli-obk how long do we want to wait from the MCP seconding to landing the PR?

@oli-obk
Copy link
Contributor

oli-obk commented Sep 17, 2023

MCPs are 10 days

@bors
Copy link
Collaborator

bors commented Sep 19, 2023

☔ The latest upstream changes (presumably #115865) made this pull request unmergeable. Please resolve the merge conflicts.

@RalfJung RalfJung added the relnotes Marks issues that should be documented in the release notes of the next release. label Sep 19, 2023
@BoxyUwU
Copy link
Member

BoxyUwU commented Sep 21, 2023

the MCP has been accepted for 10 days now this can be merged after a rebase presumably

@oli-obk
Copy link
Contributor

oli-obk commented Sep 22, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 22, 2023

📌 Commit c4ec12f has been approved by oli-obk

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 Sep 22, 2023
@bors
Copy link
Collaborator

bors commented Sep 22, 2023

⌛ Testing commit c4ec12f with merge 959b2c7...

@bors
Copy link
Collaborator

bors commented Sep 22, 2023

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 22, 2023
@bors bors merged commit 959b2c7 into rust-lang:master Sep 22, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (959b2c7): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results

This 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.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.0% [-3.4%, -2.6%] 4
All ❌✅ (primary) - - 0

Cycles

Results

This 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.

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

Binary size

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

Bootstrap: 633.528s -> 635.567s (0.32%)
Artifact size: 317.74 MiB -> 317.55 MiB (-0.06%)

@RalfJung RalfJung deleted the closure-ty-print branch September 23, 2023 05:35
flip1995 pushed a commit to flip1995/rust that referenced this pull request Sep 25, 2023
adjust how closure/generator types are printed

I saw `&[closure@$DIR/issue-20862.rs:2:5]` and I thought it is a slice type, because that's usually what `&[_]` is... it took me a while to realize that this is just a confusing printer and actually there's no slice. Let's use something that cannot be mistaken for a regular type.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. 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
Development

Successfully merging this pull request may close these issues.

7 participants