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

Sync rustfmt #98412

Merged
merged 20 commits into from
Jun 25, 2022
Merged

Sync rustfmt #98412

merged 20 commits into from
Jun 25, 2022

Conversation

calebcartwright
Copy link
Member

We had a bug in the update we made ~1 week ago, so running a somewhat early sync to pull the fix in

calebcartwright and others added 20 commits June 12, 2022 22:03
There was recently an issue where `cargo install` was installing a newer
version of a dependency than the one listed in our Cargo.toml. The newer
version added deprecation warnings that caused our continuous integration
tests to break.

As mentioned in the `cargo help install` docs, passing the `--locked`
flag should force cargo to use the `Cargo.lock` file included with
the repository.
fix sorting of use statements with raw identifiers
There are some proposed import sorting changes for raw identifier `r#`.
These changes constitute a breaking change, and need to be version
gagted. Before version gating those changes we add the version
information to the `UseSegment`.
When useing `version=One` rustfmt will treat the leading `r#` as part of
the `UseSegment` used for sorting. When using `version=Two` rustfmt will
ignore the leading `r#` and only consider the name of the identifier
when sorting the `UseSegment`.
…-warn, r=wesleywiser,flip1995

Support lint expectations for `--force-warn` lints (RFC 2383)

Rustc has a `--force-warn` flag, which overrides lint level attributes and forces the diagnostics to always be warn. This means, that for lint expectations, the diagnostic can't be suppressed as usual. This also means that the expectation would not be fulfilled, even if a lint had been triggered in the expected scope.

This PR now also tracks the expectation ID in the `ForceWarn` level. I've also made some minor adjustments, to possibly catch more bugs and make the whole implementation more robust.

This will probably conflict with rust-lang#97718. That PR should ideally be reviewed and merged first. The conflict itself will be trivial to fix.

---

r? `@wesleywiser`

cc: `@flip1995` since you've helped with the initial review and also discussed this topic with me. 🙃

Follow-up of: rust-lang#87835

Issue: rust-lang#85549

Yeah, and that's it.
* add doc_comment_code_block_width configuration

* updated config docu

* Updated docu and changed tests to config folder
By setting this value in the Cargo.toml rust-analyzer understands that
rustfmt uses compiler-internals using `extern crate`.

This is a universal step that all developers will need to take in order to
get better type hints and code completion suggestions for
compiler-internals in their editor.

**Note**: users will also need to install the `rustc-dev` component via
`rustup` and add the following to their rust-analyzer configuration:

```json
{
    "rust-analyzer.rustcSource": "discover",
    "rust-analyzer.updates.channel": "nightly"
}
```
* config_proc_macro: fix failing doctests

* ci: include config_proc_macro crate in ci

* [review] working native windows ci

* [fix] add --locked file for ci

* [fix] quoting of cmd variables
Fixes 5399

Memoizing expressions lead to cases where rustfmt's stability guarantees
were violated.

This reverts commit a37d3ab.
Fixes 5395

In PR 5239 we switched from using `structopt` to `clap`. It seems that
the default behavior for `clap` is to override the `--version` flag,
which prevented our custom version display code from running.

The fix as outlined in clap-rs/clap#3405 was
to set `#[clap(global_setting(AppSettings::NoAutoVersion))]` to prevent
clap from setting its own default behavior for the `--version` flag.
@rust-highfive
Copy link
Collaborator

Some changes occurred in src/tools/rustfmt.

cc @rust-lang/rustfmt

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 23, 2022
@calebcartwright
Copy link
Member Author

CI failure was unrelated and transient, closing and reopening to restart the initial CI checks

@calebcartwright
Copy link
Member Author

@bors r+ p=1

subtree sync, and I want to make sure this gets landed before the next nightly build to minimize the number of nightlies containing this bug, as the rustfmt output is supposed to be stable on nightly too

@bors
Copy link
Contributor

bors commented Jun 23, 2022

📌 Commit 993ee00 has been approved by calebcartwright

@bors
Copy link
Contributor

bors commented Jun 23, 2022

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@calebcartwright
Copy link
Member Author

@bors retry

@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 Jun 25, 2022
@bors
Copy link
Contributor

bors commented Jun 25, 2022

⌛ Testing commit 993ee00 with merge 9cf699d...

@bors
Copy link
Contributor

bors commented Jun 25, 2022

☀️ Test successful - checks-actions
Approved by: calebcartwright
Pushing 9cf699d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 25, 2022
@bors bors merged commit 9cf699d into rust-lang:master Jun 25, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jun 25, 2022
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #98412!

Tested on commit 9cf699d.
Direct link to PR: #98412

🎉 miri on linux: test-fail → test-pass (cc @eddyb @RalfJung @oli-obk).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Jun 25, 2022
Tested on commit rust-lang/rust@9cf699d.
Direct link to PR: <rust-lang/rust#98412>

🎉 miri on linux: test-fail → test-pass (cc @eddyb @RalfJung @oli-obk).
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9cf699d): comparison url.

Instruction count

  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: 😿 relevant regressions found
mean1 max count2
Regressions 😿
(primary)
0.3% 0.3% 1
Regressions 😿
(secondary)
0.9% 1.5% 8
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) 0.3% 0.3% 1

Max RSS (memory usage)

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

Cycles

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 🎉 relevant improvement found
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
-3.5% -3.5% 1
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

  2. number of relevant changes 2

@rustbot rustbot added the perf-regression Performance regression. label Jun 25, 2022
@calebcartwright
Copy link
Member Author

I find several of these comments quite confusing.

This obviously has no impact on builds/tests for miri, so unclear why the bots believe that updating the rustfmt subtree would.

I'm also unsure what the proper etiquette is relative to supposed perf regressions. Similar to the miri comment, rustfmt has absolutely nothing to do with those benchmarks and to the best of my knowledge no aspect of rustfmt is included in those runs. As such I naturally disagree with the assessment, but I'm disinclined to file an issue as that seems like obvious noise.

I believe both of these can and should be chalked up to bots running amok, but let me know if anyone feels otherwise

@RalfJung
Copy link
Member

Miri has a spuriously failing test that happened to fail after the previous PR, and happened to work again after this one. The test is fixed in Miri proper but the Miri submodule here still needs updating to prevent such flip-floping of test results.

I don't know about the perf changes, but I would ignore them -- I have had perf change reports for Miri updates, which also clearly just demonstrate noise in the perf measurement.

@calebcartwright
Copy link
Member Author

Also @Mark-Simulacrum this might need to be nominated for a backport. This needs to be included in the same release as #98040, so if #98040 is indeed slated for 1.63 as the milestone suggests then I want to nominate this for what I guess is a beta backport

@Mark-Simulacrum Mark-Simulacrum added beta-nominated Nominated for backporting to the compiler in the beta channel. beta-accepted Accepted for backporting to the compiler in the beta channel. labels Jun 26, 2022
@Mark-Simulacrum
Copy link
Member

beta-accepting for 1.63 backport (mostly as coincidence that these straddled the release cut date)

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 28, 2022
…imulacrum

[beta] Prepare beta 1.63.0

This PR switches the channel of `beta` after the force-push, and backports the following PRs:

*  rust-lang#98488
*  rust-lang#98412

r? `@ghost`
@rylev
Copy link
Member

rylev commented Jun 28, 2022

Looks like there is some new noise in the keccak benchmark. Marking this as triaged.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Jun 28, 2022
@ehuss
Copy link
Contributor

ehuss commented Jul 5, 2022

Untagging, re-milestoning, this was included in #98572.

@ehuss ehuss removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 5, 2022
@ehuss ehuss modified the milestones: 1.64.0, 1.63.0 Jul 5, 2022
@calebcartwright calebcartwright deleted the sync-rustfmt branch January 24, 2023 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.