From b031c516641c53577d021da1a89d0c953acdc23a Mon Sep 17 00:00:00 2001 From: okaneco <47607823+okaneco@users.noreply.github.com> Date: Tue, 7 Oct 2025 05:52:37 -0400 Subject: [PATCH 1/6] slice/ascii: Optimize `eq_ignore_ascii_case` with auto-vectorization Refactor the current functionality into a helper function Use `as_chunks` to encourage auto-vectorization in the optimized chunk processing function Add a codegen test Add benches for `eq_ignore_ascii_case` The optimized function is initially only enabled for x86_64 which has `sse2` as part of its baseline, but none of the code is platform specific. Other platforms with SIMD instructions may also benefit from this implementation. Performance improvements only manifest for slices of 16 bytes or longer, so the optimized path is gated behind a length check for greater than or equal to 16. --- library/core/src/slice/ascii.rs | 43 ++++++++++++++ library/coretests/benches/ascii.rs | 1 + .../benches/ascii/eq_ignore_ascii_case.rs | 56 +++++++++++++++++++ .../lib-optimizations/eq_ignore_ascii_case.rs | 14 +++++ 4 files changed, 114 insertions(+) create mode 100644 library/coretests/benches/ascii/eq_ignore_ascii_case.rs create mode 100644 tests/codegen-llvm/lib-optimizations/eq_ignore_ascii_case.rs diff --git a/library/core/src/slice/ascii.rs b/library/core/src/slice/ascii.rs index e17a2e03d2dc4..1f9ca4bc66987 100644 --- a/library/core/src/slice/ascii.rs +++ b/library/core/src/slice/ascii.rs @@ -60,6 +60,18 @@ impl [u8] { return false; } + #[cfg(all(target_arch = "x86_64", target_feature = "sse2"))] + if self.len() >= 16 { + return self.eq_ignore_ascii_case_chunks(other); + } + + self.eq_ignore_ascii_case_simple(other) + } + + /// ASCII case-insensitive equality check without chunk-at-a-time + /// optimization. + #[inline] + const fn eq_ignore_ascii_case_simple(&self, other: &[u8]) -> bool { // FIXME(const-hack): This implementation can be reverted when // `core::iter::zip` is allowed in const. The original implementation: // self.len() == other.len() && iter::zip(self, other).all(|(a, b)| a.eq_ignore_ascii_case(b)) @@ -78,6 +90,37 @@ impl [u8] { true } + /// Optimized version of `eq_ignore_ascii_case` which processes chunks at a + /// time. + /// + /// Platforms that have SIMD instructions may benefit from this + /// implementation over `eq_ignore_ascii_case_simple`. + #[cfg(all(target_arch = "x86_64", target_feature = "sse2"))] + #[inline] + const fn eq_ignore_ascii_case_chunks(&self, other: &[u8]) -> bool { + const N: usize = 16; + let (a, a_rem) = self.as_chunks::(); + let (b, b_rem) = other.as_chunks::(); + + let mut i = 0; + while i < a.len() && i < b.len() { + let mut equal_ascii = true; + let mut j = 0; + while j < N { + equal_ascii &= a[i][j].eq_ignore_ascii_case(&b[i][j]); + j += 1; + } + + if !equal_ascii { + return false; + } + + i += 1; + } + + a_rem.eq_ignore_ascii_case_simple(b_rem) + } + /// Converts this slice to its ASCII upper case equivalent in-place. /// /// ASCII letters 'a' to 'z' are mapped to 'A' to 'Z', diff --git a/library/coretests/benches/ascii.rs b/library/coretests/benches/ascii.rs index 64bdc7fed118f..17a520922bfa7 100644 --- a/library/coretests/benches/ascii.rs +++ b/library/coretests/benches/ascii.rs @@ -1,3 +1,4 @@ +mod eq_ignore_ascii_case; mod is_ascii; // Lower-case ASCII 'a' is the first byte that has its highest bit set diff --git a/library/coretests/benches/ascii/eq_ignore_ascii_case.rs b/library/coretests/benches/ascii/eq_ignore_ascii_case.rs new file mode 100644 index 0000000000000..a51acb1e84637 --- /dev/null +++ b/library/coretests/benches/ascii/eq_ignore_ascii_case.rs @@ -0,0 +1,56 @@ +use test::Bencher; + +#[bench] +fn bench_str_under_8_bytes_eq(b: &mut Bencher) { + let s = "foo"; + let other = "FOo"; + b.iter(|| { + assert!(s.eq_ignore_ascii_case(other)); + }) +} + +#[bench] +fn bench_str_of_8_bytes_eq(b: &mut Bencher) { + let s = "foobar78"; + let other = "FOObAr78"; + b.iter(|| { + assert!(s.eq_ignore_ascii_case(other)); + }) +} + +#[bench] +fn bench_str_17_bytes_eq(b: &mut Bencher) { + let s = "performance-criti"; + let other = "performANce-cRIti"; + b.iter(|| { + assert!(s.eq_ignore_ascii_case(other)); + }) +} + +#[bench] +fn bench_str_31_bytes_eq(b: &mut Bencher) { + let s = "foobarbazquux02foobarbazquux025"; + let other = "fooBARbazQuuX02fooBARbazQuuX025"; + b.iter(|| { + assert!(s.eq_ignore_ascii_case(other)); + }) +} + +#[bench] +fn bench_long_str_eq(b: &mut Bencher) { + let s = "Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor \ + incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud \ + exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute \ + irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla \ + pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui \ + officia deserunt mollit anim id est laborum."; + let other = "Lorem ipsum dolor sit amet, CONSECTETUR adipisicing elit, sed do eiusmod tempor \ + incididunt ut labore et dolore MAGNA aliqua. Ut enim ad MINIM veniam, quis nostrud \ + exercitation ullamco LABORIS nisi ut aliquip ex ea commodo consequat. Duis aute \ + irure dolor in reprehenderit in voluptate velit esse cillum DOLORE eu fugiat nulla \ + pariatur. Excepteur sint occaecat CUPIDATAT non proident, sunt in culpa qui \ + officia deserunt mollit anim id est laborum."; + b.iter(|| { + assert!(s.eq_ignore_ascii_case(other)); + }) +} diff --git a/tests/codegen-llvm/lib-optimizations/eq_ignore_ascii_case.rs b/tests/codegen-llvm/lib-optimizations/eq_ignore_ascii_case.rs new file mode 100644 index 0000000000000..b733f1812c92c --- /dev/null +++ b/tests/codegen-llvm/lib-optimizations/eq_ignore_ascii_case.rs @@ -0,0 +1,14 @@ +//@ compile-flags: -Copt-level=3 +//@ only-x86_64 +#![crate_type = "lib"] + +// Ensure that the optimized variant of the function gets auto-vectorized. +// CHECK-LABEL: @eq_ignore_ascii_case_autovectorized +#[no_mangle] +pub fn eq_ignore_ascii_case_autovectorized(s: &str, other: &str) -> bool { + // CHECK: load <16 x i8> + // CHECK: load <16 x i8> + // CHECK: bitcast <16 x i1> + // CHECK-NOT: panic + s.eq_ignore_ascii_case(other) +} From 7b88f48b530f6610581a0d023199c9563e635730 Mon Sep 17 00:00:00 2001 From: okaneco <47607823+okaneco@users.noreply.github.com> Date: Tue, 7 Oct 2025 15:35:02 -0400 Subject: [PATCH 2/6] Avoid scalar fallback in chunk remainder check Refactor the eq check into an inner function for reuse in tail checking Rather than fall back to the simple implementation for tail handling, load the last 16 bytes to take advantage of vectorization. This doesn't seem to negatively impact check time even when the remainder count is low. --- library/core/src/slice/ascii.rs | 35 +++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/library/core/src/slice/ascii.rs b/library/core/src/slice/ascii.rs index 1f9ca4bc66987..3f3f5a8c441dd 100644 --- a/library/core/src/slice/ascii.rs +++ b/library/core/src/slice/ascii.rs @@ -90,8 +90,8 @@ impl [u8] { true } - /// Optimized version of `eq_ignore_ascii_case` which processes chunks at a - /// time. + /// Optimized version of `eq_ignore_ascii_case` for byte lengths of at least + /// 16 bytes, which processes chunks at a time. /// /// Platforms that have SIMD instructions may benefit from this /// implementation over `eq_ignore_ascii_case_simple`. @@ -99,26 +99,41 @@ impl [u8] { #[inline] const fn eq_ignore_ascii_case_chunks(&self, other: &[u8]) -> bool { const N: usize = 16; - let (a, a_rem) = self.as_chunks::(); - let (b, b_rem) = other.as_chunks::(); + let (self_chunks, self_rem) = self.as_chunks::(); + let (other_chunks, _) = other.as_chunks::(); - let mut i = 0; - while i < a.len() && i < b.len() { + // Branchless check to encourage auto-vectorization + const fn eq_ignore_ascii_inner(lhs: &[u8; N], rhs: &[u8; N]) -> bool { let mut equal_ascii = true; let mut j = 0; while j < N { - equal_ascii &= a[i][j].eq_ignore_ascii_case(&b[i][j]); + equal_ascii &= lhs[j].eq_ignore_ascii_case(&rhs[j]); j += 1; } - if !equal_ascii { + equal_ascii + } + + // Process the chunks, returning early if an inequality is found + let mut i = 0; + while i < self_chunks.len() && i < other_chunks.len() { + if !eq_ignore_ascii_inner(&self_chunks[i], &other_chunks[i]) { return false; } - i += 1; } - a_rem.eq_ignore_ascii_case_simple(b_rem) + // If there are remaining tails, load the last N bytes in the slices to + // avoid falling back to per-byte checking. + if !self_rem.is_empty() { + if let (Some(a_rem), Some(b_rem)) = (self.last_chunk::(), other.last_chunk::()) { + if !eq_ignore_ascii_inner(a_rem, b_rem) { + return false; + } + } + } + + true } /// Converts this slice to its ASCII upper case equivalent in-place. From a5ba24843d6e4ceda580e49344bef73baec14204 Mon Sep 17 00:00:00 2001 From: okaneco <47607823+okaneco@users.noreply.github.com> Date: Tue, 7 Oct 2025 19:04:32 -0400 Subject: [PATCH 3/6] Relocate bench and use str corpora for data Add #[inline(always)] to inner function and check not for filecheck test --- library/core/src/slice/ascii.rs | 1 + library/coretests/benches/ascii.rs | 1 - .../benches/ascii/eq_ignore_ascii_case.rs | 56 ------------------- library/coretests/benches/str.rs | 1 + .../benches/str/eq_ignore_ascii_case.rs | 45 +++++++++++++++ .../lib-optimizations/eq_ignore_ascii_case.rs | 4 +- 6 files changed, 50 insertions(+), 58 deletions(-) delete mode 100644 library/coretests/benches/ascii/eq_ignore_ascii_case.rs create mode 100644 library/coretests/benches/str/eq_ignore_ascii_case.rs diff --git a/library/core/src/slice/ascii.rs b/library/core/src/slice/ascii.rs index 3f3f5a8c441dd..8b713947cd9c7 100644 --- a/library/core/src/slice/ascii.rs +++ b/library/core/src/slice/ascii.rs @@ -103,6 +103,7 @@ impl [u8] { let (other_chunks, _) = other.as_chunks::(); // Branchless check to encourage auto-vectorization + #[inline(always)] const fn eq_ignore_ascii_inner(lhs: &[u8; N], rhs: &[u8; N]) -> bool { let mut equal_ascii = true; let mut j = 0; diff --git a/library/coretests/benches/ascii.rs b/library/coretests/benches/ascii.rs index 17a520922bfa7..64bdc7fed118f 100644 --- a/library/coretests/benches/ascii.rs +++ b/library/coretests/benches/ascii.rs @@ -1,4 +1,3 @@ -mod eq_ignore_ascii_case; mod is_ascii; // Lower-case ASCII 'a' is the first byte that has its highest bit set diff --git a/library/coretests/benches/ascii/eq_ignore_ascii_case.rs b/library/coretests/benches/ascii/eq_ignore_ascii_case.rs deleted file mode 100644 index a51acb1e84637..0000000000000 --- a/library/coretests/benches/ascii/eq_ignore_ascii_case.rs +++ /dev/null @@ -1,56 +0,0 @@ -use test::Bencher; - -#[bench] -fn bench_str_under_8_bytes_eq(b: &mut Bencher) { - let s = "foo"; - let other = "FOo"; - b.iter(|| { - assert!(s.eq_ignore_ascii_case(other)); - }) -} - -#[bench] -fn bench_str_of_8_bytes_eq(b: &mut Bencher) { - let s = "foobar78"; - let other = "FOObAr78"; - b.iter(|| { - assert!(s.eq_ignore_ascii_case(other)); - }) -} - -#[bench] -fn bench_str_17_bytes_eq(b: &mut Bencher) { - let s = "performance-criti"; - let other = "performANce-cRIti"; - b.iter(|| { - assert!(s.eq_ignore_ascii_case(other)); - }) -} - -#[bench] -fn bench_str_31_bytes_eq(b: &mut Bencher) { - let s = "foobarbazquux02foobarbazquux025"; - let other = "fooBARbazQuuX02fooBARbazQuuX025"; - b.iter(|| { - assert!(s.eq_ignore_ascii_case(other)); - }) -} - -#[bench] -fn bench_long_str_eq(b: &mut Bencher) { - let s = "Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor \ - incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud \ - exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute \ - irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla \ - pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui \ - officia deserunt mollit anim id est laborum."; - let other = "Lorem ipsum dolor sit amet, CONSECTETUR adipisicing elit, sed do eiusmod tempor \ - incididunt ut labore et dolore MAGNA aliqua. Ut enim ad MINIM veniam, quis nostrud \ - exercitation ullamco LABORIS nisi ut aliquip ex ea commodo consequat. Duis aute \ - irure dolor in reprehenderit in voluptate velit esse cillum DOLORE eu fugiat nulla \ - pariatur. Excepteur sint occaecat CUPIDATAT non proident, sunt in culpa qui \ - officia deserunt mollit anim id est laborum."; - b.iter(|| { - assert!(s.eq_ignore_ascii_case(other)); - }) -} diff --git a/library/coretests/benches/str.rs b/library/coretests/benches/str.rs index 2f7d9d56a70b7..bf45a8f0a79bc 100644 --- a/library/coretests/benches/str.rs +++ b/library/coretests/benches/str.rs @@ -5,6 +5,7 @@ use test::{Bencher, black_box}; mod char_count; mod corpora; mod debug; +mod eq_ignore_ascii_case; mod iter; #[bench] diff --git a/library/coretests/benches/str/eq_ignore_ascii_case.rs b/library/coretests/benches/str/eq_ignore_ascii_case.rs new file mode 100644 index 0000000000000..29129b933bc46 --- /dev/null +++ b/library/coretests/benches/str/eq_ignore_ascii_case.rs @@ -0,0 +1,45 @@ +use test::{Bencher, black_box}; + +use super::corpora::*; + +#[bench] +fn bench_str_under_8_bytes_eq(b: &mut Bencher) { + let s = black_box("foo"); + let other = black_box("foo"); + b.iter(|| assert!(s.eq_ignore_ascii_case(other))) +} + +#[bench] +fn bench_str_of_8_bytes_eq(b: &mut Bencher) { + let s = black_box(en::TINY); + let other = black_box(en::TINY); + b.iter(|| assert!(s.eq_ignore_ascii_case(other))) +} + +#[bench] +fn bench_str_17_bytes_eq(b: &mut Bencher) { + let s = black_box(&en::SMALL[..17]); + let other = black_box(&en::SMALL[..17]); + b.iter(|| assert!(s.eq_ignore_ascii_case(other))) +} + +#[bench] +fn bench_str_31_bytes_eq(b: &mut Bencher) { + let s = black_box(&en::SMALL[..31]); + let other = black_box(&en::SMALL[..31]); + b.iter(|| assert!(s.eq_ignore_ascii_case(other))) +} + +#[bench] +fn bench_medium_str_eq(b: &mut Bencher) { + let s = black_box(en::MEDIUM); + let other = black_box(en::MEDIUM); + b.iter(|| assert!(s.eq_ignore_ascii_case(other))) +} + +#[bench] +fn bench_large_str_eq(b: &mut Bencher) { + let s = black_box(en::LARGE); + let other = black_box(en::LARGE); + b.iter(|| assert!(s.eq_ignore_ascii_case(other))) +} diff --git a/tests/codegen-llvm/lib-optimizations/eq_ignore_ascii_case.rs b/tests/codegen-llvm/lib-optimizations/eq_ignore_ascii_case.rs index b733f1812c92c..d4ac5d64585db 100644 --- a/tests/codegen-llvm/lib-optimizations/eq_ignore_ascii_case.rs +++ b/tests/codegen-llvm/lib-optimizations/eq_ignore_ascii_case.rs @@ -2,13 +2,15 @@ //@ only-x86_64 #![crate_type = "lib"] -// Ensure that the optimized variant of the function gets auto-vectorized. +// Ensure that the optimized variant of the function gets auto-vectorized and +// that the inner helper function is inlined. // CHECK-LABEL: @eq_ignore_ascii_case_autovectorized #[no_mangle] pub fn eq_ignore_ascii_case_autovectorized(s: &str, other: &str) -> bool { // CHECK: load <16 x i8> // CHECK: load <16 x i8> // CHECK: bitcast <16 x i1> + // CHECK-NOT: call {{.*}}eq_ignore_ascii_inner // CHECK-NOT: panic s.eq_ignore_ascii_case(other) } From 31bf836c74e5b53e654bed70158cabfdfdca8acf Mon Sep 17 00:00:00 2001 From: okaneco <47607823+okaneco@users.noreply.github.com> Date: Tue, 14 Oct 2025 02:23:44 -0400 Subject: [PATCH 4/6] Use a const generic parameter for the function chunk size Add comments for the optimized function invariants to the caller Add const-hack fixme for using while-loops Document the invariant for the `_chunks` function Add a debug assert for the tail handling invariant --- library/core/src/slice/ascii.rs | 35 +++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/library/core/src/slice/ascii.rs b/library/core/src/slice/ascii.rs index 8b713947cd9c7..0f3cf243f1647 100644 --- a/library/core/src/slice/ascii.rs +++ b/library/core/src/slice/ascii.rs @@ -61,8 +61,15 @@ impl [u8] { } #[cfg(all(target_arch = "x86_64", target_feature = "sse2"))] - if self.len() >= 16 { - return self.eq_ignore_ascii_case_chunks(other); + { + const CHUNK_SIZE: usize = 16; + // The following function has two invariants: + // 1. The slice lengths must be equal, which we checked above. + // 2. The slice lengths must greater than or equal to N, which this + // if-statement is checking. + if self.len() >= CHUNK_SIZE { + return self.eq_ignore_ascii_case_chunks::(other); + } } self.eq_ignore_ascii_case_simple(other) @@ -90,24 +97,30 @@ impl [u8] { true } - /// Optimized version of `eq_ignore_ascii_case` for byte lengths of at least - /// 16 bytes, which processes chunks at a time. + /// Optimized version of `eq_ignore_ascii_case` to process chunks at a time. /// /// Platforms that have SIMD instructions may benefit from this /// implementation over `eq_ignore_ascii_case_simple`. + /// + /// # Invariants + /// + /// The caller must guarantee that the slices are equal in length, and the + /// slice lengths are greater than or equal to `N` bytes. #[cfg(all(target_arch = "x86_64", target_feature = "sse2"))] #[inline] - const fn eq_ignore_ascii_case_chunks(&self, other: &[u8]) -> bool { - const N: usize = 16; + const fn eq_ignore_ascii_case_chunks(&self, other: &[u8]) -> bool { + // FIXME(const-hack): The while-loops that follow should be replaced by + // for-loops when available in const. + let (self_chunks, self_rem) = self.as_chunks::(); let (other_chunks, _) = other.as_chunks::(); // Branchless check to encourage auto-vectorization #[inline(always)] - const fn eq_ignore_ascii_inner(lhs: &[u8; N], rhs: &[u8; N]) -> bool { + const fn eq_ignore_ascii_inner(lhs: &[u8; L], rhs: &[u8; L]) -> bool { let mut equal_ascii = true; let mut j = 0; - while j < N { + while j < L { equal_ascii &= lhs[j].eq_ignore_ascii_case(&rhs[j]); j += 1; } @@ -124,6 +137,12 @@ impl [u8] { i += 1; } + // Check the length invariant which is necessary for the tail-handling + // logic to be correct. This should have been upheld by the caller, + // otherwise lengths less than N will compare as true without any + // checking. + debug_assert!(self.len() >= N); + // If there are remaining tails, load the last N bytes in the slices to // avoid falling back to per-byte checking. if !self_rem.is_empty() { From 76b6623267dc944fd61e06741327f74ee4820a21 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 23 Jan 2026 11:06:57 +0000 Subject: [PATCH 5/6] target: fix destabilising target-spec-json --- compiler/rustc_target/src/spec/mod.rs | 3 +++ tests/run-make/rust-lld-custom-target/rmake.rs | 6 +++++- tests/run-make/rustdoc-target-spec-json-path/rmake.rs | 8 +++++++- tests/run-make/target-specs/rmake.rs | 9 ++++++++- tests/ui/check-cfg/values-target-json.rs | 3 ++- 5 files changed, 25 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index ef475630d4943..164db15d24e09 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -3352,6 +3352,9 @@ impl Target { Err(format!("could not find specification for target {target_tuple:?}")) } + TargetTuple::TargetJson { ref contents, .. } if !unstable_options => { + Err("custom targets are unstable and require `-Zunstable-options`".to_string()) + } TargetTuple::TargetJson { ref contents, .. } => Target::from_json(contents), } } diff --git a/tests/run-make/rust-lld-custom-target/rmake.rs b/tests/run-make/rust-lld-custom-target/rmake.rs index 90ba424ffe940..d281d820f47b4 100644 --- a/tests/run-make/rust-lld-custom-target/rmake.rs +++ b/tests/run-make/rust-lld-custom-target/rmake.rs @@ -15,7 +15,11 @@ fn main() { // Compile to a custom target spec with rust-lld enabled by default. We'll check that by asking // the linker to display its version number with a link-arg. assert_rustc_uses_lld( - rustc().crate_type("cdylib").target("custom-target.json").input("lib.rs"), + rustc() + .crate_type("cdylib") + .target("custom-target.json") + .arg("-Zunstable-options") + .input("lib.rs"), ); // But it can also be disabled via linker features. diff --git a/tests/run-make/rustdoc-target-spec-json-path/rmake.rs b/tests/run-make/rustdoc-target-spec-json-path/rmake.rs index d43aa9b90ac72..8660556564f1e 100644 --- a/tests/run-make/rustdoc-target-spec-json-path/rmake.rs +++ b/tests/run-make/rustdoc-target-spec-json-path/rmake.rs @@ -5,8 +5,14 @@ use run_make_support::{cwd, rustc, rustdoc}; fn main() { let out_dir = "rustdoc-target-spec-json-path"; - rustc().crate_type("lib").input("dummy_core.rs").target("target.json").run(); + rustc() + .arg("-Zunstable-options") + .crate_type("lib") + .input("dummy_core.rs") + .target("target.json") + .run(); rustdoc() + .arg("-Zunstable-options") .input("my_crate.rs") .out_dir(out_dir) .library_search_path(cwd()) diff --git a/tests/run-make/target-specs/rmake.rs b/tests/run-make/target-specs/rmake.rs index 69292af5fd69c..6c88f3164e9e4 100644 --- a/tests/run-make/target-specs/rmake.rs +++ b/tests/run-make/target-specs/rmake.rs @@ -20,13 +20,20 @@ fn main() { .target("my-incomplete-platform.json") .run_fail() .assert_stderr_contains("missing field `llvm-target`"); - let test_platform = rustc() + let _ = rustc() .input("foo.rs") .target("my-x86_64-unknown-linux-gnu-platform") .crate_type("lib") .emit("asm") .run_fail() .assert_stderr_contains("custom targets are unstable and require `-Zunstable-options`"); + let _ = rustc() + .input("foo.rs") + .target("my-awesome-platform.json") + .crate_type("lib") + .emit("asm") + .run_fail() + .assert_stderr_contains("custom targets are unstable and require `-Zunstable-options`"); rustc() .arg("-Zunstable-options") .env("RUST_TARGET_PATH", ".") diff --git a/tests/ui/check-cfg/values-target-json.rs b/tests/ui/check-cfg/values-target-json.rs index defb286ed19be..21b6af9b55605 100644 --- a/tests/ui/check-cfg/values-target-json.rs +++ b/tests/ui/check-cfg/values-target-json.rs @@ -4,7 +4,8 @@ //@ check-pass //@ no-auto-check-cfg //@ needs-llvm-components: x86 -//@ compile-flags: --crate-type=lib --check-cfg=cfg() --target={{src-base}}/check-cfg/my-awesome-platform.json +//@ compile-flags: --crate-type=lib --check-cfg=cfg() +//@ compile-flags: -Zunstable-options --target={{src-base}}/check-cfg/my-awesome-platform.json //@ ignore-backends: gcc #![feature(lang_items, no_core, auto_traits, rustc_attrs)] From 0385e26e7d1e6a984954d1daa448471e9ef1051e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 20 Jan 2026 11:56:19 +1100 Subject: [PATCH 6/6] Reintroduce `QueryStackFrame` split. I tried removing it in #151203, to replace it with something simpler. But a couple of fuzzing failures have come up and I don't have a clear picture on how to fix them. So I'm reverting the main part of #151203. This commit also adds the two fuzzing tests. Fixes #151226, #151358. --- compiler/rustc_middle/src/query/mod.rs | 2 +- compiler/rustc_middle/src/query/plumbing.rs | 2 +- compiler/rustc_middle/src/ty/print/pretty.rs | 4 +- compiler/rustc_middle/src/values.rs | 4 +- compiler/rustc_query_impl/src/lib.rs | 9 +- compiler/rustc_query_impl/src/plumbing.rs | 96 ++++++++----- .../rustc_query_system/src/query/config.rs | 5 +- compiler/rustc_query_system/src/query/job.rs | 131 ++++++++++-------- compiler/rustc_query_system/src/query/mod.rs | 105 ++++++++++++-- .../rustc_query_system/src/query/plumbing.rs | 61 ++++---- .../query-cycle-printing-issue-151226.rs | 8 ++ .../query-cycle-printing-issue-151226.stderr | 36 +++++ .../query-cycle-printing-issue-151358.rs | 7 + .../query-cycle-printing-issue-151358.stderr | 9 ++ tests/ui/resolve/query-cycle-issue-124901.rs | 2 +- .../resolve/query-cycle-issue-124901.stderr | 9 +- 16 files changed, 341 insertions(+), 149 deletions(-) create mode 100644 tests/ui/query-system/query-cycle-printing-issue-151226.rs create mode 100644 tests/ui/query-system/query-cycle-printing-issue-151226.stderr create mode 100644 tests/ui/query-system/query-cycle-printing-issue-151358.rs create mode 100644 tests/ui/query-system/query-cycle-printing-issue-151358.stderr diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 2f83f3078e89f..9d17c998a8f29 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -88,7 +88,7 @@ use rustc_index::IndexVec; use rustc_lint_defs::LintId; use rustc_macros::rustc_queries; use rustc_query_system::ich::StableHashingContext; -use rustc_query_system::query::{QueryMode, QueryState}; +use rustc_query_system::query::{QueryMode, QueryStackDeferred, QueryState}; use rustc_session::Limits; use rustc_session::config::{EntryFnType, OptLevel, OutputFilenames, SymbolManglingVersion}; use rustc_session::cstore::{ diff --git a/compiler/rustc_middle/src/query/plumbing.rs b/compiler/rustc_middle/src/query/plumbing.rs index 17330f4e14bee..9ee8d743e64a4 100644 --- a/compiler/rustc_middle/src/query/plumbing.rs +++ b/compiler/rustc_middle/src/query/plumbing.rs @@ -427,7 +427,7 @@ macro_rules! define_callbacks { #[derive(Default)] pub struct QueryStates<'tcx> { $( - pub $name: QueryState<$($K)*>, + pub $name: QueryState<$($K)*, QueryStackDeferred<'tcx>>, )* } diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs index de13c4f836a5f..76a4f61e67148 100644 --- a/compiler/rustc_middle/src/ty/print/pretty.rs +++ b/compiler/rustc_middle/src/ty/print/pretty.rs @@ -159,9 +159,7 @@ pub macro with_types_for_signature($e:expr) {{ /// Avoids running any queries during prints. pub macro with_no_queries($e:expr) {{ $crate::ty::print::with_reduced_queries!($crate::ty::print::with_forced_impl_filename_line!( - $crate::ty::print::with_no_trimmed_paths!($crate::ty::print::with_no_visible_paths!( - $crate::ty::print::with_forced_impl_filename_line!($e) - )) + $crate::ty::print::with_no_trimmed_paths!($crate::ty::print::with_no_visible_paths!($e)) )) }} diff --git a/compiler/rustc_middle/src/values.rs b/compiler/rustc_middle/src/values.rs index 8d614a535498f..bc73d36216ef4 100644 --- a/compiler/rustc_middle/src/values.rs +++ b/compiler/rustc_middle/src/values.rs @@ -88,7 +88,7 @@ impl<'tcx> Value> for Representability { if info.query.dep_kind == dep_kinds::representability && let Some(field_id) = info.query.def_id && let Some(field_id) = field_id.as_local() - && let Some(DefKind::Field) = info.query.def_kind + && let Some(DefKind::Field) = info.query.info.def_kind { let parent_id = tcx.parent(field_id.to_def_id()); let item_id = match tcx.def_kind(parent_id) { @@ -224,7 +224,7 @@ impl<'tcx, T> Value> for Result> continue; }; let frame_span = - frame.query.default_span(cycle[(i + 1) % cycle.len()].span); + frame.query.info.default_span(cycle[(i + 1) % cycle.len()].span); if frame_span.is_dummy() { continue; } diff --git a/compiler/rustc_query_impl/src/lib.rs b/compiler/rustc_query_impl/src/lib.rs index 57027e937a4a9..e8983bfa1ddba 100644 --- a/compiler/rustc_query_impl/src/lib.rs +++ b/compiler/rustc_query_impl/src/lib.rs @@ -23,7 +23,7 @@ use rustc_query_system::dep_graph::SerializedDepNodeIndex; use rustc_query_system::ich::StableHashingContext; use rustc_query_system::query::{ CycleError, CycleErrorHandling, HashResult, QueryCache, QueryConfig, QueryMap, QueryMode, - QueryState, get_query_incr, get_query_non_incr, + QueryStackDeferred, QueryState, get_query_incr, get_query_non_incr, }; use rustc_span::{ErrorGuaranteed, Span}; @@ -79,7 +79,10 @@ where } #[inline(always)] - fn query_state<'a>(self, qcx: QueryCtxt<'tcx>) -> &'a QueryState + fn query_state<'a>( + self, + qcx: QueryCtxt<'tcx>, + ) -> &'a QueryState> where QueryCtxt<'tcx>: 'a, { @@ -88,7 +91,7 @@ where unsafe { &*(&qcx.tcx.query_system.states as *const QueryStates<'tcx>) .byte_add(self.dynamic.query_state) - .cast::>() + .cast::>>() } } diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 246152f5390c6..847b5f901d7ff 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -6,6 +6,7 @@ use std::num::NonZero; use rustc_data_structures::jobserver::Proxy; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; +use rustc_data_structures::sync::{DynSend, DynSync}; use rustc_data_structures::unord::UnordMap; use rustc_hashes::Hash64; use rustc_hir::limit::Limit; @@ -26,8 +27,8 @@ use rustc_middle::ty::{self, TyCtxt}; use rustc_query_system::dep_graph::{DepNodeParams, HasDepContext}; use rustc_query_system::ich::StableHashingContext; use rustc_query_system::query::{ - QueryCache, QueryConfig, QueryContext, QueryJobId, QueryMap, QuerySideEffect, QueryStackFrame, - force_query, + QueryCache, QueryConfig, QueryContext, QueryJobId, QueryMap, QuerySideEffect, + QueryStackDeferred, QueryStackFrame, QueryStackFrameExtra, force_query, }; use rustc_query_system::{QueryOverflow, QueryOverflowNote}; use rustc_serialize::{Decodable, Encodable}; @@ -66,7 +67,9 @@ impl<'tcx> HasDepContext for QueryCtxt<'tcx> { } } -impl QueryContext for QueryCtxt<'_> { +impl<'tcx> QueryContext for QueryCtxt<'tcx> { + type QueryInfo = QueryStackDeferred<'tcx>; + #[inline] fn jobserver_proxy(&self) -> &Proxy { &*self.jobserver_proxy @@ -95,7 +98,10 @@ impl QueryContext for QueryCtxt<'_> { /// Prefer passing `false` to `require_complete` to avoid potential deadlocks, /// especially when called from within a deadlock handler, unless a /// complete map is needed and no deadlock is possible at this call site. - fn collect_active_jobs(self, require_complete: bool) -> Result { + fn collect_active_jobs( + self, + require_complete: bool, + ) -> Result>, QueryMap>> { let mut jobs = QueryMap::default(); let mut complete = true; @@ -108,6 +114,13 @@ impl QueryContext for QueryCtxt<'_> { if complete { Ok(jobs) } else { Err(jobs) } } + fn lift_query_info( + self, + info: &QueryStackDeferred<'tcx>, + ) -> rustc_query_system::query::QueryStackFrameExtra { + info.extract() + } + // Interactions with on_disk_cache fn load_side_effect( self, @@ -168,7 +181,10 @@ impl QueryContext for QueryCtxt<'_> { self.sess.dcx().emit_fatal(QueryOverflow { span: info.job.span, - note: QueryOverflowNote { desc: info.query.description, depth }, + note: QueryOverflowNote { + desc: self.lift_query_info(&info.query.info).description, + depth, + }, suggested_limit, crate_name: self.crate_name(LOCAL_CRATE), }); @@ -305,16 +321,17 @@ macro_rules! should_ever_cache_on_disk { }; } -pub(crate) fn create_query_frame< - 'tcx, - K: Copy + Key + for<'a> HashStable>, ->( - tcx: TyCtxt<'tcx>, - do_describe: fn(TyCtxt<'tcx>, K) -> String, - key: K, - kind: DepKind, - name: &'static str, -) -> QueryStackFrame { +fn create_query_frame_extra<'tcx, K: Key + Copy + 'tcx>( + (tcx, key, kind, name, do_describe): ( + TyCtxt<'tcx>, + K, + DepKind, + &'static str, + fn(TyCtxt<'tcx>, K) -> String, + ), +) -> QueryStackFrameExtra { + let def_id = key.key_as_def_id(); + // If reduced queries are requested, we may be printing a query stack due // to a panic. Avoid using `default_span` and `def_kind` in that case. let reduce_queries = with_reduced_queries(); @@ -326,36 +343,49 @@ pub(crate) fn create_query_frame< } else { description }; - - let span = if reduce_queries { + let span = if kind == dep_graph::dep_kinds::def_span || reduce_queries { // The `def_span` query is used to calculate `default_span`, // so exit to avoid infinite recursion. None } else { - Some(tcx.with_reduced_queries(|| key.default_span(tcx))) + Some(key.default_span(tcx)) }; - let def_id = key.key_as_def_id(); - - let def_kind = if reduce_queries { + let def_kind = if kind == dep_graph::dep_kinds::def_kind || reduce_queries { // Try to avoid infinite recursion. None } else { - def_id - .and_then(|def_id| def_id.as_local()) - .map(|def_id| tcx.with_reduced_queries(|| tcx.def_kind(def_id))) + def_id.and_then(|def_id| def_id.as_local()).map(|def_id| tcx.def_kind(def_id)) }; + QueryStackFrameExtra::new(description, span, def_kind) +} + +pub(crate) fn create_query_frame< + 'tcx, + K: Copy + DynSend + DynSync + Key + for<'a> HashStable> + 'tcx, +>( + tcx: TyCtxt<'tcx>, + do_describe: fn(TyCtxt<'tcx>, K) -> String, + key: K, + kind: DepKind, + name: &'static str, +) -> QueryStackFrame> { + let def_id = key.key_as_def_id(); + let hash = || { + tcx.with_stable_hashing_context(|mut hcx| { + let mut hasher = StableHasher::new(); + kind.as_usize().hash_stable(&mut hcx, &mut hasher); + key.hash_stable(&mut hcx, &mut hasher); + hasher.finish::() + }) + }; let def_id_for_ty_in_cycle = key.def_id_for_ty_in_cycle(); - let hash = tcx.with_stable_hashing_context(|mut hcx| { - let mut hasher = StableHasher::new(); - kind.as_usize().hash_stable(&mut hcx, &mut hasher); - key.hash_stable(&mut hcx, &mut hasher); - hasher.finish::() - }); + let info = + QueryStackDeferred::new((tcx, key, kind, name, do_describe), create_query_frame_extra); - QueryStackFrame::new(description, span, def_id, def_kind, kind, def_id_for_ty_in_cycle, hash) + QueryStackFrame::new(info, kind, hash, def_id, def_id_for_ty_in_cycle) } pub(crate) fn encode_query_results<'a, 'tcx, Q>( @@ -710,7 +740,7 @@ macro_rules! define_queries { pub(crate) fn collect_active_jobs<'tcx>( tcx: TyCtxt<'tcx>, - qmap: &mut QueryMap, + qmap: &mut QueryMap>, require_complete: bool, ) -> Option<()> { let make_query = |tcx, key| { @@ -794,7 +824,7 @@ macro_rules! define_queries { // These arrays are used for iteration and can't be indexed by `DepKind`. const COLLECT_ACTIVE_JOBS: &[ - for<'tcx> fn(TyCtxt<'tcx>, &mut QueryMap, bool) -> Option<()> + for<'tcx> fn(TyCtxt<'tcx>, &mut QueryMap>, bool) -> Option<()> ] = &[$(query_impl::$name::collect_active_jobs),*]; diff --git a/compiler/rustc_query_system/src/query/config.rs b/compiler/rustc_query_system/src/query/config.rs index 739e8e3a8f26c..66b04aa7b467b 100644 --- a/compiler/rustc_query_system/src/query/config.rs +++ b/compiler/rustc_query_system/src/query/config.rs @@ -6,6 +6,7 @@ use std::hash::Hash; use rustc_data_structures::fingerprint::Fingerprint; use rustc_span::ErrorGuaranteed; +use super::QueryStackFrameExtra; use crate::dep_graph::{DepKind, DepNode, DepNodeParams, SerializedDepNodeIndex}; use crate::ich::StableHashingContext; use crate::query::caches::QueryCache; @@ -26,7 +27,7 @@ pub trait QueryConfig: Copy { fn format_value(self) -> fn(&Self::Value) -> String; // Don't use this method to access query results, instead use the methods on TyCtxt - fn query_state<'a>(self, tcx: Qcx) -> &'a QueryState + fn query_state<'a>(self, tcx: Qcx) -> &'a QueryState where Qcx: 'a; @@ -56,7 +57,7 @@ pub trait QueryConfig: Copy { fn value_from_cycle_error( self, tcx: Qcx::DepContext, - cycle_error: &CycleError, + cycle_error: &CycleError, guar: ErrorGuaranteed, ) -> Self::Value; diff --git a/compiler/rustc_query_system/src/query/job.rs b/compiler/rustc_query_system/src/query/job.rs index 0431151c74c95..5810ce0cbe668 100644 --- a/compiler/rustc_query_system/src/query/job.rs +++ b/compiler/rustc_query_system/src/query/job.rs @@ -1,3 +1,4 @@ +use std::fmt::Debug; use std::hash::Hash; use std::io::Write; use std::iter; @@ -11,6 +12,7 @@ use rustc_hir::def::DefKind; use rustc_session::Session; use rustc_span::{DUMMY_SP, Span}; +use super::QueryStackFrameExtra; use crate::dep_graph::DepContext; use crate::error::CycleStack; use crate::query::plumbing::CycleError; @@ -18,45 +20,54 @@ use crate::query::{QueryContext, QueryStackFrame}; /// Represents a span and a query key. #[derive(Clone, Debug)] -pub struct QueryInfo { +pub struct QueryInfo { /// The span corresponding to the reason for which this query was required. pub span: Span, - pub query: QueryStackFrame, + pub query: QueryStackFrame, } -pub type QueryMap = FxHashMap; +impl QueryInfo { + pub(crate) fn lift>( + &self, + qcx: Qcx, + ) -> QueryInfo { + QueryInfo { span: self.span, query: self.query.lift(qcx) } + } +} + +pub type QueryMap = FxHashMap>; /// A value uniquely identifying an active query job. #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug)] pub struct QueryJobId(pub NonZero); impl QueryJobId { - fn query(self, map: &QueryMap) -> QueryStackFrame { + fn query(self, map: &QueryMap) -> QueryStackFrame { map.get(&self).unwrap().query.clone() } - fn span(self, map: &QueryMap) -> Span { + fn span(self, map: &QueryMap) -> Span { map.get(&self).unwrap().job.span } - fn parent(self, map: &QueryMap) -> Option { + fn parent(self, map: &QueryMap) -> Option { map.get(&self).unwrap().job.parent } - fn latch(self, map: &QueryMap) -> Option<&QueryLatch> { + fn latch(self, map: &QueryMap) -> Option<&QueryLatch> { map.get(&self).unwrap().job.latch.as_ref() } } #[derive(Clone, Debug)] -pub struct QueryJobInfo { - pub query: QueryStackFrame, - pub job: QueryJob, +pub struct QueryJobInfo { + pub query: QueryStackFrame, + pub job: QueryJob, } /// Represents an active query job. #[derive(Debug)] -pub struct QueryJob { +pub struct QueryJob { pub id: QueryJobId, /// The span corresponding to the reason for which this query was required. @@ -66,23 +77,23 @@ pub struct QueryJob { pub parent: Option, /// The latch that is used to wait on this job. - latch: Option, + latch: Option>, } -impl Clone for QueryJob { +impl Clone for QueryJob { fn clone(&self) -> Self { Self { id: self.id, span: self.span, parent: self.parent, latch: self.latch.clone() } } } -impl QueryJob { +impl QueryJob { /// Creates a new query job. #[inline] pub fn new(id: QueryJobId, span: Span, parent: Option) -> Self { QueryJob { id, span, parent, latch: None } } - pub(super) fn latch(&mut self) -> QueryLatch { + pub(super) fn latch(&mut self) -> QueryLatch { if self.latch.is_none() { self.latch = Some(QueryLatch::new()); } @@ -102,12 +113,12 @@ impl QueryJob { } impl QueryJobId { - pub(super) fn find_cycle_in_stack( + pub(super) fn find_cycle_in_stack( &self, - query_map: QueryMap, + query_map: QueryMap, current_job: &Option, span: Span, - ) -> CycleError { + ) -> CycleError { // Find the waitee amongst `current_job` parents let mut cycle = Vec::new(); let mut current_job = Option::clone(current_job); @@ -141,7 +152,7 @@ impl QueryJobId { #[cold] #[inline(never)] - pub fn find_dep_kind_root(&self, query_map: QueryMap) -> (QueryJobInfo, usize) { + pub fn find_dep_kind_root(&self, query_map: QueryMap) -> (QueryJobInfo, usize) { let mut depth = 1; let info = query_map.get(&self).unwrap(); let dep_kind = info.query.dep_kind; @@ -161,31 +172,31 @@ impl QueryJobId { } #[derive(Debug)] -struct QueryWaiter { +struct QueryWaiter { query: Option, condvar: Condvar, span: Span, - cycle: Mutex>, + cycle: Mutex>>, } #[derive(Debug)] -struct QueryLatchInfo { +struct QueryLatchInfo { complete: bool, - waiters: Vec>, + waiters: Vec>>, } #[derive(Debug)] -pub(super) struct QueryLatch { - info: Arc>, +pub(super) struct QueryLatch { + info: Arc>>, } -impl Clone for QueryLatch { +impl Clone for QueryLatch { fn clone(&self) -> Self { Self { info: Arc::clone(&self.info) } } } -impl QueryLatch { +impl QueryLatch { fn new() -> Self { QueryLatch { info: Arc::new(Mutex::new(QueryLatchInfo { complete: false, waiters: Vec::new() })), @@ -198,7 +209,7 @@ impl QueryLatch { qcx: impl QueryContext, query: Option, span: Span, - ) -> Result<(), CycleError> { + ) -> Result<(), CycleError> { let waiter = Arc::new(QueryWaiter { query, span, cycle: Mutex::new(None), condvar: Condvar::new() }); self.wait_on_inner(qcx, &waiter); @@ -213,7 +224,7 @@ impl QueryLatch { } /// Awaits the caller on this latch by blocking the current thread. - fn wait_on_inner(&self, qcx: impl QueryContext, waiter: &Arc) { + fn wait_on_inner(&self, qcx: impl QueryContext, waiter: &Arc>) { let mut info = self.info.lock(); if !info.complete { // We push the waiter on to the `waiters` list. It can be accessed inside @@ -249,7 +260,7 @@ impl QueryLatch { /// Removes a single waiter from the list of waiters. /// This is used to break query cycles. - fn extract_waiter(&self, waiter: usize) -> Arc { + fn extract_waiter(&self, waiter: usize) -> Arc> { let mut info = self.info.lock(); debug_assert!(!info.complete); // Remove the waiter from the list of waiters @@ -269,7 +280,11 @@ type Waiter = (QueryJobId, usize); /// For visits of resumable waiters it returns Some(Some(Waiter)) which has the /// required information to resume the waiter. /// If all `visit` calls returns None, this function also returns None. -fn visit_waiters(query_map: &QueryMap, query: QueryJobId, mut visit: F) -> Option> +fn visit_waiters( + query_map: &QueryMap, + query: QueryJobId, + mut visit: F, +) -> Option> where F: FnMut(Span, QueryJobId) -> Option>, { @@ -299,8 +314,8 @@ where /// `span` is the reason for the `query` to execute. This is initially DUMMY_SP. /// If a cycle is detected, this initial value is replaced with the span causing /// the cycle. -fn cycle_check( - query_map: &QueryMap, +fn cycle_check( + query_map: &QueryMap, query: QueryJobId, span: Span, stack: &mut Vec<(Span, QueryJobId)>, @@ -339,8 +354,8 @@ fn cycle_check( /// Finds out if there's a path to the compiler root (aka. code which isn't in a query) /// from `query` without going through any of the queries in `visited`. /// This is achieved with a depth first search. -fn connected_to_root( - query_map: &QueryMap, +fn connected_to_root( + query_map: &QueryMap, query: QueryJobId, visited: &mut FxHashSet, ) -> bool { @@ -361,7 +376,7 @@ fn connected_to_root( } // Deterministically pick an query from a list -fn pick_query<'a, T, F>(query_map: &QueryMap, queries: &'a [T], f: F) -> &'a T +fn pick_query<'a, I: Clone, T, F>(query_map: &QueryMap, queries: &'a [T], f: F) -> &'a T where F: Fn(&T) -> (Span, QueryJobId), { @@ -386,10 +401,10 @@ where /// the function return true. /// If a cycle was not found, the starting query is removed from `jobs` and /// the function returns false. -fn remove_cycle( - query_map: &QueryMap, +fn remove_cycle( + query_map: &QueryMap, jobs: &mut Vec, - wakelist: &mut Vec>, + wakelist: &mut Vec>>, ) -> bool { let mut visited = FxHashSet::default(); let mut stack = Vec::new(); @@ -490,7 +505,10 @@ fn remove_cycle( /// uses a query latch and then resuming that waiter. /// There may be multiple cycles involved in a deadlock, so this searches /// all active queries for cycles before finally resuming all the waiters at once. -pub fn break_query_cycles(query_map: QueryMap, registry: &rustc_thread_pool::Registry) { +pub fn break_query_cycles( + query_map: QueryMap, + registry: &rustc_thread_pool::Registry, +) { let mut wakelist = Vec::new(); // It is OK per the comments: // - https://github.com/rust-lang/rust/pull/131200#issuecomment-2798854932 @@ -541,7 +559,7 @@ pub fn report_cycle<'a>( ) -> Diag<'a> { assert!(!stack.is_empty()); - let span = stack[0].query.default_span(stack[1 % stack.len()].span); + let span = stack[0].query.info.default_span(stack[1 % stack.len()].span); let mut cycle_stack = Vec::new(); @@ -550,31 +568,31 @@ pub fn report_cycle<'a>( for i in 1..stack.len() { let query = &stack[i].query; - let span = query.default_span(stack[(i + 1) % stack.len()].span); - cycle_stack.push(CycleStack { span, desc: query.description.to_owned() }); + let span = query.info.default_span(stack[(i + 1) % stack.len()].span); + cycle_stack.push(CycleStack { span, desc: query.info.description.to_owned() }); } let mut cycle_usage = None; if let Some((span, ref query)) = *usage { cycle_usage = Some(crate::error::CycleUsage { - span: query.default_span(span), - usage: query.description.to_string(), + span: query.info.default_span(span), + usage: query.info.description.to_string(), }); } - let alias = if stack.iter().all(|entry| matches!(entry.query.def_kind, Some(DefKind::TyAlias))) - { - Some(crate::error::Alias::Ty) - } else if stack.iter().all(|entry| entry.query.def_kind == Some(DefKind::TraitAlias)) { - Some(crate::error::Alias::Trait) - } else { - None - }; + let alias = + if stack.iter().all(|entry| matches!(entry.query.info.def_kind, Some(DefKind::TyAlias))) { + Some(crate::error::Alias::Ty) + } else if stack.iter().all(|entry| entry.query.info.def_kind == Some(DefKind::TraitAlias)) { + Some(crate::error::Alias::Trait) + } else { + None + }; let cycle_diag = crate::error::Cycle { span, cycle_stack, - stack_bottom: stack[0].query.description.to_owned(), + stack_bottom: stack[0].query.info.description.to_owned(), alias, cycle_usage, stack_count, @@ -610,11 +628,12 @@ pub fn print_query_stack( let Some(query_info) = query_map.get(&query) else { break; }; + let query_extra = qcx.lift_query_info(&query_info.query.info); if Some(count_printed) < limit_frames || limit_frames.is_none() { // Only print to stderr as many stack frames as `num_frames` when present. dcx.struct_failure_note(format!( "#{} [{:?}] {}", - count_printed, query_info.query.dep_kind, query_info.query.description + count_printed, query_info.query.dep_kind, query_extra.description )) .with_span(query_info.job.span) .emit(); @@ -627,7 +646,7 @@ pub fn print_query_stack( "#{} [{}] {}", count_total, qcx.dep_context().dep_kind_vtable(query_info.query.dep_kind).name, - query_info.query.description + query_extra.description ); } diff --git a/compiler/rustc_query_system/src/query/mod.rs b/compiler/rustc_query_system/src/query/mod.rs index 796f41d41efa7..3ff980fa9bc5c 100644 --- a/compiler/rustc_query_system/src/query/mod.rs +++ b/compiler/rustc_query_system/src/query/mod.rs @@ -1,4 +1,10 @@ +use std::fmt::Debug; +use std::marker::PhantomData; +use std::mem::transmute; +use std::sync::Arc; + use rustc_data_structures::jobserver::Proxy; +use rustc_data_structures::sync::{DynSend, DynSync}; use rustc_errors::DiagInner; use rustc_hashes::Hash64; use rustc_hir::def::DefKind; @@ -36,31 +42,59 @@ pub enum CycleErrorHandling { /// /// This is mostly used in case of cycles for error reporting. #[derive(Clone, Debug)] -pub struct QueryStackFrame { - pub description: String, - span: Option, - pub def_id: Option, - pub def_kind: Option, - /// A def-id that is extracted from a `Ty` in a query key - pub def_id_for_ty_in_cycle: Option, +pub struct QueryStackFrame { + /// This field initially stores a `QueryStackDeferred` during collection, + /// but can later be changed to `QueryStackFrameExtra` containing concrete information + /// by calling `lift`. This is done so that collecting query does not need to invoke + /// queries, instead `lift` will call queries in a more appropriate location. + pub info: I, + pub dep_kind: DepKind, /// This hash is used to deterministically pick /// a query to remove cycles in the parallel compiler. hash: Hash64, + pub def_id: Option, + /// A def-id that is extracted from a `Ty` in a query key + pub def_id_for_ty_in_cycle: Option, } -impl QueryStackFrame { +impl QueryStackFrame { #[inline] pub fn new( - description: String, - span: Option, - def_id: Option, - def_kind: Option, + info: I, dep_kind: DepKind, + hash: impl FnOnce() -> Hash64, + def_id: Option, def_id_for_ty_in_cycle: Option, - hash: Hash64, ) -> Self { - Self { description, span, def_id, def_kind, def_id_for_ty_in_cycle, dep_kind, hash } + Self { info, def_id, dep_kind, hash: hash(), def_id_for_ty_in_cycle } + } + + fn lift>( + &self, + qcx: Qcx, + ) -> QueryStackFrame { + QueryStackFrame { + info: qcx.lift_query_info(&self.info), + dep_kind: self.dep_kind, + hash: self.hash, + def_id: self.def_id, + def_id_for_ty_in_cycle: self.def_id_for_ty_in_cycle, + } + } +} + +#[derive(Clone, Debug)] +pub struct QueryStackFrameExtra { + pub description: String, + span: Option, + pub def_kind: Option, +} + +impl QueryStackFrameExtra { + #[inline] + pub fn new(description: String, span: Option, def_kind: Option) -> Self { + Self { description, span, def_kind } } // FIXME(eddyb) Get more valid `Span`s on queries. @@ -73,6 +107,40 @@ impl QueryStackFrame { } } +/// Track a 'side effect' for a particular query. +/// This is used to hold a closure which can create `QueryStackFrameExtra`. +#[derive(Clone)] +pub struct QueryStackDeferred<'tcx> { + _dummy: PhantomData<&'tcx ()>, + + // `extract` may contain references to 'tcx, but we can't tell drop checking that it won't + // access it in the destructor. + extract: Arc QueryStackFrameExtra + DynSync + DynSend>, +} + +impl<'tcx> QueryStackDeferred<'tcx> { + pub fn new( + context: C, + extract: fn(C) -> QueryStackFrameExtra, + ) -> Self { + let extract: Arc QueryStackFrameExtra + DynSync + DynSend + 'tcx> = + Arc::new(move || extract(context)); + // SAFETY: The `extract` closure does not access 'tcx in its destructor as the only + // captured variable is `context` which is Copy and cannot have a destructor. + Self { _dummy: PhantomData, extract: unsafe { transmute(extract) } } + } + + pub fn extract(&self) -> QueryStackFrameExtra { + (self.extract)() + } +} + +impl<'tcx> Debug for QueryStackDeferred<'tcx> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("QueryStackDeferred") + } +} + /// Tracks 'side effects' for a particular query. /// This struct is saved to disk along with the query result, /// and loaded from disk if we mark the query as green. @@ -92,6 +160,8 @@ pub enum QuerySideEffect { } pub trait QueryContext: HasDepContext { + type QueryInfo: Clone; + /// Gets a jobserver reference which is used to release then acquire /// a token while waiting on a query. fn jobserver_proxy(&self) -> &Proxy; @@ -101,7 +171,12 @@ pub trait QueryContext: HasDepContext { /// Get the query information from the TLS context. fn current_query_job(self) -> Option; - fn collect_active_jobs(self, require_complete: bool) -> Result; + fn collect_active_jobs( + self, + require_complete: bool, + ) -> Result, QueryMap>; + + fn lift_query_info(self, info: &Self::QueryInfo) -> QueryStackFrameExtra; /// Load a side effect associated to the node in the previous session. fn load_side_effect( diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index 150ad238dad9f..5be4ee1452082 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -18,7 +18,7 @@ use rustc_errors::{Diag, FatalError, StashKey}; use rustc_span::{DUMMY_SP, Span}; use tracing::instrument; -use super::QueryConfig; +use super::{QueryConfig, QueryStackFrameExtra}; use crate::dep_graph::{DepContext, DepGraphData, DepNode, DepNodeIndex, DepNodeParams}; use crate::ich::StableHashingContext; use crate::query::caches::QueryCache; @@ -32,23 +32,23 @@ fn equivalent_key(k: &K) -> impl Fn(&(K, V)) -> bool + '_ { move |x| x.0 == *k } -pub struct QueryState { - active: Sharded>, +pub struct QueryState { + active: Sharded)>>, } /// Indicates the state of a query for a given key in a query map. -enum QueryResult { +enum QueryResult { /// An already executing query. The query job can be used to await for its completion. - Started(QueryJob), + Started(QueryJob), /// The query panicked. Queries trying to wait on this will raise a fatal error which will /// silently panic. Poisoned, } -impl QueryResult { +impl QueryResult { /// Unwraps the query job expecting that it has started. - fn expect_job(self) -> QueryJob { + fn expect_job(self) -> QueryJob { match self { Self::Started(job) => job, Self::Poisoned => { @@ -58,7 +58,7 @@ impl QueryResult { } } -impl QueryState +impl QueryState where K: Eq + Hash + Copy + Debug, { @@ -69,13 +69,13 @@ where pub fn collect_active_jobs( &self, qcx: Qcx, - make_query: fn(Qcx, K) -> QueryStackFrame, - jobs: &mut QueryMap, + make_query: fn(Qcx, K) -> QueryStackFrame, + jobs: &mut QueryMap, require_complete: bool, ) -> Option<()> { let mut active = Vec::new(); - let mut collect = |iter: LockGuard<'_, HashTable<(K, QueryResult)>>| { + let mut collect = |iter: LockGuard<'_, HashTable<(K, QueryResult)>>| { for (k, v) in iter.iter() { if let QueryResult::Started(ref job) = *v { active.push((*k, job.clone())); @@ -106,19 +106,19 @@ where } } -impl Default for QueryState { - fn default() -> QueryState { +impl Default for QueryState { + fn default() -> QueryState { QueryState { active: Default::default() } } } /// A type representing the responsibility to execute the job in the `job` field. /// This will poison the relevant query if dropped. -struct JobOwner<'tcx, K> +struct JobOwner<'tcx, K, I> where K: Eq + Hash + Copy, { - state: &'tcx QueryState, + state: &'tcx QueryState, key: K, } @@ -159,7 +159,7 @@ where } CycleErrorHandling::Stash => { let guar = if let Some(root) = cycle_error.cycle.first() - && let Some(span) = root.query.span + && let Some(span) = root.query.info.span { error.stash(span, StashKey::Cycle).unwrap() } else { @@ -170,7 +170,7 @@ where } } -impl<'tcx, K> JobOwner<'tcx, K> +impl<'tcx, K, I> JobOwner<'tcx, K, I> where K: Eq + Hash + Copy, { @@ -207,7 +207,7 @@ where } } -impl<'tcx, K> Drop for JobOwner<'tcx, K> +impl<'tcx, K, I> Drop for JobOwner<'tcx, K, I> where K: Eq + Hash + Copy, { @@ -235,10 +235,19 @@ where } #[derive(Clone, Debug)] -pub struct CycleError { +pub struct CycleError { /// The query and related span that uses the cycle. - pub usage: Option<(Span, QueryStackFrame)>, - pub cycle: Vec, + pub usage: Option<(Span, QueryStackFrame)>, + pub cycle: Vec>, +} + +impl CycleError { + fn lift>(&self, qcx: Qcx) -> CycleError { + CycleError { + usage: self.usage.as_ref().map(|(span, frame)| (*span, frame.lift(qcx))), + cycle: self.cycle.iter().map(|info| info.lift(qcx)).collect(), + } + } } /// Checks whether there is already a value for this key in the in-memory @@ -275,10 +284,10 @@ where { // Ensure there was no errors collecting all active jobs. // We need the complete map to ensure we find a cycle to break. - let query_map = qcx.collect_active_jobs(false).expect("failed to collect active queries"); + let query_map = qcx.collect_active_jobs(false).ok().expect("failed to collect active queries"); let error = try_execute.find_cycle_in_stack(query_map, &qcx.current_query_job(), span); - (mk_cycle(query, qcx, error), None) + (mk_cycle(query, qcx, error.lift(qcx)), None) } #[inline(always)] @@ -287,7 +296,7 @@ fn wait_for_query( qcx: Qcx, span: Span, key: Q::Key, - latch: QueryLatch, + latch: QueryLatch, current: Option, ) -> (Q::Value, Option) where @@ -327,7 +336,7 @@ where (v, Some(index)) } - Err(cycle) => (mk_cycle(query, qcx, cycle), None), + Err(cycle) => (mk_cycle(query, qcx, cycle.lift(qcx)), None), } } @@ -405,7 +414,7 @@ where fn execute_job( query: Q, qcx: Qcx, - state: &QueryState, + state: &QueryState, key: Q::Key, key_hash: u64, id: QueryJobId, diff --git a/tests/ui/query-system/query-cycle-printing-issue-151226.rs b/tests/ui/query-system/query-cycle-printing-issue-151226.rs new file mode 100644 index 0000000000000..9d0a20737c9fa --- /dev/null +++ b/tests/ui/query-system/query-cycle-printing-issue-151226.rs @@ -0,0 +1,8 @@ +struct A(std::sync::OnceLock); +//~^ ERROR recursive type `A` has infinite size +//~| ERROR cycle detected when computing layout of `A<()>` + +static B: A<()> = todo!(); +//~^ ERROR cycle occurred during layout computation + +fn main() {} diff --git a/tests/ui/query-system/query-cycle-printing-issue-151226.stderr b/tests/ui/query-system/query-cycle-printing-issue-151226.stderr new file mode 100644 index 0000000000000..7e574b5911a39 --- /dev/null +++ b/tests/ui/query-system/query-cycle-printing-issue-151226.stderr @@ -0,0 +1,36 @@ +error[E0072]: recursive type `A` has infinite size + --> $DIR/query-cycle-printing-issue-151226.rs:1:1 + | +LL | struct A(std::sync::OnceLock); + | ^^^^^^^^^^^ ------------------------- recursive without indirection + | +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to break the cycle + | +LL | struct A(Box>); + | ++++ + + +error[E0391]: cycle detected when computing layout of `A<()>` + | + = note: ...which requires computing layout of `std::sync::once_lock::OnceLock>`... + = note: ...which requires computing layout of `core::cell::UnsafeCell>>`... + = note: ...which requires computing layout of `core::mem::maybe_uninit::MaybeUninit>`... + = note: ...which requires computing layout of `core::mem::manually_drop::ManuallyDrop>`... + = note: ...which requires computing layout of `core::mem::maybe_dangling::MaybeDangling>`... + = note: ...which again requires computing layout of `A<()>`, completing the cycle +note: cycle used when checking that `B` is well-formed + --> $DIR/query-cycle-printing-issue-151226.rs:5:1 + | +LL | static B: A<()> = todo!(); + | ^^^^^^^^^^^^^^^ + = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information + +error[E0080]: a cycle occurred during layout computation + --> $DIR/query-cycle-printing-issue-151226.rs:5:1 + | +LL | static B: A<()> = todo!(); + | ^^^^^^^^^^^^^^^ evaluation of `B` failed here + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0072, E0080, E0391. +For more information about an error, try `rustc --explain E0072`. diff --git a/tests/ui/query-system/query-cycle-printing-issue-151358.rs b/tests/ui/query-system/query-cycle-printing-issue-151358.rs new file mode 100644 index 0000000000000..04d8664420be8 --- /dev/null +++ b/tests/ui/query-system/query-cycle-printing-issue-151358.rs @@ -0,0 +1,7 @@ +//~ ERROR: cycle detected when looking up span for `Default` +trait Default {} +use std::num::NonZero; +fn main() { + NonZero(); + format!("{}", 0); +} diff --git a/tests/ui/query-system/query-cycle-printing-issue-151358.stderr b/tests/ui/query-system/query-cycle-printing-issue-151358.stderr new file mode 100644 index 0000000000000..9c1d7b1de33a5 --- /dev/null +++ b/tests/ui/query-system/query-cycle-printing-issue-151358.stderr @@ -0,0 +1,9 @@ +error[E0391]: cycle detected when looking up span for `Default` + | + = note: ...which immediately requires looking up span for `Default` again + = note: cycle used when perform lints prior to AST lowering + = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0391`. diff --git a/tests/ui/resolve/query-cycle-issue-124901.rs b/tests/ui/resolve/query-cycle-issue-124901.rs index ccaee0e6bc6fd..6cb1e58b6258f 100644 --- a/tests/ui/resolve/query-cycle-issue-124901.rs +++ b/tests/ui/resolve/query-cycle-issue-124901.rs @@ -1,4 +1,4 @@ -//~ ERROR: cycle detected when getting HIR ID of `Default` +//~ ERROR: cycle detected when looking up span for `Default` trait Default { type Id; diff --git a/tests/ui/resolve/query-cycle-issue-124901.stderr b/tests/ui/resolve/query-cycle-issue-124901.stderr index 3679925c6db42..9c1d7b1de33a5 100644 --- a/tests/ui/resolve/query-cycle-issue-124901.stderr +++ b/tests/ui/resolve/query-cycle-issue-124901.stderr @@ -1,10 +1,7 @@ -error[E0391]: cycle detected when getting HIR ID of `Default` +error[E0391]: cycle detected when looking up span for `Default` | - = note: ...which requires getting the crate HIR... - = note: ...which requires perform lints prior to AST lowering... - = note: ...which requires looking up span for `Default`... - = note: ...which again requires getting HIR ID of `Default`, completing the cycle - = note: cycle used when getting the resolver for lowering + = note: ...which immediately requires looking up span for `Default` again + = note: cycle used when perform lints prior to AST lowering = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information error: aborting due to 1 previous error