From 1f01d9b3db949340c614cb0a9cf8319808088054 Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Sat, 5 Jul 2025 10:19:38 +0530 Subject: [PATCH 01/12] std: sys: net: uefi: tcp4: Implement read A blocking implementation of tcp4 read. Signed-off-by: Ayush Singh --- .../std/src/sys/net/connection/uefi/mod.rs | 13 ++++--- .../std/src/sys/net/connection/uefi/tcp.rs | 6 +++ .../std/src/sys/net/connection/uefi/tcp4.rs | 39 +++++++++++++++++++ 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/library/std/src/sys/net/connection/uefi/mod.rs b/library/std/src/sys/net/connection/uefi/mod.rs index 2d465bd0efaa3..6835ba44ee242 100644 --- a/library/std/src/sys/net/connection/uefi/mod.rs +++ b/library/std/src/sys/net/connection/uefi/mod.rs @@ -38,16 +38,17 @@ impl TcpStream { unsupported() } - pub fn read(&self, _: &mut [u8]) -> io::Result { - unsupported() + pub fn read(&self, buf: &mut [u8]) -> io::Result { + self.0.read(buf) } - pub fn read_buf(&self, _buf: BorrowedCursor<'_>) -> io::Result<()> { - unsupported() + pub fn read_buf(&self, cursor: BorrowedCursor<'_>) -> io::Result<()> { + crate::io::default_read_buf(|buf| self.read(buf), cursor) } - pub fn read_vectored(&self, _: &mut [IoSliceMut<'_>]) -> io::Result { - unsupported() + pub fn read_vectored(&self, buf: &mut [IoSliceMut<'_>]) -> io::Result { + // FIXME: UEFI does support vectored read, so implement that. + crate::io::default_read_vectored(|b| self.read(b), buf) } pub fn is_read_vectored(&self) -> bool { diff --git a/library/std/src/sys/net/connection/uefi/tcp.rs b/library/std/src/sys/net/connection/uefi/tcp.rs index 9c3462e3468cd..55b6dbf2490bd 100644 --- a/library/std/src/sys/net/connection/uefi/tcp.rs +++ b/library/std/src/sys/net/connection/uefi/tcp.rs @@ -24,4 +24,10 @@ impl Tcp { Self::V4(client) => client.write(buf), } } + + pub(crate) fn read(&self, buf: &mut [u8]) -> io::Result { + match self { + Self::V4(client) => client.read(buf), + } + } } diff --git a/library/std/src/sys/net/connection/uefi/tcp4.rs b/library/std/src/sys/net/connection/uefi/tcp4.rs index d0f0d27d975f3..af1ba2be47adb 100644 --- a/library/std/src/sys/net/connection/uefi/tcp4.rs +++ b/library/std/src/sys/net/connection/uefi/tcp4.rs @@ -128,6 +128,45 @@ impl Tcp4 { } } + pub(crate) fn read(&self, buf: &mut [u8]) -> io::Result { + let evt = unsafe { self.create_evt() }?; + let completion_token = + tcp4::CompletionToken { event: evt.as_ptr(), status: Status::SUCCESS }; + let data_len = u32::try_from(buf.len()).unwrap_or(u32::MAX); + + let fragment = tcp4::FragmentData { + fragment_length: data_len, + fragment_buffer: buf.as_mut_ptr().cast::(), + }; + let mut tx_data = tcp4::ReceiveData { + urgent_flag: r_efi::efi::Boolean::FALSE, + data_length: data_len, + fragment_count: 1, + fragment_table: [fragment], + }; + + let protocol = self.protocol.as_ptr(); + let mut token = tcp4::IoToken { + completion_token, + packet: tcp4::IoTokenPacket { + rx_data: (&raw mut tx_data).cast::>(), + }, + }; + + let r = unsafe { ((*protocol).receive)(protocol, &mut token) }; + if r.is_error() { + return Err(io::Error::from_raw_os_error(r.as_usize())); + } + + self.wait_for_flag(); + + if completion_token.status.is_error() { + Err(io::Error::from_raw_os_error(completion_token.status.as_usize())) + } else { + Ok(data_len as usize) + } + } + unsafe fn create_evt(&self) -> io::Result { self.flag.store(false, Ordering::Relaxed); helpers::OwnedEvent::new( From 226b0fbe11812c71c8002b10a40063571cf52b3f Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Sat, 5 Jul 2025 08:26:32 +0200 Subject: [PATCH 02/12] use `is_multiple_of` instead of manual modulo --- compiler/rustc_borrowck/src/polonius/legacy/location.rs | 2 +- compiler/rustc_codegen_llvm/src/abi.rs | 2 +- compiler/rustc_const_eval/src/interpret/memory.rs | 4 ++-- compiler/rustc_query_system/src/query/plumbing.rs | 2 +- compiler/rustc_target/src/callconv/sparc64.rs | 6 +++--- compiler/rustc_ty_utils/src/layout/invariant.rs | 2 +- library/core/src/slice/mod.rs | 8 ++++---- library/core/src/slice/sort/shared/smallsort.rs | 2 +- library/core/src/str/count.rs | 2 +- library/core/src/str/validations.rs | 2 +- 10 files changed, 16 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_borrowck/src/polonius/legacy/location.rs b/compiler/rustc_borrowck/src/polonius/legacy/location.rs index 5f816bb9bbdc4..618119a6a3d62 100644 --- a/compiler/rustc_borrowck/src/polonius/legacy/location.rs +++ b/compiler/rustc_borrowck/src/polonius/legacy/location.rs @@ -109,6 +109,6 @@ impl PoloniusLocationTable { impl LocationIndex { fn is_start(self) -> bool { // even indices are start points; odd indices are mid points - (self.index() % 2) == 0 + self.index().is_multiple_of(2) } } diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index 4b07c8aef915b..009e7e2487b66 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -146,7 +146,7 @@ impl LlvmType for CastTarget { "total size {:?} cannot be divided into units of zero size", self.rest.total ); - if self.rest.total.bytes() % self.rest.unit.size.bytes() != 0 { + if !self.rest.total.bytes().is_multiple_of(self.rest.unit.size.bytes()) { assert_eq!(self.rest.unit.kind, RegKind::Integer, "only int regs can be split"); } self.rest.total.bytes().div_ceil(self.rest.unit.size.bytes()) diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index ff822b52a8df4..c97d53a45de7d 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -537,7 +537,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { #[inline] fn is_offset_misaligned(offset: u64, align: Align) -> Option { - if offset % align.bytes() == 0 { + if offset.is_multiple_of(align.bytes()) { None } else { // The biggest power of two through which `offset` is divisible. @@ -1554,7 +1554,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { // If the allocation is N-aligned, and the offset is not divisible by N, // then `base + offset` has a non-zero remainder after division by `N`, // which means `base + offset` cannot be null. - if offset.bytes() % info.align.bytes() != 0 { + if !offset.bytes().is_multiple_of(info.align.bytes()) { return interp_ok(false); } // We don't know enough, this might be null. diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index 3c1fc7317848c..06e59eb4cccde 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -597,7 +597,7 @@ where // from disk. Re-hashing results is fairly expensive, so we can't // currently afford to verify every hash. This subset should still // give us some coverage of potential bugs though. - let try_verify = prev_fingerprint.split().1.as_u64() % 32 == 0; + let try_verify = prev_fingerprint.split().1.as_u64().is_multiple_of(32); if std::intrinsics::unlikely( try_verify || qcx.dep_context().sess().opts.unstable_opts.incremental_verify_ich, ) { diff --git a/compiler/rustc_target/src/callconv/sparc64.rs b/compiler/rustc_target/src/callconv/sparc64.rs index 186826c08fcb8..ecc9067ced35e 100644 --- a/compiler/rustc_target/src/callconv/sparc64.rs +++ b/compiler/rustc_target/src/callconv/sparc64.rs @@ -90,7 +90,7 @@ where _ => {} } - if (offset.bytes() % 4) != 0 + if !offset.bytes().is_multiple_of(4) && matches!(scalar2.primitive(), Primitive::Float(Float::F32 | Float::F64)) { offset += Size::from_bytes(4 - (offset.bytes() % 4)); @@ -181,7 +181,7 @@ where // Structure { float, int, int } doesn't like to be handled like // { float, long int }. Other way around it doesn't mind. if data.last_offset < arg.layout.size - && (data.last_offset.bytes() % 8) != 0 + && !data.last_offset.bytes().is_multiple_of(8) && data.prefix_index < data.prefix.len() { data.prefix[data.prefix_index] = Some(Reg::i32()); @@ -190,7 +190,7 @@ where } let mut rest_size = arg.layout.size - data.last_offset; - if (rest_size.bytes() % 8) != 0 && data.prefix_index < data.prefix.len() { + if !rest_size.bytes().is_multiple_of(8) && data.prefix_index < data.prefix.len() { data.prefix[data.prefix_index] = Some(Reg::i32()); rest_size = rest_size - Reg::i32().size; } diff --git a/compiler/rustc_ty_utils/src/layout/invariant.rs b/compiler/rustc_ty_utils/src/layout/invariant.rs index 4b65c05d0e9f9..1311ee31182c6 100644 --- a/compiler/rustc_ty_utils/src/layout/invariant.rs +++ b/compiler/rustc_ty_utils/src/layout/invariant.rs @@ -8,7 +8,7 @@ use rustc_middle::ty::layout::{HasTyCtxt, LayoutCx, TyAndLayout}; pub(super) fn layout_sanity_check<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLayout<'tcx>) { let tcx = cx.tcx(); - if layout.size.bytes() % layout.align.abi.bytes() != 0 { + if !layout.size.bytes().is_multiple_of(layout.align.abi.bytes()) { bug!("size is not a multiple of align, in the following layout:\n{layout:#?}"); } if layout.size.bytes() >= tcx.data_layout.obj_size_bound() { diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 3a3f44c6b8546..dc09ba8d788fc 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -1316,7 +1316,7 @@ impl [T] { assert_unsafe_precondition!( check_language_ub, "slice::as_chunks_unchecked requires `N != 0` and the slice to split exactly into `N`-element chunks", - (n: usize = N, len: usize = self.len()) => n != 0 && len % n == 0, + (n: usize = N, len: usize = self.len()) => n != 0 && len.is_multiple_of(n), ); // SAFETY: Caller must guarantee that `N` is nonzero and exactly divides the slice length let new_len = unsafe { exact_div(self.len(), N) }; @@ -1512,7 +1512,7 @@ impl [T] { assert_unsafe_precondition!( check_language_ub, "slice::as_chunks_unchecked requires `N != 0` and the slice to split exactly into `N`-element chunks", - (n: usize = N, len: usize = self.len()) => n != 0 && len % n == 0 + (n: usize = N, len: usize = self.len()) => n != 0 && len.is_multiple_of(n) ); // SAFETY: Caller must guarantee that `N` is nonzero and exactly divides the slice length let new_len = unsafe { exact_div(self.len(), N) }; @@ -4866,7 +4866,7 @@ impl [T] { let byte_offset = elem_start.wrapping_sub(self_start); - if byte_offset % size_of::() != 0 { + if !byte_offset.is_multiple_of(size_of::()) { return None; } @@ -4920,7 +4920,7 @@ impl [T] { let byte_start = subslice_start.wrapping_sub(self_start); - if byte_start % size_of::() != 0 { + if !byte_start.is_multiple_of(size_of::()) { return None; } diff --git a/library/core/src/slice/sort/shared/smallsort.rs b/library/core/src/slice/sort/shared/smallsort.rs index 4280f7570db4c..400daba16c1b8 100644 --- a/library/core/src/slice/sort/shared/smallsort.rs +++ b/library/core/src/slice/sort/shared/smallsort.rs @@ -823,7 +823,7 @@ unsafe fn bidirectional_merge bool>( let right_end = right_rev.wrapping_add(1); // Odd length, so one element is left unconsumed in the input. - if len % 2 != 0 { + if !len.is_multiple_of(2) { let left_nonempty = left < left_end; let last_src = if left_nonempty { left } else { right }; ptr::copy_nonoverlapping(last_src, dst, 1); diff --git a/library/core/src/str/count.rs b/library/core/src/str/count.rs index 452403b23dee1..f59ad3e66b43b 100644 --- a/library/core/src/str/count.rs +++ b/library/core/src/str/count.rs @@ -52,7 +52,7 @@ fn do_count_chars(s: &str) -> usize { // Check the properties of `CHUNK_SIZE` and `UNROLL_INNER` that are required // for correctness. const _: () = assert!(CHUNK_SIZE < 256); - const _: () = assert!(CHUNK_SIZE % UNROLL_INNER == 0); + const _: () = assert!(CHUNK_SIZE.is_multiple_of(UNROLL_INNER)); // SAFETY: transmuting `[u8]` to `[usize]` is safe except for size // differences which are handled by `align_to`. diff --git a/library/core/src/str/validations.rs b/library/core/src/str/validations.rs index 8174e4ff97dfc..b54d6478e584d 100644 --- a/library/core/src/str/validations.rs +++ b/library/core/src/str/validations.rs @@ -219,7 +219,7 @@ pub(super) const fn run_utf8_validation(v: &[u8]) -> Result<(), Utf8Error> { // Ascii case, try to skip forward quickly. // When the pointer is aligned, read 2 words of data per iteration // until we find a word containing a non-ascii byte. - if align != usize::MAX && align.wrapping_sub(index) % USIZE_BYTES == 0 { + if align != usize::MAX && align.wrapping_sub(index).is_multiple_of(USIZE_BYTES) { let ptr = v.as_ptr(); while index < blocks_end { // SAFETY: since `align - index` and `ascii_block_size` are From ed3711ea29398b09483e4e2a3930567e9ba81d93 Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Sat, 5 Jul 2025 08:36:27 +0200 Subject: [PATCH 03/12] use `div_ceil` instead of manual logic --- compiler/rustc_abi/src/lib.rs | 3 +-- compiler/rustc_codegen_llvm/src/va_arg.rs | 4 ++-- compiler/rustc_index/src/bit_set.rs | 4 ++-- compiler/rustc_passes/src/liveness/rwu_table.rs | 2 +- compiler/rustc_serialize/src/leb128.rs | 2 +- compiler/rustc_span/src/edit_distance.rs | 2 +- compiler/rustc_target/src/callconv/x86.rs | 2 +- compiler/rustc_target/src/callconv/x86_64.rs | 2 +- compiler/rustc_target/src/callconv/xtensa.rs | 2 +- library/core/src/slice/sort/stable/drift.rs | 4 ++-- library/core/src/str/iter.rs | 6 +++--- library/portable-simd/crates/core_simd/src/lane_count.rs | 2 +- 12 files changed, 17 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index 0df8921c9b721..a438545c76f1c 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -527,8 +527,7 @@ impl Size { /// not a multiple of 8. pub fn from_bits(bits: impl TryInto) -> Size { let bits = bits.try_into().ok().unwrap(); - // Avoid potential overflow from `bits + 7`. - Size { raw: bits / 8 + ((bits % 8) + 7) / 8 } + Size { raw: bits.div_ceil(8) } } #[inline] diff --git a/compiler/rustc_codegen_llvm/src/va_arg.rs b/compiler/rustc_codegen_llvm/src/va_arg.rs index 4fe4c9bcbf263..486dc894a4e35 100644 --- a/compiler/rustc_codegen_llvm/src/va_arg.rs +++ b/compiler/rustc_codegen_llvm/src/va_arg.rs @@ -172,10 +172,10 @@ fn emit_aapcs_va_arg<'ll, 'tcx>( let gr_type = target_ty.is_any_ptr() || target_ty.is_integral(); let (reg_off, reg_top, slot_size) = if gr_type { - let nreg = (layout.size.bytes() + 7) / 8; + let nreg = layout.size.bytes().div_ceil(8); (gr_offs, gr_top, nreg * 8) } else { - let nreg = (layout.size.bytes() + 15) / 16; + let nreg = layout.size.bytes().div_ceil(16); (vr_offs, vr_top, nreg * 16) }; diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index a4885aabe1ffc..645d95b1dba99 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -1744,13 +1744,13 @@ impl SparseBitMatrix { #[inline] fn num_words(domain_size: T) -> usize { - (domain_size.index() + WORD_BITS - 1) / WORD_BITS + domain_size.index().div_ceil(WORD_BITS) } #[inline] fn num_chunks(domain_size: T) -> usize { assert!(domain_size.index() > 0); - (domain_size.index() + CHUNK_BITS - 1) / CHUNK_BITS + domain_size.index().div_ceil(CHUNK_BITS) } #[inline] diff --git a/compiler/rustc_passes/src/liveness/rwu_table.rs b/compiler/rustc_passes/src/liveness/rwu_table.rs index 4c1f6ea141e66..a1177946f8684 100644 --- a/compiler/rustc_passes/src/liveness/rwu_table.rs +++ b/compiler/rustc_passes/src/liveness/rwu_table.rs @@ -44,7 +44,7 @@ impl RWUTable { const WORD_RWU_COUNT: usize = Self::WORD_BITS / Self::RWU_BITS; pub(super) fn new(live_nodes: usize, vars: usize) -> RWUTable { - let live_node_words = (vars + Self::WORD_RWU_COUNT - 1) / Self::WORD_RWU_COUNT; + let live_node_words = vars.div_ceil(Self::WORD_RWU_COUNT); Self { live_nodes, vars, live_node_words, words: vec![0u8; live_node_words * live_nodes] } } diff --git a/compiler/rustc_serialize/src/leb128.rs b/compiler/rustc_serialize/src/leb128.rs index 954c1f728f2f5..da328dcea0336 100644 --- a/compiler/rustc_serialize/src/leb128.rs +++ b/compiler/rustc_serialize/src/leb128.rs @@ -7,7 +7,7 @@ use crate::serialize::Decoder; /// Returns the length of the longest LEB128 encoding for `T`, assuming `T` is an integer type pub const fn max_leb128_len() -> usize { // The longest LEB128 encoding for an integer uses 7 bits per byte. - (size_of::() * 8 + 6) / 7 + (size_of::() * 8).div_ceil(7) } /// Returns the length of the longest LEB128 encoding of all supported integer types. diff --git a/compiler/rustc_span/src/edit_distance.rs b/compiler/rustc_span/src/edit_distance.rs index 4f3202b694c1c..416e9daa8fb09 100644 --- a/compiler/rustc_span/src/edit_distance.rs +++ b/compiler/rustc_span/src/edit_distance.rs @@ -130,7 +130,7 @@ pub fn edit_distance_with_substrings(a: &str, b: &str, limit: usize) -> Option( continue; } - let size_in_regs = (arg.layout.size.bits() + 31) / 32; + let size_in_regs = arg.layout.size.bits().div_ceil(32); if size_in_regs == 0 { continue; diff --git a/compiler/rustc_target/src/callconv/x86_64.rs b/compiler/rustc_target/src/callconv/x86_64.rs index 700ee73c8fdc1..d8db7ed6e4c0f 100644 --- a/compiler/rustc_target/src/callconv/x86_64.rs +++ b/compiler/rustc_target/src/callconv/x86_64.rs @@ -95,7 +95,7 @@ where Ok(()) } - let n = ((arg.layout.size.bytes() + 7) / 8) as usize; + let n = arg.layout.size.bytes().div_ceil(8) as usize; if n > MAX_EIGHTBYTES { return Err(Memory); } diff --git a/compiler/rustc_target/src/callconv/xtensa.rs b/compiler/rustc_target/src/callconv/xtensa.rs index b687f0e20c660..a73a70a1a0c07 100644 --- a/compiler/rustc_target/src/callconv/xtensa.rs +++ b/compiler/rustc_target/src/callconv/xtensa.rs @@ -54,7 +54,7 @@ where // Determine the number of GPRs needed to pass the current argument // according to the ABI. 2*XLen-aligned varargs are passed in "aligned" // register pairs, so may consume 3 registers. - let mut needed_arg_gprs = (size + 32 - 1) / 32; + let mut needed_arg_gprs = size.div_ceil(32); if needed_align == 64 { needed_arg_gprs += *arg_gprs_left % 2; } diff --git a/library/core/src/slice/sort/stable/drift.rs b/library/core/src/slice/sort/stable/drift.rs index cf1df1e91a50d..1edffe095a89d 100644 --- a/library/core/src/slice/sort/stable/drift.rs +++ b/library/core/src/slice/sort/stable/drift.rs @@ -158,7 +158,7 @@ fn merge_tree_scale_factor(n: usize) -> u64 { panic!("Platform not supported"); } - ((1 << 62) + n as u64 - 1) / n as u64 + (1u64 << 62).div_ceil(n as u64) } // Note: merge_tree_depth output is < 64 when left < right as f*x and f*y must @@ -182,7 +182,7 @@ fn sqrt_approx(n: usize) -> usize { // Finally we note that the exponentiation / division can be done directly // with shifts. We OR with 1 to avoid zero-checks in the integer log. let ilog = (n | 1).ilog2(); - let shift = (1 + ilog) / 2; + let shift = ilog.div_ceil(2); ((1 << shift) + (n >> shift)) / 2 } diff --git a/library/core/src/str/iter.rs b/library/core/src/str/iter.rs index 425c4eaee28ee..bcf886484add4 100644 --- a/library/core/src/str/iter.rs +++ b/library/core/src/str/iter.rs @@ -102,7 +102,7 @@ impl<'a> Iterator for Chars<'a> { // `(len + 3)` can't overflow, because we know that the `slice::Iter` // belongs to a slice in memory which has a maximum length of // `isize::MAX` (that's well below `usize::MAX`). - ((len + 3) / 4, Some(len)) + (len.div_ceil(4), Some(len)) } #[inline] @@ -1532,11 +1532,11 @@ impl<'a> Iterator for EncodeUtf16<'a> { // belongs to a slice in memory which has a maximum length of // `isize::MAX` (that's well below `usize::MAX`) if self.extra == 0 { - ((len + 2) / 3, Some(len)) + (len.div_ceil(3), Some(len)) } else { // We're in the middle of a surrogate pair, so add the remaining // surrogate to the bounds. - ((len + 2) / 3 + 1, Some(len + 1)) + (len.div_ceil(3) + 1, Some(len + 1)) } } } diff --git a/library/portable-simd/crates/core_simd/src/lane_count.rs b/library/portable-simd/crates/core_simd/src/lane_count.rs index 280b27bc9bc6f..bbdfd5f5f3ed3 100644 --- a/library/portable-simd/crates/core_simd/src/lane_count.rs +++ b/library/portable-simd/crates/core_simd/src/lane_count.rs @@ -8,7 +8,7 @@ pub struct LaneCount; impl LaneCount { /// The number of bytes in a bitmask with this many lanes. - pub const BITMASK_LEN: usize = (N + 7) / 8; + pub const BITMASK_LEN: usize = N.div_ceil(8); } /// Statically guarantees that a lane count is marked as supported. From 6b71399d3850216f9fab05bc99d63854df731e6a Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Thu, 3 Jul 2025 21:36:43 -0700 Subject: [PATCH 04/12] mbe: Introduce an enum for which part of a rule we're parsing Rather than a `bool` that's `true` for the LHS and `false` for the RHS, use a self-documenting enum. --- compiler/rustc_expand/src/mbe/macro_rules.rs | 5 +- compiler/rustc_expand/src/mbe/quoted.rs | 48 ++++++++++++++------ 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 2ffd4e3cf285e..16b9fef6a21c8 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -36,6 +36,7 @@ use crate::base::{ }; use crate::expand::{AstFragment, AstFragmentKind, ensure_complete_parse, parse_ast_fragment}; use crate::mbe::macro_parser::{Error, ErrorReported, Failure, MatcherLoc, Success, TtParser}; +use crate::mbe::quoted::RulePart; use crate::mbe::transcribe::transcribe; use crate::mbe::{self, KleeneOp, macro_check}; @@ -396,7 +397,7 @@ pub fn compile_declarative_macro( let lhs_tt = p.parse_token_tree(); let lhs_tt = mbe::quoted::parse( &TokenStream::new(vec![lhs_tt]), - true, // LHS + RulePart::Pattern, sess, node_id, features, @@ -423,7 +424,7 @@ pub fn compile_declarative_macro( let rhs_tt = p.parse_token_tree(); let rhs_tt = mbe::quoted::parse( &TokenStream::new(vec![rhs_tt]), - false, // RHS + RulePart::Body, sess, node_id, features, diff --git a/compiler/rustc_expand/src/mbe/quoted.rs b/compiler/rustc_expand/src/mbe/quoted.rs index 2daa4e7155843..065ae23a262d0 100644 --- a/compiler/rustc_expand/src/mbe/quoted.rs +++ b/compiler/rustc_expand/src/mbe/quoted.rs @@ -16,6 +16,27 @@ pub(crate) const VALID_FRAGMENT_NAMES_MSG: &str = "valid fragment specifiers are `ident`, `block`, `stmt`, `expr`, `pat`, `ty`, `lifetime`, `literal`, `path`, \ `meta`, `tt`, `item` and `vis`, along with `expr_2021` and `pat_param` for edition compatibility"; +/// Which part of a macro rule we're parsing +#[derive(Copy, Clone)] +pub(crate) enum RulePart { + /// The left-hand side, with patterns and metavar definitions with types + Pattern, + /// The right-hand side body, with metavar references and metavar expressions + Body, +} + +impl RulePart { + #[inline(always)] + fn is_pattern(&self) -> bool { + matches!(self, Self::Pattern) + } + + #[inline(always)] + fn is_body(&self) -> bool { + matches!(self, Self::Body) + } +} + /// Takes a `tokenstream::TokenStream` and returns a `Vec`. Specifically, this /// takes a generic `TokenStream`, such as is used in the rest of the compiler, and returns a /// collection of `TokenTree` for use in parsing a macro. @@ -23,8 +44,8 @@ pub(crate) const VALID_FRAGMENT_NAMES_MSG: &str = "valid fragment specifiers are /// # Parameters /// /// - `input`: a token stream to read from, the contents of which we are parsing. -/// - `parsing_patterns`: `parse` can be used to parse either the "patterns" or the "body" of a -/// macro. Both take roughly the same form _except_ that: +/// - `part`: whether we're parsing the patterns or the body of a macro. Both take roughly the same +/// form _except_ that: /// - In a pattern, metavars are declared with their "matcher" type. For example `$var:expr` or /// `$id:ident`. In this example, `expr` and `ident` are "matchers". They are not present in the /// body of a macro rule -- just in the pattern. @@ -38,7 +59,7 @@ pub(crate) const VALID_FRAGMENT_NAMES_MSG: &str = "valid fragment specifiers are /// A collection of `self::TokenTree`. There may also be some errors emitted to `sess`. pub(super) fn parse( input: &tokenstream::TokenStream, - parsing_patterns: bool, + part: RulePart, sess: &Session, node_id: NodeId, features: &Features, @@ -53,9 +74,9 @@ pub(super) fn parse( while let Some(tree) = iter.next() { // Given the parsed tree, if there is a metavar and we are expecting matchers, actually // parse out the matcher (i.e., in `$id:ident` this would parse the `:` and `ident`). - let tree = parse_tree(tree, &mut iter, parsing_patterns, sess, node_id, features, edition); + let tree = parse_tree(tree, &mut iter, part, sess, node_id, features, edition); - if !parsing_patterns { + if part.is_body() { // No matchers allowed, nothing to process here result.push(tree); continue; @@ -157,13 +178,13 @@ fn maybe_emit_macro_metavar_expr_concat_feature(features: &Features, sess: &Sess /// - `tree`: the tree we wish to convert. /// - `outer_iter`: an iterator over trees. We may need to read more tokens from it in order to finish /// converting `tree` -/// - `parsing_patterns`: same as [parse]. +/// - `part`: same as [parse]. /// - `sess`: the parsing session. Any errors will be emitted to this session. /// - `features`: language features so we can do feature gating. fn parse_tree<'a>( tree: &'a tokenstream::TokenTree, outer_iter: &mut TokenStreamIter<'a>, - parsing_patterns: bool, + part: RulePart, sess: &Session, node_id: NodeId, features: &Features, @@ -189,7 +210,7 @@ fn parse_tree<'a>( match next { // `tree` is followed by a delimited set of token trees. Some(&tokenstream::TokenTree::Delimited(delim_span, _, delim, ref tts)) => { - if parsing_patterns { + if part.is_pattern() { if delim != Delimiter::Parenthesis { span_dollar_dollar_or_metavar_in_the_lhs_err( sess, @@ -244,13 +265,13 @@ fn parse_tree<'a>( // If we didn't find a metavar expression above, then we must have a // repetition sequence in the macro (e.g. `$(pat)*`). Parse the // contents of the sequence itself - let sequence = parse(tts, parsing_patterns, sess, node_id, features, edition); + let sequence = parse(tts, part, sess, node_id, features, edition); // Get the Kleene operator and optional separator let (separator, kleene) = parse_sep_and_kleene_op(&mut iter, delim_span.entire(), sess); // Count the number of captured "names" (i.e., named metavars) let num_captures = - if parsing_patterns { count_metavar_decls(&sequence) } else { 0 }; + if part.is_pattern() { count_metavar_decls(&sequence) } else { 0 }; TokenTree::Sequence( delim_span, SequenceRepetition { tts: sequence, separator, kleene, num_captures }, @@ -274,7 +295,7 @@ fn parse_tree<'a>( Token { kind: token::Dollar, span: dollar_span2 }, _, )) => { - if parsing_patterns { + if part.is_pattern() { span_dollar_dollar_or_metavar_in_the_lhs_err( sess, &Token { kind: token::Dollar, span: dollar_span2 }, @@ -306,10 +327,7 @@ fn parse_tree<'a>( &tokenstream::TokenTree::Delimited(span, spacing, delim, ref tts) => TokenTree::Delimited( span, spacing, - Delimited { - delim, - tts: parse(tts, parsing_patterns, sess, node_id, features, edition), - }, + Delimited { delim, tts: parse(tts, part, sess, node_id, features, edition) }, ), } } From 971feb618518d5b53dcdc43dcc873b5f7be3f88c Mon Sep 17 00:00:00 2001 From: binarycat Date: Sat, 5 Jul 2025 10:17:31 -0500 Subject: [PATCH 05/12] tidy: use --bless for tidy spellcheck instead of spellcheck:fix previous behavior was inconsistent with existing extra checks. --- src/tools/tidy/src/ext_tool_checks.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/tools/tidy/src/ext_tool_checks.rs b/src/tools/tidy/src/ext_tool_checks.rs index 2904908fd436e..f76403711b148 100644 --- a/src/tools/tidy/src/ext_tool_checks.rs +++ b/src/tools/tidy/src/ext_tool_checks.rs @@ -72,8 +72,7 @@ fn check_impl( let shell_lint = lint_args.contains(&"shell:lint") || shell_all; let cpp_all = lint_args.contains(&"cpp"); let cpp_fmt = lint_args.contains(&"cpp:fmt") || cpp_all; - let spellcheck_all = lint_args.contains(&"spellcheck"); - let spellcheck_fix = lint_args.contains(&"spellcheck:fix"); + let spellcheck = lint_args.contains(&"spellcheck"); let mut py_path = None; @@ -226,7 +225,7 @@ fn check_impl( shellcheck_runner(&merge_args(&cfg_args, &file_args_shc))?; } - if spellcheck_all || spellcheck_fix { + if spellcheck { let config_path = root_path.join("typos.toml"); // sync target files with .github/workflows/spellcheck.yml let mut args = vec![ @@ -238,11 +237,11 @@ fn check_impl( "./src/librustdoc", ]; - if spellcheck_all { - eprintln!("spellcheck files"); - } else if spellcheck_fix { + if bless { eprintln!("spellcheck files and fix"); args.push("--write-changes"); + } else { + eprintln!("spellcheck files"); } spellcheck_runner(&args)?; } From c1ed2ac8a27bce8ccc655bd87102752d9d8015d6 Mon Sep 17 00:00:00 2001 From: binarycat Date: Sat, 5 Jul 2025 10:24:17 -0500 Subject: [PATCH 06/12] tidy: add specific error message for trying to use `spellcheck:fix`. --- src/tools/tidy/src/ext_tool_checks.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/tools/tidy/src/ext_tool_checks.rs b/src/tools/tidy/src/ext_tool_checks.rs index f76403711b148..d2da63a970311 100644 --- a/src/tools/tidy/src/ext_tool_checks.rs +++ b/src/tools/tidy/src/ext_tool_checks.rs @@ -65,6 +65,13 @@ fn check_impl( None => vec![], }; + if lint_args.contains(&"spellcheck:fix") { + return Err(Error::Generic( + "`spellcheck:fix` is no longer valid, use `--extra=check=spellcheck --bless`" + .to_string(), + )); + } + let python_all = lint_args.contains(&"py"); let python_lint = lint_args.contains(&"py:lint") || python_all; let python_fmt = lint_args.contains(&"py:fmt") || python_all; From 9c9c5b041b807ddef7d34d128b15ed10c55e751b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 5 Jul 2025 22:21:23 +0200 Subject: [PATCH 07/12] compiletest: print slightly more information on fs::write failure --- src/tools/compiletest/src/runtest.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index f8bf4ee3022e1..a03dd23b56bf3 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1918,7 +1918,8 @@ impl<'test> TestCx<'test> { fn dump_output_file(&self, out: &str, extension: &str) { let outfile = self.make_out_name(extension); - fs::write(outfile.as_std_path(), out).unwrap(); + fs::write(outfile.as_std_path(), out) + .unwrap_or_else(|err| panic!("failed to write {outfile}: {err:?}")); } /// Creates a filename for output with the given extension. From 9f78173bb5270e6687a9b594e3ddb922f7b11bb1 Mon Sep 17 00:00:00 2001 From: binarycat Date: Sat, 5 Jul 2025 14:46:34 -0500 Subject: [PATCH 08/12] bootstrap: add change_tracker entry for removal of spellcheck:fix --- src/bootstrap/src/utils/change_tracker.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/bootstrap/src/utils/change_tracker.rs b/src/bootstrap/src/utils/change_tracker.rs index 68312a503eec6..f5a958618f99f 100644 --- a/src/bootstrap/src/utils/change_tracker.rs +++ b/src/bootstrap/src/utils/change_tracker.rs @@ -446,4 +446,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[ severity: ChangeSeverity::Info, summary: "Added new option `build.tidy-extra-checks` to specify a default value for the --extra-checks cli flag.", }, + ChangeInfo { + change_id: 143493, + severity: ChangeSeverity::Warning, + summary: "The `spellcheck:fix` tidy extra check argument has been removed, use `--bless` instead", + }, ]; From a7382eae3fccb2db5353640454c5d09840d3089e Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Thu, 3 Jul 2025 21:55:12 -0700 Subject: [PATCH 09/12] mbe: Add a helper to parse a single `TokenTree` The parser repeatedly invokes the `parse` function, constructing a one-entry vector, and assuming that the return value will be a one-entry vector. Add a helper for that case. This will simplify adding additional callers, and put all the logic in one place to allow potential future simplification of the one-TT case. --- compiler/rustc_expand/src/mbe/macro_rules.rs | 24 +++----------------- compiler/rustc_expand/src/mbe/quoted.rs | 18 ++++++++++++++- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 16b9fef6a21c8..44201d48147b7 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -36,7 +36,7 @@ use crate::base::{ }; use crate::expand::{AstFragment, AstFragmentKind, ensure_complete_parse, parse_ast_fragment}; use crate::mbe::macro_parser::{Error, ErrorReported, Failure, MatcherLoc, Success, TtParser}; -use crate::mbe::quoted::RulePart; +use crate::mbe::quoted::{RulePart, parse_one_tt}; use crate::mbe::transcribe::transcribe; use crate::mbe::{self, KleeneOp, macro_check}; @@ -395,16 +395,7 @@ pub fn compile_declarative_macro( while p.token != token::Eof { let lhs_tt = p.parse_token_tree(); - let lhs_tt = mbe::quoted::parse( - &TokenStream::new(vec![lhs_tt]), - RulePart::Pattern, - sess, - node_id, - features, - edition, - ) - .pop() - .unwrap(); + let lhs_tt = parse_one_tt(lhs_tt, RulePart::Pattern, sess, node_id, features, edition); // We don't handle errors here, the driver will abort after parsing/expansion. We can // report every error in every macro this way. check_emission(check_lhs_nt_follows(sess, node_id, &lhs_tt)); @@ -422,16 +413,7 @@ pub fn compile_declarative_macro( return dummy_syn_ext(guar); } let rhs_tt = p.parse_token_tree(); - let rhs_tt = mbe::quoted::parse( - &TokenStream::new(vec![rhs_tt]), - RulePart::Body, - sess, - node_id, - features, - edition, - ) - .pop() - .unwrap(); + let rhs_tt = parse_one_tt(rhs_tt, RulePart::Body, sess, node_id, features, edition); check_emission(check_rhs(sess, &rhs_tt)); check_emission(macro_check::check_meta_variables(&sess.psess, node_id, &lhs_tt, &rhs_tt)); lhses.push(lhs_tt); diff --git a/compiler/rustc_expand/src/mbe/quoted.rs b/compiler/rustc_expand/src/mbe/quoted.rs index 065ae23a262d0..eb874a27cece5 100644 --- a/compiler/rustc_expand/src/mbe/quoted.rs +++ b/compiler/rustc_expand/src/mbe/quoted.rs @@ -57,7 +57,7 @@ impl RulePart { /// # Returns /// /// A collection of `self::TokenTree`. There may also be some errors emitted to `sess`. -pub(super) fn parse( +fn parse( input: &tokenstream::TokenStream, part: RulePart, sess: &Session, @@ -152,6 +152,22 @@ pub(super) fn parse( result } +/// Takes a `tokenstream::TokenTree` and returns a `self::TokenTree`. Like `parse`, but for a +/// single token tree. Emits errors to `sess` if needed. +#[inline] +pub(super) fn parse_one_tt( + input: tokenstream::TokenTree, + part: RulePart, + sess: &Session, + node_id: NodeId, + features: &Features, + edition: Edition, +) -> TokenTree { + parse(&tokenstream::TokenStream::new(vec![input]), part, sess, node_id, features, edition) + .pop() + .unwrap() +} + /// Asks for the `macro_metavar_expr` feature if it is not enabled fn maybe_emit_macro_metavar_expr_feature(features: &Features, sess: &Session, span: Span) { if !features.macro_metavar_expr() { From 0d5ab3e46ce3be1e5e3c45ac0ff7d355303c4353 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Thu, 3 Jul 2025 22:57:06 -0700 Subject: [PATCH 10/12] mbe: Simplify a match to a let-else --- compiler/rustc_expand/src/mbe/macro_rules.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 44201d48147b7..731a4a08ab00f 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -213,9 +213,8 @@ fn expand_macro<'cx>( match try_success_result { Ok((i, named_matches)) => { - let (rhs, rhs_span): (&mbe::Delimited, DelimSpan) = match &rhses[i] { - mbe::TokenTree::Delimited(span, _, delimited) => (&delimited, *span), - _ => cx.dcx().span_bug(sp, "malformed macro rhs"), + let mbe::TokenTree::Delimited(rhs_span, _, ref rhs) = rhses[i] else { + cx.dcx().span_bug(sp, "malformed macro rhs"); }; let arm_span = rhses[i].span(); From 63cfb3af37d74d312829d4e260e03128eb7e3f27 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Fri, 4 Jul 2025 01:22:37 -0700 Subject: [PATCH 11/12] mbe: Defer checks for `compile_error!` until reporting an unused macro rule The MBE parser checks rules at initial parse time to see if their RHS has `compile_error!` in it, and returns a list of rule indexes and LHS spans that don't map to `compile_error!`, for use in unused macro rule checking. Instead, have the unused macro rule reporting ask the macro for the rule to report, and let the macro check at that time. That avoids checking rules unless they're unused. In the process, refactor the data structure used to store macro rules, to group the LHS and RHS (and LHS span) of each rule together, and refactor the unused rule tracking to only track rule indexes. This ends up being a net simplification, and reduction in code size. --- compiler/rustc_expand/src/base.rs | 4 + compiler/rustc_expand/src/mbe/diagnostics.rs | 10 +- compiler/rustc_expand/src/mbe/macro_rules.rs | 114 +++++++----------- .../rustc_resolve/src/build_reduced_graph.rs | 8 +- compiler/rustc_resolve/src/lib.rs | 6 +- compiler/rustc_resolve/src/macros.rs | 30 +++-- 6 files changed, 80 insertions(+), 92 deletions(-) diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 80f6e9d9fc487..d6d898088395e 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -348,6 +348,10 @@ pub trait TTMacroExpander { span: Span, input: TokenStream, ) -> MacroExpanderResult<'cx>; + + fn get_unused_rule(&self, _rule_i: usize) -> Option<(&Ident, Span)> { + None + } } pub type MacroExpanderResult<'cx> = ExpandResult, ()>; diff --git a/compiler/rustc_expand/src/mbe/diagnostics.rs b/compiler/rustc_expand/src/mbe/diagnostics.rs index c607a3a3652c9..7a280d671f413 100644 --- a/compiler/rustc_expand/src/mbe/diagnostics.rs +++ b/compiler/rustc_expand/src/mbe/diagnostics.rs @@ -10,7 +10,7 @@ use rustc_span::source_map::SourceMap; use rustc_span::{ErrorGuaranteed, Ident, Span}; use tracing::debug; -use super::macro_rules::{NoopTracker, parser_from_cx}; +use super::macro_rules::{MacroRule, NoopTracker, parser_from_cx}; use crate::expand::{AstFragmentKind, parse_ast_fragment}; use crate::mbe::macro_parser::ParseResult::*; use crate::mbe::macro_parser::{MatcherLoc, NamedParseResult, TtParser}; @@ -22,14 +22,14 @@ pub(super) fn failed_to_match_macro( def_span: Span, name: Ident, arg: TokenStream, - lhses: &[Vec], + rules: &[MacroRule], ) -> (Span, ErrorGuaranteed) { debug!("failed to match macro"); // An error occurred, try the expansion again, tracking the expansion closely for better // diagnostics. let mut tracker = CollectTrackerAndEmitter::new(psess.dcx(), sp); - let try_success_result = try_match_macro(psess, name, &arg, lhses, &mut tracker); + let try_success_result = try_match_macro(psess, name, &arg, rules, &mut tracker); if try_success_result.is_ok() { // Nonterminal parser recovery might turn failed matches into successful ones, @@ -80,12 +80,12 @@ pub(super) fn failed_to_match_macro( // Check whether there's a missing comma in this macro call, like `println!("{}" a);` if let Some((arg, comma_span)) = arg.add_comma() { - for lhs in lhses { + for rule in rules { let parser = parser_from_cx(psess, arg.clone(), Recovery::Allowed); let mut tt_parser = TtParser::new(name); if let Success(_) = - tt_parser.parse_tt(&mut Cow::Borrowed(&parser), lhs, &mut NoopTracker) + tt_parser.parse_tt(&mut Cow::Borrowed(&parser), &rule.lhs, &mut NoopTracker) { if comma_span.is_dummy() { err.note("you might be missing a comma"); diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 731a4a08ab00f..52cdcc5c747e9 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -98,13 +98,18 @@ impl<'a> ParserAnyMacro<'a> { } } +pub(super) struct MacroRule { + pub(super) lhs: Vec, + lhs_span: Span, + rhs: mbe::TokenTree, +} + struct MacroRulesMacroExpander { node_id: NodeId, name: Ident, span: Span, transparency: Transparency, - lhses: Vec>, - rhses: Vec, + rules: Vec, } impl TTMacroExpander for MacroRulesMacroExpander { @@ -122,10 +127,15 @@ impl TTMacroExpander for MacroRulesMacroExpander { self.name, self.transparency, input, - &self.lhses, - &self.rhses, + &self.rules, )) } + + fn get_unused_rule(&self, rule_i: usize) -> Option<(&Ident, Span)> { + // If the rhs contains an invocation like `compile_error!`, don't report it as unused. + let rule = &self.rules[rule_i]; + if has_compile_error_macro(&rule.rhs) { None } else { Some((&self.name, rule.lhs_span)) } + } } struct DummyExpander(ErrorGuaranteed); @@ -184,9 +194,8 @@ impl<'matcher> Tracker<'matcher> for NoopTracker { } } -/// Expands the rules based macro defined by `lhses` and `rhses` for a given -/// input `arg`. -#[instrument(skip(cx, transparency, arg, lhses, rhses))] +/// Expands the rules based macro defined by `rules` for a given input `arg`. +#[instrument(skip(cx, transparency, arg, rules))] fn expand_macro<'cx>( cx: &'cx mut ExtCtxt<'_>, sp: Span, @@ -195,8 +204,7 @@ fn expand_macro<'cx>( name: Ident, transparency: Transparency, arg: TokenStream, - lhses: &[Vec], - rhses: &[mbe::TokenTree], + rules: &[MacroRule], ) -> Box { let psess = &cx.sess.psess; // Macros defined in the current crate have a real node id, @@ -209,14 +217,14 @@ fn expand_macro<'cx>( } // Track nothing for the best performance. - let try_success_result = try_match_macro(psess, name, &arg, lhses, &mut NoopTracker); + let try_success_result = try_match_macro(psess, name, &arg, rules, &mut NoopTracker); match try_success_result { - Ok((i, named_matches)) => { - let mbe::TokenTree::Delimited(rhs_span, _, ref rhs) = rhses[i] else { + Ok((i, rule, named_matches)) => { + let mbe::TokenTree::Delimited(rhs_span, _, ref rhs) = rule.rhs else { cx.dcx().span_bug(sp, "malformed macro rhs"); }; - let arm_span = rhses[i].span(); + let arm_span = rule.rhs.span(); // rhs has holes ( `$id` and `$(...)` that need filled) let id = cx.current_expansion.id; @@ -262,7 +270,7 @@ fn expand_macro<'cx>( Err(CanRetry::Yes) => { // Retry and emit a better error. let (span, guar) = - diagnostics::failed_to_match_macro(cx.psess(), sp, def_span, name, arg, lhses); + diagnostics::failed_to_match_macro(cx.psess(), sp, def_span, name, arg, rules); cx.trace_macros_diag(); DummyResult::any(span, guar) } @@ -278,14 +286,14 @@ pub(super) enum CanRetry { /// Try expanding the macro. Returns the index of the successful arm and its named_matches if it was successful, /// and nothing if it failed. On failure, it's the callers job to use `track` accordingly to record all errors /// correctly. -#[instrument(level = "debug", skip(psess, arg, lhses, track), fields(tracking = %T::description()))] +#[instrument(level = "debug", skip(psess, arg, rules, track), fields(tracking = %T::description()))] pub(super) fn try_match_macro<'matcher, T: Tracker<'matcher>>( psess: &ParseSess, name: Ident, arg: &TokenStream, - lhses: &'matcher [Vec], + rules: &'matcher [MacroRule], track: &mut T, -) -> Result<(usize, NamedMatches), CanRetry> { +) -> Result<(usize, &'matcher MacroRule, NamedMatches), CanRetry> { // We create a base parser that can be used for the "black box" parts. // Every iteration needs a fresh copy of that parser. However, the parser // is not mutated on many of the iterations, particularly when dealing with @@ -308,7 +316,7 @@ pub(super) fn try_match_macro<'matcher, T: Tracker<'matcher>>( let parser = parser_from_cx(psess, arg.clone(), T::recovery()); // Try each arm's matchers. let mut tt_parser = TtParser::new(name); - for (i, lhs) in lhses.iter().enumerate() { + for (i, rule) in rules.iter().enumerate() { let _tracing_span = trace_span!("Matching arm", %i); // Take a snapshot of the state of pre-expansion gating at this point. @@ -317,7 +325,7 @@ pub(super) fn try_match_macro<'matcher, T: Tracker<'matcher>>( // are not recorded. On the first `Success(..)`ful matcher, the spans are merged. let mut gated_spans_snapshot = mem::take(&mut *psess.gated_spans.spans.borrow_mut()); - let result = tt_parser.parse_tt(&mut Cow::Borrowed(&parser), lhs, track); + let result = tt_parser.parse_tt(&mut Cow::Borrowed(&parser), &rule.lhs, track); track.after_arm(&result); @@ -328,7 +336,7 @@ pub(super) fn try_match_macro<'matcher, T: Tracker<'matcher>>( // Merge the gated spans from parsing the matcher with the preexisting ones. psess.gated_spans.merge(gated_spans_snapshot); - return Ok((i, named_matches)); + return Ok((i, rule, named_matches)); } Failure(_) => { trace!("Failed to match arm, trying the next one"); @@ -364,7 +372,7 @@ pub fn compile_declarative_macro( span: Span, node_id: NodeId, edition: Edition, -) -> (SyntaxExtension, Vec<(usize, Span)>) { +) -> (SyntaxExtension, usize) { let mk_syn_ext = |expander| { SyntaxExtension::new( sess, @@ -377,7 +385,7 @@ pub fn compile_declarative_macro( node_id != DUMMY_NODE_ID, ) }; - let dummy_syn_ext = |guar| (mk_syn_ext(Arc::new(DummyExpander(guar))), Vec::new()); + let dummy_syn_ext = |guar| (mk_syn_ext(Arc::new(DummyExpander(guar))), 0); let macro_rules = macro_def.macro_rules; let exp_sep = if macro_rules { exp!(Semi) } else { exp!(Comma) }; @@ -389,8 +397,7 @@ pub fn compile_declarative_macro( let mut guar = None; let mut check_emission = |ret: Result<(), ErrorGuaranteed>| guar = guar.or(ret.err()); - let mut lhses = Vec::new(); - let mut rhses = Vec::new(); + let mut rules = Vec::new(); while p.token != token::Eof { let lhs_tt = p.parse_token_tree(); @@ -415,8 +422,15 @@ pub fn compile_declarative_macro( let rhs_tt = parse_one_tt(rhs_tt, RulePart::Body, sess, node_id, features, edition); check_emission(check_rhs(sess, &rhs_tt)); check_emission(macro_check::check_meta_variables(&sess.psess, node_id, &lhs_tt, &rhs_tt)); - lhses.push(lhs_tt); - rhses.push(rhs_tt); + let lhs_span = lhs_tt.span(); + // Convert the lhs into `MatcherLoc` form, which is better for doing the + // actual matching. + let lhs = if let mbe::TokenTree::Delimited(.., delimited) = lhs_tt { + mbe::macro_parser::compute_locs(&delimited.tts) + } else { + return dummy_syn_ext(guar.unwrap()); + }; + rules.push(MacroRule { lhs, lhs_span, rhs: rhs_tt }); if p.token == token::Eof { break; } @@ -425,7 +439,7 @@ pub fn compile_declarative_macro( } } - if lhses.is_empty() { + if rules.is_empty() { let guar = sess.dcx().span_err(span, "macros must contain at least one rule"); return dummy_syn_ext(guar); } @@ -439,48 +453,12 @@ pub fn compile_declarative_macro( return dummy_syn_ext(guar); } - // Compute the spans of the macro rules for unused rule linting. - // Also, we are only interested in non-foreign macros. - let rule_spans = if node_id != DUMMY_NODE_ID { - lhses - .iter() - .zip(rhses.iter()) - .enumerate() - // If the rhs contains an invocation like compile_error!, - // don't consider the rule for the unused rule lint. - .filter(|(_idx, (_lhs, rhs))| !has_compile_error_macro(rhs)) - // We only take the span of the lhs here, - // so that the spans of created warnings are smaller. - .map(|(idx, (lhs, _rhs))| (idx, lhs.span())) - .collect::>() - } else { - Vec::new() - }; + // Return the number of rules for unused rule linting, if this is a local macro. + let nrules = if node_id != DUMMY_NODE_ID { rules.len() } else { 0 }; - // Convert the lhses into `MatcherLoc` form, which is better for doing the - // actual matching. - let lhses = lhses - .iter() - .map(|lhs| { - // Ignore the delimiters around the matcher. - match lhs { - mbe::TokenTree::Delimited(.., delimited) => { - mbe::macro_parser::compute_locs(&delimited.tts) - } - _ => sess.dcx().span_bug(span, "malformed macro lhs"), - } - }) - .collect(); - - let expander = Arc::new(MacroRulesMacroExpander { - name: ident, - span, - node_id, - transparency, - lhses, - rhses, - }); - (mk_syn_ext(expander), rule_spans) + let expander = + Arc::new(MacroRulesMacroExpander { name: ident, span, node_id, transparency, rules }); + (mk_syn_ext(expander), nrules) } fn check_lhs_nt_follows( diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 650a827ba5644..eeb8cb893d75b 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -1202,12 +1202,8 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { fn insert_unused_macro(&mut self, ident: Ident, def_id: LocalDefId, node_id: NodeId) { if !ident.as_str().starts_with('_') { self.r.unused_macros.insert(def_id, (node_id, ident)); - for (rule_i, rule_span) in &self.r.macro_map[&def_id.to_def_id()].rule_spans { - self.r - .unused_macro_rules - .entry(node_id) - .or_default() - .insert(*rule_i, (ident, *rule_span)); + for rule_i in 0..self.r.macro_map[&def_id.to_def_id()].nrules { + self.r.unused_macro_rules.entry(node_id).or_default().insert(rule_i); } } } diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 7162f3a77d350..3f865d7c2daca 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1014,13 +1014,13 @@ struct DeriveData { struct MacroData { ext: Arc, - rule_spans: Vec<(usize, Span)>, + nrules: usize, macro_rules: bool, } impl MacroData { fn new(ext: Arc) -> MacroData { - MacroData { ext, rule_spans: Vec::new(), macro_rules: false } + MacroData { ext, nrules: 0, macro_rules: false } } } @@ -1135,7 +1135,7 @@ pub struct Resolver<'ra, 'tcx> { ast_transform_scopes: FxHashMap>, unused_macros: FxIndexMap, /// A map from the macro to all its potentially unused arms. - unused_macro_rules: FxIndexMap>, + unused_macro_rules: FxIndexMap>, proc_macro_stubs: FxHashSet, /// Traces collected during macro resolution and validated when it's complete. single_segment_macro_resolutions: diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 3d33a02a9c6da..9bc96403559ee 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -351,13 +351,23 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> { } for (&node_id, unused_arms) in self.unused_macro_rules.iter() { - for (&arm_i, &(ident, rule_span)) in unused_arms.to_sorted_stable_ord() { - self.lint_buffer.buffer_lint( - UNUSED_MACRO_RULES, - node_id, - rule_span, - BuiltinLintDiag::MacroRuleNeverUsed(arm_i, ident.name), - ); + if unused_arms.is_empty() { + continue; + } + let def_id = self.local_def_id(node_id).to_def_id(); + let m = &self.macro_map[&def_id]; + let SyntaxExtensionKind::LegacyBang(ref ext) = m.ext.kind else { + continue; + }; + for &arm_i in unused_arms.to_sorted_stable_ord() { + if let Some((ident, rule_span)) = ext.get_unused_rule(arm_i) { + self.lint_buffer.buffer_lint( + UNUSED_MACRO_RULES, + node_id, + rule_span, + BuiltinLintDiag::MacroRuleNeverUsed(arm_i, ident.name), + ); + } } } } @@ -1146,7 +1156,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { node_id: NodeId, edition: Edition, ) -> MacroData { - let (mut ext, mut rule_spans) = compile_declarative_macro( + let (mut ext, mut nrules) = compile_declarative_macro( self.tcx.sess, self.tcx.features(), macro_def, @@ -1163,13 +1173,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // The macro is a built-in, replace its expander function // while still taking everything else from the source code. ext.kind = builtin_ext_kind.clone(); - rule_spans = Vec::new(); + nrules = 0; } else { self.dcx().emit_err(errors::CannotFindBuiltinMacroWithName { span, ident }); } } - MacroData { ext: Arc::new(ext), rule_spans, macro_rules: macro_def.macro_rules } + MacroData { ext: Arc::new(ext), nrules, macro_rules: macro_def.macro_rules } } fn path_accessible( From 5bbab8967d08362a36ceb5622dd7daac0c5692f4 Mon Sep 17 00:00:00 2001 From: bohan Date: Sat, 5 Jul 2025 18:17:03 +0800 Subject: [PATCH 12/12] distinguish the duplicate item of rpitit --- compiler/rustc_ty_utils/src/assoc.rs | 31 ++++++++++- .../rpitit-duplicate-associated-fn.rs | 30 +++++++++++ .../rpitit-duplicate-associated-fn.stderr | 53 +++++++++++++++++++ 3 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 tests/ui/impl-trait/in-trait/rpitit-duplicate-associated-fn.rs create mode 100644 tests/ui/impl-trait/in-trait/rpitit-duplicate-associated-fn.stderr diff --git a/compiler/rustc_ty_utils/src/assoc.rs b/compiler/rustc_ty_utils/src/assoc.rs index a65f9b347dc6d..2fb3c5ff945af 100644 --- a/compiler/rustc_ty_utils/src/assoc.rs +++ b/compiler/rustc_ty_utils/src/assoc.rs @@ -195,12 +195,39 @@ fn associated_types_for_impl_traits_in_associated_fn( match tcx.def_kind(parent_def_id) { DefKind::Trait => { if let Some(output) = tcx.hir_get_fn_output(fn_def_id) { - let data = DefPathData::AnonAssocTy(tcx.item_name(fn_def_id.to_def_id())); + let def_path_id = |def_id: LocalDefId| tcx.item_name(def_id.to_def_id()); + let def_path_data = def_path_id(fn_def_id); + + let (.., trait_item_refs) = tcx.hir_expect_item(parent_def_id).expect_trait(); + // The purpose of `disambiguator_idx` is to ensure there are + // no duplicate `def_id` in certain cases, such as: + // ``` + // trait Foo { + // fn bar() -> impl Trait; + // fn bar() -> impl Trait; + // // ~~~~~~~~~~ It will generate the same ID if we don’t disambiguate it. + // } + // ``` + let disambiguator_idx = trait_item_refs + .iter() + .take_while(|item| item.id.owner_id.def_id != fn_def_id) + .fold(0, |acc, item| { + if !matches!(item.kind, hir::AssocItemKind::Fn { .. }) { + acc + } else if def_path_id(item.id.owner_id.def_id) == def_path_data { + tcx.def_key(item.id.owner_id.def_id).disambiguated_data.disambiguator + + 1 + } else { + acc + } + }); + + let data = DefPathData::AnonAssocTy(def_path_data); let mut visitor = RPITVisitor { tcx, synthetics: vec![], data, - disambiguator: DisambiguatorState::with(parent_def_id, data, 0), + disambiguator: DisambiguatorState::with(parent_def_id, data, disambiguator_idx), }; visitor.visit_fn_ret_ty(output); tcx.arena.alloc_from_iter( diff --git a/tests/ui/impl-trait/in-trait/rpitit-duplicate-associated-fn.rs b/tests/ui/impl-trait/in-trait/rpitit-duplicate-associated-fn.rs new file mode 100644 index 0000000000000..4fddd7c4ac8c6 --- /dev/null +++ b/tests/ui/impl-trait/in-trait/rpitit-duplicate-associated-fn.rs @@ -0,0 +1,30 @@ +// issue#140796 + +trait Bar { + fn method() -> impl Sized; + fn method() -> impl Sized; //~ ERROR: the name `method` is defined multiple times +} + +impl Bar for () { //~ ERROR: not all trait items implemented, missing: `method` + fn method() -> impl Sized { + 42 + } + fn method() -> impl Sized { //~ ERROR: duplicate definitions with name `method` + 42 + } +} + +trait T { + fn method() -> impl Sized; +} + +impl T for () { + fn method() -> impl Sized { + 42 + } + fn method() -> impl Sized { //~ ERROR: duplicate definitions with name `method` + 42 + } +} + +fn main() {} diff --git a/tests/ui/impl-trait/in-trait/rpitit-duplicate-associated-fn.stderr b/tests/ui/impl-trait/in-trait/rpitit-duplicate-associated-fn.stderr new file mode 100644 index 0000000000000..b58e8136479c1 --- /dev/null +++ b/tests/ui/impl-trait/in-trait/rpitit-duplicate-associated-fn.stderr @@ -0,0 +1,53 @@ +error[E0428]: the name `method` is defined multiple times + --> $DIR/rpitit-duplicate-associated-fn.rs:5:5 + | +LL | fn method() -> impl Sized; + | -------------------------- previous definition of the value `method` here +LL | fn method() -> impl Sized; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ `method` redefined here + | + = note: `method` must be defined only once in the value namespace of this trait + +error[E0201]: duplicate definitions with name `method`: + --> $DIR/rpitit-duplicate-associated-fn.rs:12:5 + | +LL | fn method() -> impl Sized; + | -------------------------- item in trait +... +LL | / fn method() -> impl Sized { +LL | | 42 +LL | | } + | |_____- previous definition here +LL | / fn method() -> impl Sized { +LL | | 42 +LL | | } + | |_____^ duplicate definition + +error[E0201]: duplicate definitions with name `method`: + --> $DIR/rpitit-duplicate-associated-fn.rs:25:5 + | +LL | fn method() -> impl Sized; + | -------------------------- item in trait +... +LL | / fn method() -> impl Sized { +LL | | 42 +LL | | } + | |_____- previous definition here +LL | / fn method() -> impl Sized { +LL | | 42 +LL | | } + | |_____^ duplicate definition + +error[E0046]: not all trait items implemented, missing: `method` + --> $DIR/rpitit-duplicate-associated-fn.rs:8:1 + | +LL | fn method() -> impl Sized; + | -------------------------- `method` from trait +... +LL | impl Bar for () { + | ^^^^^^^^^^^^^^^ missing `method` in implementation + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0046, E0201, E0428. +For more information about an error, try `rustc --explain E0046`.