Rollup of 9 pull requests #120837
Closed
Rollup of 9 pull requests #120837
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
That is, change `diagnostic_outside_of_impl` and `untranslatable_diagnostic` from `allow` to `deny`, because more than half of the compiler has be converted to use translated diagnostics. This commit removes more `deny` attributes than it adds `allow` attributes, which proves that this change is warranted.
This rewrite makes the cache-updating nature of the function slightly clearer, using the Entry API into the hash table for region names to capture the update-insert nature of the method. May be marginally more efficient since it only runtime-borrows the map once, but in this context the performance impact is almost certainly completely negligible.
…visit This prevents ICEs from happening in the future when this code path is actually used
When you stash an error, the error count is incremented. You can then
use the non-zero error count to get an `ErrorGuaranteed`. You can then
steal the error, which decrements the error count. You can then cancel
the error.
Example code:
```
fn unsound(dcx: &DiagCtxt) -> ErrorGuaranteed {
let sp = rustc_span::DUMMY_SP;
let k = rustc_errors::StashKey::Cycle;
dcx.struct_err("bogus").stash(sp, k); // increment error count on stash
let guar = dcx.has_errors().unwrap(); // ErrorGuaranteed from error count > 0
let err = dcx.steal_diagnostic(sp, k).unwrap(); // decrement error count on steal
err.cancel(); // cancel error
guar // ErrorGuaranteed with no error emitted!
}
```
This commit fixes the problem in the simplest way: by not counting
stashed errors in `DiagCtxt::{err_count,has_errors}`.
However, just doing this without any other changes leads to over 40 ui
test failures. Mostly because of uninteresting extra errors (many saying
"type annotations needed" when type inference fails), and in a few
cases, due to delayed bugs causing ICEs when no normal errors are
printed.
To fix these, this commit adds `DiagCtxt::stashed_err_count`, and uses
it in three places alongside `DiagCtxt::{has_errors,err_count}`. It's
dodgy to rely on it, because unlike `DiagCtxt::err_count` it can go up
and down. But it's needed to preserve existing behaviour, and at least
the three places that need it are now obvious.
When launching tests with --keep-stage option, startup objects such as rsbegin.o an rsend.o may disappear from the corresponding stageN compiler. Fix issue rust-lang#120784
Introduce `run-make` V2 infrastructure, a `run_make_support` library and port over 2 tests as example See [issue rust-lang#40713: Switch run-make tests from Makefiles to rust](rust-lang#40713) for more context. ### Basic Description of `run-make` V2 `run-make` V2 aims to eliminate the dependency on `make` and `Makefile`s for building `run-make`-style tests. Makefiles are replaced by *recipes* (`rmake.rs`). The current implementation runs `run-make` V2 tests in 3 steps: 1. We build the support library `run_make_support` which the `rmake.rs` recipes depend on as a tool dylib. 2. We build the recipe `rmake.rs` and link in the support library. 3. We run the recipe to build and run the tests. `rmake.rs` is basically a replacement for `Makefile`, and allows running arbitrary Rust code. ### Planned Changes - [x] Get rid of the builder style patterns in `rmake_support` and instead use something like ```rust let output = rustc!(scx, "--cfg x -Cprefer-dynamic -Csymbol-mangling-version=legacy - Zunstable-options"); ``` as per Nils' suggestion. This can probably use something like `xshell`. - [x] Make `run_make_support` into a proper crate so it can have external dependencies like `xshell`. - [x] Instead of having an entire alternative directory `run-make-v2`, change how V2 tests are collected based on presence of `rmake.rs` recipe file. This should ease migration and prevent git history from being messed up by big moves. ### Disclaimer The current implementation is very much a **very very rough prototype** just to get the 2 example tests working. I would appreciate any feedback on the design and implementation.
…ochenkov Make privacy visitor use types more (instead of HIR) r? `@petrochenkov` This is a prerequisite to normalizing projections, as otherwise we have too many invalid bound vars (hir_ty_to_ty is creating types that have bound vars, but no binder). The commits are still chaotic, I'm gonna clean them up, but I just wanted to let you know about the general direction and wondering if we could land this before adding normalization, as normalization is where behavioral changes happen, and I'd like to keep that part as minimal as possible. [context can be found on zulip](https://rust-lang.zulipchat.com/#narrow/stream/315482-t-compiler.2Fetc.2Fopaque-types/topic/weak.20type.20aliases.20and.20privacy)
…ou-se core/time: avoid divisions in Duration::new In our (decently large) code base, we use `SystemTime::UNIX_EPOCH.elapsed()` in a lot of places & often in a loop or in the hot path. On [Unix](https://github.com/rust-lang/rust/blob/1.75.0/library/std/src/sys/unix/time.rs#L153-L162) at least, it seems we do calculations before hand to ensure that nanos is within the valid range, yet `Duration::new()` still checks it again, using 2 divisions. It seems like adding a branch can make this function 33% faster on ARM64 in the cases where nanos is already in the valid range & seems to have no effect in the other case. Benchmarks: M1 Pro (14-inch base model): ``` duration/current/checked time: [1.5945 ns 1.6167 ns 1.6407 ns] Found 5 outliers among 100 measurements (5.00%) 2 (2.00%) high mild 3 (3.00%) high severe duration/current/unchecked time: [1.5941 ns 1.6051 ns 1.6179 ns] Found 2 outliers among 100 measurements (2.00%) 1 (1.00%) high mild 1 (1.00%) high severe duration/branched/checked time: [1.1997 ns 1.2048 ns 1.2104 ns] Found 8 outliers among 100 measurements (8.00%) 4 (4.00%) high mild 4 (4.00%) high severe duration/branched/unchecked time: [1.5881 ns 1.5957 ns 1.6039 ns] Found 6 outliers among 100 measurements (6.00%) 3 (3.00%) high mild 3 (3.00%) high severe ``` EC2 c7gd.16xlarge (Graviton 3): ``` duration/current/checked time: [2.7996 ns 2.8000 ns 2.8003 ns] Found 5 outliers among 100 measurements (5.00%) 2 (2.00%) low severe 3 (3.00%) low mild duration/current/unchecked time: [2.9922 ns 2.9925 ns 2.9928 ns] Found 7 outliers among 100 measurements (7.00%) 4 (4.00%) low severe 1 (1.00%) low mild 2 (2.00%) high mild duration/branched/checked time: [2.0830 ns 2.0843 ns 2.0857 ns] Found 3 outliers among 100 measurements (3.00%) 1 (1.00%) low severe 1 (1.00%) low mild 1 (1.00%) high mild duration/branched/unchecked time: [2.9879 ns 2.9886 ns 2.9893 ns] Found 5 outliers among 100 measurements (5.00%) 3 (3.00%) low severe 2 (2.00%) low mild ``` EC2 r7iz.16xlarge (Intel Xeon Scalable-based (Sapphire Rapids)): ``` duration/current/checked time: [980.60 ps 980.79 ps 980.99 ps] Found 10 outliers among 100 measurements (10.00%) 4 (4.00%) low severe 2 (2.00%) low mild 3 (3.00%) high mild 1 (1.00%) high severe duration/current/unchecked time: [979.53 ps 979.74 ps 979.96 ps] Found 6 outliers among 100 measurements (6.00%) 2 (2.00%) low severe 1 (1.00%) low mild 2 (2.00%) high mild 1 (1.00%) high severe duration/branched/checked time: [938.72 ps 938.96 ps 939.22 ps] Found 4 outliers among 100 measurements (4.00%) 1 (1.00%) low mild 1 (1.00%) high mild 2 (2.00%) high severe duration/branched/unchecked time: [1.0103 ns 1.0110 ns 1.0118 ns] Found 10 outliers among 100 measurements (10.00%) 2 (2.00%) low mild 7 (7.00%) high mild 1 (1.00%) high severe ``` Bench code (ran using stable 1.75.0 & criterion latest 0.5.1): I couldn't find any benches for `Duration` in this repo, so I just copied the relevant types & recreated it. ```rust use criterion::{black_box, criterion_group, criterion_main, Criterion}; pub fn duration_bench(c: &mut Criterion) { const NANOS_PER_SEC: u32 = 1_000_000_000; #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] #[repr(transparent)] struct Nanoseconds(u32); impl Default for Nanoseconds { #[inline] fn default() -> Self { // SAFETY: 0 is within the valid range unsafe { Nanoseconds(0) } } } #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Default)] pub struct Duration { secs: u64, nanos: Nanoseconds, // Always 0 <= nanos < NANOS_PER_SEC } impl Duration { #[inline] pub const fn new_current(secs: u64, nanos: u32) -> Duration { let secs = match secs.checked_add((nanos / NANOS_PER_SEC) as u64) { Some(secs) => secs, None => panic!("overflow in Duration::new"), }; let nanos = nanos % NANOS_PER_SEC; // SAFETY: nanos % NANOS_PER_SEC < NANOS_PER_SEC, therefore nanos is within the valid range Duration { secs, nanos: unsafe { Nanoseconds(nanos) } } } #[inline] pub const fn new_branched(secs: u64, nanos: u32) -> Duration { if nanos < NANOS_PER_SEC { // SAFETY: nanos < NANOS_PER_SEC, therefore nanos is within the valid range Duration { secs, nanos: unsafe { Nanoseconds(nanos) } } } else { let secs = match secs.checked_add((nanos / NANOS_PER_SEC) as u64) { Some(secs) => secs, None => panic!("overflow in Duration::new"), }; let nanos = nanos % NANOS_PER_SEC; // SAFETY: nanos % NANOS_PER_SEC < NANOS_PER_SEC, therefore nanos is within the valid range Duration { secs, nanos: unsafe { Nanoseconds(nanos) } } } } } let mut group = c.benchmark_group("duration/current"); group.bench_function("checked", |b| { b.iter(|| black_box(Duration::new_current(black_box(1_000_000_000), black_box(1_000_000)))); }); group.bench_function("unchecked", |b| { b.iter(|| { black_box(Duration::new_current(black_box(1_000_000_000), black_box(2_000_000_000))) }); }); drop(group); let mut group = c.benchmark_group("duration/branched"); group.bench_function("checked", |b| { b.iter(|| { black_box(Duration::new_branched(black_box(1_000_000_000), black_box(1_000_000))) }); }); group.bench_function("unchecked", |b| { b.iter(|| { black_box(Duration::new_branched(black_box(1_000_000_000), black_box(2_000_000_000))) }); }); } criterion_group!(duration_benches, duration_bench); criterion_main!(duration_benches); ```
…s, r=davidtwco Invert diagnostic lints. That is, change `diagnostic_outside_of_impl` and `untranslatable_diagnostic` from `allow` to `deny`, because more than half of the compiler has been converted to use translated diagnostics. This commit removes more `deny` attributes than it adds `allow` attributes, which proves that this change is warranted. r? ```@davidtwco```
…write, r=compiler-errors A drive-by rewrite of `give_region_a_name()` This drive-by rewrite makes the cache-updating nature of the method clearer, using the Entry API into the hash table for region names to capture the update-insert nature of the method. May be marginally more efficient since it only runtime-borrows and indexes the map once, but in this context the performance impact is almost certainly completely negligible. Note that this commit should preserve all externally visible behaviour. Notably, it preserves the debug logging: 1. printing even in the case of a `None` for the new computed name, and 2. only printing on new values, begin silent on reused values
…ructors, r=Nilstrieb Use `transmute_unchecked` in `NonZero::new`. Tracking issue: rust-lang#120257 See rust-lang#120521 (comment).
…=oli-obk Fix more `ty::Error` ICEs in MIR passes Fixes rust-lang#120791 - Add a check for `ty::Error` in the `ByMove` coroutine pass Fixes rust-lang#120816 - Add a check for `ty::Error` in the MIR validator Also a drive-by fix for a FIXME I had asked oli to add r? oli-obk
…-obk
Fix `ErrorGuaranteed` unsoundness with stash/steal.
When you stash an error, the error count is incremented. You can then use the non-zero error count to get an `ErrorGuaranteed`. You can then steal the error, which decrements the error count. You can then cancel the error.
Example code:
```
fn unsound(dcx: &DiagCtxt) -> ErrorGuaranteed {
let sp = rustc_span::DUMMY_SP;
let k = rustc_errors::StashKey::Cycle;
dcx.struct_err("bogus").stash(sp, k); // increment error count on stash
let guar = dcx.has_errors().unwrap(); // ErrorGuaranteed from error count > 0
let err = dcx.steal_diagnostic(sp, k).unwrap(); // decrement error count on steal
err.cancel(); // cancel error
guar // ErrorGuaranteed with no error emitted!
}
```
This commit fixes the problem in the simplest way: by not counting stashed errors in `DiagCtxt::{err_count,has_errors}`.
However, just doing this without any other changes leads to over 40 ui test failures. Mostly because of uninteresting extra errors (many saying "type annotations needed" when type inference fails), and in a few cases, due to delayed bugs causing ICEs when no normal errors are printed.
To fix these, this commit adds `DiagCtxt::stashed_err_count`, and uses it in three places alongside `DiagCtxt::{has_errors,err_count}`. It's dodgy to rely on it, because unlike `DiagCtxt::err_count` it can go up and down. But it's needed to preserve existing behaviour, and at least the three places that need it are now obvious.
r? oli-obk
…objects, r=onur-ozkan Startup objects disappearing from sysroot When launching tests with --keep-stage option, startup objects such as rsbegin.o an rsend.o may disappear from the corresponding stageN compiler. Fix issue rust-lang#120784
rustbot
added
A-query-system
rustbot
added
A-translation
|
@bors r+ rollup=never p=9 |
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 9, 2024
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#113026 (Introduce `run-make` V2 infrastructure, a `run_make_support` library and port over 2 tests as example) - rust-lang#113671 (Make privacy visitor use types more (instead of HIR)) - rust-lang#120308 (core/time: avoid divisions in Duration::new) - rust-lang#120693 (Invert diagnostic lints.) - rust-lang#120704 (A drive-by rewrite of `give_region_a_name()`) - rust-lang#120809 (Use `transmute_unchecked` in `NonZero::new`.) - rust-lang#120817 (Fix more `ty::Error` ICEs in MIR passes) - rust-lang#120828 (Fix `ErrorGuaranteed` unsoundness with stash/steal.) - rust-lang#120831 (Startup objects disappearing from sysroot) r? `@ghost` `@rustbot` modify labels: rollup
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
run-makeV2 infrastructure, arun_make_supportlibrary and port over 2 tests as example #113026 (Introducerun-makeV2 infrastructure, arun_make_supportlibrary and port over 2 tests as example)give_region_a_name()#120704 (A drive-by rewrite ofgive_region_a_name())transmute_uncheckedinNonZero::new. #120809 (Usetransmute_uncheckedinNonZero::new.)ty::ErrorICEs in MIR passes #120817 (Fix morety::ErrorICEs in MIR passes)ErrorGuaranteedunsoundness with stash/steal. #120828 (FixErrorGuaranteedunsoundness with stash/steal.)r? @ghost
@rustbot modify labels: rollup
Create a similar rollup