Skip to content
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

Rollup of 5 pull requests #133280

Merged
merged 22 commits into from
Nov 21, 2024
Merged

Rollup of 5 pull requests #133280

merged 22 commits into from
Nov 21, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

hoodmane and others added 22 commits November 11, 2024 17:25
When linking an executable without dynamic linking, this is a pure improvement.
It significantly reduces code size and avoids a lot of buggy behaviors. It is
supported in all browsers for many years and in all maintained versions of
Node.

It does change the ABI, so people who are dynamically linking with a library
or executable that uses the old ABI may need to turn it off. It can be disabled
if needed by passing `-Clink-arg -sWASM_BIGINT=0` to `rustc`. But few people
will want to turn it off.
`bat` is known as `batcat` on Ubuntu and Debian, not `catbat`.
It's not necessary because `show_md_content_with_pager` is only ever
called if `is_terminal` is true.
I think the control flow in this function is complicated and confusing,
largely due to the use of two booleans `print_formatted` and
`fallback_to_println` that are set in multiple places and then used to
guide proceedings.

As well as hurting readability, this leads to at least one bug: if the
`write_termcolor_buf` call fails and the pager also fails, the function
will try to print color output to stdout, but that output will be empty
because `write_termcolor_buf` failed. I.e. the `if fallback_to_println`
body fails to check `print_formatted`.

This commit rewrites the function to be neater and more Rust-y, e.g. by
putting the result of `write_termcolor_buf` into an `Option` so it can
only be used on success, and by using `?` more. It also changes
terminology a little, using "pretty" to mean "formatted and colorized".
The result is a little shorter, more readable, and less buggy.
…=workingjubilee

Emscripten: link with -sWASM_BIGINT

When linking an executable without dynamic linking, this is a pure improvement. It significantly reduces code size and avoids a lot of buggy behaviors. It is supported in all browsers for many years and in all maintained versions of Node.

It does change the ABI, so people who are dynamically linking with a library or executable that uses the old ABI may need to turn it off. It can be disabled if needed by passing `-Clink-arg -sWASM_BIGINT=0` to `rustc`. But few people will want to turn it off.

Note this includes a libc bump to 0.2.162!
…ent, r=petrochenkov

Store resolution for self and crate root module segments

Let's make sure to record the segment resolution for `self::`, `crate::` and `$crate::`.

I'm actually somewhat surprised that the only diagnostic that uses this is the one that errors on invalid generics on a module segment... but seems strictly more correct regardless, and there may be other diagnostics using these segments resolutions that just haven't been tested for `self`. Also includes a drive-by on `report_prohibit_generics_error`.
Add visits to nodes that already have flat_maps in ast::MutVisitor

This PR aims to add `visit_` methods for every node that has a `flat_map_` in MutVisitor, giving implementers free choice over overriding `flat_map` for 1-to-n conversions or `visit` for a 1-to-1.

There is one major problem: `flat_map_stmt`.
While all other default implementations of `flat_map`s are 1-to-1 conversion, as they either only call visits or a internal 1-to-many conversions are natural, `flat_map_stmt` doesn't follow this pattern.

`flat_map_stmt`'s default implementation is a 1-to-n conversion that panics if n > 1 (effectively being a 1-to-[0;1]). This means that it cannot be used as is for a default `visit_stmt`, which would be required to be a 1-to-1.

Implementing `visit_stmt` without runtime checks would require it to reach over a potential `flat_map_item` or `filter_map_expr` overrides and call for their `visit` counterparts directly.
Other than that, if we want to keep the behavior of `flat_map_stmt` it cannot call `visit_stmt` internally.

To me, it seems reasonable to make all default implementations 1-to-1 conversions and let implementers handle `visit_stmt` if they need it, but I don't know if calling `visit` directly when a 1-to-1 is required is ok or not.

related to rust-lang#128974 & rust-lang#127615

r? ``@petrochenkov``
…e1-dead

Implement `~const` item bounds in RPIT

an RPIT in a `const fn` is allowed to be conditionally const itself :)

r? fee1-dead or reroll
…t_with_pager, r=tgross35

Rewrite `show_md_content_with_pager`

`show_md_content_with_pager` is complex and has a couple of bugs. This PR improves it.

r? ``@tgross35``
@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) rollup A PR which is a rollup labels Nov 21, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Nov 21, 2024

📌 Commit c83b6a3 has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 21, 2024
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 21, 2024
@bors
Copy link
Contributor

bors commented Nov 21, 2024

⌛ Testing commit c83b6a3 with merge 0b1bf71...

@bors
Copy link
Contributor

bors commented Nov 21, 2024

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 0b1bf71 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 21, 2024
@bors bors merged commit 0b1bf71 into rust-lang:master Nov 21, 2024
7 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 21, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#131736 Emscripten: link with -sWASM_BIGINT 383296c38099889310d42fb778efa59aec5e0dd6 (link)
#132207 Store resolution for self and crate root module segments 630a95c1d0e5bd384794708515a10556b7bae2f4 (link)
#133153 Add visits to nodes that already have flat_maps in ast::Mut… 8969b39169701ed4b1013ef9ab1f00f1530de7aa (link)
#133218 Implement ~const item bounds in RPIT 00d46d23dfe3753f33440e2123cd6dc15f811618 (link)
#133228 Rewrite show_md_content_with_pager c720b45837106c5872f7c4077c7e1b48cb4e5dd3 (link)

previous master: 318f96a8cf

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0b1bf71): comparison URL.

Overall result: ❌ regressions - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

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

Max RSS (memory usage)

Results (primary -1.8%, secondary 0.2%)

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)
3.3% [3.3%, 3.3%] 1
Improvements ✅
(primary)
-1.8% [-2.6%, -0.9%] 5
Improvements ✅
(secondary)
-2.8% [-2.8%, -2.8%] 1
All ❌✅ (primary) -1.8% [-2.6%, -0.9%] 5

Cycles

Results (primary 5.1%)

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

Binary size

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

Bootstrap: 797.091s -> 796.195s (-0.11%)
Artifact size: 335.93 MiB -> 335.92 MiB (-0.00%)

@rustbot rustbot added the perf-regression Performance regression. label Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. rollup A PR which is a rollup 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants