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 13 pull requests #133760

Merged
merged 32 commits into from
Dec 2, 2024
Merged

Conversation

GuillaumeGomez
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

dtolnay and others added 30 commits November 30, 2024 17:53
There was only ever one test which used this flag, and it was removed in rust-lang#132244. I think this is a bad flag that should never have been added; comparing by subset makes the test failures extremely hard to debug. Any test that needs complicated output filtering like this should just use run-make instead.

Note that this does not remove the underlying comparison code, because it's still used if `runner` is set. I don't quite understand what's going on there, but since we still test on other platforms and in CI that the full output is accurate, I think it will be easier to debug than a test that uses compare-by-subset unconditionally.
Before this commit, the test writer has to specify platforms and
architectures by hand for targets that have differing atomic width
support. `#[cfg(target_has_atomic)]` is not quite the same because (1)
you may have to specify additional matchers manually which has to be
maintained individually, and (2) the `#[cfg]` blocks does not
communicate to compiletest that a test would be ignored for a given
target.

This commit implements a `//@ needs-target-has-atomic` directive which
admits a comma-separated list of required atomic widths that the target
must satisfy in order for the test to run.

```
//@ needs-target-has-atomic: 8, 16, ptr
```

See <rust-lang#87377>.

Co-authored-by: kei519 <masaki.keigo.q00@kyoto-u.jp>
for small rustc_driver programs, most of their imports will currently be related to diagnostics. this change simplifiers their code so it's more clear what in the driver is modified from the default.

this is especially important for external drivers which are out of tree and not updated in response to breaking changes. for these drivers, each import is a liability for future code, since it can be broken when refactors happen.

here is an example driver which is simplified by these changes:
```
diff --git a/src/main.rs b/src/main.rs
index f81aa3e..11e5f18 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,16 +1,8 @@
 #![feature(rustc_private)]
 extern crate rustc_driver;
 extern crate rustc_interface;
-extern crate rustc_errors;
-extern crate rustc_session;

 use rustc_driver::Callbacks;
-use rustc_errors::{emitter::HumanReadableErrorType, ColorConfig};
 use rustc_interface::interface;
-use rustc_session::config::ErrorOutputType;
-use rustc_session::EarlyDiagCtxt;

 struct DisableSafetyChecks;

@@ -26,11 +18,7 @@ fn main() {
         "https://github.com/jyn514/jyn514.github.io/issues/new",
         |_| (),
     );
-    let handler = EarlyDiagCtxt::new(ErrorOutputType::HumanReadable(
-        HumanReadableErrorType::Default,
-        ColorConfig::Auto,
-    ));
-    rustc_driver::init_rustc_env_logger(&handler);
+    rustc_driver::init_rustc_env_logger(&Default::default());
     std::process::exit(rustc_driver::catch_with_exit_code(move || {
         let args: Vec<String> = std::env::args().collect();
         rustc_driver::RunCompiler::new(&args, &mut DisableSafetyChecks).run()
```
Eliminate magic numbers from expression precedence

Context: see rust-lang#133140.

This PR continues on backporting Syn's expression precedence design into rustc. Rustc's design used mysterious integer quantities represented variously as `i8` or `usize` (e.g. `PREC_CLOSURE = -40i8`), a special significance around `0` that is never named, and an extra `PREC_FORCE_PAREN` precedence level that does not correspond to any expression. Syn's design uses a C-like enum with variants that clearly correspond to specific sets of expression kinds.

This PR is a refactoring that has no intended behavior change on its own, but it unblocks other precedence work that rustc's precedence design was poorly suited to accommodate.

- Asymmetrical precedence, so that a pretty-printer can tell `(return 1) + 1` needs parens but `1 + return 1` does not.

- Squashing the `Closure` and `Jump` cases into a single precedence level.

- Numerous remaining false positives and false negatives in rustc pretty-printer's parenthesization of macro metavariables, for example in `$e < rhs` where $e is `lhs as Thing<T>`.

FYI `@fmease` &mdash; you don't need to review if rustbot picks someone else, but you mentioned being interested in the followup PRs.
…umeGomez

rustdoc-json: Include safety of `static`s

