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 9 pull requests #119662

Merged
merged 23 commits into from
Jan 6, 2024
Merged

Rollup of 9 pull requests #119662

merged 23 commits into from
Jan 6, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

RalfJung and others added 23 commits December 9, 2023 14:49
Needle and haystack are actually not the same, they remain constant.
The lint is not yet reported due to span issues, but will be reported later.
In the past it did create a fresh expansion, but now, after surviving a number of refactorings, it does not.
Now it's just a thin wrapper around `apply_mark`.
Generally, `tt` and `ident` should behave identically, modulo the latter accepting only a subset of token trees.
By using FxIndexMap instead of FxHashMap, so that the order of visiting
of locals is deterministic.

We also need to bless
copy_propagation_arg.foo.DestinationPropagation.panic*.diff. Do not
review the diff of the diff. Instead look at the diff file before and
after this commit. Both before and after this commit, 3 statements are
replaced with nop. It's just that due to change in ordering, different
statements are replaced. But the net result is the same.
The bootstrap bump has happened, so the bootstrap compiler now contains
the new diagnotic.
This cuts down on the amount of data we need to ship and users need to
keep on disk for each Rust toolchain. As noted in the added comment,
there's not much going on in these executables, so the added benefit of
symbols and debuginfo isn't large, while the cost is not insignificant.

This takes each of the binaries (we store 4 identical copies under
different names) from 3.7MB to 384KB.
…GuillaumeGomez

rustdoc: search for tuples and unit by type with `()`

This feature extends rustdoc to support the syntax that most users will naturally attempt to use to search for tuples. Part of rust-lang#60485

Function signature searches already support tuples and unit. The explicit name `primitive:tuple` and `primitive:unit` can be used to match a tuple or unit, while `()` will match either one. It also follows the direction set by the actual language for parens as a group, so `(u8,)` will only match a tuple, while `(u8)` will match a plain, unwrapped byte—thanks to loose search semantics, it will also match the tuple.

## Preview

