From d51e703534c49b5b658a954fe6d387c33ba0c5e3 Mon Sep 17 00:00:00 2001 From: CKingX Date: Thu, 8 Feb 2024 17:15:11 -0800 Subject: [PATCH 01/37] As Windows 10 requires certain features like CMPXCHG16B and a few others and Rust plans to set Windows 10 as the minimum supported OS for target x86_64-pc-windows-msvc, I have added the cmpxchg16b and sse3 feature (as CPUs that meet the Windows 10 64-bit requirement also support SSE3. See https://walbourn.github.io/directxmath-sse3-and-ssse3/ ) --- compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs index 357261073a8..e3db187e7eb 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs @@ -3,6 +3,7 @@ use crate::spec::{base, SanitizerSet, Target}; pub fn target() -> Target { let mut base = base::windows_msvc::opts(); base.cpu = "x86-64".into(); + base.features = "+cmpxchg16b,+sse3".into(); base.plt_by_default = false; base.max_atomic_width = Some(64); base.supported_sanitizers = SanitizerSet::ADDRESS; From d6766e2bc8eb6cfd695f2c9e931c7a712da474ff Mon Sep 17 00:00:00 2001 From: CKingX Date: Fri, 9 Feb 2024 07:59:38 -0800 Subject: [PATCH 02/37] Update x86_64_pc_windows_msvc.rs Fixed a bug where adding CMPXCHG16B would fail due to different names in Rustc and LLVM --- .../rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs index e3db187e7eb..f7dc60df0f4 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs @@ -3,7 +3,7 @@ use crate::spec::{base, SanitizerSet, Target}; pub fn target() -> Target { let mut base = base::windows_msvc::opts(); base.cpu = "x86-64".into(); - base.features = "+cmpxchg16b,+sse3".into(); + base.features = "+cx16,+sse3".into(); base.plt_by_default = false; base.max_atomic_width = Some(64); base.supported_sanitizers = SanitizerSet::ADDRESS; From fcb06f7ca2a20d030fdd5f9c53b684557f17c00c Mon Sep 17 00:00:00 2001 From: CKingX Date: Fri, 9 Feb 2024 09:19:59 -0800 Subject: [PATCH 03/37] Update x86_64_pc_windows_msvc.rs As CMPXCHG16B is supported, I updated the max atomic width to 128-bits from 64-bits --- .../rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs index f7dc60df0f4..e72e97db775 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs @@ -5,7 +5,7 @@ pub fn target() -> Target { base.cpu = "x86-64".into(); base.features = "+cx16,+sse3".into(); base.plt_by_default = false; - base.max_atomic_width = Some(64); + base.max_atomic_width = Some(128); base.supported_sanitizers = SanitizerSet::ADDRESS; Target { From abeac8fbc1cac4bad60094c64179366a802ec949 Mon Sep 17 00:00:00 2001 From: CKingX Date: Fri, 9 Feb 2024 12:25:17 -0800 Subject: [PATCH 04/37] Update x86_64_uwp_windows_gnu.rs Updated x86_64-uwp-windows-gnu to use CMPXCHG16B and SSE3 --- .../rustc_target/src/spec/targets/x86_64_uwp_windows_gnu.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_gnu.rs b/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_gnu.rs index c2981ddbad6..ee95c67496d 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_gnu.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_gnu.rs @@ -3,6 +3,7 @@ use crate::spec::{base, Cc, LinkerFlavor, Lld, Target}; pub fn target() -> Target { let mut base = base::windows_uwp_gnu::opts(); base.cpu = "x86-64".into(); + base.features = "+cx16,+sse3".into(); base.plt_by_default = false; // Use high-entropy 64 bit address space for ASLR base.add_pre_link_args( @@ -10,7 +11,7 @@ pub fn target() -> Target { &["-m", "i386pep", "--high-entropy-va"], ); base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m64", "-Wl,--high-entropy-va"]); - base.max_atomic_width = Some(64); + base.max_atomic_width = Some(128); Target { llvm_target: "x86_64-pc-windows-gnu".into(), From 1c6dda72774d19e79d14b7a51c047414b3126b7b Mon Sep 17 00:00:00 2001 From: Chiragroop Date: Fri, 9 Feb 2024 12:54:38 -0800 Subject: [PATCH 05/37] Possibly removed merge policy --- .../rustc_target/src/spec/targets/x86_64_pc_windows_gnu.rs | 3 ++- .../rustc_target/src/spec/targets/x86_64_pc_windows_gnullvm.rs | 3 ++- .../rustc_target/src/spec/targets/x86_64_uwp_windows_msvc.rs | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnu.rs b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnu.rs index 9e964d248bf..d16e5905eac 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnu.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnu.rs @@ -3,6 +3,7 @@ use crate::spec::{base, Cc, LinkerFlavor, Lld, Target}; pub fn target() -> Target { let mut base = base::windows_gnu::opts(); base.cpu = "x86-64".into(); + base.features = "+cx16,+sse3".into(); base.plt_by_default = false; // Use high-entropy 64 bit address space for ASLR base.add_pre_link_args( @@ -10,7 +11,7 @@ pub fn target() -> Target { &["-m", "i386pep", "--high-entropy-va"], ); base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m64", "-Wl,--high-entropy-va"]); - base.max_atomic_width = Some(64); + base.max_atomic_width = Some(128); base.linker = Some("x86_64-w64-mingw32-gcc".into()); Target { diff --git a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnullvm.rs b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnullvm.rs index 1facf9450cd..ea7f7df8120 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnullvm.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnullvm.rs @@ -3,9 +3,10 @@ use crate::spec::{base, Cc, LinkerFlavor, Lld, Target}; pub fn target() -> Target { let mut base = base::windows_gnullvm::opts(); base.cpu = "x86-64".into(); + base.features = "+cx16,+sse3".into(); base.plt_by_default = false; base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m64"]); - base.max_atomic_width = Some(64); + base.max_atomic_width = Some(128); base.linker = Some("x86_64-w64-mingw32-clang".into()); Target { diff --git a/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_msvc.rs b/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_msvc.rs index 3f0702c7ad6..acdf969375e 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_msvc.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_msvc.rs @@ -3,8 +3,9 @@ use crate::spec::{base, Target}; pub fn target() -> Target { let mut base = base::windows_uwp_msvc::opts(); base.cpu = "x86-64".into(); + base.features = "+cx16,+sse3".into(); base.plt_by_default = false; - base.max_atomic_width = Some(64); + base.max_atomic_width = Some(128); Target { llvm_target: "x86_64-pc-windows-msvc".into(), From 376c7b98921b427699b99fc36496cb81ed40c946 Mon Sep 17 00:00:00 2001 From: CKingX Date: Tue, 13 Feb 2024 12:08:30 -0800 Subject: [PATCH 06/37] Added sahf feature to windows targets --- compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnu.rs | 2 +- .../rustc_target/src/spec/targets/x86_64_pc_windows_gnullvm.rs | 2 +- .../rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs | 2 +- .../rustc_target/src/spec/targets/x86_64_uwp_windows_gnu.rs | 2 +- .../rustc_target/src/spec/targets/x86_64_uwp_windows_msvc.rs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnu.rs b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnu.rs index d16e5905eac..d6b44e7cfd4 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnu.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnu.rs @@ -3,7 +3,7 @@ use crate::spec::{base, Cc, LinkerFlavor, Lld, Target}; pub fn target() -> Target { let mut base = base::windows_gnu::opts(); base.cpu = "x86-64".into(); - base.features = "+cx16,+sse3".into(); + base.features = "+cx16,+sse3,+sahf".into(); base.plt_by_default = false; // Use high-entropy 64 bit address space for ASLR base.add_pre_link_args( diff --git a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnullvm.rs b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnullvm.rs index ea7f7df8120..b84a9a5a8a1 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnullvm.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_gnullvm.rs @@ -3,7 +3,7 @@ use crate::spec::{base, Cc, LinkerFlavor, Lld, Target}; pub fn target() -> Target { let mut base = base::windows_gnullvm::opts(); base.cpu = "x86-64".into(); - base.features = "+cx16,+sse3".into(); + base.features = "+cx16,+sse3,+sahf".into(); base.plt_by_default = false; base.add_pre_link_args(LinkerFlavor::Gnu(Cc::Yes, Lld::No), &["-m64"]); base.max_atomic_width = Some(128); diff --git a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs index e72e97db775..51807bdba5d 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_pc_windows_msvc.rs @@ -3,7 +3,7 @@ use crate::spec::{base, SanitizerSet, Target}; pub fn target() -> Target { let mut base = base::windows_msvc::opts(); base.cpu = "x86-64".into(); - base.features = "+cx16,+sse3".into(); + base.features = "+cx16,+sse3,+sahf".into(); base.plt_by_default = false; base.max_atomic_width = Some(128); base.supported_sanitizers = SanitizerSet::ADDRESS; diff --git a/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_gnu.rs b/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_gnu.rs index ee95c67496d..b37d33601e6 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_gnu.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_gnu.rs @@ -3,7 +3,7 @@ use crate::spec::{base, Cc, LinkerFlavor, Lld, Target}; pub fn target() -> Target { let mut base = base::windows_uwp_gnu::opts(); base.cpu = "x86-64".into(); - base.features = "+cx16,+sse3".into(); + base.features = "+cx16,+sse3,+sahf".into(); base.plt_by_default = false; // Use high-entropy 64 bit address space for ASLR base.add_pre_link_args( diff --git a/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_msvc.rs b/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_msvc.rs index acdf969375e..fe2b4c8e92d 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_msvc.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_uwp_windows_msvc.rs @@ -3,7 +3,7 @@ use crate::spec::{base, Target}; pub fn target() -> Target { let mut base = base::windows_uwp_msvc::opts(); base.cpu = "x86-64".into(); - base.features = "+cx16,+sse3".into(); + base.features = "+cx16,+sse3,+sahf".into(); base.plt_by_default = false; base.max_atomic_width = Some(128); From 2d25c3b3697728ea42eb26f79e6f80907470ca22 Mon Sep 17 00:00:00 2001 From: CKingX Date: Mon, 19 Feb 2024 21:59:13 -0800 Subject: [PATCH 07/37] Updated test to account for added previous features (thanks erikdesjardins!) --- tests/codegen/sse42-implies-crc32.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/codegen/sse42-implies-crc32.rs b/tests/codegen/sse42-implies-crc32.rs index 56079d32a8d..f84d390b4e4 100644 --- a/tests/codegen/sse42-implies-crc32.rs +++ b/tests/codegen/sse42-implies-crc32.rs @@ -12,4 +12,4 @@ pub unsafe fn crc32sse(v: u8) -> u32 { _mm_crc32_u8(out, v) } -// CHECK: attributes #0 {{.*"target-features"="\+sse4.2,\+crc32"}} +// CHECK: attributes #0 {{.*"target-features"=".*\+sse4.2,\+crc32"}} From 2fc091f510ce3b41de0c3c076b587badb687c458 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sat, 24 Feb 2024 13:56:14 -0300 Subject: [PATCH 08/37] Use volatile to make `p_thread_callback` used --- .../src/sys/pal/windows/thread_local_key.rs | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/library/std/src/sys/pal/windows/thread_local_key.rs b/library/std/src/sys/pal/windows/thread_local_key.rs index 5eee4a9667b..81741cc5f78 100644 --- a/library/std/src/sys/pal/windows/thread_local_key.rs +++ b/library/std/src/sys/pal/windows/thread_local_key.rs @@ -1,7 +1,7 @@ use crate::cell::UnsafeCell; use crate::ptr; use crate::sync::atomic::{ - AtomicBool, AtomicPtr, AtomicU32, + AtomicPtr, AtomicU32, Ordering::{AcqRel, Acquire, Relaxed, Release}, }; use crate::sys::c; @@ -9,10 +9,6 @@ use crate::sys::c; #[cfg(test)] mod tests; -/// An optimization hint. The compiler is often smart enough to know if an atomic -/// is never set and can remove dead code based on that fact. -static HAS_DTORS: AtomicBool = AtomicBool::new(false); - // Using a per-thread list avoids the problems in synchronizing global state. #[thread_local] #[cfg(target_thread_local)] @@ -24,12 +20,11 @@ static DESTRUCTORS: crate::cell::RefCell dtors.push((t, dtor)), Err(_) => rtabort!("global allocator may not use TLS"), } - - HAS_DTORS.store(true, Relaxed); } #[inline(never)] // See comment above @@ -130,6 +125,7 @@ impl StaticKey { #[cold] unsafe fn init(&'static self) -> Key { if self.dtor.is_some() { + dtors_used(); let mut pending = c::FALSE; let r = c::InitOnceBeginInitialize(self.once.get(), 0, &mut pending, ptr::null_mut()); assert_eq!(r, c::TRUE); @@ -215,7 +211,6 @@ unsafe fn register_dtor(key: &'static StaticKey) { Err(new) => head = new, } } - HAS_DTORS.store(true, Release); } // ------------------------------------------------------------------------- @@ -281,17 +276,16 @@ unsafe fn register_dtor(key: &'static StaticKey) { // the address of the symbol to ensure it sticks around. #[link_section = ".CRT$XLB"] -#[allow(dead_code, unused_variables)] -#[used] // we don't want LLVM eliminating this symbol for any reason, and -// when the symbol makes it to the linker the linker will take over pub static p_thread_callback: unsafe extern "system" fn(c::LPVOID, c::DWORD, c::LPVOID) = on_tls_callback; -#[allow(dead_code, unused_variables)] -unsafe extern "system" fn on_tls_callback(h: c::LPVOID, dwReason: c::DWORD, pv: c::LPVOID) { - if !HAS_DTORS.load(Acquire) { - return; - } +fn dtors_used() { + // we don't want LLVM eliminating p_thread_callback when destructors are used. + // when the symbol makes it to the linker the linker will take over + unsafe { crate::intrinsics::volatile_load(&p_thread_callback) }; +} + +unsafe extern "system" fn on_tls_callback(_h: c::LPVOID, dwReason: c::DWORD, _pv: c::LPVOID) { if dwReason == c::DLL_THREAD_DETACH || dwReason == c::DLL_PROCESS_DETACH { #[cfg(not(target_thread_local))] run_dtors(); @@ -313,7 +307,7 @@ unsafe extern "system" fn on_tls_callback(h: c::LPVOID, dwReason: c::DWORD, pv: unsafe fn reference_tls_used() {} } -#[allow(dead_code)] // actually called below +#[cfg(not(target_thread_local))] unsafe fn run_dtors() { for _ in 0..5 { let mut any_run = false; From ad4c4f4654bf78bb51638e58a9c418afcdd6ee15 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sat, 24 Feb 2024 14:09:22 -0300 Subject: [PATCH 09/37] Remove _tls_used trickery unless needed --- library/std/src/sys/pal/windows/thread_local_key.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/library/std/src/sys/pal/windows/thread_local_key.rs b/library/std/src/sys/pal/windows/thread_local_key.rs index 81741cc5f78..1d9cb316a45 100644 --- a/library/std/src/sys/pal/windows/thread_local_key.rs +++ b/library/std/src/sys/pal/windows/thread_local_key.rs @@ -295,16 +295,13 @@ unsafe extern "system" fn on_tls_callback(_h: c::LPVOID, dwReason: c::DWORD, _pv // See comments above for what this is doing. Note that we don't need this // trickery on GNU windows, just on MSVC. - reference_tls_used(); - #[cfg(target_env = "msvc")] - unsafe fn reference_tls_used() { + #[cfg(all(target_env = "msvc", not(target_thread_local)))] + { extern "C" { static _tls_used: u8; } crate::intrinsics::volatile_load(&_tls_used); } - #[cfg(not(target_env = "msvc"))] - unsafe fn reference_tls_used() {} } #[cfg(not(target_thread_local))] From 205319d962b3467e44927150faad462d7313fd64 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 20 Feb 2024 05:19:11 +0100 Subject: [PATCH 10/37] Skip unnecessary comparison with half-open ranges --- compiler/rustc_middle/src/thir.rs | 16 ------ .../rustc_mir_build/src/build/matches/test.rs | 54 +++++++++++-------- 2 files changed, 31 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index e1e5d68148f..7ffa88afa15 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -958,22 +958,6 @@ impl<'tcx> PatRangeBoundary<'tcx> { Self::NegInfinity | Self::PosInfinity => None, } } - #[inline] - pub fn to_const(self, ty: Ty<'tcx>, tcx: TyCtxt<'tcx>) -> mir::Const<'tcx> { - match self { - Self::Finite(value) => value, - Self::NegInfinity => { - // Unwrap is ok because the type is known to be numeric. - let c = ty.numeric_min_val(tcx).unwrap(); - mir::Const::from_ty_const(c, tcx) - } - Self::PosInfinity => { - // Unwrap is ok because the type is known to be numeric. - let c = ty.numeric_max_val(tcx).unwrap(); - mir::Const::from_ty_const(c, tcx) - } - } - } pub fn eval_bits(self, ty: Ty<'tcx>, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> u128 { match self { Self::Finite(value) => value.eval_bits(tcx, param_env), diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index 1c97de58863..1b6994966d1 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -291,33 +291,41 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } TestKind::Range(ref range) => { - let lower_bound_success = self.cfg.start_new_block(); - - // Test `val` by computing `lo <= val && val <= hi`, using primitive comparisons. - // FIXME: skip useless comparison when the range is half-open. - let lo = range.lo.to_const(range.ty, self.tcx); - let hi = range.hi.to_const(range.ty, self.tcx); - let lo = self.literal_operand(test.span, lo); - let hi = self.literal_operand(test.span, hi); - let val = Operand::Copy(place); - let [success, fail] = *target_blocks else { bug!("`TestKind::Range` should have two target blocks"); }; - self.compare( - block, - lower_bound_success, - fail, - source_info, - BinOp::Le, - lo, - val.clone(), - ); - let op = match range.end { - RangeEnd::Included => BinOp::Le, - RangeEnd::Excluded => BinOp::Lt, + // Test `val` by computing `lo <= val && val <= hi`, using primitive comparisons. + let val = Operand::Copy(place); + + let intermediate_block = if !range.lo.is_finite() { + block + } else if !range.hi.is_finite() { + success + } else { + self.cfg.start_new_block() }; - self.compare(lower_bound_success, success, fail, source_info, op, val, hi); + + if let Some(lo) = range.lo.as_finite() { + let lo = self.literal_operand(test.span, lo); + self.compare( + block, + intermediate_block, + fail, + source_info, + BinOp::Le, + lo, + val.clone(), + ); + }; + + if let Some(hi) = range.hi.as_finite() { + let hi = self.literal_operand(test.span, hi); + let op = match range.end { + RangeEnd::Included => BinOp::Le, + RangeEnd::Excluded => BinOp::Lt, + }; + self.compare(intermediate_block, success, fail, source_info, op, val, hi); + } } TestKind::Len { len, op } => { From 7c6960e289c627bc5533c03769ff45b03a141be9 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 27 Feb 2024 17:23:06 +0100 Subject: [PATCH 11/37] Document invariant in `thir::PatRange` --- compiler/rustc_middle/src/thir.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index 7ffa88afa15..ee11a75529b 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -815,7 +815,9 @@ pub enum PatKind<'tcx> { /// The boundaries must be of the same type and that type must be numeric. #[derive(Clone, Debug, PartialEq, HashStable, TypeVisitable)] pub struct PatRange<'tcx> { + /// Must not be `PosInfinity`. pub lo: PatRangeBoundary<'tcx>, + /// Must not be `NegInfinity`. pub hi: PatRangeBoundary<'tcx>, #[type_visitable(ignore)] pub end: RangeEnd, From be01e28dceb5e8e32bb1c97f3be5c5488eed8f4f Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 6 Feb 2024 02:35:59 +0100 Subject: [PATCH 12/37] Push down the decision to skip fields --- compiler/rustc_pattern_analysis/src/rustc.rs | 38 +++++++++++--------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 5b62731e202..1b33ef5472e 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -165,13 +165,14 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { &self, ty: RevealedTy<'tcx>, variant: &'tcx VariantDef, - ) -> impl Iterator)> + Captures<'p> + Captures<'_> { + ) -> impl Iterator, bool)> + Captures<'p> + Captures<'_> + { let cx = self; let ty::Adt(adt, args) = ty.kind() else { bug!() }; - // Whether we must not match the fields of this variant exhaustively. + // Whether we must avoid matching the fields of this variant exhaustively. let is_non_exhaustive = variant.is_field_list_non_exhaustive() && !adt.did().is_local(); - variant.fields.iter().enumerate().filter_map(move |(i, field)| { + variant.fields.iter().enumerate().map(move |(i, field)| { let ty = field.ty(cx.tcx, args); // `field.ty()` doesn't normalize after instantiating. let ty = cx.tcx.normalize_erasing_regions(cx.param_env, ty); @@ -180,12 +181,9 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { || cx.tcx.features().min_exhaustive_patterns) && cx.is_uninhabited(ty); - if is_uninhabited && (!is_visible || is_non_exhaustive) { - None - } else { - let ty = cx.reveal_opaque_ty(ty); - Some((FieldIdx::new(i), ty)) - } + let skip = is_uninhabited && (!is_visible || is_non_exhaustive); + let ty = cx.reveal_opaque_ty(ty); + (FieldIdx::new(i), ty, skip) }) } @@ -229,7 +227,10 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { } else { let variant = &adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt)); - let tys = cx.list_variant_nonhidden_fields(ty, variant).map(|(_, ty)| ty); + let tys = cx + .list_variant_nonhidden_fields(ty, variant) + .filter(|(_, _, skip)| !skip) + .map(|(_, ty, _)| ty); cx.dropless_arena.alloc_from_iter(tys) } } @@ -276,7 +277,9 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { } else { let variant = &adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt)); - self.list_variant_nonhidden_fields(ty, variant).count() + self.list_variant_nonhidden_fields(ty, variant) + .filter(|(_, _, skip)| !skip) + .count() } } _ => bug!("Unexpected type for constructor `{ctor:?}`: {ty:?}"), @@ -523,12 +526,14 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { // For each field in the variant, we store the relevant index into `self.fields` if any. let mut field_id_to_id: Vec> = (0..variant.fields.len()).map(|_| None).collect(); - let tys = cx.list_variant_nonhidden_fields(ty, variant).enumerate().map( - |(i, (field, ty))| { + let tys = cx + .list_variant_nonhidden_fields(ty, variant) + .filter(|(_, _, skip)| !skip) + .enumerate() + .map(|(i, (field, ty, _))| { field_id_to_id[field.index()] = Some(i); ty - }, - ); + }); fields = tys.map(|ty| DeconstructedPat::wildcard(ty)).collect(); for pat in subpatterns { if let Some(i) = field_id_to_id[pat.field.index()] { @@ -778,8 +783,9 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { let variant = &adt_def.variant(variant_index); let subpatterns = cx .list_variant_nonhidden_fields(*pat.ty(), variant) + .filter(|(_, _, skip)| !skip) .zip(subpatterns) - .map(|((field, _ty), pattern)| FieldPat { field, pattern }) + .map(|((field, _ty, _), pattern)| FieldPat { field, pattern }) .collect(); if adt_def.is_enum() { From ab06037269da9c5fc83083fb7d9d9638294e3d63 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 6 Feb 2024 02:44:48 +0100 Subject: [PATCH 13/37] Push the decision to skip fields further down --- compiler/rustc_pattern_analysis/src/lib.rs | 10 +++++++--- compiler/rustc_pattern_analysis/src/pat.rs | 8 ++++++-- compiler/rustc_pattern_analysis/src/rustc.rs | 16 +++++++++------- .../rustc_pattern_analysis/src/usefulness.rs | 6 ++++-- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index 164dc36b679..0b4bc77a976 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -82,6 +82,10 @@ use crate::usefulness::{compute_match_usefulness, ValidityConstraint}; pub trait Captures<'a> {} impl<'a, T: ?Sized> Captures<'a> for T {} +/// `bool` newtype that indicates whether we should skip this field during analysis. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub struct SkipField(pub bool); + /// Context that provides type information about constructors. /// /// Most of the crate is parameterized on a type that implements this trait. @@ -105,13 +109,13 @@ pub trait TypeCx: Sized + fmt::Debug { /// The number of fields for this constructor. fn ctor_arity(&self, ctor: &Constructor, ty: &Self::Ty) -> usize; - /// The types of the fields for this constructor. The result must have a length of - /// `ctor_arity()`. + /// The types of the fields for this constructor. The result must contain `ctor_arity()`-many + /// fields that are not skipped. fn ctor_sub_tys<'a>( &'a self, ctor: &'a Constructor, ty: &'a Self::Ty, - ) -> impl Iterator + ExactSizeIterator + Captures<'a>; + ) -> impl Iterator + ExactSizeIterator + Captures<'a>; /// The set of all the constructors for `ty`. /// diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index d9b2b31643d..3e89a894419 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -5,7 +5,7 @@ use std::fmt; use smallvec::{smallvec, SmallVec}; use crate::constructor::{Constructor, Slice, SliceKind}; -use crate::TypeCx; +use crate::{SkipField, TypeCx}; use self::Constructor::*; @@ -300,7 +300,11 @@ impl WitnessPat { /// For example, if `ctor` is a `Constructor::Variant` for `Option::Some`, we get the pattern /// `Some(_)`. pub(crate) fn wild_from_ctor(cx: &Cx, ctor: Constructor, ty: Cx::Ty) -> Self { - let fields = cx.ctor_sub_tys(&ctor, &ty).map(|ty| Self::wildcard(ty)).collect(); + let fields = cx + .ctor_sub_tys(&ctor, &ty) + .filter(|(_, SkipField(skip))| !skip) + .map(|(ty, _)| Self::wildcard(ty)) + .collect(); Self::new(ctor, fields, ty) } diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 1b33ef5472e..8d55dcf2056 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -18,7 +18,7 @@ use rustc_target::abi::{FieldIdx, Integer, VariantIdx, FIRST_VARIANT}; use crate::constructor::{ IntRange, MaybeInfiniteInt, OpaqueId, RangeEnd, Slice, SliceKind, VariantVisibility, }; -use crate::{errors, Captures, TypeCx}; +use crate::{errors, Captures, SkipField, TypeCx}; use crate::constructor::Constructor::*; @@ -208,12 +208,15 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { &'a self, ctor: &'a Constructor<'p, 'tcx>, ty: RevealedTy<'tcx>, - ) -> impl Iterator> + ExactSizeIterator + Captures<'a> { + ) -> impl Iterator, SkipField)> + ExactSizeIterator + Captures<'a> + { fn reveal_and_alloc<'a, 'tcx>( cx: &'a RustcMatchCheckCtxt<'_, 'tcx>, iter: impl Iterator>, - ) -> &'a [RevealedTy<'tcx>] { - cx.dropless_arena.alloc_from_iter(iter.map(|ty| cx.reveal_opaque_ty(ty))) + ) -> &'a [(RevealedTy<'tcx>, SkipField)] { + cx.dropless_arena.alloc_from_iter( + iter.map(|ty| cx.reveal_opaque_ty(ty)).map(|ty| (ty, SkipField(false))), + ) } let cx = self; let slice = match ctor { @@ -229,8 +232,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { &adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt)); let tys = cx .list_variant_nonhidden_fields(ty, variant) - .filter(|(_, _, skip)| !skip) - .map(|(_, ty, _)| ty); + .map(|(_, ty, skip)| (ty, SkipField(skip))); cx.dropless_arena.alloc_from_iter(tys) } } @@ -872,7 +874,7 @@ impl<'p, 'tcx: 'p> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> { &'a self, ctor: &'a crate::constructor::Constructor, ty: &'a Self::Ty, - ) -> impl Iterator + ExactSizeIterator + Captures<'a> { + ) -> impl Iterator + ExactSizeIterator + Captures<'a> { self.ctor_sub_tys(ctor, *ty) } fn ctors_for_ty( diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index f672051be5a..ec9f3bb0db9 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -716,7 +716,7 @@ use std::fmt; use crate::constructor::{Constructor, ConstructorSet, IntRange}; use crate::pat::{DeconstructedPat, PatId, PatOrWild, WitnessPat}; -use crate::{Captures, MatchArm, TypeCx}; +use crate::{Captures, MatchArm, SkipField, TypeCx}; use self::ValidityConstraint::*; @@ -833,7 +833,9 @@ impl PlaceInfo { ) -> impl Iterator + ExactSizeIterator + Captures<'a> { let ctor_sub_tys = cx.ctor_sub_tys(ctor, &self.ty); let ctor_sub_validity = self.validity.specialize(ctor); - ctor_sub_tys.map(move |ty| PlaceInfo { + // Collect to keep the `ExactSizeIterator` bound. This is a temporary measure. + let tmp: Vec<_> = ctor_sub_tys.filter(|(_, SkipField(skip))| !skip).collect(); + tmp.into_iter().map(move |(ty, _)| PlaceInfo { ty, validity: ctor_sub_validity, is_scrutinee: false, From 4f7f06777bf67212aa960017ced0b911ab54bbf8 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 6 Feb 2024 02:53:37 +0100 Subject: [PATCH 14/37] Add special `Skip` constructor --- .../rustc_pattern_analysis/src/constructor.rs | 7 +++++ compiler/rustc_pattern_analysis/src/pat.rs | 4 ++- compiler/rustc_pattern_analysis/src/rustc.rs | 26 ++++--------------- 3 files changed, 15 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/constructor.rs b/compiler/rustc_pattern_analysis/src/constructor.rs index 483986969d1..26b0f5fc45d 100644 --- a/compiler/rustc_pattern_analysis/src/constructor.rs +++ b/compiler/rustc_pattern_analysis/src/constructor.rs @@ -688,6 +688,10 @@ pub enum Constructor { /// Fake extra constructor for constructors that are not seen in the matrix, as explained at the /// top of the file. Missing, + /// Fake extra constructor that indicates that we should skip the column entirely. This is used + /// when a private field is empty, so that we don't observe its emptiness. Only used for + /// specialization. + Skip, } impl Clone for Constructor { @@ -709,6 +713,7 @@ impl Clone for Constructor { Constructor::NonExhaustive => Constructor::NonExhaustive, Constructor::Hidden => Constructor::Hidden, Constructor::Missing => Constructor::Missing, + Constructor::Skip => Constructor::Skip, } } } @@ -763,6 +768,8 @@ impl Constructor { } // Wildcards cover anything (_, Wildcard) => true, + // `Skip` skips everything. + (Skip, _) => true, // Only a wildcard pattern can match these special constructors. (Missing { .. } | NonExhaustive | Hidden, _) => false, diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index 3e89a894419..642ab74b8b9 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -84,6 +84,8 @@ impl DeconstructedPat { match (&self.ctor, other_ctor) { // Return a wildcard for each field of `other_ctor`. (Wildcard, _) => wildcard_sub_tys(), + // Skip this column. + (_, Skip) => smallvec![], // The only non-trivial case: two slices of different arity. `other_slice` is // guaranteed to have a larger arity, so we fill the middle part with enough // wildcards to reach the length of the new, larger slice. @@ -192,7 +194,7 @@ impl fmt::Debug for DeconstructedPat { } Ok(()) } - Wildcard | Missing { .. } | NonExhaustive | Hidden => write!(f, "_ : {:?}", pat.ty()), + Wildcard | Missing | NonExhaustive | Hidden | Skip => write!(f, "_ : {:?}", pat.ty()), } } } diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 8d55dcf2056..3b68dd503ad 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -249,16 +249,8 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { } _ => bug!("bad slice pattern {:?} {:?}", ctor, ty), }, - Bool(..) - | IntRange(..) - | F32Range(..) - | F64Range(..) - | Str(..) - | Opaque(..) - | NonExhaustive - | Hidden - | Missing { .. } - | Wildcard => &[], + Bool(..) | IntRange(..) | F32Range(..) | F64Range(..) | Str(..) | Opaque(..) + | NonExhaustive | Hidden | Missing | Skip | Wildcard => &[], Or => { bug!("called `Fields::wildcards` on an `Or` ctor") } @@ -288,16 +280,8 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { }, Ref => 1, Slice(slice) => slice.arity(), - Bool(..) - | IntRange(..) - | F32Range(..) - | F64Range(..) - | Str(..) - | Opaque(..) - | NonExhaustive - | Hidden - | Missing { .. } - | Wildcard => 0, + Bool(..) | IntRange(..) | F32Range(..) | F64Range(..) | Str(..) | Opaque(..) + | NonExhaustive | Hidden | Missing | Skip | Wildcard => 0, Or => bug!("The `Or` constructor doesn't have a fixed arity"), } } @@ -838,7 +822,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { } } &Str(value) => PatKind::Constant { value }, - Wildcard | NonExhaustive | Hidden => PatKind::Wild, + Wildcard | NonExhaustive | Hidden | Skip => PatKind::Wild, Missing { .. } => bug!( "trying to convert a `Missing` constructor into a `Pat`; this is probably a bug, `Missing` should have been processed in `apply_constructors`" From ea381663900b90c0f78c6a64cd5e0b1876047714 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 6 Feb 2024 03:50:22 +0100 Subject: [PATCH 15/37] Don't filter out skipped fields --- compiler/rustc_pattern_analysis/src/lib.rs | 3 +-- compiler/rustc_pattern_analysis/src/pat.rs | 5 ---- compiler/rustc_pattern_analysis/src/rustc.rs | 15 ++++------- .../rustc_pattern_analysis/src/usefulness.rs | 27 +++++++++++++++---- 4 files changed, 28 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index 0b4bc77a976..e58e322a70d 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -109,8 +109,7 @@ pub trait TypeCx: Sized + fmt::Debug { /// The number of fields for this constructor. fn ctor_arity(&self, ctor: &Constructor, ty: &Self::Ty) -> usize; - /// The types of the fields for this constructor. The result must contain `ctor_arity()`-many - /// fields that are not skipped. + /// The types of the fields for this constructor. The result must contain `ctor_arity()` fields. fn ctor_sub_tys<'a>( &'a self, ctor: &'a Constructor, diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index 642ab74b8b9..8e1c22b92c8 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -23,11 +23,6 @@ impl PatId { /// Values and patterns can be represented as a constructor applied to some fields. This represents /// a pattern in this form. A `DeconstructedPat` will almost always come from user input; the only /// exception are some `Wildcard`s introduced during pattern lowering. -/// -/// Note that the number of fields may not match the fields declared in the original struct/variant. -/// This happens if a private or `non_exhaustive` field is uninhabited, because the code mustn't -/// observe that it is uninhabited. In that case that field is not included in `fields`. Care must -/// be taken when converting to/from `thir::Pat`. pub struct DeconstructedPat { ctor: Constructor, fields: Vec>, diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 3b68dd503ad..4e90d1a7406 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -271,9 +271,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { } else { let variant = &adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt)); - self.list_variant_nonhidden_fields(ty, variant) - .filter(|(_, _, skip)| !skip) - .count() + self.list_variant_nonhidden_fields(ty, variant).count() } } _ => bug!("Unexpected type for constructor `{ctor:?}`: {ty:?}"), @@ -512,14 +510,12 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { // For each field in the variant, we store the relevant index into `self.fields` if any. let mut field_id_to_id: Vec> = (0..variant.fields.len()).map(|_| None).collect(); - let tys = cx - .list_variant_nonhidden_fields(ty, variant) - .filter(|(_, _, skip)| !skip) - .enumerate() - .map(|(i, (field, ty, _))| { + let tys = cx.list_variant_nonhidden_fields(ty, variant).enumerate().map( + |(i, (field, ty, _))| { field_id_to_id[field.index()] = Some(i); ty - }); + }, + ); fields = tys.map(|ty| DeconstructedPat::wildcard(ty)).collect(); for pat in subpatterns { if let Some(i) = field_id_to_id[pat.field.index()] { @@ -769,7 +765,6 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { let variant = &adt_def.variant(variant_index); let subpatterns = cx .list_variant_nonhidden_fields(*pat.ty(), variant) - .filter(|(_, _, skip)| !skip) .zip(subpatterns) .map(|((field, _ty, _), pattern)| FieldPat { field, pattern }) .collect(); diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index ec9f3bb0db9..99d1f46e804 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -817,6 +817,9 @@ impl fmt::Display for ValidityConstraint { struct PlaceInfo { /// The type of the place. ty: Cx::Ty, + /// Whether we must skip this field during analysis. This is used when a private field is empty, + /// so that we don't observe its emptiness. + skip: SkipField, /// Whether the place is known to contain valid data. validity: ValidityConstraint, /// Whether the place is the scrutinee itself or a subplace of it. @@ -833,10 +836,9 @@ impl PlaceInfo { ) -> impl Iterator + ExactSizeIterator + Captures<'a> { let ctor_sub_tys = cx.ctor_sub_tys(ctor, &self.ty); let ctor_sub_validity = self.validity.specialize(ctor); - // Collect to keep the `ExactSizeIterator` bound. This is a temporary measure. - let tmp: Vec<_> = ctor_sub_tys.filter(|(_, SkipField(skip))| !skip).collect(); - tmp.into_iter().map(move |(ty, _)| PlaceInfo { + ctor_sub_tys.map(move |(ty, skip)| PlaceInfo { ty, + skip, validity: ctor_sub_validity, is_scrutinee: false, }) @@ -858,6 +860,11 @@ impl PlaceInfo { where Cx: 'a, { + if matches!(self.skip, SkipField(true)) { + // Skip the whole column + return Ok((smallvec![Constructor::Skip], vec![])); + } + let ctors_for_ty = cx.ctors_for_ty(&self.ty)?; // We treat match scrutinees of type `!` or `EmptyEnum` differently. @@ -916,7 +923,12 @@ impl PlaceInfo { impl Clone for PlaceInfo { fn clone(&self) -> Self { - Self { ty: self.ty.clone(), validity: self.validity, is_scrutinee: self.is_scrutinee } + Self { + ty: self.ty.clone(), + skip: self.skip, + validity: self.validity, + is_scrutinee: self.is_scrutinee, + } } } @@ -1123,7 +1135,12 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { scrut_ty: Cx::Ty, scrut_validity: ValidityConstraint, ) -> Self { - let place_info = PlaceInfo { ty: scrut_ty, validity: scrut_validity, is_scrutinee: true }; + let place_info = PlaceInfo { + ty: scrut_ty, + skip: SkipField(false), + validity: scrut_validity, + is_scrutinee: true, + }; let mut matrix = Matrix { rows: Vec::with_capacity(arms.len()), place_info: smallvec![place_info], From 39441e4cdd46c61be6b86e4bfe352d1e7d5af6fb Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 6 Feb 2024 03:12:21 +0100 Subject: [PATCH 16/37] Simplify --- compiler/rustc_pattern_analysis/src/rustc.rs | 80 +++++++++----------- 1 file changed, 34 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 4e90d1a7406..4f5f0383890 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -10,7 +10,7 @@ use rustc_middle::mir::interpret::Scalar; use rustc_middle::mir::{self, Const}; use rustc_middle::thir::{FieldPat, Pat, PatKind, PatRange, PatRangeBoundary}; use rustc_middle::ty::layout::IntegerExt; -use rustc_middle::ty::{self, OpaqueTypeKey, Ty, TyCtxt, TypeVisitableExt, VariantDef}; +use rustc_middle::ty::{self, FieldDef, OpaqueTypeKey, Ty, TyCtxt, TypeVisitableExt, VariantDef}; use rustc_session::lint; use rustc_span::{ErrorGuaranteed, Span, DUMMY_SP}; use rustc_target::abi::{FieldIdx, Integer, VariantIdx, FIRST_VARIANT}; @@ -158,32 +158,19 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { } } - // In the cases of either a `#[non_exhaustive]` field list or a non-public field, we hide - // uninhabited fields in order not to reveal the uninhabitedness of the whole variant. - // This lists the fields we keep along with their types. - pub(crate) fn list_variant_nonhidden_fields( + pub(crate) fn variant_sub_tys( &self, ty: RevealedTy<'tcx>, variant: &'tcx VariantDef, - ) -> impl Iterator, bool)> + Captures<'p> + Captures<'_> + ) -> impl Iterator)> + Captures<'p> + Captures<'_> { - let cx = self; - let ty::Adt(adt, args) = ty.kind() else { bug!() }; - // Whether we must avoid matching the fields of this variant exhaustively. - let is_non_exhaustive = variant.is_field_list_non_exhaustive() && !adt.did().is_local(); - - variant.fields.iter().enumerate().map(move |(i, field)| { - let ty = field.ty(cx.tcx, args); + let ty::Adt(_, args) = ty.kind() else { bug!() }; + variant.fields.iter().map(move |field| { + let ty = field.ty(self.tcx, args); // `field.ty()` doesn't normalize after instantiating. - let ty = cx.tcx.normalize_erasing_regions(cx.param_env, ty); - let is_visible = adt.is_enum() || field.vis.is_accessible_from(cx.module, cx.tcx); - let is_uninhabited = (cx.tcx.features().exhaustive_patterns - || cx.tcx.features().min_exhaustive_patterns) - && cx.is_uninhabited(ty); - - let skip = is_uninhabited && (!is_visible || is_non_exhaustive); - let ty = cx.reveal_opaque_ty(ty); - (FieldIdx::new(i), ty, skip) + let ty = self.tcx.normalize_erasing_regions(self.param_env, ty); + let ty = self.reveal_opaque_ty(ty); + (field, ty) }) } @@ -230,9 +217,21 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { } else { let variant = &adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt)); - let tys = cx - .list_variant_nonhidden_fields(ty, variant) - .map(|(_, ty, skip)| (ty, SkipField(skip))); + + // In the cases of either a `#[non_exhaustive]` field list or a non-public + // field, we skip uninhabited fields in order not to reveal the + // uninhabitedness of the whole variant. + let is_non_exhaustive = + variant.is_field_list_non_exhaustive() && !adt.did().is_local(); + let tys = cx.variant_sub_tys(ty, variant).map(|(field, ty)| { + let is_visible = + adt.is_enum() || field.vis.is_accessible_from(cx.module, cx.tcx); + let is_uninhabited = (cx.tcx.features().exhaustive_patterns + || cx.tcx.features().min_exhaustive_patterns) + && cx.is_uninhabited(*ty); + let skip = is_uninhabited && (!is_visible || is_non_exhaustive); + (ty, SkipField(skip)) + }); cx.dropless_arena.alloc_from_iter(tys) } } @@ -269,9 +268,8 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { // patterns. If we're here we can assume this is a box pattern. 1 } else { - let variant = - &adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt)); - self.list_variant_nonhidden_fields(ty, variant).count() + let variant_idx = RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt); + adt.variant(variant_idx).fields.len() } } _ => bug!("Unexpected type for constructor `{ctor:?}`: {ty:?}"), @@ -507,20 +505,12 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { }; let variant = &adt.variant(RustcMatchCheckCtxt::variant_index_for_adt(&ctor, *adt)); - // For each field in the variant, we store the relevant index into `self.fields` if any. - let mut field_id_to_id: Vec> = - (0..variant.fields.len()).map(|_| None).collect(); - let tys = cx.list_variant_nonhidden_fields(ty, variant).enumerate().map( - |(i, (field, ty, _))| { - field_id_to_id[field.index()] = Some(i); - ty - }, - ); - fields = tys.map(|ty| DeconstructedPat::wildcard(ty)).collect(); + fields = cx + .variant_sub_tys(ty, variant) + .map(|(_, ty)| DeconstructedPat::wildcard(ty)) + .collect(); for pat in subpatterns { - if let Some(i) = field_id_to_id[pat.field.index()] { - fields[i] = self.lower_pat(&pat.pattern); - } + fields[pat.field.index()] = self.lower_pat(&pat.pattern); } } _ => bug!("pattern has unexpected type: pat: {:?}, ty: {:?}", pat, ty), @@ -762,11 +752,9 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { ty::Adt(adt_def, args) => { let variant_index = RustcMatchCheckCtxt::variant_index_for_adt(&pat.ctor(), *adt_def); - let variant = &adt_def.variant(variant_index); - let subpatterns = cx - .list_variant_nonhidden_fields(*pat.ty(), variant) - .zip(subpatterns) - .map(|((field, _ty, _), pattern)| FieldPat { field, pattern }) + let subpatterns = subpatterns + .enumerate() + .map(|(i, pattern)| FieldPat { field: FieldIdx::new(i), pattern }) .collect(); if adt_def.is_enum() { From c918893b63022c1d810a71e8b7fa211b6aecd782 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 28 Feb 2024 17:56:01 +0100 Subject: [PATCH 17/37] Rename `Skip` to `PrivateUninhabited` --- .../rustc_pattern_analysis/src/constructor.rs | 13 +++++------ compiler/rustc_pattern_analysis/src/lib.rs | 7 +++--- compiler/rustc_pattern_analysis/src/pat.rs | 10 ++++---- compiler/rustc_pattern_analysis/src/rustc.rs | 23 +++++++++++-------- .../rustc_pattern_analysis/src/usefulness.rs | 18 +++++++-------- 5 files changed, 38 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/constructor.rs b/compiler/rustc_pattern_analysis/src/constructor.rs index 26b0f5fc45d..1286022fe4d 100644 --- a/compiler/rustc_pattern_analysis/src/constructor.rs +++ b/compiler/rustc_pattern_analysis/src/constructor.rs @@ -688,10 +688,9 @@ pub enum Constructor { /// Fake extra constructor for constructors that are not seen in the matrix, as explained at the /// top of the file. Missing, - /// Fake extra constructor that indicates that we should skip the column entirely. This is used - /// when a private field is empty, so that we don't observe its emptiness. Only used for - /// specialization. - Skip, + /// Fake extra constructor that indicates and empty field that is private. When we encounter one + /// we skip the column entirely so we don't observe its emptiness. Only used for specialization. + PrivateUninhabited, } impl Clone for Constructor { @@ -713,7 +712,7 @@ impl Clone for Constructor { Constructor::NonExhaustive => Constructor::NonExhaustive, Constructor::Hidden => Constructor::Hidden, Constructor::Missing => Constructor::Missing, - Constructor::Skip => Constructor::Skip, + Constructor::PrivateUninhabited => Constructor::PrivateUninhabited, } } } @@ -768,8 +767,8 @@ impl Constructor { } // Wildcards cover anything (_, Wildcard) => true, - // `Skip` skips everything. - (Skip, _) => true, + // `PrivateUninhabited` skips everything. + (PrivateUninhabited, _) => true, // Only a wildcard pattern can match these special constructors. (Missing { .. } | NonExhaustive | Hidden, _) => false, diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index e58e322a70d..d4b38d260e7 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -82,9 +82,10 @@ use crate::usefulness::{compute_match_usefulness, ValidityConstraint}; pub trait Captures<'a> {} impl<'a, T: ?Sized> Captures<'a> for T {} -/// `bool` newtype that indicates whether we should skip this field during analysis. +/// `bool` newtype that indicates whether this is a privately uninhabited field that we should skip +/// during analysis. #[derive(Copy, Clone, Debug, PartialEq, Eq)] -pub struct SkipField(pub bool); +pub struct PrivateUninhabitedField(pub bool); /// Context that provides type information about constructors. /// @@ -114,7 +115,7 @@ pub trait TypeCx: Sized + fmt::Debug { &'a self, ctor: &'a Constructor, ty: &'a Self::Ty, - ) -> impl Iterator + ExactSizeIterator + Captures<'a>; + ) -> impl Iterator + ExactSizeIterator + Captures<'a>; /// The set of all the constructors for `ty`. /// diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index 8e1c22b92c8..decbfa5c0cf 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -5,7 +5,7 @@ use std::fmt; use smallvec::{smallvec, SmallVec}; use crate::constructor::{Constructor, Slice, SliceKind}; -use crate::{SkipField, TypeCx}; +use crate::{PrivateUninhabitedField, TypeCx}; use self::Constructor::*; @@ -80,7 +80,7 @@ impl DeconstructedPat { // Return a wildcard for each field of `other_ctor`. (Wildcard, _) => wildcard_sub_tys(), // Skip this column. - (_, Skip) => smallvec![], + (_, PrivateUninhabited) => smallvec![], // The only non-trivial case: two slices of different arity. `other_slice` is // guaranteed to have a larger arity, so we fill the middle part with enough // wildcards to reach the length of the new, larger slice. @@ -189,7 +189,9 @@ impl fmt::Debug for DeconstructedPat { } Ok(()) } - Wildcard | Missing | NonExhaustive | Hidden | Skip => write!(f, "_ : {:?}", pat.ty()), + Wildcard | Missing | NonExhaustive | Hidden | PrivateUninhabited => { + write!(f, "_ : {:?}", pat.ty()) + } } } } @@ -299,7 +301,7 @@ impl WitnessPat { pub(crate) fn wild_from_ctor(cx: &Cx, ctor: Constructor, ty: Cx::Ty) -> Self { let fields = cx .ctor_sub_tys(&ctor, &ty) - .filter(|(_, SkipField(skip))| !skip) + .filter(|(_, PrivateUninhabitedField(skip))| !skip) .map(|(ty, _)| Self::wildcard(ty)) .collect(); Self::new(ctor, fields, ty) diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index 4f5f0383890..7a0562e12f1 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -18,7 +18,7 @@ use rustc_target::abi::{FieldIdx, Integer, VariantIdx, FIRST_VARIANT}; use crate::constructor::{ IntRange, MaybeInfiniteInt, OpaqueId, RangeEnd, Slice, SliceKind, VariantVisibility, }; -use crate::{errors, Captures, SkipField, TypeCx}; +use crate::{errors, Captures, PrivateUninhabitedField, TypeCx}; use crate::constructor::Constructor::*; @@ -195,14 +195,16 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { &'a self, ctor: &'a Constructor<'p, 'tcx>, ty: RevealedTy<'tcx>, - ) -> impl Iterator, SkipField)> + ExactSizeIterator + Captures<'a> - { + ) -> impl Iterator, PrivateUninhabitedField)> + + ExactSizeIterator + + Captures<'a> { fn reveal_and_alloc<'a, 'tcx>( cx: &'a RustcMatchCheckCtxt<'_, 'tcx>, iter: impl Iterator>, - ) -> &'a [(RevealedTy<'tcx>, SkipField)] { + ) -> &'a [(RevealedTy<'tcx>, PrivateUninhabitedField)] { cx.dropless_arena.alloc_from_iter( - iter.map(|ty| cx.reveal_opaque_ty(ty)).map(|ty| (ty, SkipField(false))), + iter.map(|ty| cx.reveal_opaque_ty(ty)) + .map(|ty| (ty, PrivateUninhabitedField(false))), ) } let cx = self; @@ -230,7 +232,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { || cx.tcx.features().min_exhaustive_patterns) && cx.is_uninhabited(*ty); let skip = is_uninhabited && (!is_visible || is_non_exhaustive); - (ty, SkipField(skip)) + (ty, PrivateUninhabitedField(skip)) }); cx.dropless_arena.alloc_from_iter(tys) } @@ -249,7 +251,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { _ => bug!("bad slice pattern {:?} {:?}", ctor, ty), }, Bool(..) | IntRange(..) | F32Range(..) | F64Range(..) | Str(..) | Opaque(..) - | NonExhaustive | Hidden | Missing | Skip | Wildcard => &[], + | NonExhaustive | Hidden | Missing | PrivateUninhabited | Wildcard => &[], Or => { bug!("called `Fields::wildcards` on an `Or` ctor") } @@ -277,7 +279,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { Ref => 1, Slice(slice) => slice.arity(), Bool(..) | IntRange(..) | F32Range(..) | F64Range(..) | Str(..) | Opaque(..) - | NonExhaustive | Hidden | Missing | Skip | Wildcard => 0, + | NonExhaustive | Hidden | Missing | PrivateUninhabited | Wildcard => 0, Or => bug!("The `Or` constructor doesn't have a fixed arity"), } } @@ -805,7 +807,7 @@ impl<'p, 'tcx: 'p> RustcMatchCheckCtxt<'p, 'tcx> { } } &Str(value) => PatKind::Constant { value }, - Wildcard | NonExhaustive | Hidden | Skip => PatKind::Wild, + Wildcard | NonExhaustive | Hidden | PrivateUninhabited => PatKind::Wild, Missing { .. } => bug!( "trying to convert a `Missing` constructor into a `Pat`; this is probably a bug, `Missing` should have been processed in `apply_constructors`" @@ -841,7 +843,8 @@ impl<'p, 'tcx: 'p> TypeCx for RustcMatchCheckCtxt<'p, 'tcx> { &'a self, ctor: &'a crate::constructor::Constructor, ty: &'a Self::Ty, - ) -> impl Iterator + ExactSizeIterator + Captures<'a> { + ) -> impl Iterator + ExactSizeIterator + Captures<'a> + { self.ctor_sub_tys(ctor, *ty) } fn ctors_for_ty( diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 99d1f46e804..bbe02f94c0a 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -716,7 +716,7 @@ use std::fmt; use crate::constructor::{Constructor, ConstructorSet, IntRange}; use crate::pat::{DeconstructedPat, PatId, PatOrWild, WitnessPat}; -use crate::{Captures, MatchArm, SkipField, TypeCx}; +use crate::{Captures, MatchArm, PrivateUninhabitedField, TypeCx}; use self::ValidityConstraint::*; @@ -817,9 +817,9 @@ impl fmt::Display for ValidityConstraint { struct PlaceInfo { /// The type of the place. ty: Cx::Ty, - /// Whether we must skip this field during analysis. This is used when a private field is empty, + /// Whether the place is a private uninhabited field. If so we skip this field during analysis /// so that we don't observe its emptiness. - skip: SkipField, + private_uninhabited: bool, /// Whether the place is known to contain valid data. validity: ValidityConstraint, /// Whether the place is the scrutinee itself or a subplace of it. @@ -836,9 +836,9 @@ impl PlaceInfo { ) -> impl Iterator + ExactSizeIterator + Captures<'a> { let ctor_sub_tys = cx.ctor_sub_tys(ctor, &self.ty); let ctor_sub_validity = self.validity.specialize(ctor); - ctor_sub_tys.map(move |(ty, skip)| PlaceInfo { + ctor_sub_tys.map(move |(ty, PrivateUninhabitedField(private_uninhabited))| PlaceInfo { ty, - skip, + private_uninhabited, validity: ctor_sub_validity, is_scrutinee: false, }) @@ -860,9 +860,9 @@ impl PlaceInfo { where Cx: 'a, { - if matches!(self.skip, SkipField(true)) { + if self.private_uninhabited { // Skip the whole column - return Ok((smallvec![Constructor::Skip], vec![])); + return Ok((smallvec![Constructor::PrivateUninhabited], vec![])); } let ctors_for_ty = cx.ctors_for_ty(&self.ty)?; @@ -925,7 +925,7 @@ impl Clone for PlaceInfo { fn clone(&self) -> Self { Self { ty: self.ty.clone(), - skip: self.skip, + private_uninhabited: self.private_uninhabited, validity: self.validity, is_scrutinee: self.is_scrutinee, } @@ -1137,7 +1137,7 @@ impl<'p, Cx: TypeCx> Matrix<'p, Cx> { ) -> Self { let place_info = PlaceInfo { ty: scrut_ty, - skip: SkipField(false), + private_uninhabited: false, validity: scrut_validity, is_scrutinee: true, }; From c4ec196c7edf3f6d193b8b8da49ead7ceffa9799 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 26 Feb 2024 16:54:53 +1100 Subject: [PATCH 18/37] Don't cancel stashed `OpaqueHiddenTypeMismatch` errors. This gives one extra error message on one test, but is necessary to fix bigger problems caused by the cancellation of stashed errors. (Note: why not just avoid stashing altogether? Because that resulted in additional output changes.) --- compiler/rustc_middle/src/ty/mod.rs | 6 +++++- .../different_defining_uses_never_type-2.rs | 1 + .../different_defining_uses_never_type-2.stderr | 14 +++++++++++++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index d97f0e4c321..9089d992cd5 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -852,7 +852,11 @@ impl<'tcx> OpaqueHiddenType<'tcx> { .dcx() .steal_diagnostic(tcx.def_span(opaque_def_id), StashKey::OpaqueHiddenTypeMismatch) { - diag.cancel(); + // We used to cancel here for slightly better error messages, but + // cancelling stashed diagnostics is no longer allowed because it + // causes problems when tracking whether errors have actually + // occurred. + diag.emit(); } (self.ty, other.ty).error_reported()?; // Found different concrete types for the opaque type. diff --git a/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-2.rs b/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-2.rs index b2842df150a..4b5f455e381 100644 --- a/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-2.rs +++ b/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-2.rs @@ -11,6 +11,7 @@ fn foo<'a, 'b>() -> Tait<'a> { } let x: Tait<'a> = (); x + //~^ ERROR concrete type differs from previous defining opaque type use } fn main() {} diff --git a/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-2.stderr b/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-2.stderr index e5cee49cf29..6f5be5467f7 100644 --- a/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-2.stderr +++ b/tests/ui/type-alias-impl-trait/different_defining_uses_never_type-2.stderr @@ -1,3 +1,15 @@ +error: concrete type differs from previous defining opaque type use + --> $DIR/different_defining_uses_never_type-2.rs:13:5 + | +LL | x + | ^ expected `i32`, got `()` + | +note: previous use here + --> $DIR/different_defining_uses_never_type-2.rs:8:31 + | +LL | let y: Tait<'b> = 1i32; + | ^^^^ + error: concrete type differs from previous defining opaque type use --> $DIR/different_defining_uses_never_type-2.rs:8:31 | @@ -10,5 +22,5 @@ note: previous use here LL | if { return } { | ^^^^^^ -error: aborting due to 1 previous error +error: aborting due to 2 previous errors From ec25d6db53768a4bb68f621be3584941ac3fe416 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 26 Feb 2024 17:22:24 +1100 Subject: [PATCH 19/37] Don't cancel stashed `TraitMissingMethod` errors. This gives one extra error message on two tests, but is necessary to fix bigger problems caused by the cancellation of stashed errors. (Note: why not just avoid stashing altogether? Because that resulted in additional output changes.) --- compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs | 11 ++++------- tests/ui/resolve/issue-111312.rs | 4 +++- tests/ui/resolve/issue-111312.stderr | 16 ++++++++++++++-- tests/ui/resolve/issue-111727.rs | 4 +++- tests/ui/resolve/issue-111727.stderr | 16 ++++++++++++++-- 5 files changed, 38 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index a5892dea1a5..1e6467deb01 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs @@ -879,17 +879,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } - // emit or cancel the diagnostic for bare traits + // Emit the diagnostic for bare traits. (We used to cancel for slightly better + // error messages, but cancelling stashed diagnostics is no longer allowed because + // it causes problems when tracking whether errors have actually occurred.) if span.edition().at_least_rust_2021() && let Some(diag) = self.dcx().steal_diagnostic(qself.span, StashKey::TraitMissingMethod) { - if trait_missing_method { - // cancel the diag for bare traits when meeting `MyTrait::missing_method` - diag.cancel(); - } else { - diag.emit(); - } + diag.emit(); } if item_name.name != kw::Empty { diff --git a/tests/ui/resolve/issue-111312.rs b/tests/ui/resolve/issue-111312.rs index 68fc8573dde..79c6f67dadd 100644 --- a/tests/ui/resolve/issue-111312.rs +++ b/tests/ui/resolve/issue-111312.rs @@ -7,5 +7,7 @@ trait Has { trait HasNot {} fn main() { - HasNot::has(); //~ ERROR + HasNot::has(); + //~^ ERROR trait objects must include the `dyn` keyword + //~| ERROR no function or associated item named `has` found for trait `HasNot` } diff --git a/tests/ui/resolve/issue-111312.stderr b/tests/ui/resolve/issue-111312.stderr index 7e7ef22ae61..431802ead30 100644 --- a/tests/ui/resolve/issue-111312.stderr +++ b/tests/ui/resolve/issue-111312.stderr @@ -1,3 +1,14 @@ +error[E0782]: trait objects must include the `dyn` keyword + --> $DIR/issue-111312.rs:10:5 + | +LL | HasNot::has(); + | ^^^^^^ + | +help: add `dyn` keyword before this trait + | +LL | ::has(); + | ++++ + + error[E0599]: no function or associated item named `has` found for trait `HasNot` --> $DIR/issue-111312.rs:10:13 | @@ -10,6 +21,7 @@ note: `Has` defines an item `has` LL | trait Has { | ^^^^^^^^^ -error: aborting due to 1 previous error +error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0599`. +Some errors have detailed explanations: E0599, E0782. +For more information about an error, try `rustc --explain E0599`. diff --git a/tests/ui/resolve/issue-111727.rs b/tests/ui/resolve/issue-111727.rs index 740037fe434..fcab924b809 100644 --- a/tests/ui/resolve/issue-111727.rs +++ b/tests/ui/resolve/issue-111727.rs @@ -1,5 +1,7 @@ //@ edition: 2021 fn main() { - std::any::Any::create(); //~ ERROR + std::any::Any::create(); + //~^ ERROR trait objects must include the `dyn` keyword + //~| ERROR no function or associated item named `create` found for trait `Any` } diff --git a/tests/ui/resolve/issue-111727.stderr b/tests/ui/resolve/issue-111727.stderr index b58168d0e75..1ef5a1a1d5e 100644 --- a/tests/ui/resolve/issue-111727.stderr +++ b/tests/ui/resolve/issue-111727.stderr @@ -1,9 +1,21 @@ +error[E0782]: trait objects must include the `dyn` keyword + --> $DIR/issue-111727.rs:4:5 + | +LL | std::any::Any::create(); + | ^^^^^^^^^^^^^ + | +help: add `dyn` keyword before this trait + | +LL | ::create(); + | ++++ + + error[E0599]: no function or associated item named `create` found for trait `Any` --> $DIR/issue-111727.rs:4:20 | LL | std::any::Any::create(); | ^^^^^^ function or associated item not found in `Any` -error: aborting due to 1 previous error +error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0599`. +Some errors have detailed explanations: E0599, E0782. +For more information about an error, try `rustc --explain E0599`. From 260ae701405f1278202de219bcdd0d60e8060da9 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 26 Feb 2024 15:21:01 +1100 Subject: [PATCH 20/37] Overhaul how stashed diagnostics work, again. Stashed errors used to be counted as errors, but could then be cancelled, leading to `ErrorGuaranteed` soundness holes. #120828 changed that, closing the soundness hole. But it introduced other difficulties because you sometimes have to account for pending stashed errors when making decisions about whether errors have occured/will occur and it's easy to overlook these. This commit aims for a middle ground. - Stashed errors (not warnings) are counted immediately as emitted errors, avoiding the possibility of forgetting to consider them. - The ability to cancel (or downgrade) stashed errors is eliminated, by disallowing the use of `steal_diagnostic` with errors, and introducing the more restrictive methods `try_steal_{modify,replace}_and_emit_err` that can be used instead. Other things: - `DiagnosticBuilder::stash` and `DiagCtxt::stash_diagnostic` now both return `Option`, which enables the removal of two `delayed_bug` calls and one `Ty::new_error_with_message` call. This is possible because we store error guarantees in `DiagCtxt::stashed_diagnostics`. - Storing the guarantees also saves us having to maintain a counter. - Calls to the `stashed_err_count` method are no longer necessary alongside calls to `has_errors`, which is a nice simplification, and eliminates two more `span_delayed_bug` calls and one FIXME comment. - Tests are added for three of the four fixed PRs mentioned below. - `issue-121108.rs`'s output improved slightly, omitting a non-useful error message. Fixes #121451. Fixes #121477. Fixes #121504. Fixes #121508. --- compiler/rustc_errors/src/diagnostic.rs | 8 +- compiler/rustc_errors/src/lib.rs | 211 +++++++++++++----- .../rustc_hir_analysis/src/astconv/lint.rs | 12 +- .../rustc_hir_analysis/src/check/check.rs | 3 +- .../rustc_hir_analysis/src/collect/type_of.rs | 22 +- compiler/rustc_hir_typeck/src/callee.rs | 17 +- compiler/rustc_hir_typeck/src/expr.rs | 45 ++-- .../rustc_hir_typeck/src/fn_ctxt/_impl.rs | 22 +- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 76 +++---- .../rustc_hir_typeck/src/method/suggest.rs | 100 ++++----- compiler/rustc_hir_typeck/src/writeback.rs | 4 - compiler/rustc_infer/src/infer/at.rs | 1 - compiler/rustc_infer/src/infer/mod.rs | 15 -- compiler/rustc_middle/src/ty/mod.rs | 20 +- compiler/rustc_middle/src/ty/region.rs | 4 +- compiler/rustc_middle/src/ty/sty.rs | 4 +- compiler/rustc_parse/src/parser/expr.rs | 29 +-- compiler/rustc_passes/src/check_attr.rs | 20 +- .../rustc_query_system/src/query/plumbing.rs | 3 +- compiler/rustc_session/src/parse.rs | 3 +- .../error_reporting/type_err_ctxt_ext.rs | 18 +- .../ui/crashes/unreachable-array-or-slice.rs | 8 + .../crashes/unreachable-array-or-slice.stderr | 9 + tests/ui/foreign/stashed-issue-121451.rs | 4 + tests/ui/foreign/stashed-issue-121451.stderr | 9 + .../impl-trait/stashed-diag-issue-121504.rs | 13 ++ .../stashed-diag-issue-121504.stderr | 9 + tests/ui/lowering/issue-121108.rs | 2 +- tests/ui/lowering/issue-121108.stderr | 10 +- 29 files changed, 406 insertions(+), 295 deletions(-) create mode 100644 src/tools/clippy/tests/ui/crashes/unreachable-array-or-slice.rs create mode 100644 src/tools/clippy/tests/ui/crashes/unreachable-array-or-slice.stderr create mode 100644 tests/ui/foreign/stashed-issue-121451.rs create mode 100644 tests/ui/foreign/stashed-issue-121451.stderr create mode 100644 tests/ui/impl-trait/stashed-diag-issue-121504.rs create mode 100644 tests/ui/impl-trait/stashed-diag-issue-121504.stderr diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 01f36ad6a78..207c616cefc 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -1289,11 +1289,9 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { drop(self); } - /// Stashes diagnostic for possible later improvement in a different, - /// later stage of the compiler. The diagnostic can be accessed with - /// the provided `span` and `key` through [`DiagCtxt::steal_diagnostic()`]. - pub fn stash(mut self, span: Span, key: StashKey) { - self.dcx.stash_diagnostic(span, key, self.take_diag()); + /// See `DiagCtxt::stash_diagnostic` for details. + pub fn stash(mut self, span: Span, key: StashKey) -> Option { + self.dcx.stash_diagnostic(span, key, self.take_diag()) } /// Delay emission of this diagnostic as a bug. diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index bc338b01d8b..60bda9f4c94 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -434,10 +434,6 @@ struct DiagCtxtInner { /// The delayed bugs and their error guarantees. delayed_bugs: Vec<(DelayedDiagInner, ErrorGuaranteed)>, - /// The number of stashed errors. Unlike the other counts, this can go up - /// and down, so it doesn't guarantee anything. - stashed_err_count: usize, - /// The error count shown to the user at the end. deduplicated_err_count: usize, /// The warning count shown to the user at the end. @@ -475,7 +471,7 @@ struct DiagCtxtInner { /// add more information). All stashed diagnostics must be emitted with /// `emit_stashed_diagnostics` by the time the `DiagCtxtInner` is dropped, /// otherwise an assertion failure will occur. - stashed_diagnostics: FxIndexMap<(Span, StashKey), DiagInner>, + stashed_diagnostics: FxIndexMap<(Span, StashKey), (DiagInner, Option)>, future_breakage_diagnostics: Vec, @@ -561,8 +557,14 @@ impl Drop for DiagCtxtInner { fn drop(&mut self) { // Any stashed diagnostics should have been handled by // `emit_stashed_diagnostics` by now. + // + // Important: it is sound to produce an `ErrorGuaranteed` when stashing + // errors because they are guaranteed to have been emitted by here. assert!(self.stashed_diagnostics.is_empty()); + // Important: it is sound to produce an `ErrorGuaranteed` when emitting + // delayed bugs because they are guaranteed to be emitted here if + // necessary. if self.err_guars.is_empty() { self.flush_delayed() } @@ -615,7 +617,6 @@ impl DiagCtxt { err_guars: Vec::new(), lint_err_guars: Vec::new(), delayed_bugs: Vec::new(), - stashed_err_count: 0, deduplicated_err_count: 0, deduplicated_warn_count: 0, emitter, @@ -676,7 +677,6 @@ impl DiagCtxt { err_guars, lint_err_guars, delayed_bugs, - stashed_err_count, deduplicated_err_count, deduplicated_warn_count, emitter: _, @@ -699,7 +699,6 @@ impl DiagCtxt { *err_guars = Default::default(); *lint_err_guars = Default::default(); *delayed_bugs = Default::default(); - *stashed_err_count = 0; *deduplicated_err_count = 0; *deduplicated_warn_count = 0; *must_produce_diag = false; @@ -715,39 +714,111 @@ impl DiagCtxt { *fulfilled_expectations = Default::default(); } - /// Stash a given diagnostic with the given `Span` and [`StashKey`] as the key. - /// Retrieve a stashed diagnostic with `steal_diagnostic`. - pub fn stash_diagnostic(&self, span: Span, key: StashKey, diag: DiagInner) { - let mut inner = self.inner.borrow_mut(); - - let key = (span.with_parent(None), key); - - if diag.is_error() { - if diag.is_lint.is_none() { - inner.stashed_err_count += 1; - } - } + /// Stashes a diagnostic for possible later improvement in a different, + /// later stage of the compiler. Possible actions depend on the diagnostic + /// level: + /// - Level::Error: immediately counted as an error that has occurred, because it + /// is guaranteed to be emitted eventually. Can be later accessed with the + /// provided `span` and `key` through + /// [`DiagCtxt::try_steal_modify_and_emit_err`] or + /// [`DiagCtxt::try_steal_replace_and_emit_err`]. These do not allow + /// cancellation or downgrading of the error. Returns + /// `Some(ErrorGuaranteed)`. + /// - Level::Warning and lower (i.e. !is_error()): can be accessed with the + /// provided `span` and `key` through [`DiagCtxt::steal_non_err()`]. This + /// allows cancelling and downgrading of the diagnostic. Returns `None`. + /// - Others: not allowed, will trigger a panic. + pub fn stash_diagnostic( + &self, + span: Span, + key: StashKey, + diag: DiagInner, + ) -> Option { + let guar = if diag.level() == Level::Error { + // This `unchecked_error_guaranteed` is valid. It is where the + // `ErrorGuaranteed` for stashed errors originates. See + // `DiagCtxtInner::drop`. + #[allow(deprecated)] + Some(ErrorGuaranteed::unchecked_error_guaranteed()) + } else if !diag.is_error() { + None + } else { + self.span_bug(span, format!("invalid level in `stash_diagnostic`: {}", diag.level)); + }; // FIXME(Centril, #69537): Consider reintroducing panic on overwriting a stashed diagnostic // if/when we have a more robust macro-friendly replacement for `(span, key)` as a key. // See the PR for a discussion. - inner.stashed_diagnostics.insert(key, diag); + let key = (span.with_parent(None), key); + self.inner.borrow_mut().stashed_diagnostics.insert(key, (diag, guar)); + + guar } - /// Steal a previously stashed diagnostic with the given `Span` and [`StashKey`] as the key. - pub fn steal_diagnostic(&self, span: Span, key: StashKey) -> Option> { - let mut inner = self.inner.borrow_mut(); + /// Steal a previously stashed non-error diagnostic with the given `Span` + /// and [`StashKey`] as the key. Panics if the found diagnostic is an + /// error. + pub fn steal_non_err(&self, span: Span, key: StashKey) -> Option> { let key = (span.with_parent(None), key); // FIXME(#120456) - is `swap_remove` correct? - let diag = inner.stashed_diagnostics.swap_remove(&key)?; - if diag.is_error() { - if diag.is_lint.is_none() { - inner.stashed_err_count -= 1; - } - } + let (diag, guar) = self.inner.borrow_mut().stashed_diagnostics.swap_remove(&key)?; + assert!(!diag.is_error()); + assert!(guar.is_none()); Some(Diag::new_diagnostic(self, diag)) } + /// Steals a previously stashed error with the given `Span` and + /// [`StashKey`] as the key, modifies it, and emits it. Returns `None` if + /// no matching diagnostic is found. Panics if the found diagnostic's level + /// isn't `Level::Error`. + pub fn try_steal_modify_and_emit_err( + &self, + span: Span, + key: StashKey, + mut modify_err: F, + ) -> Option + where + F: FnMut(&mut Diag<'_>), + { + let key = (span.with_parent(None), key); + // FIXME(#120456) - is `swap_remove` correct? + let err = self.inner.borrow_mut().stashed_diagnostics.swap_remove(&key); + err.map(|(err, guar)| { + // The use of `::` is safe because level is `Level::Error`. + assert_eq!(err.level, Level::Error); + assert!(guar.is_some()); + let mut err = Diag::::new_diagnostic(self, err); + modify_err(&mut err); + assert_eq!(err.level, Level::Error); + err.emit() + }) + } + + /// Steals a previously stashed error with the given `Span` and + /// [`StashKey`] as the key, cancels it if found, and emits `new_err`. + /// Panics if the found diagnostic's level isn't `Level::Error`. + pub fn try_steal_replace_and_emit_err( + &self, + span: Span, + key: StashKey, + new_err: Diag<'_>, + ) -> ErrorGuaranteed { + let key = (span.with_parent(None), key); + // FIXME(#120456) - is `swap_remove` correct? + let old_err = self.inner.borrow_mut().stashed_diagnostics.swap_remove(&key); + match old_err { + Some((old_err, guar)) => { + assert_eq!(old_err.level, Level::Error); + assert!(guar.is_some()); + // Because `old_err` has already been counted, it can only be + // safely cancelled because the `new_err` supplants it. + Diag::::new_diagnostic(self, old_err).cancel(); + } + None => {} + }; + new_err.emit() + } + pub fn has_stashed_diagnostic(&self, span: Span, key: StashKey) -> bool { self.inner.borrow().stashed_diagnostics.get(&(span.with_parent(None), key)).is_some() } @@ -757,41 +828,40 @@ impl DiagCtxt { self.inner.borrow_mut().emit_stashed_diagnostics() } - /// This excludes lint errors, delayed bugs and stashed errors. + /// This excludes lint errors, and delayed bugs. #[inline] pub fn err_count_excluding_lint_errs(&self) -> usize { - self.inner.borrow().err_guars.len() + let inner = self.inner.borrow(); + inner.err_guars.len() + + inner + .stashed_diagnostics + .values() + .filter(|(diag, guar)| guar.is_some() && diag.is_lint.is_none()) + .count() } - /// This excludes delayed bugs and stashed errors. + /// This excludes delayed bugs. #[inline] pub fn err_count(&self) -> usize { let inner = self.inner.borrow(); - inner.err_guars.len() + inner.lint_err_guars.len() + inner.err_guars.len() + + inner.lint_err_guars.len() + + inner.stashed_diagnostics.values().filter(|(_diag, guar)| guar.is_some()).count() } - /// This excludes normal errors, lint errors, and delayed bugs. Unless - /// absolutely necessary, avoid using this. It's dubious because stashed - /// errors can later be cancelled, so the presence of a stashed error at - /// some point of time doesn't guarantee anything -- there are no - /// `ErrorGuaranteed`s here. - pub fn stashed_err_count(&self) -> usize { - self.inner.borrow().stashed_err_count - } - - /// This excludes lint errors, delayed bugs, and stashed errors. Unless - /// absolutely necessary, prefer `has_errors` to this method. + /// This excludes lint errors and delayed bugs. Unless absolutely + /// necessary, prefer `has_errors` to this method. pub fn has_errors_excluding_lint_errors(&self) -> Option { self.inner.borrow().has_errors_excluding_lint_errors() } - /// This excludes delayed bugs and stashed errors. + /// This excludes delayed bugs. pub fn has_errors(&self) -> Option { self.inner.borrow().has_errors() } - /// This excludes stashed errors. Unless absolutely necessary, prefer - /// `has_errors` to this method. + /// This excludes nothing. Unless absolutely necessary, prefer `has_errors` + /// to this method. pub fn has_errors_or_delayed_bugs(&self) -> Option { self.inner.borrow().has_errors_or_delayed_bugs() } @@ -876,10 +946,10 @@ impl DiagCtxt { } } - /// This excludes delayed bugs and stashed errors. Used for early aborts - /// after errors occurred -- e.g. because continuing in the face of errors is - /// likely to lead to bad results, such as spurious/uninteresting - /// additional errors -- when returning an error `Result` is difficult. + /// This excludes delayed bugs. Used for early aborts after errors occurred + /// -- e.g. because continuing in the face of errors is likely to lead to + /// bad results, such as spurious/uninteresting additional errors -- when + /// returning an error `Result` is difficult. pub fn abort_if_errors(&self) { if self.has_errors().is_some() { FatalError.raise(); @@ -963,7 +1033,7 @@ impl DiagCtxt { inner .stashed_diagnostics .values_mut() - .for_each(|diag| diag.update_unstable_expectation_id(unstable_to_stable)); + .for_each(|(diag, _guar)| diag.update_unstable_expectation_id(unstable_to_stable)); inner .future_breakage_diagnostics .iter_mut() @@ -1270,12 +1340,8 @@ impl DiagCtxtInner { fn emit_stashed_diagnostics(&mut self) -> Option { let mut guar = None; let has_errors = !self.err_guars.is_empty(); - for (_, diag) in std::mem::take(&mut self.stashed_diagnostics).into_iter() { - if diag.is_error() { - if diag.is_lint.is_none() { - self.stashed_err_count -= 1; - } - } else { + for (_, (diag, _guar)) in std::mem::take(&mut self.stashed_diagnostics).into_iter() { + if !diag.is_error() { // Unless they're forced, don't flush stashed warnings when // there are errors, to avoid causing warning overload. The // stash would've been stolen already if it were important. @@ -1334,7 +1400,8 @@ impl DiagCtxtInner { } else { let backtrace = std::backtrace::Backtrace::capture(); // This `unchecked_error_guaranteed` is valid. It is where the - // `ErrorGuaranteed` for delayed bugs originates. + // `ErrorGuaranteed` for delayed bugs originates. See + // `DiagCtxtInner::drop`. #[allow(deprecated)] let guar = ErrorGuaranteed::unchecked_error_guaranteed(); self.delayed_bugs @@ -1446,11 +1513,31 @@ impl DiagCtxtInner { } fn has_errors_excluding_lint_errors(&self) -> Option { - self.err_guars.get(0).copied() + self.err_guars.get(0).copied().or_else(|| { + if let Some((_diag, guar)) = self + .stashed_diagnostics + .values() + .find(|(diag, guar)| guar.is_some() && diag.is_lint.is_none()) + { + *guar + } else { + None + } + }) } fn has_errors(&self) -> Option { - self.has_errors_excluding_lint_errors().or_else(|| self.lint_err_guars.get(0).copied()) + self.err_guars.get(0).copied().or_else(|| self.lint_err_guars.get(0).copied()).or_else( + || { + if let Some((_diag, guar)) = + self.stashed_diagnostics.values().find(|(_diag, guar)| guar.is_some()) + { + *guar + } else { + None + } + }, + ) } fn has_errors_or_delayed_bugs(&self) -> Option { diff --git a/compiler/rustc_hir_analysis/src/astconv/lint.rs b/compiler/rustc_hir_analysis/src/astconv/lint.rs index fb5f3426cea..227254b4cc8 100644 --- a/compiler/rustc_hir_analysis/src/astconv/lint.rs +++ b/compiler/rustc_hir_analysis/src/astconv/lint.rs @@ -1,5 +1,5 @@ use rustc_ast::TraitObjectSyntax; -use rustc_errors::{codes::*, Diag, EmissionGuarantee, StashKey}; +use rustc_errors::{codes::*, Diag, EmissionGuarantee, Level, StashKey}; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_lint_defs::{builtin::BARE_TRAIT_OBJECTS, Applicability}; @@ -237,7 +237,15 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { } // check if the impl trait that we are considering is a impl of a local trait self.maybe_lint_blanket_trait_impl(self_ty, &mut diag); - diag.stash(self_ty.span, StashKey::TraitMissingMethod); + match diag.level() { + Level::Error => { + diag.stash(self_ty.span, StashKey::TraitMissingMethod); + } + Level::DelayedBug => { + diag.emit(); + } + _ => unreachable!(), + } } else { let msg = "trait objects without an explicit `dyn` are deprecated"; tcx.node_span_lint(BARE_TRAIT_OBJECTS, self_ty.hir_id, self_ty.span, msg, |lint| { diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 81e84860b82..96bebda5828 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -1350,8 +1350,7 @@ fn check_type_alias_type_params_are_used<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalD let ty = tcx.type_of(def_id).instantiate_identity(); if ty.references_error() { // If there is already another error, do not emit an error for not using a type parameter. - // Without the `stashed_err_count` part this can fail (#120856). - assert!(tcx.dcx().has_errors().is_some() || tcx.dcx().stashed_err_count() > 0); + assert!(tcx.dcx().has_errors().is_some()); return; } diff --git a/compiler/rustc_hir_analysis/src/collect/type_of.rs b/compiler/rustc_hir_analysis/src/collect/type_of.rs index c0128afe2bf..417f0fceaa8 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of.rs @@ -596,10 +596,11 @@ fn infer_placeholder_type<'a>( // then the user may have written e.g. `const A = 42;`. // In this case, the parser has stashed a diagnostic for // us to improve in typeck so we do that now. - match tcx.dcx().steal_diagnostic(span, StashKey::ItemNoType) { - Some(mut err) => { + let guar = tcx + .dcx() + .try_steal_modify_and_emit_err(span, StashKey::ItemNoType, |err| { if !ty.references_error() { - // Only suggest adding `:` if it was missing (and suggested by parsing diagnostic) + // Only suggest adding `:` if it was missing (and suggested by parsing diagnostic). let colon = if span == item_ident.span.shrink_to_hi() { ":" } else { "" }; // The parser provided a sub-optimal `HasPlaceholders` suggestion for the type. @@ -622,12 +623,8 @@ fn infer_placeholder_type<'a>( )); } } - - err.emit(); - // diagnostic stashing loses the information of whether something is a hard error. - Ty::new_error_with_message(tcx, span, "ItemNoType is a hard error") - } - None => { + }) + .unwrap_or_else(|| { let mut diag = bad_placeholder(tcx, vec![span], kind); if !ty.references_error() { @@ -645,10 +642,9 @@ fn infer_placeholder_type<'a>( )); } } - - Ty::new_error(tcx, diag.emit()) - } - } + diag.emit() + }); + Ty::new_error(tcx, guar) } fn check_feature_inherent_assoc_ty(tcx: TyCtxt<'_>, span: Span) { diff --git a/compiler/rustc_hir_typeck/src/callee.rs b/compiler/rustc_hir_typeck/src/callee.rs index bb6d1ecae02..b87d71e3533 100644 --- a/compiler/rustc_hir_typeck/src/callee.rs +++ b/compiler/rustc_hir_typeck/src/callee.rs @@ -484,12 +484,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = &callee_expr.kind && let [segment] = path.segments - && let Some(mut diag) = - self.dcx().steal_diagnostic(segment.ident.span, StashKey::CallIntoMethod) { - // Try suggesting `foo(a)` -> `a.foo()` if possible. - self.suggest_call_as_method(&mut diag, segment, arg_exprs, call_expr, expected); - diag.emit(); + self.dcx().try_steal_modify_and_emit_err( + segment.ident.span, + StashKey::CallIntoMethod, + |err| { + // Try suggesting `foo(a)` -> `a.foo()` if possible. + self.suggest_call_as_method( + err, segment, arg_exprs, call_expr, expected, + ); + }, + ); } let err = self.report_invalid_callee(call_expr, callee_expr, callee_ty, arg_exprs); @@ -601,7 +606,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// and suggesting the fix if the method probe is successful. fn suggest_call_as_method( &self, - diag: &mut Diag<'_, ()>, + diag: &mut Diag<'_>, segment: &'tcx hir::PathSegment<'tcx>, arg_exprs: &'tcx [hir::Expr<'tcx>], call_expr: &'tcx hir::Expr<'tcx>, diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 2b7bce3f485..054be89a7c4 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -1460,18 +1460,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { && let hir::ArrayLen::Body(hir::AnonConst { hir_id, .. }) = length { let span = self.tcx.hir().span(hir_id); - match self.dcx().steal_diagnostic(span, StashKey::UnderscoreForArrayLengths) { - Some(mut err) => { + self.dcx().try_steal_modify_and_emit_err( + span, + StashKey::UnderscoreForArrayLengths, + |err| { err.span_suggestion( span, "consider specifying the array length", array_len, Applicability::MaybeIncorrect, ); - err.emit(); - } - None => (), - } + }, + ); } } @@ -1751,11 +1751,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let (_, diag) = self.demand_coerce_diag(field.expr, ty, field_type, None, AllowTwoPhase::No); - if let Some(mut diag) = diag { + if let Some(diag) = diag { if idx == ast_fields.len() - 1 { if remaining_fields.is_empty() { - self.suggest_fru_from_range(field, variant, args, &mut diag); - diag.emit(); + self.suggest_fru_from_range_and_emit(field, variant, args, diag); } else { diag.stash(field.span, StashKey::MaybeFruTypo); } @@ -1986,20 +1985,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err.span_label(span, format!("missing {remaining_fields_names}{truncated_fields_error}")); if let Some(last) = ast_fields.last() { - self.suggest_fru_from_range(last, variant, args, &mut err); + self.suggest_fru_from_range_and_emit(last, variant, args, err); + } else { + err.emit(); } - - err.emit(); } /// If the last field is a range literal, but it isn't supposed to be, then they probably /// meant to use functional update syntax. - fn suggest_fru_from_range( + fn suggest_fru_from_range_and_emit( &self, last_expr_field: &hir::ExprField<'tcx>, variant: &ty::VariantDef, args: GenericArgsRef<'tcx>, - err: &mut Diag<'_>, + mut err: Diag<'_>, ) { // I don't use 'is_range_literal' because only double-sided, half-open ranges count. if let ExprKind::Struct(QPath::LangItem(LangItem::Range, ..), [range_start, range_end], _) = @@ -2012,16 +2011,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .map(|adt| adt.did()) != range_def_id { - // Suppress any range expr type mismatches - if let Some(diag) = - self.dcx().steal_diagnostic(last_expr_field.span, StashKey::MaybeFruTypo) - { - diag.delay_as_bug(); - } - // Use a (somewhat arbitrary) filtering heuristic to avoid printing // expressions that are either too long, or have control character - //such as newlines in them. + // such as newlines in them. let expr = self .tcx .sess @@ -2043,6 +2035,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.dcx(), TypeMismatchFruTypo { expr_span: range_start.span, fru_span, expr }, ); + + // Suppress any range expr type mismatches + self.dcx().try_steal_replace_and_emit_err( + last_expr_field.span, + StashKey::MaybeFruTypo, + err, + ); + } else { + err.emit(); } } diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index 1e6467deb01..dd44fdd8893 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs @@ -848,11 +848,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .resolve_fully_qualified_call(span, item_name, ty.normalized, qself.span, hir_id) .map(|r| { // lint bare trait if the method is found in the trait - if span.edition().at_least_rust_2021() - && let Some(diag) = - self.dcx().steal_diagnostic(qself.span, StashKey::TraitMissingMethod) - { - diag.emit(); + if span.edition().at_least_rust_2021() { + self.dcx().try_steal_modify_and_emit_err( + qself.span, + StashKey::TraitMissingMethod, + |_err| {}, + ); } r }) @@ -882,11 +883,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Emit the diagnostic for bare traits. (We used to cancel for slightly better // error messages, but cancelling stashed diagnostics is no longer allowed because // it causes problems when tracking whether errors have actually occurred.) - if span.edition().at_least_rust_2021() - && let Some(diag) = - self.dcx().steal_diagnostic(qself.span, StashKey::TraitMissingMethod) - { - diag.emit(); + if span.edition().at_least_rust_2021() { + self.dcx().try_steal_modify_and_emit_err( + qself.span, + StashKey::TraitMissingMethod, + |_err| {}, + ); } if item_name.name != kw::Empty { diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 61c52422d19..1311cc8968a 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -1941,53 +1941,49 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { errors_causecode: Vec<(Span, ObligationCauseCode<'tcx>)>, ) { for (span, code) in errors_causecode { - let Some(mut diag) = self.dcx().steal_diagnostic(span, StashKey::MaybeForgetReturn) - else { - continue; - }; - - if let Some(fn_sig) = self.body_fn_sig() - && let ExprBindingObligation(_, _, hir_id, ..) = code - && !fn_sig.output().is_unit() - { - let mut block_num = 0; - let mut found_semi = false; - for (_, node) in self.tcx.hir().parent_iter(hir_id) { - match node { - hir::Node::Stmt(stmt) => { - if let hir::StmtKind::Semi(expr) = stmt.kind { - let expr_ty = self.typeck_results.borrow().expr_ty(expr); - let return_ty = fn_sig.output(); - if !matches!(expr.kind, hir::ExprKind::Ret(..)) - && self.can_coerce(expr_ty, return_ty) - { - found_semi = true; + self.dcx().try_steal_modify_and_emit_err(span, StashKey::MaybeForgetReturn, |err| { + if let Some(fn_sig) = self.body_fn_sig() + && let ExprBindingObligation(_, _, hir_id, ..) = code + && !fn_sig.output().is_unit() + { + let mut block_num = 0; + let mut found_semi = false; + for (_, node) in self.tcx.hir().parent_iter(hir_id) { + match node { + hir::Node::Stmt(stmt) => { + if let hir::StmtKind::Semi(expr) = stmt.kind { + let expr_ty = self.typeck_results.borrow().expr_ty(expr); + let return_ty = fn_sig.output(); + if !matches!(expr.kind, hir::ExprKind::Ret(..)) + && self.can_coerce(expr_ty, return_ty) + { + found_semi = true; + } } } - } - hir::Node::Block(_block) => { - if found_semi { - block_num += 1; + hir::Node::Block(_block) => { + if found_semi { + block_num += 1; + } } - } - hir::Node::Item(item) => { - if let hir::ItemKind::Fn(..) = item.kind { - break; + hir::Node::Item(item) => { + if let hir::ItemKind::Fn(..) = item.kind { + break; + } } + _ => {} } - _ => {} + } + if block_num > 1 && found_semi { + err.span_suggestion_verbose( + span.shrink_to_lo(), + "you might have meant to return this to infer its type parameters", + "return ", + Applicability::MaybeIncorrect, + ); } } - if block_num > 1 && found_semi { - diag.span_suggestion_verbose( - span.shrink_to_lo(), - "you might have meant to return this to infer its type parameters", - "return ", - Applicability::MaybeIncorrect, - ); - } - } - diag.emit(); + }); } } diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index 28771ae40f5..e5b0d0ae0da 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -2194,59 +2194,59 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let [seg1, seg2] = segs else { return; }; - let Some(mut diag) = - self.dcx().steal_diagnostic(seg1.ident.span, StashKey::CallAssocMethod) - else { - return; - }; - - let map = self.infcx.tcx.hir(); - let body_id = self.tcx.hir().body_owned_by(self.body_id); - let body = map.body(body_id); - struct LetVisitor<'a> { - result: Option<&'a hir::Expr<'a>>, - ident_name: Symbol, - } - - // FIXME: This really should be taking scoping, etc into account. - impl<'v> Visitor<'v> for LetVisitor<'v> { - fn visit_stmt(&mut self, ex: &'v hir::Stmt<'v>) { - if let hir::StmtKind::Local(hir::Local { pat, init, .. }) = &ex.kind - && let Binding(_, _, ident, ..) = pat.kind - && ident.name == self.ident_name - { - self.result = *init; - } else { - hir::intravisit::walk_stmt(self, ex); + self.dcx().try_steal_modify_and_emit_err( + seg1.ident.span, + StashKey::CallAssocMethod, + |err| { + let map = self.infcx.tcx.hir(); + let body_id = self.tcx.hir().body_owned_by(self.body_id); + let body = map.body(body_id); + struct LetVisitor<'a> { + result: Option<&'a hir::Expr<'a>>, + ident_name: Symbol, } - } - } - let mut visitor = LetVisitor { result: None, ident_name: seg1.ident.name }; - visitor.visit_body(body); + // FIXME: This really should be taking scoping, etc into account. + impl<'v> Visitor<'v> for LetVisitor<'v> { + fn visit_stmt(&mut self, ex: &'v hir::Stmt<'v>) { + if let hir::StmtKind::Local(hir::Local { pat, init, .. }) = &ex.kind + && let Binding(_, _, ident, ..) = pat.kind + && ident.name == self.ident_name + { + self.result = *init; + } else { + hir::intravisit::walk_stmt(self, ex); + } + } + } - if let Node::Expr(call_expr) = self.tcx.parent_hir_node(seg1.hir_id) - && let Some(expr) = visitor.result - && let Some(self_ty) = self.node_ty_opt(expr.hir_id) - { - let probe = self.lookup_probe_for_diagnostic( - seg2.ident, - self_ty, - call_expr, - ProbeScope::TraitsInScope, - None, - ); - if probe.is_ok() { - let sm = self.infcx.tcx.sess.source_map(); - diag.span_suggestion_verbose( - sm.span_extend_while(seg1.ident.span.shrink_to_hi(), |c| c == ':').unwrap(), - "you may have meant to call an instance method", - ".", - Applicability::MaybeIncorrect, - ); - } - } - diag.emit(); + let mut visitor = LetVisitor { result: None, ident_name: seg1.ident.name }; + visitor.visit_body(body); + + if let Node::Expr(call_expr) = self.tcx.parent_hir_node(seg1.hir_id) + && let Some(expr) = visitor.result + && let Some(self_ty) = self.node_ty_opt(expr.hir_id) + { + let probe = self.lookup_probe_for_diagnostic( + seg2.ident, + self_ty, + call_expr, + ProbeScope::TraitsInScope, + None, + ); + if probe.is_ok() { + let sm = self.infcx.tcx.sess.source_map(); + err.span_suggestion_verbose( + sm.span_extend_while(seg1.ident.span.shrink_to_hi(), |c| c == ':') + .unwrap(), + "you may have meant to call an instance method", + ".", + Applicability::MaybeIncorrect, + ); + } + } + }, + ); } /// Suggest calling a method on a field i.e. `a.field.bar()` instead of `a.bar()` diff --git a/compiler/rustc_hir_typeck/src/writeback.rs b/compiler/rustc_hir_typeck/src/writeback.rs index ed102a7fac0..0740a3fd3e9 100644 --- a/compiler/rustc_hir_typeck/src/writeback.rs +++ b/compiler/rustc_hir_typeck/src/writeback.rs @@ -772,10 +772,6 @@ impl<'cx, 'tcx> Resolver<'cx, 'tcx> { fn report_error(&self, p: impl Into>) -> ErrorGuaranteed { if let Some(guar) = self.fcx.dcx().has_errors() { guar - } else if self.fcx.dcx().stashed_err_count() > 0 { - // Without this case we sometimes get uninteresting and extraneous - // "type annotations needed" errors. - self.fcx.dcx().delayed_bug("error in Resolver") } else { self.fcx .err_ctxt() diff --git a/compiler/rustc_infer/src/infer/at.rs b/compiler/rustc_infer/src/infer/at.rs index cc09a094688..a086d89b933 100644 --- a/compiler/rustc_infer/src/infer/at.rs +++ b/compiler/rustc_infer/src/infer/at.rs @@ -87,7 +87,6 @@ impl<'tcx> InferCtxt<'tcx> { reported_signature_mismatch: self.reported_signature_mismatch.clone(), tainted_by_errors: self.tainted_by_errors.clone(), err_count_on_creation: self.err_count_on_creation, - stashed_err_count_on_creation: self.stashed_err_count_on_creation, universe: self.universe.clone(), intercrate, next_trait_solver: self.next_trait_solver, diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index d7e16488508..6f52ded3551 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -306,12 +306,6 @@ pub struct InferCtxt<'tcx> { // FIXME(matthewjasper) Merge into `tainted_by_errors` err_count_on_creation: usize, - /// Track how many errors were stashed when this infcx is created. - /// Used for the same purpose as `err_count_on_creation`, even - /// though it's weaker because the count can go up and down. - // FIXME(matthewjasper) Merge into `tainted_by_errors` - stashed_err_count_on_creation: usize, - /// What is the innermost universe we have created? Starts out as /// `UniverseIndex::root()` but grows from there as we enter /// universal quantifiers. @@ -717,7 +711,6 @@ impl<'tcx> InferCtxtBuilder<'tcx> { reported_signature_mismatch: Default::default(), tainted_by_errors: Cell::new(None), err_count_on_creation: tcx.dcx().err_count_excluding_lint_errs(), - stashed_err_count_on_creation: tcx.dcx().stashed_err_count(), universe: Cell::new(ty::UniverseIndex::ROOT), intercrate, next_trait_solver, @@ -1274,14 +1267,6 @@ impl<'tcx> InferCtxt<'tcx> { let guar = self.dcx().has_errors().unwrap(); self.set_tainted_by_errors(guar); Some(guar) - } else if self.dcx().stashed_err_count() > self.stashed_err_count_on_creation { - // Errors stashed since this infcx was made. Not entirely reliable - // because the count of stashed errors can go down. But without - // this case we get a moderate number of uninteresting and - // extraneous "type annotations needed" errors. - let guar = self.dcx().delayed_bug("tainted_by_errors: stashed bug awaiting emission"); - self.set_tainted_by_errors(guar); - Some(guar) } else { None } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 9089d992cd5..6fdb03c0bab 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -847,17 +847,15 @@ impl<'tcx> OpaqueHiddenType<'tcx> { opaque_def_id: LocalDefId, tcx: TyCtxt<'tcx>, ) -> Result, ErrorGuaranteed> { - if let Some(diag) = tcx - .sess - .dcx() - .steal_diagnostic(tcx.def_span(opaque_def_id), StashKey::OpaqueHiddenTypeMismatch) - { - // We used to cancel here for slightly better error messages, but - // cancelling stashed diagnostics is no longer allowed because it - // causes problems when tracking whether errors have actually - // occurred. - diag.emit(); - } + // We used to cancel here for slightly better error messages, but + // cancelling stashed diagnostics is no longer allowed because it + // causes problems when tracking whether errors have actually + // occurred. + tcx.sess.dcx().try_steal_modify_and_emit_err( + tcx.def_span(opaque_def_id), + StashKey::OpaqueHiddenTypeMismatch, + |_err| {}, + ); (self.ty, other.ty).error_reported()?; // Found different concrete types for the opaque type. let sub_diag = if self.span == other.span { diff --git a/compiler/rustc_middle/src/ty/region.rs b/compiler/rustc_middle/src/ty/region.rs index b206727f051..51a4a9f411c 100644 --- a/compiler/rustc_middle/src/ty/region.rs +++ b/compiler/rustc_middle/src/ty/region.rs @@ -91,8 +91,8 @@ impl<'tcx> Region<'tcx> { /// Constructs a `RegionKind::ReError` region. #[track_caller] - pub fn new_error(tcx: TyCtxt<'tcx>, reported: ErrorGuaranteed) -> Region<'tcx> { - tcx.intern_region(ty::ReError(reported)) + pub fn new_error(tcx: TyCtxt<'tcx>, guar: ErrorGuaranteed) -> Region<'tcx> { + tcx.intern_region(ty::ReError(guar)) } /// Constructs a `RegionKind::ReError` region and registers a delayed bug to ensure it gets diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 27c78d18d19..06be8191dc4 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -1528,8 +1528,8 @@ impl<'tcx> Ty<'tcx> { } /// Constructs a `TyKind::Error` type with current `ErrorGuaranteed` - pub fn new_error(tcx: TyCtxt<'tcx>, reported: ErrorGuaranteed) -> Ty<'tcx> { - Ty::new(tcx, Error(reported)) + pub fn new_error(tcx: TyCtxt<'tcx>, guar: ErrorGuaranteed) -> Ty<'tcx> { + Ty::new(tcx, Error(guar)) } /// Constructs a `TyKind::Error` type and registers a `span_delayed_bug` to ensure it gets used. diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index f5a7bfd42ff..316a9c4f8df 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1762,24 +1762,25 @@ impl<'a> Parser<'a> { err: impl FnOnce(&Self) -> Diag<'a>, ) -> L { assert!(could_be_unclosed_char_literal(ident)); - if let Some(diag) = self.dcx().steal_diagnostic(ident.span, StashKey::LifetimeIsChar) { - diag.with_span_suggestion_verbose( - ident.span.shrink_to_hi(), - "add `'` to close the char literal", - "'", - Applicability::MaybeIncorrect, - ) - .emit(); - } else { - err(self) - .with_span_suggestion_verbose( + self.dcx() + .try_steal_modify_and_emit_err(ident.span, StashKey::LifetimeIsChar, |err| { + err.span_suggestion_verbose( ident.span.shrink_to_hi(), "add `'` to close the char literal", "'", Applicability::MaybeIncorrect, - ) - .emit(); - } + ); + }) + .unwrap_or_else(|| { + err(self) + .with_span_suggestion_verbose( + ident.span.shrink_to_hi(), + "add `'` to close the char literal", + "'", + Applicability::MaybeIncorrect, + ) + .emit() + }); let name = ident.without_first_quote().name; mk_lit_char(name, ident.span) } diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index c12d35ec73c..04c0cf7436e 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -2520,14 +2520,6 @@ fn check_invalid_crate_level_attr(tcx: TyCtxt<'_>, attrs: &[Attribute]) { if attr.style == AttrStyle::Inner { for attr_to_check in ATTRS_TO_CHECK { if attr.has_name(*attr_to_check) { - if let AttrKind::Normal(ref p) = attr.kind - && let Some(diag) = tcx.dcx().steal_diagnostic( - p.item.path.span, - StashKey::UndeterminedMacroResolution, - ) - { - diag.cancel(); - } let item = tcx .hir() .items() @@ -2537,7 +2529,7 @@ fn check_invalid_crate_level_attr(tcx: TyCtxt<'_>, attrs: &[Attribute]) { span: item.ident.span, kind: item.kind.descr(), }); - tcx.dcx().emit_err(errors::InvalidAttrAtCrateLevel { + let err = tcx.dcx().create_err(errors::InvalidAttrAtCrateLevel { span: attr.span, sugg_span: tcx .sess @@ -2553,6 +2545,16 @@ fn check_invalid_crate_level_attr(tcx: TyCtxt<'_>, attrs: &[Attribute]) { name: *attr_to_check, item, }); + + if let AttrKind::Normal(ref p) = attr.kind { + tcx.dcx().try_steal_replace_and_emit_err( + p.item.path.span, + StashKey::UndeterminedMacroResolution, + err, + ); + } else { + err.emit(); + } } } } diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index e4c596b10b8..40517407ee6 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -149,8 +149,7 @@ where let guar = if let Some(root) = cycle_error.cycle.first() && let Some(span) = root.query.span { - error.stash(span, StashKey::Cycle); - qcx.dep_context().sess().dcx().span_delayed_bug(span, "delayed cycle error") + error.stash(span, StashKey::Cycle).unwrap() } else { error.emit() }; diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index 752bb05f3d7..1b79786c1b8 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/compiler/rustc_session/src/parse.rs @@ -105,8 +105,7 @@ pub fn feature_err_issue( // Cancel an earlier warning for this same error, if it exists. if let Some(span) = span.primary_span() { - if let Some(err) = sess.parse_sess.dcx.steal_diagnostic(span, StashKey::EarlySyntaxWarning) - { + if let Some(err) = sess.parse_sess.dcx.steal_non_err(span, StashKey::EarlySyntaxWarning) { err.cancel() } } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index dcbb63f00f7..d9e49f1eb0a 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -889,7 +889,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } } - SelectionError::OpaqueTypeAutoTraitLeakageUnknown(def_id) => self.report_opaque_type_auto_trait_leakage( + SelectionError::OpaqueTypeAutoTraitLeakageUnknown(def_id) => return self.report_opaque_type_auto_trait_leakage( &obligation, def_id, ), @@ -2252,8 +2252,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { ErrorCode::E0282, false, ); - err.stash(span, StashKey::MaybeForgetReturn); - return self.dcx().delayed_bug("stashed error never reported"); + return err.stash(span, StashKey::MaybeForgetReturn).unwrap(); } Some(e) => return e, } @@ -2766,7 +2765,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { self.suggest_unsized_bound_if_applicable(err, obligation); if let Some(span) = err.span.primary_span() && let Some(mut diag) = - self.tcx.dcx().steal_diagnostic(span, StashKey::AssociatedTypeSuggestion) + self.tcx.dcx().steal_non_err(span, StashKey::AssociatedTypeSuggestion) && let Ok(ref mut s1) = err.suggestions && let Ok(ref mut s2) = diag.suggestions { @@ -3291,7 +3290,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { &self, obligation: &PredicateObligation<'tcx>, def_id: DefId, - ) -> Diag<'tcx> { + ) -> ErrorGuaranteed { let name = match self.tcx.opaque_type_origin(def_id.expect_local()) { hir::OpaqueTyOrigin::FnReturn(_) | hir::OpaqueTyOrigin::AsyncFn(_) => { "opaque type".to_string() @@ -3318,12 +3317,9 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } }; - if let Some(diag) = self.dcx().steal_diagnostic(self.tcx.def_span(def_id), StashKey::Cycle) - { - diag.cancel(); - } - - err + self.note_obligation_cause(&mut err, &obligation); + self.point_at_returns_when_relevant(&mut err, &obligation); + self.dcx().try_steal_replace_and_emit_err(self.tcx.def_span(def_id), StashKey::Cycle, err) } fn report_signature_mismatch_error( diff --git a/src/tools/clippy/tests/ui/crashes/unreachable-array-or-slice.rs b/src/tools/clippy/tests/ui/crashes/unreachable-array-or-slice.rs new file mode 100644 index 00000000000..b56abccbd41 --- /dev/null +++ b/src/tools/clippy/tests/ui/crashes/unreachable-array-or-slice.rs @@ -0,0 +1,8 @@ +struct Foo(isize, isize, isize, isize); + +pub fn main() { + let Self::anything_here_kills_it(a, b, ..) = Foo(5, 5, 5, 5); + match [5, 5, 5, 5] { + [..] => { } + } +} diff --git a/src/tools/clippy/tests/ui/crashes/unreachable-array-or-slice.stderr b/src/tools/clippy/tests/ui/crashes/unreachable-array-or-slice.stderr new file mode 100644 index 00000000000..9e0d3b934b8 --- /dev/null +++ b/src/tools/clippy/tests/ui/crashes/unreachable-array-or-slice.stderr @@ -0,0 +1,9 @@ +error[E0433]: failed to resolve: `Self` is only available in impls, traits, and type definitions + --> tests/ui/crashes/unreachable-array-or-slice.rs:4:9 + | +LL | let Self::anything_here_kills_it(a, b, ..) = Foo(5, 5, 5, 5); + | ^^^^ `Self` is only available in impls, traits, and type definitions + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0433`. diff --git a/tests/ui/foreign/stashed-issue-121451.rs b/tests/ui/foreign/stashed-issue-121451.rs new file mode 100644 index 00000000000..97a4af37475 --- /dev/null +++ b/tests/ui/foreign/stashed-issue-121451.rs @@ -0,0 +1,4 @@ +extern "C" fn _f() -> libc::uintptr_t {} +//~^ ERROR failed to resolve: use of undeclared crate or module `libc` + +fn main() {} diff --git a/tests/ui/foreign/stashed-issue-121451.stderr b/tests/ui/foreign/stashed-issue-121451.stderr new file mode 100644 index 00000000000..440d98d6f46 --- /dev/null +++ b/tests/ui/foreign/stashed-issue-121451.stderr @@ -0,0 +1,9 @@ +error[E0433]: failed to resolve: use of undeclared crate or module `libc` + --> $DIR/stashed-issue-121451.rs:1:23 + | +LL | extern "C" fn _f() -> libc::uintptr_t {} + | ^^^^ use of undeclared crate or module `libc` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0433`. diff --git a/tests/ui/impl-trait/stashed-diag-issue-121504.rs b/tests/ui/impl-trait/stashed-diag-issue-121504.rs new file mode 100644 index 00000000000..4ac8ffe8931 --- /dev/null +++ b/tests/ui/impl-trait/stashed-diag-issue-121504.rs @@ -0,0 +1,13 @@ +//@ edition: 2021 + +trait MyTrait { + async fn foo(self) -> (Self, i32); +} + +impl MyTrait for xyz::T { //~ ERROR failed to resolve: use of undeclared crate or module `xyz` + async fn foo(self, key: i32) -> (u32, i32) { + (self, key) + } +} + +fn main() {} diff --git a/tests/ui/impl-trait/stashed-diag-issue-121504.stderr b/tests/ui/impl-trait/stashed-diag-issue-121504.stderr new file mode 100644 index 00000000000..6a881dc7f9f --- /dev/null +++ b/tests/ui/impl-trait/stashed-diag-issue-121504.stderr @@ -0,0 +1,9 @@ +error[E0433]: failed to resolve: use of undeclared crate or module `xyz` + --> $DIR/stashed-diag-issue-121504.rs:7:18 + | +LL | impl MyTrait for xyz::T { + | ^^^ use of undeclared crate or module `xyz` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0433`. diff --git a/tests/ui/lowering/issue-121108.rs b/tests/ui/lowering/issue-121108.rs index 6b2dd24e4a8..6dfb7e00821 100644 --- a/tests/ui/lowering/issue-121108.rs +++ b/tests/ui/lowering/issue-121108.rs @@ -3,7 +3,7 @@ use std::ptr::addr_of; const UNINHABITED_VARIANT: () = unsafe { - let v = *addr_of!(data).cast(); //~ ERROR cannot determine resolution for the macro `addr_of` + let v = *addr_of!(data).cast(); }; fn main() {} diff --git a/tests/ui/lowering/issue-121108.stderr b/tests/ui/lowering/issue-121108.stderr index c2c5746d6f1..e4942e8cb07 100644 --- a/tests/ui/lowering/issue-121108.stderr +++ b/tests/ui/lowering/issue-121108.stderr @@ -13,13 +13,5 @@ LL - #![derive(Clone, Copy)] LL + #[derive(Clone, Copy)] | -error: cannot determine resolution for the macro `addr_of` - --> $DIR/issue-121108.rs:6:14 - | -LL | let v = *addr_of!(data).cast(); - | ^^^^^^^ - | - = note: import resolution is stuck, try simplifying macro imports - -error: aborting due to 2 previous errors +error: aborting due to 1 previous error From b4e9f93eb47fa7405749e74d2c17c2add245a877 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 27 Feb 2024 15:02:54 +1100 Subject: [PATCH 21/37] Mark some once-again-unreachable paths as unreachable. This undoes the changes from #121482 and #121586, now that stashed errors will trigger more early aborts. --- compiler/rustc_lint/src/array_into_iter.rs | 14 +++++--------- compiler/rustc_privacy/src/lib.rs | 5 +---- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_lint/src/array_into_iter.rs b/compiler/rustc_lint/src/array_into_iter.rs index 993b1d739a1..3a5c585366a 100644 --- a/compiler/rustc_lint/src/array_into_iter.rs +++ b/compiler/rustc_lint/src/array_into_iter.rs @@ -70,15 +70,11 @@ impl<'tcx> LateLintPass<'tcx> for ArrayIntoIter { // Check if the method call actually calls the libcore // `IntoIterator::into_iter`. - let trait_id = cx - .typeck_results() - .type_dependent_def_id(expr.hir_id) - .and_then(|did| cx.tcx.trait_of_item(did)); - if trait_id.is_none() - || !cx.tcx.is_diagnostic_item(sym::IntoIterator, trait_id.unwrap()) - { - return; - } + let def_id = cx.typeck_results().type_dependent_def_id(expr.hir_id).unwrap(); + match cx.tcx.trait_of_item(def_id) { + Some(trait_id) if cx.tcx.is_diagnostic_item(sym::IntoIterator, trait_id) => {} + _ => return, + }; // As this is a method call expression, we have at least one argument. let receiver_ty = cx.typeck_results().expr_ty(receiver_arg); diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 1c6bd887128..9d8a9f5fce3 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -988,10 +988,7 @@ impl<'tcx> Visitor<'tcx> for NamePrivacyVisitor<'tcx> { fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) { if let hir::ExprKind::Struct(qpath, fields, ref base) = expr.kind { let res = self.typeck_results().qpath_res(qpath, expr.hir_id); - let Some(adt) = self.typeck_results().expr_ty(expr).ty_adt_def() else { - self.tcx.dcx().span_delayed_bug(expr.span, "no adt_def for expression"); - return; - }; + let adt = self.typeck_results().expr_ty(expr).ty_adt_def().unwrap(); let variant = adt.variant_of_res(res); if let Some(base) = *base { // If the expression uses FRU we need to make sure all the unmentioned fields From 9aff357e5398320962137a8885b924231904ef08 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 27 Feb 2024 15:52:29 +1100 Subject: [PATCH 22/37] Stop miri if delayed bugs are present. Seems wise, since it shouldn't proceed in that case. --- src/tools/miri/src/bin/miri.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 8a7133fea43..281a32b77c5 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -68,7 +68,7 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { queries: &'tcx rustc_interface::Queries<'tcx>, ) -> Compilation { queries.global_ctxt().unwrap().enter(|tcx| { - if tcx.sess.dcx().has_errors().is_some() { + if tcx.sess.dcx().has_errors_or_delayed_bugs().is_some() { tcx.dcx().fatal("miri cannot be run on programs that fail compilation"); } From 82961c0abcd7b9ea73fb87fd049e2e853abd5787 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 28 Feb 2024 11:00:27 +1100 Subject: [PATCH 23/37] Reinstate `emit_stashed_diagnostics` in `DiagCtxtInner::drop`. I removed it in #121206 because I thought thought it wasn't necessary. But then I had to add an `emit_stashed_diagnostics` call elsewhere in rustfmt to avoid the assertion failure (which took two attempts to get right, #121487 and #121615), and now there's an assertion failure in clippy as well (https://github.com/rust-lang/rust-clippy/issues/12364). So this commit just reinstates the call in `DiagCtxtInner::drop`. It also reverts the rustfmt changes from #121487 and #121615, though it keeps the tests added for those PRs. --- compiler/rustc_errors/src/lib.rs | 10 ++++++---- src/tools/rustfmt/src/formatting.rs | 21 ++++++--------------- src/tools/rustfmt/src/parse/session.rs | 8 +------- 3 files changed, 13 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 60bda9f4c94..186ec41f7cd 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -555,12 +555,14 @@ pub struct DiagCtxtFlags { impl Drop for DiagCtxtInner { fn drop(&mut self) { - // Any stashed diagnostics should have been handled by - // `emit_stashed_diagnostics` by now. + // For tools using `interface::run_compiler` (e.g. rustc, rustdoc) + // stashed diagnostics will have already been emitted. But for others + // that don't use `interface::run_compiler` (e.g. rustfmt, some clippy + // lints) this fallback is necessary. // // Important: it is sound to produce an `ErrorGuaranteed` when stashing - // errors because they are guaranteed to have been emitted by here. - assert!(self.stashed_diagnostics.is_empty()); + // errors because they are guaranteed to be emitted here or earlier. + self.emit_stashed_diagnostics(); // Important: it is sound to produce an `ErrorGuaranteed` when emitting // delayed bugs because they are guaranteed to be emitted here if diff --git a/src/tools/rustfmt/src/formatting.rs b/src/tools/rustfmt/src/formatting.rs index 323ae83fe6e..1c64921b1a6 100644 --- a/src/tools/rustfmt/src/formatting.rs +++ b/src/tools/rustfmt/src/formatting.rs @@ -109,7 +109,7 @@ fn format_project( let main_file = input.file_name(); let input_is_stdin = main_file == FileName::Stdin; - let mut parse_session = ParseSess::new(config)?; + let parse_session = ParseSess::new(config)?; if config.skip_children() && parse_session.ignore_file(&main_file) { return Ok(FormatReport::new()); } @@ -118,20 +118,11 @@ fn format_project( let mut report = FormatReport::new(); let directory_ownership = input.to_directory_ownership(); - // rustfmt doesn't use `run_compiler` like other tools, so it must emit any - // stashed diagnostics itself, otherwise the `DiagCtxt` will assert when - // dropped. The final result here combines the parsing result and the - // `emit_stashed_diagnostics` result. - let parse_res = Parser::parse_crate(input, &parse_session); - let stashed_res = parse_session.emit_stashed_diagnostics(); - let krate = match (parse_res, stashed_res) { - (Ok(krate), None) => krate, - (parse_res, _) => { - // Surface parse error via Session (errors are merged there from report). - let forbid_verbose = match parse_res { - Err(e) if e != ParserError::ParsePanicError => true, - _ => input_is_stdin, - }; + let krate = match Parser::parse_crate(input, &parse_session) { + Ok(krate) => krate, + // Surface parse error via Session (errors are merged there from report) + Err(e) => { + let forbid_verbose = input_is_stdin || e != ParserError::ParsePanicError; should_emit_verbose(forbid_verbose, config, || { eprintln!("The Rust parser panicked"); }); diff --git a/src/tools/rustfmt/src/parse/session.rs b/src/tools/rustfmt/src/parse/session.rs index e33f1ca755c..f5defe63c13 100644 --- a/src/tools/rustfmt/src/parse/session.rs +++ b/src/tools/rustfmt/src/parse/session.rs @@ -5,9 +5,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use rustc_data_structures::sync::{IntoDynSyncSend, Lrc}; use rustc_errors::emitter::{DynEmitter, Emitter, HumanEmitter}; use rustc_errors::translation::Translate; -use rustc_errors::{ - ColorConfig, Diag, DiagCtxt, DiagInner, ErrorGuaranteed, Level as DiagnosticLevel, -}; +use rustc_errors::{ColorConfig, Diag, DiagCtxt, DiagInner, Level as DiagnosticLevel}; use rustc_session::parse::ParseSess as RawParseSess; use rustc_span::{ source_map::{FilePathMapping, SourceMap}, @@ -230,10 +228,6 @@ impl ParseSess { self.ignore_path_set.as_ref().is_match(path) } - pub(crate) fn emit_stashed_diagnostics(&mut self) -> Option { - self.parse_sess.dcx.emit_stashed_diagnostics() - } - pub(crate) fn set_silent_emitter(&mut self) { self.parse_sess.dcx = DiagCtxt::with_emitter(silent_emitter()); } From c1f01638af37ec287fe09ae41a6c784b42875438 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Feb 2024 09:09:19 +1100 Subject: [PATCH 24/37] Minor visibility and formatting improvements. --- compiler/rustc_errors/src/emitter.rs | 20 +++++++++++--------- compiler/rustc_errors/src/error.rs | 3 +++ compiler/rustc_errors/src/lib.rs | 12 ++++++------ 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 304922018eb..967b7674e05 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -130,8 +130,8 @@ impl Margin { fn was_cut_right(&self, line_len: usize) -> bool { let right = if self.computed_right == self.span_right || self.computed_right == self.label_right { - // Account for the "..." padding given above. Otherwise we end up with code lines that - // do fit but end in "..." as if they were trimmed. + // Account for the "..." padding given above. Otherwise we end up with code lines + // that do fit but end in "..." as if they were trimmed. self.computed_right - 6 } else { self.computed_right @@ -657,9 +657,9 @@ pub struct HumanEmitter { } #[derive(Debug)] -pub struct FileWithAnnotatedLines { - pub file: Lrc, - pub lines: Vec, +pub(crate) struct FileWithAnnotatedLines { + pub(crate) file: Lrc, + pub(crate) lines: Vec, multiline_depth: usize, } @@ -724,8 +724,9 @@ impl HumanEmitter { .skip(left) .take_while(|ch| { // Make sure that the trimming on the right will fall within the terminal width. - // FIXME: `unicode_width` sometimes disagrees with terminals on how wide a `char` is. - // For now, just accept that sometimes the code line will be longer than desired. + // FIXME: `unicode_width` sometimes disagrees with terminals on how wide a `char` + // is. For now, just accept that sometimes the code line will be longer than + // desired. let next = unicode_width::UnicodeWidthChar::width(*ch).unwrap_or(1); if taken + next > right - left { return false; @@ -2228,8 +2229,8 @@ impl HumanEmitter { buffer.puts(*row_num - 1, max_line_num_len + 3, &line, Style::NoStyle); *row_num += 1; } - // If the last line is exactly equal to the line we need to add, we can skip both of them. - // This allows us to avoid output like the following: + // If the last line is exactly equal to the line we need to add, we can skip both of + // them. This allows us to avoid output like the following: // 2 - & // 2 + if true { true } else { false } // 3 - if true { true } else { false } @@ -2586,6 +2587,7 @@ fn num_overlap( let extra = if inclusive { 1 } else { 0 }; (b_start..b_end + extra).contains(&a_start) || (a_start..a_end + extra).contains(&b_start) } + fn overlaps(a1: &Annotation, a2: &Annotation, padding: usize) -> bool { num_overlap( a1.start_col.display, diff --git a/compiler/rustc_errors/src/error.rs b/compiler/rustc_errors/src/error.rs index ec0a2fe8cd8..ca818a4d832 100644 --- a/compiler/rustc_errors/src/error.rs +++ b/compiler/rustc_errors/src/error.rs @@ -23,9 +23,11 @@ impl<'args> TranslateError<'args> { pub fn message(id: &'args Cow<'args, str>, args: &'args FluentArgs<'args>) -> Self { Self::One { id, args, kind: TranslateErrorKind::MessageMissing } } + pub fn primary(id: &'args Cow<'args, str>, args: &'args FluentArgs<'args>) -> Self { Self::One { id, args, kind: TranslateErrorKind::PrimaryBundleMissing } } + pub fn attribute( id: &'args Cow<'args, str>, args: &'args FluentArgs<'args>, @@ -33,6 +35,7 @@ impl<'args> TranslateError<'args> { ) -> Self { Self::One { id, args, kind: TranslateErrorKind::AttributeMissing { attr } } } + pub fn value(id: &'args Cow<'args, str>, args: &'args FluentArgs<'args>) -> Self { Self::One { id, args, kind: TranslateErrorKind::ValueMissing } } diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index bc338b01d8b..bb6f031cec6 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -217,10 +217,10 @@ impl CodeSuggestion { use rustc_span::{CharPos, Pos}; - /// Extracts a substring from the provided `line_opt` based on the specified low and high indices, - /// appends it to the given buffer `buf`, and returns the count of newline characters in the substring - /// for accurate highlighting. - /// If `line_opt` is `None`, a newline character is appended to the buffer, and 0 is returned. + /// Extracts a substring from the provided `line_opt` based on the specified low and high + /// indices, appends it to the given buffer `buf`, and returns the count of newline + /// characters in the substring for accurate highlighting. If `line_opt` is `None`, a + /// newline character is appended to the buffer, and 0 is returned. /// /// ## Returns /// @@ -486,8 +486,8 @@ struct DiagCtxtInner { /// have been converted. check_unstable_expect_diagnostics: bool, - /// Expected [`DiagInner`][struct@diagnostic::DiagInner]s store a [`LintExpectationId`] as part of - /// the lint level. [`LintExpectationId`]s created early during the compilation + /// Expected [`DiagInner`][struct@diagnostic::DiagInner]s store a [`LintExpectationId`] as part + /// of the lint level. [`LintExpectationId`]s created early during the compilation /// (before `HirId`s have been defined) are not stable and can therefore not be /// stored on disk. This buffer stores these diagnostics until the ID has been /// replaced by a stable [`LintExpectationId`]. The [`DiagInner`][struct@diagnostic::DiagInner]s From 869bd03a04fa2c6f95dc78523d098ed6b1e88b04 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Feb 2024 09:27:11 +1100 Subject: [PATCH 25/37] Simplify `UnusedExterns` lifetimes. In practice, 'a and 'b and 'c are always the same. This change makes `UnusedExterns` more like `ArtifactNotification`, which uses a single lifetime 'a in multiple ways. --- compiler/rustc_errors/src/json.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 88a83c8bf78..35030638f4c 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -162,7 +162,7 @@ enum EmitTyped<'a> { Diagnostic(Diagnostic), Artifact(ArtifactNotification<'a>), FutureIncompat(FutureIncompatReport<'a>), - UnusedExtern(UnusedExterns<'a, 'a, 'a>), + UnusedExtern(UnusedExterns<'a>), } impl Translate for JsonEmitter { @@ -332,11 +332,11 @@ struct FutureIncompatReport<'a> { // We could unify this struct the one in rustdoc but they have different // ownership semantics, so doing so would create wasteful allocations. #[derive(Serialize)] -struct UnusedExterns<'a, 'b, 'c> { +struct UnusedExterns<'a> { /// The severity level of the unused dependencies lint lint_level: &'a str, /// List of unused externs by their names. - unused_extern_names: &'b [&'c str], + unused_extern_names: &'a [&'a str], } impl Diagnostic { From 3c3f15cafe2945bd3b0db899b5288c678d392b2c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Feb 2024 10:14:21 +1100 Subject: [PATCH 26/37] Use `Destination` more. --- compiler/rustc_errors/src/emitter.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 967b7674e05..cbe973330dc 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -60,7 +60,7 @@ impl HumanReadableErrorType { } pub fn new_emitter( self, - mut dst: Box, + mut dst: Destination, fallback_bundle: LazyFallbackBundle, ) -> HumanEmitter { let (short, color_config) = self.unzip(); @@ -686,10 +686,7 @@ impl HumanEmitter { } } - pub fn new( - dst: Box, - fallback_bundle: LazyFallbackBundle, - ) -> HumanEmitter { + pub fn new(dst: Destination, fallback_bundle: LazyFallbackBundle) -> HumanEmitter { Self::create(dst, fallback_bundle) } @@ -2634,7 +2631,7 @@ fn emit_to_destination( Ok(()) } -pub type Destination = Box<(dyn WriteColor + Send)>; +pub type Destination = Box; struct Buffy { buffer_writer: BufferWriter, From 805e50e71bfdb870011b4bb128929c6103af602f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Feb 2024 13:36:46 +1100 Subject: [PATCH 27/37] Remove unnecessary `diagnostic_width` call. This `HumanEmitter` is only created to test if it supports colour. The diagnostic width isn't relevant. --- src/librustdoc/doctest.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index e2b4ec5908b..8390b34dba9 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -576,9 +576,8 @@ pub(crate) fn make_test( rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), false, ); - supports_color = HumanEmitter::stderr(ColorConfig::Auto, fallback_bundle.clone()) - .diagnostic_width(Some(80)) - .supports_color(); + supports_color = + HumanEmitter::stderr(ColorConfig::Auto, fallback_bundle.clone()).supports_color(); let emitter = HumanEmitter::new(Box::new(io::sink()), fallback_bundle); From ca5b79ddf7325fa7f6a88c170edbffa1757b149e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Feb 2024 13:55:16 +1100 Subject: [PATCH 28/37] Remove unnecessary `output` local variable. --- compiler/rustc_errors/src/json.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 35030638f4c..94d0c5d060e 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -405,9 +405,8 @@ impl Diagnostic { .collect(); let buf = BufWriter::default(); - let output = buf.clone(); je.json_rendered - .new_emitter(Box::new(buf), je.fallback_bundle.clone()) + .new_emitter(Box::new(buf.clone()), je.fallback_bundle.clone()) .sm(Some(je.sm.clone())) .fluent_bundle(je.fluent_bundle.clone()) .diagnostic_width(je.diagnostic_width) @@ -417,8 +416,8 @@ impl Diagnostic { .ui_testing(je.ui_testing) .ignored_directories_in_source_blocks(je.ignored_directories_in_source_blocks.clone()) .emit_diagnostic(diag); - let output = Arc::try_unwrap(output.0).unwrap().into_inner().unwrap(); - let output = String::from_utf8(output).unwrap(); + let buf = Arc::try_unwrap(buf.0).unwrap().into_inner().unwrap(); + let buf = String::from_utf8(buf).unwrap(); Diagnostic { message: translated_message.to_string(), @@ -426,7 +425,7 @@ impl Diagnostic { level, spans, children, - rendered: Some(output), + rendered: Some(buf), } } From f9eef38e32f5e362006122d4a11d8e1f41e2f033 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Feb 2024 11:40:50 +1100 Subject: [PATCH 29/37] Inline and remove `DiagCtxt::with_tty_emitter` It only has two call sites, and one of those doesn't set the source map. --- compiler/rustc_errors/src/lib.rs | 11 ++--------- compiler/rustc_session/src/parse.rs | 12 ++++++++---- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index bb6f031cec6..72b3c88ee03 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -59,11 +59,11 @@ pub use snippet::Style; // See https://github.com/rust-lang/rust/pull/115393. pub use termcolor::{Color, ColorSpec, WriteColor}; -use emitter::{is_case_difference, DynEmitter, Emitter, HumanEmitter}; +use emitter::{is_case_difference, DynEmitter, Emitter}; use registry::Registry; use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet}; use rustc_data_structures::stable_hasher::{Hash128, StableHasher}; -use rustc_data_structures::sync::{Lock, Lrc}; +use rustc_data_structures::sync::Lock; use rustc_data_structures::AtomicRef; use rustc_lint_defs::LintExpectationId; use rustc_span::source_map::SourceMap; @@ -586,13 +586,6 @@ impl Drop for DiagCtxtInner { } impl DiagCtxt { - pub fn with_tty_emitter( - sm: Option>, - fallback_bundle: LazyFallbackBundle, - ) -> Self { - let emitter = Box::new(HumanEmitter::stderr(ColorConfig::Auto, fallback_bundle).sm(sm)); - Self::with_emitter(emitter) - } pub fn disable_warnings(mut self) -> Self { self.inner.get_mut().flags.can_emit_warnings = false; self diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index 752bb05f3d7..c88a1186965 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/compiler/rustc_session/src/parse.rs @@ -13,9 +13,10 @@ use crate::Session; use rustc_ast::node_id::NodeId; use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet}; use rustc_data_structures::sync::{AppendOnlyVec, Lock, Lrc}; -use rustc_errors::{emitter::SilentEmitter, DiagCtxt}; +use rustc_errors::emitter::{HumanEmitter, SilentEmitter}; use rustc_errors::{ - fallback_fluent_bundle, Diag, DiagnosticMessage, EmissionGuarantee, MultiSpan, StashKey, + fallback_fluent_bundle, ColorConfig, Diag, DiagCtxt, DiagnosticMessage, EmissionGuarantee, + MultiSpan, StashKey, }; use rustc_feature::{find_feature_issue, GateIssue, UnstableFeatures}; use rustc_span::edition::Edition; @@ -236,7 +237,9 @@ impl ParseSess { pub fn new(locale_resources: Vec<&'static str>, file_path_mapping: FilePathMapping) -> Self { let fallback_bundle = fallback_fluent_bundle(locale_resources, false); let sm = Lrc::new(SourceMap::new(file_path_mapping)); - let dcx = DiagCtxt::with_tty_emitter(Some(sm.clone()), fallback_bundle); + let emitter = + Box::new(HumanEmitter::stderr(ColorConfig::Auto, fallback_bundle).sm(Some(sm.clone()))); + let dcx = DiagCtxt::with_emitter(emitter); ParseSess::with_dcx(dcx, sm) } @@ -265,7 +268,8 @@ impl ParseSess { pub fn with_silent_emitter(fatal_note: String) -> Self { let fallback_bundle = fallback_fluent_bundle(Vec::new(), false); let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); - let fatal_dcx = DiagCtxt::with_tty_emitter(None, fallback_bundle).disable_warnings(); + let emitter = Box::new(HumanEmitter::stderr(ColorConfig::Auto, fallback_bundle)); + let fatal_dcx = DiagCtxt::with_emitter(emitter); let dcx = DiagCtxt::with_emitter(Box::new(SilentEmitter { fatal_dcx, fatal_note })) .disable_warnings(); ParseSess::with_dcx(dcx, sm) From 880c1c585f1252a8b63b402b44def940cac6f134 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Feb 2024 11:50:32 +1100 Subject: [PATCH 30/37] Rename `DiagCtxt::with_emitter` as `DiagCtxt::new`. Because it's now the only constructor. --- compiler/rustc_codegen_ssa/src/back/write.rs | 2 +- compiler/rustc_driver_impl/src/lib.rs | 2 +- compiler/rustc_errors/src/json/tests.rs | 2 +- compiler/rustc_errors/src/lib.rs | 2 +- compiler/rustc_expand/src/tests.rs | 2 +- compiler/rustc_session/src/parse.rs | 8 ++++---- compiler/rustc_session/src/session.rs | 8 ++++---- src/librustdoc/core.rs | 2 +- src/librustdoc/doctest.rs | 4 ++-- src/librustdoc/passes/lint/check_code_block_syntax.rs | 2 +- .../clippy/clippy_lints/src/doc/needless_doctest_main.rs | 2 +- src/tools/rustfmt/src/parse/session.rs | 4 ++-- 12 files changed, 20 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index c784ee48675..97089dff31b 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -373,7 +373,7 @@ pub struct CodegenContext { impl CodegenContext { pub fn create_dcx(&self) -> DiagCtxt { - DiagCtxt::with_emitter(Box::new(self.diag_emitter.clone())) + DiagCtxt::new(Box::new(self.diag_emitter.clone())) } pub fn config(&self, kind: ModuleKind) -> &ModuleConfig { diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 692c059beb0..10188026a97 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -1388,7 +1388,7 @@ fn report_ice( rustc_errors::ColorConfig::Auto, fallback_bundle, )); - let dcx = rustc_errors::DiagCtxt::with_emitter(emitter); + let dcx = rustc_errors::DiagCtxt::new(emitter); // a .span_bug or .bug call has already printed what // it wants to print. diff --git a/compiler/rustc_errors/src/json/tests.rs b/compiler/rustc_errors/src/json/tests.rs index 303de0a93f6..ba8d58e159e 100644 --- a/compiler/rustc_errors/src/json/tests.rs +++ b/compiler/rustc_errors/src/json/tests.rs @@ -61,7 +61,7 @@ fn test_positions(code: &str, span: (u32, u32), expected_output: SpanTestData) { ); let span = Span::with_root_ctxt(BytePos(span.0), BytePos(span.1)); - let dcx = DiagCtxt::with_emitter(Box::new(je)); + let dcx = DiagCtxt::new(Box::new(je)); dcx.span_err(span, "foo"); let bytes = output.lock().unwrap(); diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 72b3c88ee03..a5e6aed13eb 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -601,7 +601,7 @@ impl DiagCtxt { self } - pub fn with_emitter(emitter: Box) -> Self { + pub fn new(emitter: Box) -> Self { Self { inner: Lock::new(DiagCtxtInner { flags: DiagCtxtFlags { can_emit_warnings: true, ..Default::default() }, diff --git a/compiler/rustc_expand/src/tests.rs b/compiler/rustc_expand/src/tests.rs index 3c14ad5e7b8..b242ce795fd 100644 --- a/compiler/rustc_expand/src/tests.rs +++ b/compiler/rustc_expand/src/tests.rs @@ -33,7 +33,7 @@ fn create_test_handler() -> (DiagCtxt, Lrc, Arc>>) { let emitter = HumanEmitter::new(Box::new(Shared { data: output.clone() }), fallback_bundle) .sm(Some(source_map.clone())) .diagnostic_width(Some(140)); - let dcx = DiagCtxt::with_emitter(Box::new(emitter)); + let dcx = DiagCtxt::new(Box::new(emitter)); (dcx, source_map, output) } diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index c88a1186965..5e8acf693dd 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/compiler/rustc_session/src/parse.rs @@ -239,7 +239,7 @@ impl ParseSess { let sm = Lrc::new(SourceMap::new(file_path_mapping)); let emitter = Box::new(HumanEmitter::stderr(ColorConfig::Auto, fallback_bundle).sm(Some(sm.clone()))); - let dcx = DiagCtxt::with_emitter(emitter); + let dcx = DiagCtxt::new(emitter); ParseSess::with_dcx(dcx, sm) } @@ -269,9 +269,9 @@ impl ParseSess { let fallback_bundle = fallback_fluent_bundle(Vec::new(), false); let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); let emitter = Box::new(HumanEmitter::stderr(ColorConfig::Auto, fallback_bundle)); - let fatal_dcx = DiagCtxt::with_emitter(emitter); - let dcx = DiagCtxt::with_emitter(Box::new(SilentEmitter { fatal_dcx, fatal_note })) - .disable_warnings(); + let fatal_dcx = DiagCtxt::new(emitter); + let dcx = + DiagCtxt::new(Box::new(SilentEmitter { fatal_dcx, fatal_note })).disable_warnings(); ParseSess::with_dcx(dcx, sm) } diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index e9db96dc356..aa45958c07b 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -1080,8 +1080,8 @@ pub fn build_session( ); let emitter = default_emitter(&sopts, registry, source_map.clone(), bundle, fallback_bundle); - let mut dcx = DiagCtxt::with_emitter(emitter) - .with_flags(sopts.unstable_opts.dcx_flags(can_emit_warnings)); + let mut dcx = + DiagCtxt::new(emitter).with_flags(sopts.unstable_opts.dcx_flags(can_emit_warnings)); if let Some(ice_file) = ice_file { dcx = dcx.with_ice_file(ice_file); } @@ -1402,7 +1402,7 @@ pub struct EarlyDiagCtxt { impl EarlyDiagCtxt { pub fn new(output: ErrorOutputType) -> Self { let emitter = mk_emitter(output); - Self { dcx: DiagCtxt::with_emitter(emitter) } + Self { dcx: DiagCtxt::new(emitter) } } /// Swap out the underlying dcx once we acquire the user's preference on error emission @@ -1412,7 +1412,7 @@ impl EarlyDiagCtxt { self.dcx.abort_if_errors(); let emitter = mk_emitter(output); - self.dcx = DiagCtxt::with_emitter(emitter); + self.dcx = DiagCtxt::new(emitter); } #[allow(rustc::untranslatable_diagnostic)] diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index c662f054d05..65ce24fe10e 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -172,7 +172,7 @@ pub(crate) fn new_dcx( } }; - rustc_errors::DiagCtxt::with_emitter(emitter).with_flags(unstable_opts.dcx_flags(true)) + rustc_errors::DiagCtxt::new(emitter).with_flags(unstable_opts.dcx_flags(true)) } /// Parse, resolve, and typecheck the given crate. diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 8390b34dba9..05df32812c3 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -582,7 +582,7 @@ pub(crate) fn make_test( let emitter = HumanEmitter::new(Box::new(io::sink()), fallback_bundle); // FIXME(misdreavus): pass `-Z treat-err-as-bug` to the doctest parser - let dcx = DiagCtxt::with_emitter(Box::new(emitter)).disable_warnings(); + let dcx = DiagCtxt::new(Box::new(emitter)).disable_warnings(); let sess = ParseSess::with_dcx(dcx, sm); let mut found_main = false; @@ -767,7 +767,7 @@ fn check_if_attr_is_complete(source: &str, edition: Edition) -> bool { let emitter = HumanEmitter::new(Box::new(io::sink()), fallback_bundle); - let dcx = DiagCtxt::with_emitter(Box::new(emitter)).disable_warnings(); + let dcx = DiagCtxt::new(Box::new(emitter)).disable_warnings(); let sess = ParseSess::with_dcx(dcx, sm); let mut parser = match maybe_new_parser_from_source_str(&sess, filename, source.to_owned()) { diff --git a/src/librustdoc/passes/lint/check_code_block_syntax.rs b/src/librustdoc/passes/lint/check_code_block_syntax.rs index c6571e72fc5..5145d8babef 100644 --- a/src/librustdoc/passes/lint/check_code_block_syntax.rs +++ b/src/librustdoc/passes/lint/check_code_block_syntax.rs @@ -42,7 +42,7 @@ fn check_rust_syntax( let emitter = BufferEmitter { buffer: Lrc::clone(&buffer), fallback_bundle }; let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); - let dcx = DiagCtxt::with_emitter(Box::new(emitter)).disable_warnings(); + let dcx = DiagCtxt::new(Box::new(emitter)).disable_warnings(); let source = dox[code_block.code].to_owned(); let sess = ParseSess::with_dcx(dcx, sm); diff --git a/src/tools/clippy/clippy_lints/src/doc/needless_doctest_main.rs b/src/tools/clippy/clippy_lints/src/doc/needless_doctest_main.rs index 58656140352..fdb9ceb7179 100644 --- a/src/tools/clippy/clippy_lints/src/doc/needless_doctest_main.rs +++ b/src/tools/clippy/clippy_lints/src/doc/needless_doctest_main.rs @@ -45,7 +45,7 @@ pub fn check( let fallback_bundle = rustc_errors::fallback_fluent_bundle(rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), false); let emitter = HumanEmitter::new(Box::new(io::sink()), fallback_bundle); - let dcx = DiagCtxt::with_emitter(Box::new(emitter)).disable_warnings(); + let dcx = DiagCtxt::new(Box::new(emitter)).disable_warnings(); #[expect(clippy::arc_with_non_send_sync)] // `Lrc` is expected by with_dcx let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); let sess = ParseSess::with_dcx(dcx, sm); diff --git a/src/tools/rustfmt/src/parse/session.rs b/src/tools/rustfmt/src/parse/session.rs index e33f1ca755c..ad36e022b8d 100644 --- a/src/tools/rustfmt/src/parse/session.rs +++ b/src/tools/rustfmt/src/parse/session.rs @@ -154,7 +154,7 @@ fn default_dcx( ); Box::new(HumanEmitter::stderr(emit_color, fallback_bundle).sm(Some(source_map.clone()))) }; - DiagCtxt::with_emitter(Box::new(SilentOnIgnoredFilesEmitter { + DiagCtxt::new(Box::new(SilentOnIgnoredFilesEmitter { has_non_ignorable_parser_errors: false, source_map, emitter, @@ -235,7 +235,7 @@ impl ParseSess { } pub(crate) fn set_silent_emitter(&mut self) { - self.parse_sess.dcx = DiagCtxt::with_emitter(silent_emitter()); + self.parse_sess.dcx = DiagCtxt::new(silent_emitter()); } pub(crate) fn span_to_filename(&self, span: Span) -> FileName { From d3727413ed96a8feedcca9931b9b8abb72632492 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Feb 2024 10:17:28 +1100 Subject: [PATCH 31/37] Merge HumanEmitter::{new,create}. They have the same signature, and the former just calls the latter. --- compiler/rustc_errors/src/emitter.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index cbe973330dc..89015a3ca2e 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -665,11 +665,10 @@ pub(crate) struct FileWithAnnotatedLines { impl HumanEmitter { pub fn stderr(color_config: ColorConfig, fallback_bundle: LazyFallbackBundle) -> HumanEmitter { - let dst = from_stderr(color_config); - Self::create(dst, fallback_bundle) + Self::new(from_stderr(color_config), fallback_bundle) } - fn create(dst: Destination, fallback_bundle: LazyFallbackBundle) -> HumanEmitter { + pub fn new(dst: Destination, fallback_bundle: LazyFallbackBundle) -> HumanEmitter { HumanEmitter { dst: IntoDynSyncSend(dst), sm: None, @@ -686,10 +685,6 @@ impl HumanEmitter { } } - pub fn new(dst: Destination, fallback_bundle: LazyFallbackBundle) -> HumanEmitter { - Self::create(dst, fallback_bundle) - } - fn maybe_anonymized(&self, line_num: usize) -> Cow<'static, str> { if self.ui_testing { Cow::Borrowed(ANONYMIZED_LINE_NUM) From 437325bdd4e5fc54b0f78491e63fa386ebfffbba Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Feb 2024 14:47:09 +1100 Subject: [PATCH 32/37] Inline and remove `HumanReadableErrorType::new_emitter`. And likewise with `ColorConfig::suggests_using_colors`. They both have a single call site. And note that `BufWriter::supports_color()` always returns false, which enables a small bit of constant folding along the way. --- compiler/rustc_errors/src/emitter.rs | 20 +------------------- compiler/rustc_errors/src/json.rs | 18 +++++++++++++++--- compiler/rustc_errors/src/json/tests.rs | 1 - 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 89015a3ca2e..ff105b4d46f 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -35,7 +35,7 @@ use std::io::prelude::*; use std::io::{self, IsTerminal}; use std::iter; use std::path::Path; -use termcolor::{Ansi, Buffer, BufferWriter, ColorChoice, ColorSpec, StandardStream}; +use termcolor::{Buffer, BufferWriter, ColorChoice, ColorSpec, StandardStream}; use termcolor::{Color, WriteColor}; /// Default column width, used in tests and when terminal dimensions cannot be determined. @@ -58,18 +58,6 @@ impl HumanReadableErrorType { HumanReadableErrorType::AnnotateSnippet(cc) => (false, cc), } } - pub fn new_emitter( - self, - mut dst: Destination, - fallback_bundle: LazyFallbackBundle, - ) -> HumanEmitter { - let (short, color_config) = self.unzip(); - let color = color_config.suggests_using_colors(); - if !dst.supports_color() && color { - dst = Box::new(Ansi::new(dst)); - } - HumanEmitter::new(dst, fallback_bundle).short_message(short) - } } #[derive(Clone, Copy, Debug)] @@ -628,12 +616,6 @@ impl ColorConfig { ColorConfig::Auto => ColorChoice::Never, } } - fn suggests_using_colors(self) -> bool { - match self { - ColorConfig::Always | ColorConfig::Auto => true, - ColorConfig::Never => false, - } - } } /// Handles the writing of `HumanReadableErrorType::Default` and `HumanReadableErrorType::Short` diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 94d0c5d060e..ab2ed5ebaeb 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -12,7 +12,10 @@ use rustc_span::source_map::{FilePathMapping, SourceMap}; use termcolor::{ColorSpec, WriteColor}; -use crate::emitter::{should_show_source_code, Emitter, HumanReadableErrorType}; +use crate::emitter::{ + should_show_source_code, ColorConfig, Destination, Emitter, HumanEmitter, + HumanReadableErrorType, +}; use crate::registry::Registry; use crate::translation::{to_fluent_args, Translate}; use crate::{ @@ -405,8 +408,17 @@ impl Diagnostic { .collect(); let buf = BufWriter::default(); - je.json_rendered - .new_emitter(Box::new(buf.clone()), je.fallback_bundle.clone()) + let mut dst: Destination = Box::new(buf.clone()); + let (short, color_config) = je.json_rendered.unzip(); + let color = match color_config { + ColorConfig::Always | ColorConfig::Auto => true, + ColorConfig::Never => false, + }; + if color { + dst = Box::new(termcolor::Ansi::new(dst)); + } + HumanEmitter::new(dst, je.fallback_bundle.clone()) + .short_message(short) .sm(Some(je.sm.clone())) .fluent_bundle(je.fluent_bundle.clone()) .diagnostic_width(je.diagnostic_width) diff --git a/compiler/rustc_errors/src/json/tests.rs b/compiler/rustc_errors/src/json/tests.rs index ba8d58e159e..e541f4ca7d4 100644 --- a/compiler/rustc_errors/src/json/tests.rs +++ b/compiler/rustc_errors/src/json/tests.rs @@ -1,6 +1,5 @@ use super::*; -use crate::emitter::ColorConfig; use crate::DiagCtxt; use rustc_span::BytePos; From 067d7c3d00a05989bd4b7d4b5648d61748f80848 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Feb 2024 15:37:38 +1100 Subject: [PATCH 33/37] Inline and remove `HumanEmitter::stderr`. Because `HumanEmitter::new` is enough, in conjunction with the (renamed) `stderr_destination` function. --- compiler/rustc_driver_impl/src/lib.rs | 5 +++-- compiler/rustc_errors/src/emitter.rs | 6 +----- compiler/rustc_errors/src/json/tests.rs | 1 + compiler/rustc_session/src/parse.rs | 11 +++++++---- compiler/rustc_session/src/session.rs | 9 ++++++--- src/librustdoc/core.rs | 4 ++-- src/librustdoc/doctest.rs | 4 +++- src/tools/rustfmt/src/parse/session.rs | 7 +++++-- 8 files changed, 28 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 10188026a97..410e7eba30a 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -21,6 +21,7 @@ use rustc_codegen_ssa::{traits::CodegenBackend, CodegenErrors, CodegenResults}; use rustc_data_structures::profiling::{ get_resident_set_size, print_time_passes_entry, TimePassesFormat, }; +use rustc_errors::emitter::stderr_destination; use rustc_errors::registry::Registry; use rustc_errors::{ markdown, ColorConfig, DiagCtxt, ErrCode, ErrorGuaranteed, FatalError, PResult, @@ -1384,8 +1385,8 @@ fn report_ice( ) { let fallback_bundle = rustc_errors::fallback_fluent_bundle(crate::DEFAULT_LOCALE_RESOURCES.to_vec(), false); - let emitter = Box::new(rustc_errors::emitter::HumanEmitter::stderr( - rustc_errors::ColorConfig::Auto, + let emitter = Box::new(rustc_errors::emitter::HumanEmitter::new( + stderr_destination(rustc_errors::ColorConfig::Auto), fallback_bundle, )); let dcx = rustc_errors::DiagCtxt::new(emitter); diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index ff105b4d46f..87e4a5ead4d 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -646,10 +646,6 @@ pub(crate) struct FileWithAnnotatedLines { } impl HumanEmitter { - pub fn stderr(color_config: ColorConfig, fallback_bundle: LazyFallbackBundle) -> HumanEmitter { - Self::new(from_stderr(color_config), fallback_bundle) - } - pub fn new(dst: Destination, fallback_bundle: LazyFallbackBundle) -> HumanEmitter { HumanEmitter { dst: IntoDynSyncSend(dst), @@ -2650,7 +2646,7 @@ impl WriteColor for Buffy { } } -fn from_stderr(color: ColorConfig) -> Destination { +pub fn stderr_destination(color: ColorConfig) -> Destination { let choice = color.to_color_choice(); // On Windows we'll be performing global synchronization on the entire // system for emitting rustc errors, so there's no need to buffer diff --git a/compiler/rustc_errors/src/json/tests.rs b/compiler/rustc_errors/src/json/tests.rs index e541f4ca7d4..fc9948cb2fc 100644 --- a/compiler/rustc_errors/src/json/tests.rs +++ b/compiler/rustc_errors/src/json/tests.rs @@ -1,6 +1,7 @@ use super::*; use crate::DiagCtxt; +use rustc_span::source_map::FilePathMapping; use rustc_span::BytePos; use std::str; diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index 5e8acf693dd..18c8652aaf2 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/compiler/rustc_session/src/parse.rs @@ -13,7 +13,7 @@ use crate::Session; use rustc_ast::node_id::NodeId; use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet}; use rustc_data_structures::sync::{AppendOnlyVec, Lock, Lrc}; -use rustc_errors::emitter::{HumanEmitter, SilentEmitter}; +use rustc_errors::emitter::{stderr_destination, HumanEmitter, SilentEmitter}; use rustc_errors::{ fallback_fluent_bundle, ColorConfig, Diag, DiagCtxt, DiagnosticMessage, EmissionGuarantee, MultiSpan, StashKey, @@ -237,8 +237,10 @@ impl ParseSess { pub fn new(locale_resources: Vec<&'static str>, file_path_mapping: FilePathMapping) -> Self { let fallback_bundle = fallback_fluent_bundle(locale_resources, false); let sm = Lrc::new(SourceMap::new(file_path_mapping)); - let emitter = - Box::new(HumanEmitter::stderr(ColorConfig::Auto, fallback_bundle).sm(Some(sm.clone()))); + let emitter = Box::new( + HumanEmitter::new(stderr_destination(ColorConfig::Auto), fallback_bundle) + .sm(Some(sm.clone())), + ); let dcx = DiagCtxt::new(emitter); ParseSess::with_dcx(dcx, sm) } @@ -268,7 +270,8 @@ impl ParseSess { pub fn with_silent_emitter(fatal_note: String) -> Self { let fallback_bundle = fallback_fluent_bundle(Vec::new(), false); let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); - let emitter = Box::new(HumanEmitter::stderr(ColorConfig::Auto, fallback_bundle)); + let emitter = + Box::new(HumanEmitter::new(stderr_destination(ColorConfig::Auto), fallback_bundle)); let fatal_dcx = DiagCtxt::new(emitter); let dcx = DiagCtxt::new(Box::new(SilentEmitter { fatal_dcx, fatal_note })).disable_warnings(); diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index aa45958c07b..9ac49f663ab 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -18,7 +18,7 @@ use rustc_data_structures::sync::{ AtomicU64, DynSend, DynSync, Lock, Lrc, MappedReadGuard, ReadGuard, RwLock, }; use rustc_errors::annotate_snippet_emitter_writer::AnnotateSnippetEmitter; -use rustc_errors::emitter::{DynEmitter, HumanEmitter, HumanReadableErrorType}; +use rustc_errors::emitter::{stderr_destination, DynEmitter, HumanEmitter, HumanReadableErrorType}; use rustc_errors::json::JsonEmitter; use rustc_errors::registry::Registry; use rustc_errors::{ @@ -982,7 +982,7 @@ fn default_emitter( ); Box::new(emitter.ui_testing(sopts.unstable_opts.ui_testing)) } else { - let emitter = HumanEmitter::stderr(color_config, fallback_bundle) + let emitter = HumanEmitter::new(stderr_destination(color_config), fallback_bundle) .fluent_bundle(bundle) .sm(Some(source_map)) .short_message(short) @@ -1473,7 +1473,10 @@ fn mk_emitter(output: ErrorOutputType) -> Box { let emitter: Box = match output { config::ErrorOutputType::HumanReadable(kind) => { let (short, color_config) = kind.unzip(); - Box::new(HumanEmitter::stderr(color_config, fallback_bundle).short_message(short)) + Box::new( + HumanEmitter::new(stderr_destination(color_config), fallback_bundle) + .short_message(short), + ) } config::ErrorOutputType::Json { pretty, json_rendered } => Box::new(JsonEmitter::basic( pretty, diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 65ce24fe10e..bcc72857d0a 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -1,7 +1,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::sync::Lrc; use rustc_data_structures::unord::UnordSet; -use rustc_errors::emitter::{DynEmitter, HumanEmitter}; +use rustc_errors::emitter::{stderr_destination, DynEmitter, HumanEmitter}; use rustc_errors::json::JsonEmitter; use rustc_errors::{codes::*, ErrorGuaranteed, TerminalUrl}; use rustc_feature::UnstableFeatures; @@ -141,7 +141,7 @@ pub(crate) fn new_dcx( ErrorOutputType::HumanReadable(kind) => { let (short, color_config) = kind.unzip(); Box::new( - HumanEmitter::stderr(color_config, fallback_bundle) + HumanEmitter::new(stderr_destination(color_config), fallback_bundle) .sm(source_map.map(|sm| sm as _)) .short_message(short) .teach(unstable_opts.teach) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 05df32812c3..3d92444364f 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -1,6 +1,7 @@ use rustc_ast as ast; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::sync::Lrc; +use rustc_errors::emitter::stderr_destination; use rustc_errors::{ColorConfig, ErrorGuaranteed, FatalError}; use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID, LOCAL_CRATE}; use rustc_hir::{self as hir, intravisit, CRATE_HIR_ID}; @@ -577,7 +578,8 @@ pub(crate) fn make_test( false, ); supports_color = - HumanEmitter::stderr(ColorConfig::Auto, fallback_bundle.clone()).supports_color(); + HumanEmitter::new(stderr_destination(ColorConfig::Auto), fallback_bundle.clone()) + .supports_color(); let emitter = HumanEmitter::new(Box::new(io::sink()), fallback_bundle); diff --git a/src/tools/rustfmt/src/parse/session.rs b/src/tools/rustfmt/src/parse/session.rs index ad36e022b8d..7e6517a66dd 100644 --- a/src/tools/rustfmt/src/parse/session.rs +++ b/src/tools/rustfmt/src/parse/session.rs @@ -3,7 +3,7 @@ use std::path::Path; use std::sync::atomic::{AtomicBool, Ordering}; use rustc_data_structures::sync::{IntoDynSyncSend, Lrc}; -use rustc_errors::emitter::{DynEmitter, Emitter, HumanEmitter}; +use rustc_errors::emitter::{stderr_destination, DynEmitter, Emitter, HumanEmitter}; use rustc_errors::translation::Translate; use rustc_errors::{ ColorConfig, Diag, DiagCtxt, DiagInner, ErrorGuaranteed, Level as DiagnosticLevel, @@ -152,7 +152,10 @@ fn default_dcx( rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), false, ); - Box::new(HumanEmitter::stderr(emit_color, fallback_bundle).sm(Some(source_map.clone()))) + Box::new( + HumanEmitter::new(stderr_destination(emit_color), fallback_bundle) + .sm(Some(source_map.clone())), + ) }; DiagCtxt::new(Box::new(SilentOnIgnoredFilesEmitter { has_non_ignorable_parser_errors: false, From 2999d8dc72e9ed9e895d6a9e2d3d34f1cd4370ba Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Feb 2024 09:21:02 +1100 Subject: [PATCH 34/37] Inline and remove `JsonEmitter::{basic,stderr}`. They are so similar to `JsonEmitter::new` it's not worth having separate functions, it makes the code harder to read. --- compiler/rustc_errors/src/json.rs | 56 +-------------------------- compiler/rustc_session/src/session.rs | 15 ++++--- src/librustdoc/core.rs | 4 +- 3 files changed, 14 insertions(+), 61 deletions(-) diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index ab2ed5ebaeb..e99a70c393e 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -9,7 +9,7 @@ // FIXME: spec the JSON output properly. -use rustc_span::source_map::{FilePathMapping, SourceMap}; +use rustc_span::source_map::SourceMap; use termcolor::{ColorSpec, WriteColor}; use crate::emitter::{ @@ -56,60 +56,6 @@ pub struct JsonEmitter { } impl JsonEmitter { - pub fn stderr( - registry: Option, - source_map: Lrc, - fluent_bundle: Option>, - fallback_bundle: LazyFallbackBundle, - pretty: bool, - json_rendered: HumanReadableErrorType, - diagnostic_width: Option, - macro_backtrace: bool, - track_diagnostics: bool, - terminal_url: TerminalUrl, - ) -> JsonEmitter { - JsonEmitter { - dst: IntoDynSyncSend(Box::new(io::BufWriter::new(io::stderr()))), - registry, - sm: source_map, - fluent_bundle, - fallback_bundle, - pretty, - ui_testing: false, - ignored_directories_in_source_blocks: Vec::new(), - json_rendered, - diagnostic_width, - macro_backtrace, - track_diagnostics, - terminal_url, - } - } - - pub fn basic( - pretty: bool, - json_rendered: HumanReadableErrorType, - fluent_bundle: Option>, - fallback_bundle: LazyFallbackBundle, - diagnostic_width: Option, - macro_backtrace: bool, - track_diagnostics: bool, - terminal_url: TerminalUrl, - ) -> JsonEmitter { - let file_path_mapping = FilePathMapping::empty(); - JsonEmitter::stderr( - None, - Lrc::new(SourceMap::new(file_path_mapping)), - fluent_bundle, - fallback_bundle, - pretty, - json_rendered, - diagnostic_width, - macro_backtrace, - track_diagnostics, - terminal_url, - ) - } - pub fn new( dst: Box, registry: Option, diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 9ac49f663ab..2979d880dbd 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -28,7 +28,7 @@ use rustc_errors::{ use rustc_macros::HashStable_Generic; pub use rustc_span::def_id::StableCrateId; use rustc_span::edition::Edition; -use rustc_span::source_map::{FileLoader, RealFileLoader, SourceMap}; +use rustc_span::source_map::{FileLoader, FilePathMapping, RealFileLoader, SourceMap}; use rustc_span::{SourceFileHashAlgorithm, Span, Symbol}; use rustc_target::asm::InlineAsmArch; use rustc_target::spec::{CodeModel, PanicStrategy, RelocModel, RelroLevel}; @@ -39,6 +39,7 @@ use rustc_target::spec::{ use std::any::Any; use std::env; use std::fmt; +use std::io; use std::ops::{Div, Mul}; use std::path::{Path, PathBuf}; use std::str::FromStr; @@ -998,7 +999,8 @@ fn default_emitter( } } config::ErrorOutputType::Json { pretty, json_rendered } => Box::new( - JsonEmitter::stderr( + JsonEmitter::new( + Box::new(io::BufWriter::new(io::stderr())), Some(registry), source_map, bundle, @@ -1478,11 +1480,14 @@ fn mk_emitter(output: ErrorOutputType) -> Box { .short_message(short), ) } - config::ErrorOutputType::Json { pretty, json_rendered } => Box::new(JsonEmitter::basic( - pretty, - json_rendered, + config::ErrorOutputType::Json { pretty, json_rendered } => Box::new(JsonEmitter::new( + Box::new(io::BufWriter::new(io::stderr())), + None, + Lrc::new(SourceMap::new(FilePathMapping::empty())), None, fallback_bundle, + pretty, + json_rendered, None, false, false, diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index bcc72857d0a..463b8385d43 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -20,6 +20,7 @@ use rustc_span::symbol::sym; use rustc_span::{source_map, Span}; use std::cell::RefCell; +use std::io; use std::mem; use std::rc::Rc; use std::sync::LazyLock; @@ -155,7 +156,8 @@ pub(crate) fn new_dcx( Lrc::new(source_map::SourceMap::new(source_map::FilePathMapping::empty())) }); Box::new( - JsonEmitter::stderr( + JsonEmitter::new( + Box::new(io::BufWriter::new(io::stderr())), None, source_map, None, From 9ff4487999ccd02065ec56fdc8cee665feae9680 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Feb 2024 16:03:59 +1100 Subject: [PATCH 35/37] Make `JsonEmitter` more like `HumanEmitter`. Use `derive(Setters)` to derive setters, and then change `JsonEmitter::new` to only have the arguments that are always used. --- compiler/rustc_errors/src/emitter.rs | 3 +- compiler/rustc_errors/src/json.rs | 48 ++++++++++--------------- compiler/rustc_errors/src/json/tests.rs | 8 +---- compiler/rustc_session/src/session.rs | 20 ++++------- src/librustdoc/core.rs | 11 +++--- 5 files changed, 32 insertions(+), 58 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 87e4a5ead4d..5637c05d04c 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -21,12 +21,11 @@ use crate::{ FluentBundle, LazyFallbackBundle, Level, MultiSpan, Subdiag, SubstitutionHighlight, SuggestionStyle, TerminalUrl, }; -use rustc_lint_defs::pluralize; - use derive_setters::Setters; use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet}; use rustc_data_structures::sync::{DynSend, IntoDynSyncSend, Lrc}; use rustc_error_messages::{FluentArgs, SpanLabel}; +use rustc_lint_defs::pluralize; use rustc_span::hygiene::{ExpnKind, MacroKind}; use std::borrow::Cow; use std::cmp::{max, min, Reverse}; diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index e99a70c393e..3166768e9e2 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -9,9 +9,6 @@ // FIXME: spec the JSON output properly. -use rustc_span::source_map::SourceMap; -use termcolor::{ColorSpec, WriteColor}; - use crate::emitter::{ should_show_source_code, ColorConfig, Destination, Emitter, HumanEmitter, HumanReadableErrorType, @@ -22,32 +19,39 @@ use crate::{ diagnostic::IsLint, CodeSuggestion, FluentBundle, LazyFallbackBundle, MultiSpan, SpanLabel, Subdiag, TerminalUrl, }; -use rustc_lint_defs::Applicability; - +use derive_setters::Setters; use rustc_data_structures::sync::{IntoDynSyncSend, Lrc}; use rustc_error_messages::FluentArgs; +use rustc_lint_defs::Applicability; use rustc_span::hygiene::ExpnData; +use rustc_span::source_map::SourceMap; use rustc_span::Span; +use serde::Serialize; use std::error::Report; use std::io::{self, Write}; use std::path::Path; use std::sync::{Arc, Mutex}; use std::vec; - -use serde::Serialize; +use termcolor::{ColorSpec, WriteColor}; #[cfg(test)] mod tests; +#[derive(Setters)] pub struct JsonEmitter { + #[setters(skip)] dst: IntoDynSyncSend>, registry: Option, + #[setters(skip)] sm: Lrc, fluent_bundle: Option>, + #[setters(skip)] fallback_bundle: LazyFallbackBundle, + #[setters(skip)] pretty: bool, ui_testing: bool, ignored_directories_in_source_blocks: Vec, + #[setters(skip)] json_rendered: HumanReadableErrorType, diagnostic_width: Option, macro_backtrace: bool, @@ -58,42 +62,28 @@ pub struct JsonEmitter { impl JsonEmitter { pub fn new( dst: Box, - registry: Option, - source_map: Lrc, - fluent_bundle: Option>, + sm: Lrc, fallback_bundle: LazyFallbackBundle, pretty: bool, json_rendered: HumanReadableErrorType, - diagnostic_width: Option, - macro_backtrace: bool, - track_diagnostics: bool, - terminal_url: TerminalUrl, ) -> JsonEmitter { JsonEmitter { dst: IntoDynSyncSend(dst), - registry, - sm: source_map, - fluent_bundle, + registry: None, + sm, + fluent_bundle: None, fallback_bundle, pretty, ui_testing: false, ignored_directories_in_source_blocks: Vec::new(), json_rendered, - diagnostic_width, - macro_backtrace, - track_diagnostics, - terminal_url, + diagnostic_width: None, + macro_backtrace: false, + track_diagnostics: false, + terminal_url: TerminalUrl::No, } } - pub fn ui_testing(self, ui_testing: bool) -> Self { - Self { ui_testing, ..self } - } - - pub fn ignored_directories_in_source_blocks(self, value: Vec) -> Self { - Self { ignored_directories_in_source_blocks: value, ..self } - } - fn emit(&mut self, val: EmitTyped<'_>) -> io::Result<()> { if self.pretty { serde_json::to_writer_pretty(&mut *self.dst, &val)? diff --git a/compiler/rustc_errors/src/json/tests.rs b/compiler/rustc_errors/src/json/tests.rs index fc9948cb2fc..80b4d2bf75c 100644 --- a/compiler/rustc_errors/src/json/tests.rs +++ b/compiler/rustc_errors/src/json/tests.rs @@ -48,16 +48,10 @@ fn test_positions(code: &str, span: (u32, u32), expected_output: SpanTestData) { let output = Arc::new(Mutex::new(Vec::new())); let je = JsonEmitter::new( Box::new(Shared { data: output.clone() }), - None, sm, - None, fallback_bundle, - true, + true, // pretty HumanReadableErrorType::Short(ColorConfig::Never), - None, - false, - false, - TerminalUrl::No, ); let span = Span::with_root_ctxt(BytePos(span.0), BytePos(span.1)); diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 2979d880dbd..09fb6aa5d8f 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -1001,21 +1001,21 @@ fn default_emitter( config::ErrorOutputType::Json { pretty, json_rendered } => Box::new( JsonEmitter::new( Box::new(io::BufWriter::new(io::stderr())), - Some(registry), source_map, - bundle, fallback_bundle, pretty, json_rendered, - sopts.diagnostic_width, - macro_backtrace, - track_diagnostics, - terminal_url, ) + .registry(Some(registry)) + .fluent_bundle(bundle) .ui_testing(sopts.unstable_opts.ui_testing) .ignored_directories_in_source_blocks( sopts.unstable_opts.ignore_directory_in_diagnostics_source_blocks.clone(), - ), + ) + .diagnostic_width(sopts.diagnostic_width) + .macro_backtrace(macro_backtrace) + .track_diagnostics(track_diagnostics) + .terminal_url(terminal_url), ), } } @@ -1482,16 +1482,10 @@ fn mk_emitter(output: ErrorOutputType) -> Box { } config::ErrorOutputType::Json { pretty, json_rendered } => Box::new(JsonEmitter::new( Box::new(io::BufWriter::new(io::stderr())), - None, Lrc::new(SourceMap::new(FilePathMapping::empty())), - None, fallback_bundle, pretty, json_rendered, - None, - false, - false, - TerminalUrl::No, )), }; emitter diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 463b8385d43..9ba79cf5d29 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -158,18 +158,15 @@ pub(crate) fn new_dcx( Box::new( JsonEmitter::new( Box::new(io::BufWriter::new(io::stderr())), - None, source_map, - None, fallback_bundle, pretty, json_rendered, - diagnostic_width, - false, - unstable_opts.track_diagnostics, - TerminalUrl::No, ) - .ui_testing(unstable_opts.ui_testing), + .ui_testing(unstable_opts.ui_testing) + .diagnostic_width(diagnostic_width) + .track_diagnostics(unstable_opts.track_diagnostics) + .terminal_url(TerminalUrl::No), ) } }; From 58f45059a5e39135ac5e26a662821dc13c0e4e56 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Feb 2024 16:15:44 +1100 Subject: [PATCH 36/37] Add a useful comment. It took me a while to work this out. --- compiler/rustc_errors/src/json.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 3166768e9e2..77e4f9a0767 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -279,6 +279,7 @@ struct UnusedExterns<'a> { } impl Diagnostic { + /// Converts from `rustc_errors::DiagInner` to `Diagnostic`. fn from_errors_diagnostic(diag: crate::DiagInner, je: &JsonEmitter) -> Diagnostic { let args = to_fluent_args(diag.args.iter()); let sugg = diag.suggestions.iter().flatten().map(|sugg| { From 607bf653c2bed1ec3f7979d9511f3c9cef604bc3 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Feb 2024 20:12:43 +1100 Subject: [PATCH 37/37] Avoid unnecessary `color` local variable. --- compiler/rustc_errors/src/json.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 77e4f9a0767..bc1822f83fc 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -347,12 +347,9 @@ impl Diagnostic { let buf = BufWriter::default(); let mut dst: Destination = Box::new(buf.clone()); let (short, color_config) = je.json_rendered.unzip(); - let color = match color_config { - ColorConfig::Always | ColorConfig::Auto => true, - ColorConfig::Never => false, - }; - if color { - dst = Box::new(termcolor::Ansi::new(dst)); + match color_config { + ColorConfig::Always | ColorConfig::Auto => dst = Box::new(termcolor::Ansi::new(dst)), + ColorConfig::Never => {} } HumanEmitter::new(dst, je.fallback_bundle.clone()) .short_message(short)