`static`s in an `extern` block can have an associated safety annotation ["because there is nothing guaranteeing that the bit pattern at the static’s memory is valid for the type it is declared with"](https://doc.rust-lang.org/reference/items/external-blocks.html#statics). Rustdoc already knows this and displays in for HTML. This PR also includes it in JSON.

Inspired by obi1kenobi/cargo-semver-checks#975 which needs this, but it's probably useful in other places.

r? `@GuillaumeGomez.` Possibly easier to review commit-by-commit.
…laumeGomez

rustdoc-json: Add test for `impl Trait for dyn Trait`

Found while investigating rust-lang#133719

Helps with rust-lang#81359

r? `@GuillaumeGomez`
…t, r=jieyouxu

Remove `//@ compare-output-lines-by-subset`

There was only ever one test which used this flag, and it was removed in rust-lang#132244. I think this is a bad flag that should never have been added; comparing by subset makes the test failures extremely hard to debug. Any test that needs complicated output filtering like this should just use run-make instead.

Note that this does not remove the underlying comparison code, because it's still used if `runner` is set. I don't quite understand what's going on there, but since we still test on other platforms and in CI that the full output is accurate, I think it will be easier to debug than a test that uses compare-by-subset unconditionally.

rustc-dev-guide update PR: rust-lang/rustc-dev-guide#2151
Add pretty-printer parenthesis insertion test

This test demonstrates numerous bugs in rustc_ast_pretty, including all five of:

- Failing to insert parentheses where necessary to preserve the meaning of a syntax tree, producing invalid syntax.
- Failing to insert parentheses, producing valid syntax with the wrong meaning.
- Inserting too many parentheses.
- Inserting parentheses in the wrong place, producing invalid syntax.
- Losing syntactically significant parts of the syntax tree.

These pretty-printer bugs have consequences for `-Zunpretty=expanded`. The `cargo expand` subcommand cannot work reliably unless rustc can consistently produce valid Rust output. Erroneous syntax cannot be passed through rustfmt, or queried with [syn-select](https://crates.io/crates/syn-select).

The test in this PR is a port of a test from Syn that tests the automatic parenthesis insertion performed by Syn's `ToTokens` impls. In Syn we actually run this test over every expression in every Rust source file in the whole rust-lang/rust repo, including rustc and the standard library and tools and test suites. For the test here, I have only used a small selection of interesting expressions. This will serve as an easy spot to accumulate regression tests as the various bugs get fixed. Once rustc's pretty-printer is in better shape, it's possible that the test can be expanded to cover a larger set of expressions collected automatically like in Syn.
…r=compiler-errors

Add `needs-target-has-atomic` directive

Before this PR, the test writer has to specify platforms and architectures by hand for targets that have differing atomic width support. `#[cfg(target_has_atomic="...")]` is not quite the same because (1) you may have to specify additional matchers manually which has to be maintained individually, and (2) the `#[cfg]` blocks does not communicate to compiletest that a test would be ignored for a given target.

This PR implements a `//@ needs-target-has-atomic` directive which admits a comma-separated list of required atomic widths that the target must satisfy in order for the test to run.

```
//@ needs-target-has-atomic: 8, 16, ptr
```

See <rust-lang#87377>.

This PR supersedes rust-lang#133095 and is co-authored by `@kei519,` because it was somewhat subtle, and it turned out easier to implement than to review.

rustc-dev-guide docs PR: rust-lang/rustc-dev-guide#2154
Fix docs for `<[T]>::as_array`.

Tracking issue: rust-lang#133508

This PR fixes a small typographical error in the docs entry for `<[T]>::as_array`.
Fix typo README.md

Fix typo README.md 2: Electric Boogaloo
Expands on rust-lang#131144
…r=notriddle

Remove static HashSet for default IDs list

Follow-up of rust-lang#133345.

Let's see how it impacts performance.

r? `@notriddle`
…er-errors

mir validator: don't store mir phase

it's already stored in the `Body` and we assert that they match.
…ompiler-errors

remove `Ty::is_copy_modulo_regions`

Using these functions is likely incorrect if an `InferCtxt` is available, I moved this function to `TyCtxt` (and added it to `LateContext`) and added a note to the documentation that one should prefer `Infer::type_is_copy_modulo_regions` instead.

I didn't yet move `is_sized` and `is_freeze`, though I think we should move these as well.

r? `@compiler-errors` cc rust-lang#132279
…rrors

`impl Default for EarlyDiagCtxt`

for small rustc_driver programs, most of their imports will currently be related to diagnostics. this change simplifies their code so it's more clear what in the driver is modified from the default.

this is especially important for external drivers which are out of tree and not updated in response to breaking changes. for these drivers, each import is a liability for future code, since it can be broken when refactors happen.

here is an example driver which is simplified by these changes:
```diff
diff --git a/src/main.rs b/src/main.rs
index f81aa3e..11e5f18 100644
--- a/src/main.rs
+++ b/src/main.rs
`@@` -1,16 +1,8 `@@`
 #![feature(rustc_private)]
 extern crate rustc_driver;
 extern crate rustc_interface;
-extern crate rustc_errors;
-extern crate rustc_session;

 use rustc_driver::Callbacks;
-use rustc_errors::{emitter::HumanReadableErrorType, ColorConfig};
 use rustc_interface::interface;
-use rustc_session::config::ErrorOutputType;
-use rustc_session::EarlyDiagCtxt;

 struct DisableSafetyChecks;

`@@` -26,11 +18,7 `@@` fn main() {
         "https://github.com/jyn514/jyn514.github.io/issues/new",
         |_| (),
     );
-    let handler = EarlyDiagCtxt::new(ErrorOutputType::HumanReadable(
-        HumanReadableErrorType::Default,
-        ColorConfig::Auto,
-    ));
-    rustc_driver::init_rustc_env_logger(&handler);
+    rustc_driver::init_rustc_env_logger(&Default::default());
     std::process::exit(rustc_driver::catch_with_exit_code(move || {
         let args: Vec<String> = std::env::args().collect();
         rustc_driver::RunCompiler::new(&args, &mut DisableSafetyChecks).run()
```
@rustbot rustbot added A-compiletest Area: The compiletest test runner A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc 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 Dec 2, 2024
@GuillaumeGomez
Copy link
Member Author

@bors r+ rollup=never p=10

@bors
Copy link
Contributor

bors commented Dec 2, 2024

📌 Commit 586591f has been approved by GuillaumeGomez

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 Dec 2, 2024
@bors
Copy link
Contributor

bors commented Dec 2, 2024

⌛ Testing commit 586591f with merge d49be02...

@bors
Copy link
Contributor

bors commented Dec 2, 2024

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing d49be02 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 2, 2024
@bors bors merged commit d49be02 into rust-lang:master Dec 2, 2024
7 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 2, 2024
@GuillaumeGomez GuillaumeGomez deleted the rollup-2c1y8c3 branch December 2, 2024 22:08
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#133603 Eliminate magic numbers from expression precedence a6ea373f863fe34cff04dc2410852595c5be6910 (link)
#133715 rustdoc-json: Include safety of statics 1a16e930e801ac78d6d5311487f5ea069cf95b4d (link)
#133721 rustdoc-json: Add test for impl Trait for dyn Trait c4a363da6975dbd9f491e17173b64a7537cd762f (link)
#133725 Remove //@ compare-output-lines-by-subset 0f4976f937a8e47ba548677c66eee6b24f5bcb38 (link)
#133730 Add pretty-printer parenthesis insertion test 8e78dd635b0d93684ab9de74c8fe7cb65a74b8ea (link)
#133736 Add needs-target-has-atomic directive c09e2bd2b021bf8c16909864046116e4b3ed7827 (link)
#133739 Re-add myself to rotation 180f75378bcf7c88aec26f8fe46c60c346b69591 (link)
#133743 Fix docs for <[T]>::as_array. b3a790ff33654f699796991cdd59938277f27170 (link)
#133744 Fix typo README.md be18b6ed33f51317003f87eef759e852e94dd950 (link)
#133745 Remove static HashSet for default IDs list b8371c83e733bdd0526f5a3ba5b1fdda12af168f (link)
#133749 mir validator: don't store mir phase e67debf037e11ea92e34428dc9beda4d47541253 (link)
#133751 remove Ty::is_copy_modulo_regions 7255e637b938a5659a8d2da39bc1ac92d067649d (link)
#133757 impl Default for EarlyDiagCtxt 2528f1874874e51975ecc6d69738a6cd37013f92 (link)

previous master: 32eea2f446

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 (d49be02): 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 (secondary 2.3%)

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)
2.3% [0.5%, 4.2%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

Results (secondary -2.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)
- - 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: 768.242s -> 767.322s (-0.12%)
Artifact size: 332.19 MiB -> 332.21 MiB (0.01%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. 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.