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 #99520

Merged
merged 30 commits into from
Jul 20, 2022
Merged

Rollup of 9 pull requests #99520

merged 30 commits into from
Jul 20, 2022

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

compiler-errors and others added 30 commits July 16, 2022 22:54
Change some regular comments into documentation comments.

Signed-off-by: David Wood <david.wood@huawei.com>
When an unexpected meta item is provided to `#[stable]`, the diagnostic
lists "since" and "note" as expected meta-items, however the surrounding
code actually expects "feature" and "since".

Signed-off-by: David Wood <david.wood@huawei.com>
If part of a feature is stabilized and a new feature is added for the
remaining parts, then the `implied_by` attribute can be used to indicate
which now-stable feature previously contained a item. If the now-stable
feature is still active (if the user has only just updated rustc, for
example) then there will not be an stability error for uses of the item
from the implied feature.

Signed-off-by: David Wood <david.wood@huawei.com>
Adds a simple helper function to the `SourceMap` for extending a `Span`
to encompass the entire line it is on - useful for suggestions where
removing a line is the suggested action.

Signed-off-by: David Wood <david.wood@huawei.com>
Improves the diagnostic when a feature attribute is specified
unnecessarily but the feature implies another (i.e. it was partially
stabilized) to refer to the implied feature.

Signed-off-by: David Wood <david.wood@huawei.com>
Add a check confirming that features referenced in `implied_by` meta
items actually exist.

Signed-off-by: David Wood <david.wood@huawei.com>
…, r=michaelwoerister

introduce `implied_by` in `#[unstable]` attribute

Requested by the library team [on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/better.20support.20for.20partial.20stabilizations/near/285581519).

If part of a feature is stabilized and a new feature is added for the remaining parts, then the `implied_by` meta-item can be added to `#[unstable]` to indicate which now-stable feature was used previously.

```diagnostic
error: the feature `foo` has been partially stabilized since 1.62.0 and is succeeded by the feature `foobar`
  --> $DIR/stability-attribute-implies-using-unstable.rs:3:12
   |
LL | #![feature(foo)]
   |            ^^^
   |
note: the lint level is defined here
  --> $DIR/stability-attribute-implies-using-stable.rs:2:9
   |
LL | #![deny(stable_features)]
   |         ^^^^^^^^^^^^^^^
help: if you are using features which are still unstable, change to using `foobar`
   |
LL | #![feature(foobar)]
   |            ~~~~~~
help: if you are using features which are now stable, remove this line
   |
LL - #![feature(foo)]
   |
```

When a `#![feature(..)]` attribute still exists for the now-stable attribute, then there this has two effects:

- There will not be an stability error for uses of items from the implied feature which are still unstable (until the `#![feature(..)]` is removed or updated to the new feature).
- There will be an improved diagnostic for the remaining use of the feature attribute for the now-stable feature.

```rust
        /// If part of a feature is stabilized and a new feature is added for the remaining parts,
        /// then the `implied_by` attribute is used to indicate which now-stable feature previously
        /// contained a item.
        ///
        /// ```pseudo-Rust
        /// #[unstable(feature = "foo", issue = "...")]
        /// fn foo() {}
        /// #[unstable(feature = "foo", issue = "...")]
        /// fn foobar() {}
        /// ```
        ///
        /// ...becomes...
        ///
        /// ```pseudo-Rust
        /// #[stable(feature = "foo", since = "1.XX.X")]
        /// fn foo() {}
        /// #[unstable(feature = "foobar", issue = "...", implied_by = "foo")]
        /// fn foobar() {}
        /// ```
```

In the Zulip discussion, this was envisioned as `implies` on `#[stable]` but I went with `implied_by` on `#[unstable]` because it means that only the unstable attribute needs to be changed in future, not the new stable attribute, which seems less error-prone. It also isn't particularly feasible for me to detect whether items from the implied feature are used and then only suggest updating _or_ removing the `#![feature(..)]` as appropriate, so I always do both.

There's some new information in the cross-crate metadata as a result of this change, that's a little unfortunate, but without requiring that the `#[unstable]` and `#[stable]` attributes both contain the implication information, it's necessary:

```rust
    /// This mapping is necessary unless both the `#[stable]` and `#[unstable]` attributes should
    /// specify their implications (both `implies` and `implied_by`). If only one of the two
    /// attributes do (as in the current implementation, `implied_by` in `#[unstable]`), then this
    /// mapping is necessary for diagnostics. When a "unnecessary feature attribute" error is
    /// reported, only the `#[stable]` attribute information is available, so the map is necessary
    /// to know that the feature implies another feature. If it were reversed, and the `#[stable]`
    /// attribute had an `implies` meta item, then a map would be necessary when avoiding a "use of
    /// unstable feature" error for a feature that was implied.