* [`option<t>, option<u> -> (t, u)`](<https://notriddle.com/rustdoc-html-demo-5/tuple-unit/std/index.html?search=option%3Ct%3E%2C option%3Cu%3E -%3E (t%2C u)>)
* [`[t] -> (t,)`](<https://notriddle.com/rustdoc-html-demo-5/tuple-unit/std/index.html?search=[t] -%3E (t%2C)>)
* [`(ipaddr,) -> socketaddr`](<https://notriddle.com/rustdoc-html-demo-5/tuple-unit/std/index.html?search=(ipaddr%2C) -%3E socketaddr>)

## Motivation

When type-based search was first landed, it was directly [described as incomplete][a comment].

[a comment]: rust-lang#23289 (comment)

Filling out the missing functionality is going to mean adding support for more of Rust's [type expression] syntax, such as tuples (in this PR), references, raw pointers, function pointers, and closures.

[type expression]: https://doc.rust-lang.org/reference/types.html#type-expressions

There does seem to be demand for this sort of thing, such as [this Discord message](https://discord.com/channels/442252698964721669/443150878111694848/1042145740065099796) expressing regret at rustdoc not supporting tuples in search queries.

## Reference description (from the Rustdoc book)

<table>
<thead>
  <tr>
    <th>Shorthand</th>
    <th>Explicit names</th>
  </tr>
</thead>
<tbody>
  <tr><td colspan="2">Before this PR</td></tr>
  <tr>
    <td><code>[]</code></td>
    <td><code>primitive:slice</code> and/or <code>primitive:array</code></td>
  </tr>
  <tr>
    <td><code>[T]</code></td>
    <td><code>primitive:slice&lt;T&gt;</code> and/or <code>primitive:array&lt;T&gt;</code></td>
  </tr>
  <tr>
    <td><code>!</code></td>
    <td><code>primitive:never</code></td>
  </tr>
  <tr><td colspan="2">After this PR</td></tr>
  <tr>
    <td><code>()</code></td>
    <td><code>primitive:unit</code> and/or <code>primitive:tuple</code></td>
  </tr>
  <tr>
    <td><code>(T)</code></td>
    <td><code>T</code></td>
  </tr>
  <tr>
    <td><code>(T,)</code></td>
    <td><code>primitive:tuple&lt;T&gt;</code></td>
  </tr>
</tbody>
</table>

A single type expression wrapped in parens is the same as that type expression, since parens act as the grouping operator. If they're empty, though, they will match both `unit` and `tuple`, and if there's more than one type (or a trailing or leading comma) it is the same as `primitive:tuple<...>`.

However, since items can be left out of the query, `(T)` will still return results for types that match tuples, even though it also matches the type on its own. That is, `(u32)` matches `(u32,)` for the exact same reason that it also matches `Result<u32, Error>`.

## Future direction

The [type expression grammar](https://doc.rust-lang.org/reference/types.html#type-expressions) from the Reference is given below:

<pre><code>Syntax
    Type :
        TypeNoBounds
        | <a href="https://doc.rust-lang.org/reference/types/impl-trait.html">ImplTraitType</a>
        | <a href="https://doc.rust-lang.org/reference/types/trait-object.html">TraitObjectType</a>
<br>
    TypeNoBounds :
        <a href="https://doc.rust-lang.org/reference/types.html#parenthesized-types">ParenthesizedType</a>
        | <a href="https://doc.rust-lang.org/reference/types/impl-trait.html">ImplTraitTypeOneBound</a>
        | <a href="https://doc.rust-lang.org/reference/types/trait-object.html">TraitObjectTypeOneBound</a>
        | <a href="https://doc.rust-lang.org/reference/paths.html#paths-in-types">TypePath</a>
        | <a href="https://doc.rust-lang.org/reference/types/tuple.html#tuple-types">TupleType</a>
        | <a href="https://doc.rust-lang.org/reference/types/never.html">NeverType</a>
        | <a href="https://doc.rust-lang.org/reference/types/pointer.html#raw-pointers-const-and-mut">RawPointerType</a>
        | <a href="https://doc.rust-lang.org/reference/types/pointer.html#shared-references-">ReferenceType</a>
        | <a href="https://doc.rust-lang.org/reference/types/array.html">ArrayType</a>
        | <a href="https://doc.rust-lang.org/reference/types/slice.html">SliceType</a>
        | <a href="https://doc.rust-lang.org/reference/types/inferred.html">InferredType</a>
        | <a href="https://doc.rust-lang.org/reference/paths.html#qualified-paths">QualifiedPathInType</a>
        | <a href="https://doc.rust-lang.org/reference/types/function-pointer.html">BareFunctionType</a>
        | <a href="https://doc.rust-lang.org/reference/macros.html#macro-invocation">MacroInvocation</a>
</code></pre>

ImplTraitType and TraitObjectType (and ImplTraitTypeOneBound and TraitObjectTypeOneBound) are not yet implemented. They would mostly desugar to `trait:`, similarly to how `!` desugars to `primitive:never`.

ParenthesizedType and TuplePath are added in this PR.

TypePath is already implemented (except const generics, which is not planned, and function-like trait syntax, which is planned as part of closure support).

NeverType is already implemented.

RawPointerType and ReferenceType require parsing and fixes to the search index to store this information, but otherwise their behavior seems simple enough. Just like tuples and slices, `&T` would be equivalent to `primitive:reference<T>`, `&mut T` would be equivalent to `primitive:reference<keyword:mut, T>`, `*T` would be equivalent to `primitive:pointer<T>`, `*mut T` would be equivalent to `primitive:pointer<keyword:mut, T>`, and `*const T` would be equivalent to `primitive:pointer<keyword:const, T>`. Lifetime generics support is not planned, because lifetime subtyping seems too complicated.

ArrayType is subsumed by SliceType right now. Implementing const generics is not planned, because it seems like it would require a lot of implementation complexity for not much gain.

InferredType isn't really covered right now. Its semantics in a search context are not obvious.

QualifiedPathInType is not implemented, and it is not planned. I would need a use case to justify it, and act as a guide for what the exact semantics should be.

BareFunctionType is not implemented. Along with function-like trait syntax, which is formally considered a TypePath, it's the biggest missing feature to be able to do structured searches over generic APIs like `Option`.

MacroInvocation is not parsed (macro names are, but they don't mean the same thing here at all). Those are gone by the time Rustdoc sees the source code.
…8472

merge core_panic feature into panic_internals

I don't know why those are two separate features, but it does not seem intentional. This merge is useful because with rust-lang#118123, panic_internals is recognized as an internal feature, but core_panic is not -- but core_panic definitely should be internal.
…staged, r=Mark-Simulacrum

pass allow-{dirty,staged} to clippy

Support of Clippy's `--allow-dirty` and `--allow-staged` flags on bootstrap.

Resolves rust-lang#119483
…ble, r=cjgillot

rustc_mir_transform: Make DestinationPropagation stable for queries

By using `FxIndexMap` instead of `FxHashMap`, so that the order of visiting of locals is deterministic.

We also need to bless
`copy_propagation_arg.foo.DestinationPropagation.panic*.diff`. Do not review the diff of the diff. Instead look at the diff files before and after this commit. Both before and after this commit, 3 statements are replaced with nop. It's just that due to change in ordering, different statements are replaced. But the net result is the same. In other words, compare this diff (before fix):
* https://github.com/rust-lang/rust/blob/090d5eac722000906cc00d991f2bf052b0e388c3/tests/mir-opt/dest-prop/copy_propagation_arg.foo.DestinationPropagation.panic-unwind.diff

With this diff (after fix):
* https://github.com/rust-lang/rust/blob/f603babd63a607e155609dc0277806e559626ea0/tests/mir-opt/dest-prop/copy_propagation_arg.foo.DestinationPropagation.panic-unwind.diff

and you can see that both before and after the fix, we replace 3 statements with `nop`s.

I find it _slightly_ surprising that the test this PR affects did not previously fail spuriously due to the indeterminism of `FxHashMap`, but I guess in can be explained with the predictability of small `FxHashMap`s with `usize` (`Local`) keys, or something along those lines.

This should fix [this](rust-lang#119252 (comment)) comment, but I wanted to make a separate PR for this fix for a simpler development and review process.

Part of rust-lang#84447 which is E-help-wanted.

r? `@cjgillot` who is reviewer for the highly related PR rust-lang#119252.
Fixed ambiguity in hint.rs

Needle and haystack are actually not the same, they remain constant.
…rrors

rustc_span: More consistent span combination operations

Also add more tests for using `tt` in addition to `ident`, and some other minor tweaks, see individual commits.

This is a part of rust-lang#119412 that doesn't yet add side tables for metavariable spans.
…t_support, r=petrochenkov

compiler: update Fuchsia sanitizer support.
…monster-mess, r=WaffleLapkin

Remove ignore-stage1 that was added when changing error count msg

The bootstrap bump has happened, so the bootstrap compiler now contains the new diagnostic.

this was added in rust-lang#118138
…, r=Kobzol

Strip lld-wrapper binaries

This cuts down on the amount of data we need to ship and users need to keep on disk for each Rust toolchain. As noted in the added comment, there's not much going on in these executables, so the added benefit of symbols and debuginfo isn't large, while the cost is not insignificant.

This takes each of the binaries (we store 4 identical copies under different names) from 3.7MB to 384KB.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Jan 6, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=9

@bors
Copy link
Contributor

bors commented Jan 6, 2024

📌 Commit 5f0f028 has been approved by matthiaskrgr

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 Jan 6, 2024
@bors
Copy link
Contributor

bors commented Jan 6, 2024

⌛ Testing commit 5f0f028 with merge b6a8c76...

@bors
Copy link
Contributor

bors commented Jan 6, 2024

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 6, 2024
@bors bors merged commit b6a8c76 into rust-lang:master Jan 6, 2024
12 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 6, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#118194 rustdoc: search for tuples and unit by type with () 516bbf2640c072c931dfcc75c90d3144b6df6531 (link)
#118781 merge core_panic feature into panic_internals 9193d79e6b59aedadc58472c94c143b97968240d (link)
#119486 pass allow-{dirty,staged} to clippy 2d0c651712b744e516975a5fbc6c566996911787 (link)
#119591 rustc_mir_transform: Make DestinationPropagation stable for… 4c76c162b7d5fd97234d1ee5abac601fce2c1f8f (link)
#119595 Fixed ambiguity in hint.rs fad45fa74ae70de82cda450e913c963a00ac05ce (link)
#119624 rustc_span: More consistent span combination operations 44636caceed80276dcc572665763220a1e5d19c6 (link)
#119653 compiler: update Fuchsia sanitizer support. 9263db4059fc9a4b90e13c2a2667ad27912871cc (link)
#119655 Remove ignore-stage1 that was added when changing error cou… 44a31735c41852f7fcf5643f167ef87c20e5bbfb (link)
#119661 Strip lld-wrapper binaries 5b5b6e955eb59500bb9162cd4336a3533b73edc2 (link)

previous master: 9212108a9b

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 (b6a8c76): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

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

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)
3.4% [3.4%, 3.4%] 1
Improvements ✅
(primary)
-2.2% [-2.9%, -1.5%] 2
Improvements ✅
(secondary)
-5.8% [-9.8%, -1.8%] 2
All ❌✅ (primary) -2.2% [-2.9%, -1.5%] 2

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

Binary size

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

Bootstrap: 668.835s -> 668.367s (-0.07%)
Artifact size: 308.53 MiB -> 308.49 MiB (-0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Jan 6, 2024
@Kobzol
Copy link
Contributor

Kobzol commented Jan 6, 2024

@rust-timer build 44636ca

This looks suspicious w.r.t. the tt-muncher regression.

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (44636ca): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.2% [0.2%, 0.2%] 1
Regressions ❌
(secondary)
0.7% [0.2%, 1.3%] 12
Improvements ✅
(primary)
-0.4% [-0.4%, -0.4%] 4
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.3% [-0.4%, 0.2%] 5

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)
-1.1% [-1.3%, -0.9%] 3
Improvements ✅
(secondary)
-3.5% [-4.3%, -2.6%] 2
All ❌✅ (primary) -1.1% [-1.3%, -0.9%] 3

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)
-1.7% [-1.7%, -1.7%] 1
Improvements ✅
(secondary)
-7.5% [-7.7%, -7.4%] 2
All ❌✅ (primary) -1.7% [-1.7%, -1.7%] 1

Binary size

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

Bootstrap: 668.835s -> 668.722s (-0.02%)
Artifact size: 308.53 MiB -> 308.52 MiB (-0.00%)

@Mark-Simulacrum Mark-Simulacrum added perf-regression-triaged The performance regression has been triaged. and removed perf-regression-triaged The performance regression has been triaged. labels Jan 8, 2024
@Mark-Simulacrum
Copy link
Member

Regression looks real, not sure much can be done about it. Cachegrind diff

$ ./target/release/collector profile_local cachegrind +9212108a9b32cc381995cf509dd2627123c11a25 --rustc2 +44636caceed80276dcc572665763220a1e5d19c6 --include tt-muncher --profiles Check --scenarios Full
--------------------------------------------------------------------------------
-- File:function summary
--------------------------------------------------------------------------------
  Ir________  file:function

< 31,443,976  ???:
  29,581,300    <rustc_span::span_encoding::Span>::source_callsite
   1,596,570    <rustc_span::span_encoding::Span>::to
   1,572,681    <rustc_expand::mbe::macro_rules::MacroRulesMacroExpander as rustc_expand::base::TTMacroExpander>::expand
    -580,131    <rustc_parse::parser::Parser>::parse_path_inner
    -513,825    <rustc_span::symbol::MacroRulesNormalizedIdent>::new
    -375,501    <rustc_ast_lowering::LoweringContext>::lower_qpath
     341,699    rustc_expand::mbe::transcribe::lockstep_iter_size
    -274,767    <rustc_parse::parser::Parser>::parse_expr_path_start
     163,816    <rustc_middle::ty::context::CtxtInterners>::intern_predicate
      85,008    <rustc_middle::ty::context::CtxtInterners>::intern_ty
     -58,408    <rustc_resolve::Resolver as rustc_expand::base::ResolverExpand>::resolve_macro_invocation
     -58,263    <rustc_parse::parser::Parser>::parse_expr_dot_or_call
     -36,353    <hashbrown::raw::inner::RawTable<usize>>::reserve_rehash::<indexmap::map::core::get_hash<rustc_span::SpanData, ()>::{closure#0}>
<   -583,508  ./string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:__memcpy_avx_unaligned_erms
<    -92,944  <all-jemalloc-files>:<all-jemalloc-functions>

cc @petrochenkov -- did you expect a regression from #119624? It sounds like it's supposed to be mostly a refactoring, so maybe missing some extra conditions or wants a fast path?

My inclination is to mark as triaged if we don't see obvious fixes.

@matthiaskrgr matthiaskrgr deleted the rollup-ehofh5n branch March 16, 2024 18:19
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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.