```

I also change some comments to documentation comments in the compiler, add a helper for going from a `Span` to a `Span` for the entire line, and fix a incorrect part of the pre-existing stability attribute diagnostics.

cc `@yaahc`
…neric-call, r=spastorino

Use `typeck_results` to avoid duplicate `ast_ty_to_ty` call

Comes with a bunch of improvements in spans 😍
…than-zero, r=petrochenkov

better error for bad depth parameter on macro metavar expr

Fixes rust-lang#99060
Diagnostic width span is not added when '0$' is used as width in format strings

When the following code is run rustc does not add diagnostic spans for the width argument. Such spans are necessary for a clippy lint that I am currently writing.

```rust
println!("Hello {1:0$}!", 5, "x");
//                 ^^
// Should have a span here
```
compiletest: Allow using revisions with debuginfo tests.

A small wart that came up in rust-lang#95685 (comment).
rustdoc UI fixes

The first commit fixes this bug:

![Screenshot from 2022-07-20 02-54-26](https://user-images.githubusercontent.com/3050060/179879053-fc34f27a-6248-4f5c-9fcb-80adbfc1598c.png)
![Screenshot from 2022-07-20 03-00-03](https://user-images.githubusercontent.com/3050060/179879056-1c0973a0-d535-44e7-a48e-bad692034467.png)

The second one fixes the missing change of border color when the search input is focused.

cc `@jsha`
r? `@notriddle`
…ersion-in-BuiltinLintDiagnostics, r=compiler-errors

Avoid `Symbol` to `String` conversions

follow-up to rust-lang#99342
adapt assembly/static-relocation-model test for LLVM change

After llvm/llvm-project@f0dd12e LLVM emits `movzbl` instead. Adapted this test case accordingly.

Discovered in our experimental rust + llvm at head ci:
https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/12104#0182195b-8791-4f88-853c-bb23a1e4b54c
…-issue, r=Mark-Simulacrum

Use new tracking issue for proc_macro::tracked_*.
@rustbot rustbot added T-compiler Relevant to the compiler 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 Jul 20, 2022
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=10

@bors
Copy link
Contributor

bors commented Jul 20, 2022

📌 Commit a5a6811 has been approved by matthiaskrgr

It is now in the queue for this repository.

@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 Jul 20, 2022
@bors
Copy link
Contributor

bors commented Jul 20, 2022

⌛ Testing commit a5a6811 with merge d68e7eb...

@bors
Copy link
Contributor

bors commented Jul 20, 2022

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 20, 2022
@bors bors merged commit d68e7eb into rust-lang:master Jul 20, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 20, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d68e7eb): comparison url.

Instruction count

  • Primary benchmarks: 😿 relevant regressions found
  • Secondary benchmarks: 😿 relevant regressions found
mean1 max count2
Regressions 😿
(primary)
2.0% 2.7% 4
Regressions 😿
(secondary)
1.3% 2.5% 29
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) 2.0% 2.7% 4

Max RSS (memory usage)

Results
  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
-4.4% -6.0% 2
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) -4.4% -6.0% 2

Cycles

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 😿 relevant regression found
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
4.3% 4.3% 1
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) N/A N/A 0

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@rustbot rustbot added the perf-regression Performance regression. label Jul 21, 2022
@pnkfelix
Copy link
Member

pnkfelix commented Jul 27, 2022

  • The 4 primary regressions, 3 are helloworld check, regressing by 2.5% to 2.7% on various incr scenarios. The last is ripgrep check but that only regressed by 0.36%.
  • In any case, a broad swath of regressions are signalled for incremental check builds.
  • Review of the perf-regression reported here leads me to hypothesize that it might have been injected by PR introduce implied_by in #[unstable] attribute #99212.
  • I'm going to leave a comment over in that PR asking them to double-check whether that PR might have regressed performance.

@rylev
Copy link
Member

rylev commented Jul 28, 2022

Going to try out the tooling introduced in rust-lang/rustc-perf#701. Forgive any noise!

@rust-timer make-pr-for 857afc7

@rylev
Copy link
Member

rylev commented Jul 28, 2022

Trying again! Forgive the noise.

@rust-timer make-pr-for 857afc7

@matthiaskrgr matthiaskrgr deleted the rollup-05uuv5s branch July 30, 2022 10:32
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-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.

None yet