From 077b8d5c370391d428dbb4d3d037f6e51b1349a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sun, 16 Mar 2025 23:59:10 +0100 Subject: [PATCH 01/19] Abort in deadlock handler if we fail to get a query map --- compiler/rustc_interface/src/util.rs | 13 +++++++++++-- compiler/rustc_query_impl/src/plumbing.rs | 15 +++++++++------ compiler/rustc_query_system/src/query/job.rs | 2 +- compiler/rustc_query_system/src/query/mod.rs | 2 +- compiler/rustc_query_system/src/query/plumbing.rs | 5 +++-- 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 5cccab893bb35..0904795529e3b 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -192,7 +192,16 @@ pub(crate) fn run_in_thread_pool_with_globals R + Send, // `TyCtxt` TLS reference here. let query_map = current_gcx2.access(|gcx| { tls::enter_context(&tls::ImplicitCtxt::new(gcx), || { - tls::with(|tcx| QueryCtxt::new(tcx).collect_active_jobs()) + tls::with(|tcx| { + let (query_map, complete) = QueryCtxt::new(tcx).collect_active_jobs(); + if !complete { + eprintln!("internal compiler error: failed to get query map in deadlock handler, aborting process"); + // We need to abort here as we failed to resolve the deadlock, + // otherwise the compiler could just hang, + process::abort(); + } + query_map + }) }) }); let query_map = FromDyn::from(query_map); @@ -201,7 +210,7 @@ pub(crate) fn run_in_thread_pool_with_globals R + Send, .name("rustc query cycle handler".to_string()) .spawn(move || { let on_panic = defer(|| { - eprintln!("query cycle handler thread panicked, aborting process"); + eprintln!("internal compiler error: query cycle handler thread panicked, aborting process"); // We need to abort here as we failed to resolve the deadlock, // otherwise the compiler could just hang, process::abort(); diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 2b8457ace8ee7..85abb77e53c7d 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -79,14 +79,15 @@ impl QueryContext for QueryCtxt<'_> { tls::with_related_context(self.tcx, |icx| icx.query) } - fn collect_active_jobs(self) -> QueryMap { + fn collect_active_jobs(self) -> (QueryMap, bool) { let mut jobs = QueryMap::default(); + let mut complete = true; for collect in super::TRY_COLLECT_ACTIVE_JOBS.iter() { - collect(self.tcx, &mut jobs); + collect(self.tcx, &mut jobs, &mut complete); } - jobs + (jobs, complete) } // Interactions with on_disk_cache @@ -139,7 +140,8 @@ impl QueryContext for QueryCtxt<'_> { } fn depth_limit_error(self, job: QueryJobId) { - let (info, depth) = job.find_dep_kind_root(self.collect_active_jobs()); + // FIXME: `collect_active_jobs` expects no locks to be held, which doesn't hold for this call. + let (info, depth) = job.find_dep_kind_root(self.collect_active_jobs().0); let suggested_limit = match self.recursion_limit() { Limit(0) => Limit(2), @@ -677,7 +679,7 @@ macro_rules! define_queries { } } - pub(crate) fn try_collect_active_jobs<'tcx>(tcx: TyCtxt<'tcx>, qmap: &mut QueryMap) { + pub(crate) fn try_collect_active_jobs<'tcx>(tcx: TyCtxt<'tcx>, qmap: &mut QueryMap, complete: &mut bool) { let make_query = |tcx, key| { let kind = rustc_middle::dep_graph::dep_kinds::$name; let name = stringify!($name); @@ -692,6 +694,7 @@ macro_rules! define_queries { // don't `unwrap()` here, just manually check for `None` and do best-effort error // reporting. if res.is_none() { + *complete = false; tracing::warn!( "Failed to collect active jobs for query with name `{}`!", stringify!($name) @@ -756,7 +759,7 @@ macro_rules! define_queries { // These arrays are used for iteration and can't be indexed by `DepKind`. - const TRY_COLLECT_ACTIVE_JOBS: &[for<'tcx> fn(TyCtxt<'tcx>, &mut QueryMap)] = + const TRY_COLLECT_ACTIVE_JOBS: &[for<'tcx> fn(TyCtxt<'tcx>, &mut QueryMap, &mut bool)] = &[$(query_impl::$name::try_collect_active_jobs),*]; const ALLOC_SELF_PROFILE_QUERY_STRINGS: &[ diff --git a/compiler/rustc_query_system/src/query/job.rs b/compiler/rustc_query_system/src/query/job.rs index 37b305d0a8b50..44f9325dce262 100644 --- a/compiler/rustc_query_system/src/query/job.rs +++ b/compiler/rustc_query_system/src/query/job.rs @@ -588,7 +588,7 @@ pub fn print_query_stack( // state if it was responsible for triggering the panic. let mut count_printed = 0; let mut count_total = 0; - let query_map = qcx.collect_active_jobs(); + let query_map = qcx.collect_active_jobs().0; if let Some(ref mut file) = file { let _ = writeln!(file, "\n\nquery stack during panic:"); diff --git a/compiler/rustc_query_system/src/query/mod.rs b/compiler/rustc_query_system/src/query/mod.rs index 2ed0c810b7516..3728691a6f9a9 100644 --- a/compiler/rustc_query_system/src/query/mod.rs +++ b/compiler/rustc_query_system/src/query/mod.rs @@ -86,7 +86,7 @@ pub trait QueryContext: HasDepContext { /// Get the query information from the TLS context. fn current_query_job(self) -> Option; - fn collect_active_jobs(self) -> QueryMap; + fn collect_active_jobs(self) -> (QueryMap, bool); /// Load a side effect associated to the node in the previous session. fn load_side_effect( diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index 7578cb5e2aebd..68590854343e2 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -250,8 +250,9 @@ where Q: QueryConfig, Qcx: QueryContext, { - let error = - try_execute.find_cycle_in_stack(qcx.collect_active_jobs(), &qcx.current_query_job(), span); + let (query_map, complete) = qcx.collect_active_jobs(); + assert!(complete, "failed to collect active queries"); + let error = try_execute.find_cycle_in_stack(query_map, &qcx.current_query_job(), span); (mk_cycle(query, qcx, error), None) } From 157008d7117ec9ca13b70c38058639524437112f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Tue, 18 Mar 2025 14:36:28 +0100 Subject: [PATCH 02/19] Update comments --- compiler/rustc_interface/src/util.rs | 5 +++-- compiler/rustc_query_impl/src/plumbing.rs | 2 ++ compiler/rustc_query_system/src/query/plumbing.rs | 3 +++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 0904795529e3b..249b80bb31d91 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -195,9 +195,10 @@ pub(crate) fn run_in_thread_pool_with_globals R + Send, tls::with(|tcx| { let (query_map, complete) = QueryCtxt::new(tcx).collect_active_jobs(); if !complete { + // There was an unexpected error collecting all active jobs, which we need + // to find cycles to break. + // We want to avoid panicking in the deadlock handler, so we abort instead. eprintln!("internal compiler error: failed to get query map in deadlock handler, aborting process"); - // We need to abort here as we failed to resolve the deadlock, - // otherwise the compiler could just hang, process::abort(); } query_map diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 85abb77e53c7d..301bcac4ca795 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -79,6 +79,8 @@ impl QueryContext for QueryCtxt<'_> { tls::with_related_context(self.tcx, |icx| icx.query) } + /// Returns a query map representing active query jobs and a bool being false + /// if there was an error constructing the map. fn collect_active_jobs(self) -> (QueryMap, bool) { let mut jobs = QueryMap::default(); let mut complete = true; diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index 68590854343e2..dc1d6f760faca 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -251,7 +251,10 @@ where Qcx: QueryContext, { let (query_map, complete) = qcx.collect_active_jobs(); + // Ensure there was no errors collecting all active jobs. + // We need the complete map to ensure we find a cycle to break. assert!(complete, "failed to collect active queries"); + let error = try_execute.find_cycle_in_stack(query_map, &qcx.current_query_job(), span); (mk_cycle(query, qcx, error), None) } From ea99e81485ff5d82cabba9af5d1c21293737cc16 Mon Sep 17 00:00:00 2001 From: Bardi Harborow Date: Fri, 21 Mar 2025 17:53:29 +1100 Subject: [PATCH 03/19] Recognise new IPv6 non-global range from RFC9602 This commit adds the 5f00::/16 range defined by RFC9602 to those ranges which Ipv6Addr::is_global recognises as a non-global IP. This range is used for Segment Routing (SRv6) SIDs. --- library/core/src/net/ip_addr.rs | 2 ++ library/coretests/tests/net/ip_addr.rs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/library/core/src/net/ip_addr.rs b/library/core/src/net/ip_addr.rs index 7aa5ed60d0467..2f027be692860 100644 --- a/library/core/src/net/ip_addr.rs +++ b/library/core/src/net/ip_addr.rs @@ -1623,6 +1623,8 @@ impl Ipv6Addr { // IANA says N/A. || matches!(self.segments(), [0x2002, _, _, _, _, _, _, _]) || self.is_documentation() + // Segment Routing (SRv6) SIDs (`5f00::/16`) + || matches!(self.segments(), [0x5f00, ..]) || self.is_unique_local() || self.is_unicast_link_local()) } diff --git a/library/coretests/tests/net/ip_addr.rs b/library/coretests/tests/net/ip_addr.rs index f01b43282ec42..3fec59d67b748 100644 --- a/library/coretests/tests/net/ip_addr.rs +++ b/library/coretests/tests/net/ip_addr.rs @@ -689,6 +689,8 @@ fn ipv6_properties() { check!("2002::", &[0x20, 0x02, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], unicast_global); + check!("5f00::", &[0x5f, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], unicast_global); + check!("fc00::", &[0xfc, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], unique_local); check!( From 34244c1477e4b485c51872fc3c6d846b4a3c6ffa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Fri, 21 Mar 2025 08:09:42 +0100 Subject: [PATCH 04/19] Address comments --- compiler/rustc_interface/src/util.rs | 17 +++++++------ compiler/rustc_query_impl/src/plumbing.rs | 25 ++++++++++++------- compiler/rustc_query_system/src/query/job.rs | 7 +++++- compiler/rustc_query_system/src/query/mod.rs | 2 +- .../rustc_query_system/src/query/plumbing.rs | 3 +-- 5 files changed, 33 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 249b80bb31d91..333786f0ca38f 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -193,15 +193,16 @@ pub(crate) fn run_in_thread_pool_with_globals R + Send, let query_map = current_gcx2.access(|gcx| { tls::enter_context(&tls::ImplicitCtxt::new(gcx), || { tls::with(|tcx| { - let (query_map, complete) = QueryCtxt::new(tcx).collect_active_jobs(); - if !complete { - // There was an unexpected error collecting all active jobs, which we need - // to find cycles to break. - // We want to avoid panicking in the deadlock handler, so we abort instead. - eprintln!("internal compiler error: failed to get query map in deadlock handler, aborting process"); - process::abort(); + match QueryCtxt::new(tcx).collect_active_jobs() { + Ok(query_map) => query_map, + Err(_) => { + // There was an unexpected error collecting all active jobs, which we need + // to find cycles to break. + // We want to avoid panicking in the deadlock handler, so we abort instead. + eprintln!("internal compiler error: failed to get query map in deadlock handler, aborting process"); + process::abort(); + } } - query_map }) }) }); diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 301bcac4ca795..cafe16ce77b9f 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -79,17 +79,20 @@ impl QueryContext for QueryCtxt<'_> { tls::with_related_context(self.tcx, |icx| icx.query) } - /// Returns a query map representing active query jobs and a bool being false - /// if there was an error constructing the map. - fn collect_active_jobs(self) -> (QueryMap, bool) { + /// Returns a query map representing active query jobs. + /// It returns an incomplete map as an error if it fails + /// to take locks. + fn collect_active_jobs(self) -> Result { let mut jobs = QueryMap::default(); let mut complete = true; for collect in super::TRY_COLLECT_ACTIVE_JOBS.iter() { - collect(self.tcx, &mut jobs, &mut complete); + if collect(self.tcx, &mut jobs).is_none() { + complete = false; + } } - (jobs, complete) + if complete { Ok(jobs) } else { Err(jobs) } } // Interactions with on_disk_cache @@ -143,7 +146,11 @@ impl QueryContext for QueryCtxt<'_> { fn depth_limit_error(self, job: QueryJobId) { // FIXME: `collect_active_jobs` expects no locks to be held, which doesn't hold for this call. - let (info, depth) = job.find_dep_kind_root(self.collect_active_jobs().0); + let query_map = match self.collect_active_jobs() { + Ok(query_map) => query_map, + Err(query_map) => query_map, + }; + let (info, depth) = job.find_dep_kind_root(query_map); let suggested_limit = match self.recursion_limit() { Limit(0) => Limit(2), @@ -681,7 +688,7 @@ macro_rules! define_queries { } } - pub(crate) fn try_collect_active_jobs<'tcx>(tcx: TyCtxt<'tcx>, qmap: &mut QueryMap, complete: &mut bool) { + pub(crate) fn try_collect_active_jobs<'tcx>(tcx: TyCtxt<'tcx>, qmap: &mut QueryMap) -> Option<()> { let make_query = |tcx, key| { let kind = rustc_middle::dep_graph::dep_kinds::$name; let name = stringify!($name); @@ -696,12 +703,12 @@ macro_rules! define_queries { // don't `unwrap()` here, just manually check for `None` and do best-effort error // reporting. if res.is_none() { - *complete = false; tracing::warn!( "Failed to collect active jobs for query with name `{}`!", stringify!($name) ); } + res } pub(crate) fn alloc_self_profile_query_strings<'tcx>( @@ -761,7 +768,7 @@ macro_rules! define_queries { // These arrays are used for iteration and can't be indexed by `DepKind`. - const TRY_COLLECT_ACTIVE_JOBS: &[for<'tcx> fn(TyCtxt<'tcx>, &mut QueryMap, &mut bool)] = + const TRY_COLLECT_ACTIVE_JOBS: &[for<'tcx> fn(TyCtxt<'tcx>, &mut QueryMap) -> Option<()>] = &[$(query_impl::$name::try_collect_active_jobs),*]; const ALLOC_SELF_PROFILE_QUERY_STRINGS: &[ diff --git a/compiler/rustc_query_system/src/query/job.rs b/compiler/rustc_query_system/src/query/job.rs index 44f9325dce262..f0cb378f471e8 100644 --- a/compiler/rustc_query_system/src/query/job.rs +++ b/compiler/rustc_query_system/src/query/job.rs @@ -588,7 +588,12 @@ pub fn print_query_stack( // state if it was responsible for triggering the panic. let mut count_printed = 0; let mut count_total = 0; - let query_map = qcx.collect_active_jobs().0; + + // Make use of a partial query map if we fail to take locks collecting active queries. + let query_map = match qcx.collect_active_jobs() { + Ok(query_map) => query_map, + Err(query_map) => query_map, + }; if let Some(ref mut file) = file { let _ = writeln!(file, "\n\nquery stack during panic:"); diff --git a/compiler/rustc_query_system/src/query/mod.rs b/compiler/rustc_query_system/src/query/mod.rs index 3728691a6f9a9..0d0c66aa978ef 100644 --- a/compiler/rustc_query_system/src/query/mod.rs +++ b/compiler/rustc_query_system/src/query/mod.rs @@ -86,7 +86,7 @@ pub trait QueryContext: HasDepContext { /// Get the query information from the TLS context. fn current_query_job(self) -> Option; - fn collect_active_jobs(self) -> (QueryMap, bool); + fn collect_active_jobs(self) -> Result; /// Load a side effect associated to the node in the previous session. fn load_side_effect( diff --git a/compiler/rustc_query_system/src/query/plumbing.rs b/compiler/rustc_query_system/src/query/plumbing.rs index dc1d6f760faca..a5ecc50421e77 100644 --- a/compiler/rustc_query_system/src/query/plumbing.rs +++ b/compiler/rustc_query_system/src/query/plumbing.rs @@ -250,10 +250,9 @@ where Q: QueryConfig, Qcx: QueryContext, { - let (query_map, complete) = qcx.collect_active_jobs(); // Ensure there was no errors collecting all active jobs. // We need the complete map to ensure we find a cycle to break. - assert!(complete, "failed to collect active queries"); + let query_map = qcx.collect_active_jobs().expect("failed to collect active queries"); let error = try_execute.find_cycle_in_stack(query_map, &qcx.current_query_job(), span); (mk_cycle(query, qcx, error), None) From 83b56eb059d9e47864191fc13783f44f04cb42b4 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 4 Mar 2025 14:17:16 +1100 Subject: [PATCH 05/19] coverage: Separate span-extraction from unexpansion --- .../rustc_mir_transform/src/coverage/spans.rs | 36 +++++- .../src/coverage/spans/from_mir.rs | 105 ++++++------------ 2 files changed, 66 insertions(+), 75 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index b9ed6984ddb21..f63ccff8decfb 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -6,10 +6,8 @@ use rustc_span::{DesugaringKind, ExpnKind, MacroKind, Span}; use tracing::{debug, debug_span, instrument}; use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph}; -use crate::coverage::spans::from_mir::{ - ExtractedCovspans, Hole, SpanFromMir, extract_covspans_from_mir, -}; -use crate::coverage::{ExtractedHirInfo, mappings}; +use crate::coverage::spans::from_mir::{Hole, RawSpanFromMir, SpanFromMir}; +use crate::coverage::{ExtractedHirInfo, mappings, unexpand}; mod from_mir; @@ -19,7 +17,35 @@ pub(super) fn extract_refined_covspans( graph: &CoverageGraph, code_mappings: &mut impl Extend, ) { - let ExtractedCovspans { mut covspans } = extract_covspans_from_mir(mir_body, hir_info, graph); + let &ExtractedHirInfo { body_span, .. } = hir_info; + + let raw_spans = from_mir::extract_raw_spans_from_mir(mir_body, graph); + let mut covspans = raw_spans + .into_iter() + .filter_map(|RawSpanFromMir { raw_span, bcb }| try { + let (span, expn_kind) = + unexpand::unexpand_into_body_span_with_expn_kind(raw_span, body_span)?; + // Discard any spans that fill the entire body, because they tend + // to represent compiler-inserted code, e.g. implicitly returning `()`. + if span.source_equal(body_span) { + return None; + }; + SpanFromMir { span, expn_kind, bcb } + }) + .collect::>(); + + // Only proceed if we found at least one usable span. + if covspans.is_empty() { + return; + } + + // Also add the adjusted function signature span, if available. + // Otherwise, add a fake span at the start of the body, to avoid an ugly + // gap between the start of the body and the first real span. + // FIXME: Find a more principled way to solve this problem. + covspans.push(SpanFromMir::for_fn_sig( + hir_info.fn_sig_span_extended.unwrap_or_else(|| body_span.shrink_to_lo()), + )); // First, perform the passes that need macro information. covspans.sort_by(|a, b| graph.cmp_in_dominator_order(a.bcb, b.bcb)); diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 73b68d7155cfb..1faa2171c0b02 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -1,3 +1,5 @@ +use std::iter; + use rustc_middle::bug; use rustc_middle::mir::coverage::CoverageKind; use rustc_middle::mir::{ @@ -5,87 +7,50 @@ use rustc_middle::mir::{ }; use rustc_span::{ExpnKind, Span}; -use crate::coverage::ExtractedHirInfo; -use crate::coverage::graph::{ - BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph, START_BCB, -}; +use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph, START_BCB}; use crate::coverage::spans::Covspan; -use crate::coverage::unexpand::unexpand_into_body_span_with_expn_kind; -pub(crate) struct ExtractedCovspans { - pub(crate) covspans: Vec, +#[derive(Debug)] +pub(crate) struct RawSpanFromMir { + /// A span that has been extracted from a MIR statement/terminator, but + /// hasn't been "unexpanded", so it might not lie within the function body + /// span and might be part of an expansion with a different context. + pub(crate) raw_span: Span, + pub(crate) bcb: BasicCoverageBlock, } -/// Traverses the MIR body to produce an initial collection of coverage-relevant -/// spans, each associated with a node in the coverage graph (BCB) and possibly -/// other metadata. -pub(crate) fn extract_covspans_from_mir( - mir_body: &mir::Body<'_>, - hir_info: &ExtractedHirInfo, +/// Generates an initial set of coverage spans from the statements and +/// terminators in the function's MIR body, each associated with its +/// corresponding node in the coverage graph. +/// +/// This is necessarily an inexact process, because MIR isn't designed to +/// capture source spans at the level of detail we would want for coverage, +/// but it's good enough to be better than nothing. +pub(crate) fn extract_raw_spans_from_mir<'tcx>( + mir_body: &mir::Body<'tcx>, graph: &CoverageGraph, -) -> ExtractedCovspans { - let &ExtractedHirInfo { body_span, .. } = hir_info; - - let mut covspans = vec![]; +) -> Vec { + let mut raw_spans = vec![]; + // We only care about blocks that are part of the coverage graph. for (bcb, bcb_data) in graph.iter_enumerated() { - bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data, &mut covspans); - } + let make_raw_span = |raw_span: Span| RawSpanFromMir { raw_span, bcb }; - // Only add the signature span if we found at least one span in the body. - if !covspans.is_empty() { - // If there is no usable signature span, add a fake one (before refinement) - // to avoid an ugly gap between the body start and the first real span. - // FIXME: Find a more principled way to solve this problem. - let fn_sig_span = hir_info.fn_sig_span_extended.unwrap_or_else(|| body_span.shrink_to_lo()); - covspans.push(SpanFromMir::for_fn_sig(fn_sig_span)); - } + // A coverage graph node can consist of multiple basic blocks. + for &bb in &bcb_data.basic_blocks { + let bb_data = &mir_body[bb]; - ExtractedCovspans { covspans } -} + let statements = bb_data.statements.iter(); + raw_spans.extend(statements.filter_map(filtered_statement_span).map(make_raw_span)); -// Generate a set of coverage spans from the filtered set of `Statement`s and `Terminator`s of -// the `BasicBlock`(s) in the given `BasicCoverageBlockData`. One coverage span is generated -// for each `Statement` and `Terminator`. (Note that subsequent stages of coverage analysis will -// merge some coverage spans, at which point a coverage span may represent multiple -// `Statement`s and/or `Terminator`s.) -fn bcb_to_initial_coverage_spans<'a, 'tcx>( - mir_body: &'a mir::Body<'tcx>, - body_span: Span, - bcb: BasicCoverageBlock, - bcb_data: &'a BasicCoverageBlockData, - initial_covspans: &mut Vec, -) { - for &bb in &bcb_data.basic_blocks { - let data = &mir_body[bb]; - - let unexpand = move |expn_span| { - unexpand_into_body_span_with_expn_kind(expn_span, body_span) - // Discard any spans that fill the entire body, because they tend - // to represent compiler-inserted code, e.g. implicitly returning `()`. - .filter(|(span, _)| !span.source_equal(body_span)) - }; - - let mut extract_statement_span = |statement| { - let expn_span = filtered_statement_span(statement)?; - let (span, expn_kind) = unexpand(expn_span)?; - - initial_covspans.push(SpanFromMir::new(span, expn_kind, bcb)); - Some(()) - }; - for statement in data.statements.iter() { - extract_statement_span(statement); + // There's only one terminator, but wrap it in an iterator to + // mirror the handling of statements. + let terminator = iter::once(bb_data.terminator()); + raw_spans.extend(terminator.filter_map(filtered_terminator_span).map(make_raw_span)); } - - let mut extract_terminator_span = |terminator| { - let expn_span = filtered_terminator_span(terminator)?; - let (span, expn_kind) = unexpand(expn_span)?; - - initial_covspans.push(SpanFromMir::new(span, expn_kind, bcb)); - Some(()) - }; - extract_terminator_span(data.terminator()); } + + raw_spans } /// If the MIR `Statement` has a span contributive to computing coverage spans, @@ -219,7 +184,7 @@ pub(crate) struct SpanFromMir { } impl SpanFromMir { - fn for_fn_sig(fn_sig_span: Span) -> Self { + pub(crate) fn for_fn_sig(fn_sig_span: Span) -> Self { Self::new(fn_sig_span, None, START_BCB) } From 7fdac5eef09a11fd4f35ab272d83f5ea08b15320 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 4 Mar 2025 13:53:24 +1100 Subject: [PATCH 06/19] coverage: Defer the filtering of hole spans --- .../rustc_mir_transform/src/coverage/mod.rs | 21 +++++++------------ .../rustc_mir_transform/src/coverage/spans.rs | 9 +++++++- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 572dbae8fd20d..aa4c0ef1e1f93 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -273,8 +273,9 @@ struct ExtractedHirInfo { /// Must have the same context and filename as the body span. fn_sig_span_extended: Option, body_span: Span, - /// "Holes" are regions within the body span that should not be included in - /// coverage spans for this function (e.g. closures and nested items). + /// "Holes" are regions within the function body (or its expansions) that + /// should not be included in coverage spans for this function + /// (e.g. closures and nested items). hole_spans: Vec, } @@ -323,7 +324,7 @@ fn extract_hir_info<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> ExtractedHir let function_source_hash = hash_mir_source(tcx, hir_body); - let hole_spans = extract_hole_spans_from_hir(tcx, body_span, hir_body); + let hole_spans = extract_hole_spans_from_hir(tcx, hir_body); ExtractedHirInfo { function_source_hash, @@ -340,14 +341,9 @@ fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx hir::Body<'tcx>) -> tcx.hir_owner_nodes(owner).opt_hash_including_bodies.unwrap().to_smaller_hash().as_u64() } -fn extract_hole_spans_from_hir<'tcx>( - tcx: TyCtxt<'tcx>, - body_span: Span, // Usually `hir_body.value.span`, but not always - hir_body: &hir::Body<'tcx>, -) -> Vec { +fn extract_hole_spans_from_hir<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &hir::Body<'tcx>) -> Vec { struct HolesVisitor<'tcx> { tcx: TyCtxt<'tcx>, - body_span: Span, hole_spans: Vec, } @@ -387,14 +383,11 @@ fn extract_hole_spans_from_hir<'tcx>( } impl HolesVisitor<'_> { fn visit_hole_span(&mut self, hole_span: Span) { - // Discard any holes that aren't directly visible within the body span. - if self.body_span.contains(hole_span) && self.body_span.eq_ctxt(hole_span) { - self.hole_spans.push(hole_span); - } + self.hole_spans.push(hole_span); } } - let mut visitor = HolesVisitor { tcx, body_span, hole_spans: vec![] }; + let mut visitor = HolesVisitor { tcx, hole_spans: vec![] }; visitor.visit_body(hir_body); visitor.hole_spans diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index f63ccff8decfb..8befe9c5d8dd8 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -69,7 +69,14 @@ pub(super) fn extract_refined_covspans( covspans.dedup_by(|b, a| a.span.source_equal(b.span)); // Sort the holes, and merge overlapping/adjacent holes. - let mut holes = hir_info.hole_spans.iter().map(|&span| Hole { span }).collect::>(); + let mut holes = hir_info + .hole_spans + .iter() + .copied() + // Discard any holes that aren't directly visible within the body span. + .filter(|&hole_span| body_span.contains(hole_span) && body_span.eq_ctxt(hole_span)) + .map(|span| Hole { span }) + .collect::>(); holes.sort_by(|a, b| compare_spans(a.span, b.span)); holes.dedup_by(|b, a| a.merge_if_overlapping_or_adjacent(b)); From 856a181570371e1622bcb42ef94fc23090f93a12 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Mon, 24 Mar 2025 11:53:09 +0100 Subject: [PATCH 07/19] =?UTF-8?q?Fix=20autofix=20for=20`self`=20and=20`sel?= =?UTF-8?q?f=20as=20=E2=80=A6`=20in=20`unused=5Fimports`=20lint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes two problems with the autofixes for the `unused_imports` lint: - `use std::collections::{HashMap, self as coll};` would suggest, when `HashMap` is unused, the incorrect `use std::collections::self as coll;` which does not compile. - `use std::borrow::{self, Cow};` would suggest, when `self` is unused, `use std::borrow::{Cow};`, which contains unnecessary brackets. --- compiler/rustc_resolve/src/check_unused.rs | 3 +- .../lint-unused-imports-self-single.fixed | 29 ++++++++++++ .../unused/lint-unused-imports-self-single.rs | 30 +++++++++++++ .../lint-unused-imports-self-single.stderr | 44 +++++++++++++++++++ 4 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 tests/ui/lint/unused/lint-unused-imports-self-single.fixed create mode 100644 tests/ui/lint/unused/lint-unused-imports-self-single.rs create mode 100644 tests/ui/lint/unused/lint-unused-imports-self-single.stderr diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs index 1c1e8494ffc7c..51ff4aa834baa 100644 --- a/compiler/rustc_resolve/src/check_unused.rs +++ b/compiler/rustc_resolve/src/check_unused.rs @@ -337,7 +337,8 @@ fn calc_unused_spans( } } contains_self |= use_tree.prefix == kw::SelfLower - && matches!(use_tree.kind, ast::UseTreeKind::Simple(None)); + && matches!(use_tree.kind, ast::UseTreeKind::Simple(_)) + && !unused_import.unused.contains(&use_tree_id); previous_unused = remove.is_some(); } if unused_spans.is_empty() { diff --git a/tests/ui/lint/unused/lint-unused-imports-self-single.fixed b/tests/ui/lint/unused/lint-unused-imports-self-single.fixed new file mode 100644 index 0000000000000..361548bfdc12a --- /dev/null +++ b/tests/ui/lint/unused/lint-unused-imports-self-single.fixed @@ -0,0 +1,29 @@ +//@ run-rustfix + +#![deny(unused_imports)] +#![allow(unreachable_code)] + +use std::collections::{self as coll}; +//~^ ERROR unused import: `HashMap` + +//~^ ERROR unused import: `self as std_io` + +use std::sync::Mutex; +//~^ ERROR unused import: `self as std_sync` + +use std::sync::mpsc::Sender; +//~^ ERROR unused import: `self as std_sync_mpsc` + +use std::collections::hash_map::{self as std_coll_hm}; +//~^ ERROR unused import: `Keys` + +use std::borrow::Cow; +//~^ ERROR unused import: `self` + +fn main() { + let _ = coll::BTreeSet::::default(); + let _ = Mutex::new(String::new()); + let _: Cow<'static, str> = "foo".into(); + let _: Sender = todo!(); + let _: std_coll_hm::Entry<'static, u32, u32> = todo!(); +} diff --git a/tests/ui/lint/unused/lint-unused-imports-self-single.rs b/tests/ui/lint/unused/lint-unused-imports-self-single.rs new file mode 100644 index 0000000000000..d03d3822e04e7 --- /dev/null +++ b/tests/ui/lint/unused/lint-unused-imports-self-single.rs @@ -0,0 +1,30 @@ +//@ run-rustfix + +#![deny(unused_imports)] +#![allow(unreachable_code)] + +use std::collections::{HashMap, self as coll}; +//~^ ERROR unused import: `HashMap` + +use std::io::{self as std_io}; +//~^ ERROR unused import: `self as std_io` + +use std::sync::{Mutex, self as std_sync}; +//~^ ERROR unused import: `self as std_sync` + +use std::sync::{mpsc::{self as std_sync_mpsc, Sender}}; +//~^ ERROR unused import: `self as std_sync_mpsc` + +use std::collections::{hash_map::{self as std_coll_hm, Keys}}; +//~^ ERROR unused import: `Keys` + +use std::borrow::{self, Cow}; +//~^ ERROR unused import: `self` + +fn main() { + let _ = coll::BTreeSet::::default(); + let _ = Mutex::new(String::new()); + let _: Cow<'static, str> = "foo".into(); + let _: Sender = todo!(); + let _: std_coll_hm::Entry<'static, u32, u32> = todo!(); +} diff --git a/tests/ui/lint/unused/lint-unused-imports-self-single.stderr b/tests/ui/lint/unused/lint-unused-imports-self-single.stderr new file mode 100644 index 0000000000000..70a9b78a66401 --- /dev/null +++ b/tests/ui/lint/unused/lint-unused-imports-self-single.stderr @@ -0,0 +1,44 @@ +error: unused import: `HashMap` + --> $DIR/lint-unused-imports-self-single.rs:6:24 + | +LL | use std::collections::{HashMap, self as coll}; + | ^^^^^^^ + | +note: the lint level is defined here + --> $DIR/lint-unused-imports-self-single.rs:3:9 + | +LL | #![deny(unused_imports)] + | ^^^^^^^^^^^^^^ + +error: unused import: `self as std_io` + --> $DIR/lint-unused-imports-self-single.rs:9:15 + | +LL | use std::io::{self as std_io}; + | ^^^^^^^^^^^^^^ + +error: unused import: `self as std_sync` + --> $DIR/lint-unused-imports-self-single.rs:12:24 + | +LL | use std::sync::{Mutex, self as std_sync}; + | ^^^^^^^^^^^^^^^^ + +error: unused import: `self as std_sync_mpsc` + --> $DIR/lint-unused-imports-self-single.rs:15:24 + | +LL | use std::sync::{mpsc::{self as std_sync_mpsc, Sender}}; + | ^^^^^^^^^^^^^^^^^^^^^ + +error: unused import: `Keys` + --> $DIR/lint-unused-imports-self-single.rs:18:56 + | +LL | use std::collections::{hash_map::{self as std_coll_hm, Keys}}; + | ^^^^ + +error: unused import: `self` + --> $DIR/lint-unused-imports-self-single.rs:21:19 + | +LL | use std::borrow::{self, Cow}; + | ^^^^ + +error: aborting due to 6 previous errors + From 1a6266340e53e3f663b49f71a14461dafa47de47 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 25 Mar 2025 10:51:42 +1100 Subject: [PATCH 08/19] Add a test with an empty crate name. This error was untested. --- tests/ui/attributes/crate-name-empty.rs | 5 +++++ tests/ui/attributes/crate-name-empty.stderr | 8 ++++++++ 2 files changed, 13 insertions(+) create mode 100644 tests/ui/attributes/crate-name-empty.rs create mode 100644 tests/ui/attributes/crate-name-empty.stderr diff --git a/tests/ui/attributes/crate-name-empty.rs b/tests/ui/attributes/crate-name-empty.rs new file mode 100644 index 0000000000000..dfba77a52def7 --- /dev/null +++ b/tests/ui/attributes/crate-name-empty.rs @@ -0,0 +1,5 @@ +// Ensure we reject `#![crate_name = ""]`. + +#![crate_name = ""] //~ ERROR crate name must not be empty + +fn main() {} diff --git a/tests/ui/attributes/crate-name-empty.stderr b/tests/ui/attributes/crate-name-empty.stderr new file mode 100644 index 0000000000000..509a42d05f711 --- /dev/null +++ b/tests/ui/attributes/crate-name-empty.stderr @@ -0,0 +1,8 @@ +error: crate name must not be empty + --> $DIR/crate-name-empty.rs:3:1 + | +LL | #![crate_name = ""] + | ^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + From bd61e0129f4ff393ad350f05442a9f1ac3ddbd44 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 24 Mar 2025 18:58:18 +1100 Subject: [PATCH 09/19] Use `Ident::dummy()` in `dummy_annotatable`. This is exactly the kind of case `Ident::dummy()` is for. --- compiler/rustc_builtin_macros/src/derive.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_builtin_macros/src/derive.rs b/compiler/rustc_builtin_macros/src/derive.rs index 2653a9f48b9b6..e259f5b3955be 100644 --- a/compiler/rustc_builtin_macros/src/derive.rs +++ b/compiler/rustc_builtin_macros/src/derive.rs @@ -103,7 +103,7 @@ impl MultiItemModifier for Expander { fn dummy_annotatable() -> Annotatable { Annotatable::GenericParam(ast::GenericParam { id: ast::DUMMY_NODE_ID, - ident: Ident::empty(), + ident: Ident::dummy(), attrs: Default::default(), bounds: Default::default(), is_placeholder: false, From 2469ab195a558b3a09b5ef4319ea8a3f989b621f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 24 Mar 2025 18:59:44 +1100 Subject: [PATCH 10/19] Use `Option` in `panic_call`. Instead of `kw::Empty`. It makes it clearer that this is a name that is searched for and might not be found. --- compiler/rustc_lint/src/non_fmt_panic.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_lint/src/non_fmt_panic.rs b/compiler/rustc_lint/src/non_fmt_panic.rs index ac9f8d92dacb7..16c061008085d 100644 --- a/compiler/rustc_lint/src/non_fmt_panic.rs +++ b/compiler/rustc_lint/src/non_fmt_panic.rs @@ -7,7 +7,7 @@ use rustc_parse_format::{ParseMode, Parser, Piece}; use rustc_session::lint::FutureIncompatibilityReason; use rustc_session::{declare_lint, declare_lint_pass}; use rustc_span::edition::Edition; -use rustc_span::{InnerSpan, Span, Symbol, hygiene, kw, sym}; +use rustc_span::{InnerSpan, Span, Symbol, hygiene, sym}; use rustc_trait_selection::infer::InferCtxtExt; use crate::lints::{NonFmtPanicBraces, NonFmtPanicUnused}; @@ -167,7 +167,7 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc .get_diagnostic_item(sym::Debug) .is_some_and(|t| infcx.type_implements_trait(t, [ty], param_env).may_apply()); - let suggest_panic_any = !is_str && panic == sym::std_panic_macro; + let suggest_panic_any = !is_str && panic == Some(sym::std_panic_macro); let fmt_applicability = if suggest_panic_any { // If we can use panic_any, use that as the MachineApplicable suggestion. @@ -297,10 +297,13 @@ fn find_delimiters(cx: &LateContext<'_>, span: Span) -> Option<(Span, Span, char )) } -fn panic_call<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>) -> (Span, Symbol, Symbol) { +fn panic_call<'tcx>( + cx: &LateContext<'tcx>, + f: &'tcx hir::Expr<'tcx>, +) -> (Span, Option, Symbol) { let mut expn = f.span.ctxt().outer_expn_data(); - let mut panic_macro = kw::Empty; + let mut panic_macro = None; // Unwrap more levels of macro expansion, as panic_2015!() // was likely expanded from panic!() and possibly from @@ -320,7 +323,7 @@ fn panic_call<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>) -> (Span, break; } expn = parent; - panic_macro = name; + panic_macro = Some(name); } let macro_symbol = From 3f32a321bc0962cd3c700a5625dca4e4967918e4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 25 Mar 2025 09:09:01 +1100 Subject: [PATCH 11/19] Use `Option` in `ModuleKind::Def`. This way, `None` represents "crate root without a name" instead of `kw::Empty`. This changes makes it impossible to forget to handle the exceptional case. --- compiler/rustc_resolve/src/build_reduced_graph.rs | 10 +++++----- compiler/rustc_resolve/src/diagnostics.rs | 2 +- compiler/rustc_resolve/src/lib.rs | 15 ++++++++------- compiler/rustc_resolve/src/macros.rs | 11 ++++++----- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 763e9207a126b..08648a164ab70 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -131,7 +131,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let expn_id = self.cstore().expn_that_defined_untracked(def_id, self.tcx.sess); return Some(self.new_module( parent, - ModuleKind::Def(def_kind, def_id, self.tcx.item_name(def_id)), + ModuleKind::Def(def_kind, def_id, Some(self.tcx.item_name(def_id))), expn_id, self.def_span(def_id), // FIXME: Account for `#[no_implicit_prelude]` attributes. @@ -594,7 +594,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { // HACK(eddyb) unclear how good this is, but keeping `$crate` // in `source` breaks `tests/ui/imports/import-crate-var.rs`, // while the current crate doesn't have a valid `crate_name`. - if crate_name != kw::Empty { + if let Some(crate_name) = crate_name { // `crate_name` should not be interpreted as relative. module_path.push(Segment::from_ident_and_id( Ident { name: kw::PathRoot, span: source.ident.span }, @@ -603,7 +603,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { source.ident.name = crate_name; } if rename.is_none() { - ident.name = crate_name; + ident.name = sym::dummy; } self.r.dcx().emit_err(errors::CrateImported { span: item.span }); @@ -775,7 +775,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { ItemKind::Mod(.., ref mod_kind) => { let module = self.r.new_module( Some(parent), - ModuleKind::Def(def_kind, def_id, ident.name), + ModuleKind::Def(def_kind, def_id, Some(ident.name)), expansion.to_expn_id(), item.span, parent.no_implicit_prelude @@ -811,7 +811,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { ItemKind::Enum(_, _) | ItemKind::Trait(..) => { let module = self.r.new_module( Some(parent), - ModuleKind::Def(def_kind, def_id, ident.name), + ModuleKind::Def(def_kind, def_id, Some(ident.name)), expansion.to_expn_id(), item.span, parent.no_implicit_prelude, diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 5361af98f3c74..d683fd98bd21c 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -2439,7 +2439,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let Res::Def(DefKind::Macro(MacroKind::Bang), _) = binding.res() else { return None; }; - let module_name = crate_module.kind.name().unwrap(); + let module_name = crate_module.kind.name().unwrap_or(kw::Empty); let import_snippet = match import.kind { ImportKind::Single { source, target, .. } if source != target => { format!("{source} as {target}") diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index ff31af0025b54..d5fd03e5f412b 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -503,18 +503,18 @@ enum ModuleKind { /// /// * A normal module – either `mod from_file;` or `mod from_block { }` – /// or the crate root (which is conceptually a top-level module). - /// Note that the crate root's [name][Self::name] will be [`kw::Empty`]. + /// The crate root will have `None` for the symbol. /// * A trait or an enum (it implicitly contains associated types, methods and variant /// constructors). - Def(DefKind, DefId, Symbol), + Def(DefKind, DefId, Option), } impl ModuleKind { /// Get name of the module. fn name(&self) -> Option { - match self { + match *self { ModuleKind::Block => None, - ModuleKind::Def(.., name) => Some(*name), + ModuleKind::Def(.., name) => name, } } } @@ -1402,7 +1402,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let mut module_self_bindings = FxHashMap::default(); let graph_root = arenas.new_module( None, - ModuleKind::Def(DefKind::Mod, root_def_id, kw::Empty), + ModuleKind::Def(DefKind::Mod, root_def_id, None), ExpnId::root(), crate_span, attr::contains_name(attrs, sym::no_implicit_prelude), @@ -1411,7 +1411,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ); let empty_module = arenas.new_module( None, - ModuleKind::Def(DefKind::Mod, root_def_id, kw::Empty), + ModuleKind::Def(DefKind::Mod, root_def_id, None), ExpnId::root(), DUMMY_SP, true, @@ -2286,7 +2286,8 @@ fn module_to_string(mut module: Module<'_>) -> Option { loop { if let ModuleKind::Def(.., name) = module.kind { if let Some(parent) = module.parent { - names.push(name); + // `unwrap` is safe: the presence of a parent means it's not the crate root. + names.push(name.unwrap()); module = parent } else { break; diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index c4304a7a6df6a..967cb5a0ec624 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -168,7 +168,7 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> { hygiene::update_dollar_crate_names(|ctxt| { let ident = Ident::new(kw::DollarCrate, DUMMY_SP.with_ctxt(ctxt)); match self.resolve_crate_root(ident).kind { - ModuleKind::Def(.., name) if name != kw::Empty => name, + ModuleKind::Def(.., name) if let Some(name) = name => name, _ => kw::Crate, } }); @@ -1068,11 +1068,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ); if fallback_binding.ok().and_then(|b| b.res().opt_def_id()) != Some(def_id) { let location = match parent_scope.module.kind { - ModuleKind::Def(_, _, name) if name == kw::Empty => { - "the crate root".to_string() - } ModuleKind::Def(kind, def_id, name) => { - format!("{} `{name}`", kind.descr(def_id)) + if let Some(name) = name { + format!("{} `{name}`", kind.descr(def_id)) + } else { + "the crate root".to_string() + } } ModuleKind::Block => "this scope".to_string(), }; From e576d8850de702f6c712eaca77f758f72ef13419 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 25 Mar 2025 11:10:45 +1100 Subject: [PATCH 12/19] Use `Option` in `DuplicateLangItem`. For the the symbols that might not be present, instead of `kw::Empty`. --- compiler/rustc_passes/src/errors.rs | 20 +++++++++++++------- compiler/rustc_passes/src/lang_items.rs | 14 +++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index a72f40cd843a7..0ee17430aab8e 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1348,12 +1348,12 @@ pub(crate) struct DuplicateLangItem { pub local_span: Option, pub lang_item_name: Symbol, pub crate_name: Symbol, - pub dependency_of: Symbol, + pub dependency_of: Option, pub is_local: bool, pub path: String, pub first_defined_span: Option, - pub orig_crate_name: Symbol, - pub orig_dependency_of: Symbol, + pub orig_crate_name: Option, + pub orig_dependency_of: Option, pub orig_is_local: bool, pub orig_path: String, pub(crate) duplicate: Duplicate, @@ -1374,10 +1374,16 @@ impl Diagnostic<'_, G> for DuplicateLangItem { diag.code(E0152); diag.arg("lang_item_name", self.lang_item_name); diag.arg("crate_name", self.crate_name); - diag.arg("dependency_of", self.dependency_of); + if let Some(dependency_of) = self.dependency_of { + diag.arg("dependency_of", dependency_of); + } diag.arg("path", self.path); - diag.arg("orig_crate_name", self.orig_crate_name); - diag.arg("orig_dependency_of", self.orig_dependency_of); + if let Some(orig_crate_name) = self.orig_crate_name { + diag.arg("orig_crate_name", orig_crate_name); + } + if let Some(orig_dependency_of) = self.orig_dependency_of { + diag.arg("orig_dependency_of", orig_dependency_of); + } diag.arg("orig_path", self.orig_path); if let Some(span) = self.local_span { diag.span(span); @@ -1385,7 +1391,7 @@ impl Diagnostic<'_, G> for DuplicateLangItem { if let Some(span) = self.first_defined_span { diag.span_note(span, fluent::passes_first_defined_span); } else { - if self.orig_dependency_of.is_empty() { + if self.orig_dependency_of.is_none() { diag.note(fluent::passes_first_defined_crate); } else { diag.note(fluent::passes_first_defined_crate_depends); diff --git a/compiler/rustc_passes/src/lang_items.rs b/compiler/rustc_passes/src/lang_items.rs index fe1140d893e37..9d4b46cd30660 100644 --- a/compiler/rustc_passes/src/lang_items.rs +++ b/compiler/rustc_passes/src/lang_items.rs @@ -16,7 +16,7 @@ use rustc_hir::{LangItem, LanguageItems, MethodKind, Target}; use rustc_middle::query::Providers; use rustc_middle::ty::{ResolverAstLowering, TyCtxt}; use rustc_session::cstore::ExternCrate; -use rustc_span::{Span, kw}; +use rustc_span::Span; use crate::errors::{ DuplicateLangItem, IncorrectTarget, LangItemOnIncorrectTarget, UnknownLangItem, @@ -98,7 +98,7 @@ impl<'ast, 'tcx> LanguageItemCollector<'ast, 'tcx> { { let lang_item_name = lang_item.name(); let crate_name = self.tcx.crate_name(item_def_id.krate); - let mut dependency_of = kw::Empty; + let mut dependency_of = None; let is_local = item_def_id.is_local(); let path = if is_local { String::new() @@ -112,8 +112,8 @@ impl<'ast, 'tcx> LanguageItemCollector<'ast, 'tcx> { }; let first_defined_span = self.item_spans.get(&original_def_id).copied(); - let mut orig_crate_name = kw::Empty; - let mut orig_dependency_of = kw::Empty; + let mut orig_crate_name = None; + let mut orig_dependency_of = None; let orig_is_local = original_def_id.is_local(); let orig_path = if orig_is_local { String::new() @@ -127,11 +127,11 @@ impl<'ast, 'tcx> LanguageItemCollector<'ast, 'tcx> { }; if first_defined_span.is_none() { - orig_crate_name = self.tcx.crate_name(original_def_id.krate); + orig_crate_name = Some(self.tcx.crate_name(original_def_id.krate)); if let Some(ExternCrate { dependency_of: inner_dependency_of, .. }) = self.tcx.extern_crate(original_def_id.krate) { - orig_dependency_of = self.tcx.crate_name(*inner_dependency_of); + orig_dependency_of = Some(self.tcx.crate_name(*inner_dependency_of)); } } @@ -140,7 +140,7 @@ impl<'ast, 'tcx> LanguageItemCollector<'ast, 'tcx> { } else { match self.tcx.extern_crate(item_def_id.krate) { Some(ExternCrate { dependency_of: inner_dependency_of, .. }) => { - dependency_of = self.tcx.crate_name(*inner_dependency_of); + dependency_of = Some(self.tcx.crate_name(*inner_dependency_of)); Duplicate::CrateDepends } _ => Duplicate::Crate, From 79b9664091e90c08a7abeab2bbc4a941bcdc3863 Mon Sep 17 00:00:00 2001 From: Daniel Paoliello Date: Wed, 13 Nov 2024 13:13:03 -0800 Subject: [PATCH 13/19] Reduce visibility of most items in `rustc_codegen_llvm` --- compiler/rustc_codegen_llvm/src/back/lto.rs | 2 +- compiler/rustc_codegen_llvm/src/lib.rs | 6 +- .../rustc_codegen_llvm/src/llvm/archive_ro.rs | 6 +- .../rustc_codegen_llvm/src/llvm/diagnostic.rs | 20 +- .../rustc_codegen_llvm/src/llvm/enzyme_ffi.rs | 44 ++--- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 182 +++++++++--------- compiler/rustc_codegen_llvm/src/llvm/mod.rs | 20 +- 7 files changed, 144 insertions(+), 136 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/back/lto.rs b/compiler/rustc_codegen_llvm/src/back/lto.rs index 668795191a29a..f083cfbd7d306 100644 --- a/compiler/rustc_codegen_llvm/src/back/lto.rs +++ b/compiler/rustc_codegen_llvm/src/back/lto.rs @@ -728,7 +728,7 @@ impl ThinBuffer { } } - pub unsafe fn from_raw_ptr(ptr: *mut llvm::ThinLTOBuffer) -> ThinBuffer { + pub(crate) unsafe fn from_raw_ptr(ptr: *mut llvm::ThinLTOBuffer) -> ThinBuffer { let mut ptr = NonNull::new(ptr).unwrap(); ThinBuffer(unsafe { ptr.as_mut() }) } diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index f622646a5d9d0..425381b0ffab7 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -29,7 +29,7 @@ use back::owned_target_machine::OwnedTargetMachine; use back::write::{create_informational_target_machine, create_target_machine}; use context::SimpleCx; use errors::{AutoDiffWithoutLTO, ParseTargetMachineConfig}; -pub(crate) use llvm_util::target_features_cfg; +use llvm_util::target_features_cfg; use rustc_ast::expand::allocator::AllocatorKind; use rustc_ast::expand::autodiff_attrs::AutoDiffItem; use rustc_codegen_ssa::back::lto::{LtoModuleCodegen, SerializedModule, ThinModule}; @@ -71,9 +71,7 @@ mod debuginfo; mod declare; mod errors; mod intrinsic; -// FIXME(Zalathar): Fix all the unreachable-pub warnings that would occur if -// this isn't pub, then make it not pub. -pub mod llvm; +mod llvm; mod llvm_util; mod mono_item; mod type_; diff --git a/compiler/rustc_codegen_llvm/src/llvm/archive_ro.rs b/compiler/rustc_codegen_llvm/src/llvm/archive_ro.rs index 63b2b15c5145e..51bcc4d123d31 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/archive_ro.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/archive_ro.rs @@ -5,17 +5,17 @@ use std::{slice, str}; use rustc_fs_util::path_to_c_string; -pub struct ArchiveRO { +pub(crate) struct ArchiveRO { pub raw: &'static mut super::Archive, } unsafe impl Send for ArchiveRO {} -pub struct Iter<'a> { +pub(crate) struct Iter<'a> { raw: &'a mut super::ArchiveIterator<'a>, } -pub struct Child<'a> { +pub(crate) struct Child<'a> { pub raw: &'a mut super::ArchiveChild<'a>, } diff --git a/compiler/rustc_codegen_llvm/src/llvm/diagnostic.rs b/compiler/rustc_codegen_llvm/src/llvm/diagnostic.rs index 11043b664f526..0e0f2b0eab016 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/diagnostic.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/diagnostic.rs @@ -3,13 +3,13 @@ use libc::c_uint; use rustc_span::InnerSpan; -pub use self::Diagnostic::*; -pub use self::OptimizationDiagnosticKind::*; +pub(crate) use self::Diagnostic::*; +use self::OptimizationDiagnosticKind::*; use super::{DiagnosticInfo, SMDiagnostic}; use crate::value::Value; #[derive(Copy, Clone, Debug)] -pub enum OptimizationDiagnosticKind { +pub(crate) enum OptimizationDiagnosticKind { OptimizationRemark, OptimizationMissed, OptimizationAnalysis, @@ -19,9 +19,10 @@ pub enum OptimizationDiagnosticKind { OptimizationRemarkOther, } -pub struct OptimizationDiagnostic<'ll> { +pub(crate) struct OptimizationDiagnostic<'ll> { pub kind: OptimizationDiagnosticKind, pub pass_name: String, + #[expect(dead_code)] pub function: &'ll Value, pub line: c_uint, pub column: c_uint, @@ -73,14 +74,14 @@ impl<'ll> OptimizationDiagnostic<'ll> { } } -pub struct SrcMgrDiagnostic { +pub(crate) struct SrcMgrDiagnostic { pub level: super::DiagnosticLevel, pub message: String, pub source: Option<(String, Vec)>, } impl SrcMgrDiagnostic { - pub unsafe fn unpack(diag: &SMDiagnostic) -> SrcMgrDiagnostic { + pub(crate) unsafe fn unpack(diag: &SMDiagnostic) -> SrcMgrDiagnostic { // Recover the post-substitution assembly code from LLVM for better // diagnostics. let mut have_source = false; @@ -120,7 +121,7 @@ impl SrcMgrDiagnostic { } #[derive(Clone)] -pub struct InlineAsmDiagnostic { +pub(crate) struct InlineAsmDiagnostic { pub level: super::DiagnosticLevel, pub cookie: u64, pub message: String, @@ -158,7 +159,7 @@ impl InlineAsmDiagnostic { } } -pub enum Diagnostic<'ll> { +pub(crate) enum Diagnostic<'ll> { Optimization(OptimizationDiagnostic<'ll>), InlineAsm(InlineAsmDiagnostic), PGO(&'ll DiagnosticInfo), @@ -166,11 +167,12 @@ pub enum Diagnostic<'ll> { Unsupported(&'ll DiagnosticInfo), /// LLVM has other types that we do not wrap here. + #[expect(dead_code)] UnknownDiagnostic(&'ll DiagnosticInfo), } impl<'ll> Diagnostic<'ll> { - pub unsafe fn unpack(di: &'ll DiagnosticInfo) -> Self { + pub(crate) unsafe fn unpack(di: &'ll DiagnosticInfo) -> Self { use super::DiagnosticKind as Dk; unsafe { diff --git a/compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs index f6b238629075c..79e4cc8aa7744 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs @@ -31,20 +31,20 @@ unsafe extern "C" { #[repr(C)] #[derive(Copy, Clone, PartialEq)] -pub enum LLVMRustVerifierFailureAction { +pub(crate) enum LLVMRustVerifierFailureAction { LLVMAbortProcessAction = 0, LLVMPrintMessageAction = 1, LLVMReturnStatusAction = 2, } #[cfg(llvm_enzyme)] -pub use self::Enzyme_AD::*; +pub(crate) use self::Enzyme_AD::*; #[cfg(llvm_enzyme)] -pub mod Enzyme_AD { +pub(crate) mod Enzyme_AD { use libc::c_void; unsafe extern "C" { - pub fn EnzymeSetCLBool(arg1: *mut ::std::os::raw::c_void, arg2: u8); + pub(crate) fn EnzymeSetCLBool(arg1: *mut ::std::os::raw::c_void, arg2: u8); } unsafe extern "C" { static mut EnzymePrintPerf: c_void; @@ -56,42 +56,42 @@ pub mod Enzyme_AD { static mut EnzymeInline: c_void; static mut RustTypeRules: c_void; } - pub fn set_print_perf(print: bool) { + pub(crate) fn set_print_perf(print: bool) { unsafe { EnzymeSetCLBool(std::ptr::addr_of_mut!(EnzymePrintPerf), print as u8); } } - pub fn set_print_activity(print: bool) { + pub(crate) fn set_print_activity(print: bool) { unsafe { EnzymeSetCLBool(std::ptr::addr_of_mut!(EnzymePrintActivity), print as u8); } } - pub fn set_print_type(print: bool) { + pub(crate) fn set_print_type(print: bool) { unsafe { EnzymeSetCLBool(std::ptr::addr_of_mut!(EnzymePrintType), print as u8); } } - pub fn set_print(print: bool) { + pub(crate) fn set_print(print: bool) { unsafe { EnzymeSetCLBool(std::ptr::addr_of_mut!(EnzymePrint), print as u8); } } - pub fn set_strict_aliasing(strict: bool) { + pub(crate) fn set_strict_aliasing(strict: bool) { unsafe { EnzymeSetCLBool(std::ptr::addr_of_mut!(EnzymeStrictAliasing), strict as u8); } } - pub fn set_loose_types(loose: bool) { + pub(crate) fn set_loose_types(loose: bool) { unsafe { EnzymeSetCLBool(std::ptr::addr_of_mut!(looseTypeAnalysis), loose as u8); } } - pub fn set_inline(val: bool) { + pub(crate) fn set_inline(val: bool) { unsafe { EnzymeSetCLBool(std::ptr::addr_of_mut!(EnzymeInline), val as u8); } } - pub fn set_rust_rules(val: bool) { + pub(crate) fn set_rust_rules(val: bool) { unsafe { EnzymeSetCLBool(std::ptr::addr_of_mut!(RustTypeRules), val as u8); } @@ -99,34 +99,34 @@ pub mod Enzyme_AD { } #[cfg(not(llvm_enzyme))] -pub use self::Fallback_AD::*; +pub(crate) use self::Fallback_AD::*; #[cfg(not(llvm_enzyme))] -pub mod Fallback_AD { +pub(crate) mod Fallback_AD { #![allow(unused_variables)] - pub fn set_inline(val: bool) { + pub(crate) fn set_inline(val: bool) { unimplemented!() } - pub fn set_print_perf(print: bool) { + pub(crate) fn set_print_perf(print: bool) { unimplemented!() } - pub fn set_print_activity(print: bool) { + pub(crate) fn set_print_activity(print: bool) { unimplemented!() } - pub fn set_print_type(print: bool) { + pub(crate) fn set_print_type(print: bool) { unimplemented!() } - pub fn set_print(print: bool) { + pub(crate) fn set_print(print: bool) { unimplemented!() } - pub fn set_strict_aliasing(strict: bool) { + pub(crate) fn set_strict_aliasing(strict: bool) { unimplemented!() } - pub fn set_loose_types(loose: bool) { + pub(crate) fn set_loose_types(loose: bool) { unimplemented!() } - pub fn set_rust_rules(val: bool) { + pub(crate) fn set_rust_rules(val: bool) { unimplemented!() } } diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 83efb3ea66039..3ce3761944b3a 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -32,10 +32,10 @@ use crate::llvm; /// In the LLVM-C API, boolean values are passed as `typedef int LLVMBool`, /// which has a different ABI from Rust or C++ `bool`. -pub type Bool = c_int; +pub(crate) type Bool = c_int; -pub const True: Bool = 1 as Bool; -pub const False: Bool = 0 as Bool; +pub(crate) const True: Bool = 1 as Bool; +pub(crate) const False: Bool = 0 as Bool; /// Wrapper for a raw enum value returned from LLVM's C APIs. /// @@ -44,7 +44,7 @@ pub const False: Bool = 0 as Bool; /// value and returns it. Instead, return this raw wrapper, then convert to the /// Rust-side enum explicitly. #[repr(transparent)] -pub struct RawEnum { +pub(crate) struct RawEnum { value: u32, /// We don't own or consume a `T`, but we can produce one. _rust_side_type: PhantomData T>, @@ -64,7 +64,7 @@ impl> RawEnum { #[derive(Copy, Clone, PartialEq)] #[repr(C)] #[allow(dead_code)] // Variants constructed by C++. -pub enum LLVMRustResult { +pub(crate) enum LLVMRustResult { Success, Failure, } @@ -83,7 +83,7 @@ pub enum LLVMRustResult { /// C++ API. #[derive(Copy, Clone, PartialEq)] #[repr(C)] -pub enum ModuleFlagMergeBehavior { +pub(crate) enum ModuleFlagMergeBehavior { Error = 1, Warning = 2, Require = 3, @@ -101,7 +101,7 @@ pub enum ModuleFlagMergeBehavior { /// See #[derive(Copy, Clone, PartialEq, Debug, TryFromU32)] #[repr(C)] -pub enum CallConv { +pub(crate) enum CallConv { CCallConv = 0, FastCallConv = 8, ColdCallConv = 9, @@ -126,7 +126,7 @@ pub enum CallConv { /// Must match the layout of `LLVMLinkage`. #[derive(Copy, Clone, PartialEq, TryFromU32)] #[repr(C)] -pub enum Linkage { +pub(crate) enum Linkage { ExternalLinkage = 0, AvailableExternallyLinkage = 1, LinkOnceAnyLinkage = 2, @@ -153,7 +153,7 @@ pub enum Linkage { /// Must match the layout of `LLVMVisibility`. #[repr(C)] #[derive(Copy, Clone, PartialEq, TryFromU32)] -pub enum Visibility { +pub(crate) enum Visibility { Default = 0, Hidden = 1, Protected = 2, @@ -171,8 +171,9 @@ impl Visibility { /// LLVMUnnamedAddr #[repr(C)] -pub enum UnnamedAddr { +pub(crate) enum UnnamedAddr { No, + #[expect(dead_code)] Local, Global, } @@ -180,7 +181,7 @@ pub enum UnnamedAddr { /// LLVMDLLStorageClass #[derive(Copy, Clone)] #[repr(C)] -pub enum DLLStorageClass { +pub(crate) enum DLLStorageClass { #[allow(dead_code)] Default = 0, DllImport = 1, // Function to be imported from DLL. @@ -193,7 +194,8 @@ pub enum DLLStorageClass { /// though it is not ABI compatible (since it's a C++ enum) #[repr(C)] #[derive(Copy, Clone, Debug)] -pub enum AttributeKind { +#[expect(dead_code, reason = "Some variants are unused, but are kept to match the C++")] +pub(crate) enum AttributeKind { AlwaysInline = 0, ByVal = 1, Cold = 2, @@ -241,7 +243,7 @@ pub enum AttributeKind { /// LLVMIntPredicate #[derive(Copy, Clone)] #[repr(C)] -pub enum IntPredicate { +pub(crate) enum IntPredicate { IntEQ = 32, IntNE = 33, IntUGT = 34, @@ -275,7 +277,7 @@ impl IntPredicate { /// LLVMRealPredicate #[derive(Copy, Clone)] #[repr(C)] -pub enum RealPredicate { +pub(crate) enum RealPredicate { RealPredicateFalse = 0, RealOEQ = 1, RealOGT = 2, @@ -321,7 +323,8 @@ impl RealPredicate { /// LLVMTypeKind #[derive(Copy, Clone, PartialEq, Debug)] #[repr(C)] -pub enum TypeKind { +#[expect(dead_code, reason = "Some variants are unused, but are kept to match LLVM-C")] +pub(crate) enum TypeKind { Void = 0, Half = 1, Float = 2, @@ -373,7 +376,7 @@ impl TypeKind { /// LLVMAtomicRmwBinOp #[derive(Copy, Clone)] #[repr(C)] -pub enum AtomicRmwBinOp { +pub(crate) enum AtomicRmwBinOp { AtomicXchg = 0, AtomicAdd = 1, AtomicSub = 2, @@ -409,7 +412,7 @@ impl AtomicRmwBinOp { /// LLVMAtomicOrdering #[derive(Copy, Clone)] #[repr(C)] -pub enum AtomicOrdering { +pub(crate) enum AtomicOrdering { #[allow(dead_code)] NotAtomic = 0, Unordered = 1, @@ -438,7 +441,7 @@ impl AtomicOrdering { /// LLVMRustFileType #[derive(Copy, Clone)] #[repr(C)] -pub enum FileType { +pub(crate) enum FileType { AssemblyFile, ObjectFile, } @@ -446,7 +449,8 @@ pub enum FileType { /// LLVMMetadataType #[derive(Copy, Clone)] #[repr(C)] -pub enum MetadataType { +#[expect(dead_code, reason = "Some variants are unused, but are kept to match LLVM-C")] +pub(crate) enum MetadataType { MD_dbg = 0, MD_tbaa = 1, MD_prof = 2, @@ -470,7 +474,7 @@ pub enum MetadataType { /// LLVMRustAsmDialect #[derive(Copy, Clone, PartialEq)] #[repr(C)] -pub enum AsmDialect { +pub(crate) enum AsmDialect { Att, Intel, } @@ -478,7 +482,7 @@ pub enum AsmDialect { /// LLVMRustCodeGenOptLevel #[derive(Copy, Clone, PartialEq)] #[repr(C)] -pub enum CodeGenOptLevel { +pub(crate) enum CodeGenOptLevel { None, Less, Default, @@ -487,7 +491,7 @@ pub enum CodeGenOptLevel { /// LLVMRustPassBuilderOptLevel #[repr(C)] -pub enum PassBuilderOptLevel { +pub(crate) enum PassBuilderOptLevel { O0, O1, O2, @@ -499,7 +503,7 @@ pub enum PassBuilderOptLevel { /// LLVMRustOptStage #[derive(PartialEq)] #[repr(C)] -pub enum OptStage { +pub(crate) enum OptStage { PreLinkNoLTO, PreLinkThinLTO, PreLinkFatLTO, @@ -509,7 +513,7 @@ pub enum OptStage { /// LLVMRustSanitizerOptions #[repr(C)] -pub struct SanitizerOptions { +pub(crate) struct SanitizerOptions { pub sanitize_address: bool, pub sanitize_address_recover: bool, pub sanitize_cfi: bool, @@ -530,7 +534,7 @@ pub struct SanitizerOptions { /// LLVMRustRelocModel #[derive(Copy, Clone, PartialEq)] #[repr(C)] -pub enum RelocModel { +pub(crate) enum RelocModel { Static, PIC, DynamicNoPic, @@ -542,7 +546,7 @@ pub enum RelocModel { /// LLVMRustFloatABI #[derive(Copy, Clone, PartialEq)] #[repr(C)] -pub enum FloatAbi { +pub(crate) enum FloatAbi { Default, Soft, Hard, @@ -551,7 +555,7 @@ pub enum FloatAbi { /// LLVMRustCodeModel #[derive(Copy, Clone)] #[repr(C)] -pub enum CodeModel { +pub(crate) enum CodeModel { Tiny, Small, Kernel, @@ -564,7 +568,7 @@ pub enum CodeModel { #[derive(Copy, Clone)] #[repr(C)] #[allow(dead_code)] // Variants constructed by C++. -pub enum DiagnosticKind { +pub(crate) enum DiagnosticKind { Other, InlineAsm, StackSize, @@ -587,7 +591,7 @@ pub enum DiagnosticKind { #[derive(Copy, Clone)] #[repr(C)] #[allow(dead_code)] // Variants constructed by C++. -pub enum DiagnosticLevel { +pub(crate) enum DiagnosticLevel { Error, Warning, Note, @@ -597,7 +601,7 @@ pub enum DiagnosticLevel { /// LLVMRustArchiveKind #[derive(Copy, Clone)] #[repr(C)] -pub enum ArchiveKind { +pub(crate) enum ArchiveKind { K_GNU, K_BSD, K_DARWIN, @@ -607,15 +611,15 @@ pub enum ArchiveKind { unsafe extern "C" { // LLVMRustThinLTOData - pub type ThinLTOData; + pub(crate) type ThinLTOData; // LLVMRustThinLTOBuffer - pub type ThinLTOBuffer; + pub(crate) type ThinLTOBuffer; } /// LLVMRustThinLTOModule #[repr(C)] -pub struct ThinLTOModule { +pub(crate) struct ThinLTOModule { pub identifier: *const c_char, pub data: *const u8, pub len: usize, @@ -624,7 +628,8 @@ pub struct ThinLTOModule { /// LLVMThreadLocalMode #[derive(Copy, Clone)] #[repr(C)] -pub enum ThreadLocalMode { +pub(crate) enum ThreadLocalMode { + #[expect(dead_code)] NotThreadLocal, GeneralDynamic, LocalDynamic, @@ -635,7 +640,7 @@ pub enum ThreadLocalMode { /// LLVMRustChecksumKind #[derive(Copy, Clone)] #[repr(C)] -pub enum ChecksumKind { +pub(crate) enum ChecksumKind { None, MD5, SHA1, @@ -645,7 +650,7 @@ pub enum ChecksumKind { /// LLVMRustMemoryEffects #[derive(Copy, Clone)] #[repr(C)] -pub enum MemoryEffects { +pub(crate) enum MemoryEffects { None, ReadOnly, InaccessibleMemOnly, @@ -654,7 +659,8 @@ pub enum MemoryEffects { /// LLVMOpcode #[derive(Copy, Clone, PartialEq, Eq)] #[repr(C)] -pub enum Opcode { +#[expect(dead_code, reason = "Some variants are unused, but are kept to match LLVM-C")] +pub(crate) enum Opcode { Ret = 1, Br = 2, Switch = 3, @@ -735,48 +741,48 @@ struct InvariantOpaque<'a> { // Opaque pointer types unsafe extern "C" { - pub type Module; - pub type Context; - pub type Type; - pub type Value; - pub type ConstantInt; - pub type Attribute; - pub type Metadata; - pub type BasicBlock; - pub type Comdat; + pub(crate) type Module; + pub(crate) type Context; + pub(crate) type Type; + pub(crate) type Value; + pub(crate) type ConstantInt; + pub(crate) type Attribute; + pub(crate) type Metadata; + pub(crate) type BasicBlock; + pub(crate) type Comdat; } #[repr(C)] -pub struct Builder<'a>(InvariantOpaque<'a>); +pub(crate) struct Builder<'a>(InvariantOpaque<'a>); #[repr(C)] -pub struct PassManager<'a>(InvariantOpaque<'a>); +pub(crate) struct PassManager<'a>(InvariantOpaque<'a>); unsafe extern "C" { pub type TargetMachine; - pub type Archive; + pub(crate) type Archive; } #[repr(C)] -pub struct ArchiveIterator<'a>(InvariantOpaque<'a>); +pub(crate) struct ArchiveIterator<'a>(InvariantOpaque<'a>); #[repr(C)] -pub struct ArchiveChild<'a>(InvariantOpaque<'a>); +pub(crate) struct ArchiveChild<'a>(InvariantOpaque<'a>); unsafe extern "C" { - pub type Twine; - pub type DiagnosticInfo; - pub type SMDiagnostic; + pub(crate) type Twine; + pub(crate) type DiagnosticInfo; + pub(crate) type SMDiagnostic; } #[repr(C)] -pub struct RustArchiveMember<'a>(InvariantOpaque<'a>); +pub(crate) struct RustArchiveMember<'a>(InvariantOpaque<'a>); /// Opaque pointee of `LLVMOperandBundleRef`. #[repr(C)] pub(crate) struct OperandBundle<'a>(InvariantOpaque<'a>); #[repr(C)] -pub struct Linker<'a>(InvariantOpaque<'a>); +pub(crate) struct Linker<'a>(InvariantOpaque<'a>); unsafe extern "C" { - pub type DiagnosticHandler; + pub(crate) type DiagnosticHandler; } -pub type DiagnosticHandlerTy = unsafe extern "C" fn(&DiagnosticInfo, *mut c_void); +pub(crate) type DiagnosticHandlerTy = unsafe extern "C" fn(&DiagnosticInfo, *mut c_void); -pub mod debuginfo { +pub(crate) mod debuginfo { use std::ptr; use bitflags::bitflags; @@ -793,7 +799,7 @@ pub mod debuginfo { /// builder reference typically has a shorter lifetime than the LLVM /// session (`'ll`) that it participates in. #[repr(C)] - pub struct DIBuilder<'ll>(InvariantOpaque<'ll>); + pub(crate) struct DIBuilder<'ll>(InvariantOpaque<'ll>); /// Owning pointer to a `DIBuilder<'ll>` that will dispose of the builder /// when dropped. Use `.as_ref()` to get the underlying `&DIBuilder` @@ -822,22 +828,22 @@ pub mod debuginfo { } } - pub type DIDescriptor = Metadata; - pub type DILocation = Metadata; - pub type DIScope = DIDescriptor; - pub type DIFile = DIScope; - pub type DILexicalBlock = DIScope; - pub type DISubprogram = DIScope; - pub type DIType = DIDescriptor; - pub type DIBasicType = DIType; - pub type DIDerivedType = DIType; - pub type DICompositeType = DIDerivedType; - pub type DIVariable = DIDescriptor; - pub type DIGlobalVariableExpression = DIDescriptor; - pub type DIArray = DIDescriptor; - pub type DISubrange = DIDescriptor; - pub type DIEnumerator = DIDescriptor; - pub type DITemplateTypeParameter = DIDescriptor; + pub(crate) type DIDescriptor = Metadata; + pub(crate) type DILocation = Metadata; + pub(crate) type DIScope = DIDescriptor; + pub(crate) type DIFile = DIScope; + pub(crate) type DILexicalBlock = DIScope; + pub(crate) type DISubprogram = DIScope; + pub(crate) type DIType = DIDescriptor; + pub(crate) type DIBasicType = DIType; + pub(crate) type DIDerivedType = DIType; + pub(crate) type DICompositeType = DIDerivedType; + pub(crate) type DIVariable = DIDescriptor; + pub(crate) type DIGlobalVariableExpression = DIDescriptor; + pub(crate) type DIArray = DIDescriptor; + pub(crate) type DISubrange = DIDescriptor; + pub(crate) type DIEnumerator = DIDescriptor; + pub(crate) type DITemplateTypeParameter = DIDescriptor; bitflags! { /// Must match the layout of `LLVMDIFlags` in the LLVM-C API. @@ -846,7 +852,7 @@ pub mod debuginfo { /// assertions in `RustWrapper.cpp` used by `fromRust(LLVMDIFlags)`. #[repr(transparent)] #[derive(Clone, Copy, Default)] - pub struct DIFlags: u32 { + pub(crate) struct DIFlags: u32 { const FlagZero = 0; const FlagPrivate = 1; const FlagProtected = 2; @@ -886,7 +892,7 @@ pub mod debuginfo { bitflags! { #[repr(transparent)] #[derive(Clone, Copy, Default)] - pub struct DISPFlags: u32 { + pub(crate) struct DISPFlags: u32 { const SPFlagZero = 0; const SPFlagVirtual = 1; const SPFlagPureVirtual = 2; @@ -900,7 +906,7 @@ pub mod debuginfo { /// LLVMRustDebugEmissionKind #[derive(Copy, Clone)] #[repr(C)] - pub enum DebugEmissionKind { + pub(crate) enum DebugEmissionKind { NoDebug, FullDebug, LineTablesOnly, @@ -932,8 +938,9 @@ pub mod debuginfo { /// LLVMRustDebugNameTableKind #[derive(Clone, Copy)] #[repr(C)] - pub enum DebugNameTableKind { + pub(crate) enum DebugNameTableKind { Default, + #[expect(dead_code)] Gnu, None, } @@ -943,7 +950,7 @@ pub mod debuginfo { bitflags! { #[repr(transparent)] #[derive(Default)] - pub struct AllocKindFlags : u64 { + pub(crate) struct AllocKindFlags : u64 { const Unknown = 0; const Alloc = 1; const Realloc = 1 << 1; @@ -966,19 +973,20 @@ bitflags! { } unsafe extern "C" { - pub type ModuleBuffer; + pub(crate) type ModuleBuffer; } -pub type SelfProfileBeforePassCallback = +pub(crate) type SelfProfileBeforePassCallback = unsafe extern "C" fn(*mut c_void, *const c_char, *const c_char); -pub type SelfProfileAfterPassCallback = unsafe extern "C" fn(*mut c_void); +pub(crate) type SelfProfileAfterPassCallback = unsafe extern "C" fn(*mut c_void); -pub type GetSymbolsCallback = unsafe extern "C" fn(*mut c_void, *const c_char) -> *mut c_void; -pub type GetSymbolsErrorCallback = unsafe extern "C" fn(*const c_char) -> *mut c_void; +pub(crate) type GetSymbolsCallback = + unsafe extern "C" fn(*mut c_void, *const c_char) -> *mut c_void; +pub(crate) type GetSymbolsErrorCallback = unsafe extern "C" fn(*const c_char) -> *mut c_void; #[derive(Copy, Clone)] #[repr(transparent)] -pub struct MetadataKindId(c_uint); +pub(crate) struct MetadataKindId(c_uint); impl From for MetadataKindId { fn from(value: MetadataType) -> Self { diff --git a/compiler/rustc_codegen_llvm/src/llvm/mod.rs b/compiler/rustc_codegen_llvm/src/llvm/mod.rs index a36226b25a249..6ca81c651ed42 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/mod.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/mod.rs @@ -9,18 +9,18 @@ use libc::c_uint; use rustc_abi::{Align, Size, WrappingRange}; use rustc_llvm::RustString; -pub use self::CallConv::*; -pub use self::CodeGenOptSize::*; -pub use self::MetadataType::*; -pub use self::ffi::*; +pub(crate) use self::CallConv::*; +pub(crate) use self::CodeGenOptSize::*; +pub(crate) use self::MetadataType::*; +pub(crate) use self::ffi::*; use crate::common::AsCCharPtr; -pub mod archive_ro; -pub mod diagnostic; -pub mod enzyme_ffi; +pub(crate) mod archive_ro; +pub(crate) mod diagnostic; +pub(crate) mod enzyme_ffi; mod ffi; -pub use self::enzyme_ffi::*; +pub(crate) use self::enzyme_ffi::*; impl LLVMRustResult { pub(crate) fn into_result(self) -> Result<(), ()> { @@ -127,7 +127,7 @@ pub(crate) fn CreateRangeAttr(llcx: &Context, size: Size, range: WrappingRange) } #[derive(Copy, Clone)] -pub enum AttributePlace { +pub(crate) enum AttributePlace { ReturnValue, Argument(u32), Function, @@ -145,7 +145,7 @@ impl AttributePlace { #[derive(Copy, Clone, PartialEq)] #[repr(C)] -pub enum CodeGenOptSize { +pub(crate) enum CodeGenOptSize { CodeGenOptSizeNone = 0, CodeGenOptSizeDefault = 1, CodeGenOptSizeAggressive = 2, From 867da30cc71017597d9ed731b03a45c79accd873 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 25 Mar 2025 15:50:31 +1100 Subject: [PATCH 14/19] Avoid `kw::Empty` when dealing with `rustc_allowed_through_unstable_modules`. The existing code produces `Some(kw::Empty)` for these invalid forms: - a non-name-value, e.g. `#[rustc_allowed_through_unstable_modules]` - a non-string arg, e.g. `#[rustc_allowed_through_unstable_modules = 3]` The new code avoids the `kw::Empty` and is a little shorter. It will produce `None` in those cases, which means E0789 won't be produced if the `stable` attribute is missing for these invalid forms. This doesn't matter, because these invalid forms will trigger an "malformed `rustc_allowed_through_unstable_modules` attribute" anyway. --- compiler/rustc_attr_parsing/src/attributes/stability.rs | 8 ++------ compiler/rustc_error_codes/src/error_codes/E0789.md | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_attr_parsing/src/attributes/stability.rs b/compiler/rustc_attr_parsing/src/attributes/stability.rs index 6d76456e83c80..bdad6b50186dd 100644 --- a/compiler/rustc_attr_parsing/src/attributes/stability.rs +++ b/compiler/rustc_attr_parsing/src/attributes/stability.rs @@ -5,7 +5,7 @@ use rustc_attr_data_structures::{ StableSince, UnstableReason, VERSION_PLACEHOLDER, }; use rustc_errors::ErrorGuaranteed; -use rustc_span::{Span, Symbol, kw, sym}; +use rustc_span::{Span, Symbol, sym}; use super::util::parse_version; use super::{AcceptMapping, AttributeParser, SingleAttributeParser}; @@ -61,11 +61,7 @@ impl AttributeParser for StabilityParser { }), (&[sym::rustc_allowed_through_unstable_modules], |this, cx, args| { reject_outside_std!(cx); - this.allowed_through_unstable_modules = - Some(match args.name_value().and_then(|i| i.value_as_str()) { - Some(msg) => msg, - None => kw::Empty, - }); + this.allowed_through_unstable_modules = args.name_value().and_then(|i| i.value_as_str()) }), ]; diff --git a/compiler/rustc_error_codes/src/error_codes/E0789.md b/compiler/rustc_error_codes/src/error_codes/E0789.md index 2c0018cc799f4..c7bc6cfde5134 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0789.md +++ b/compiler/rustc_error_codes/src/error_codes/E0789.md @@ -14,7 +14,7 @@ Erroneous code example: #![unstable(feature = "foo_module", reason = "...", issue = "123")] -#[rustc_allowed_through_unstable_modules] +#[rustc_allowed_through_unstable_modules = "deprecation message"] // #[stable(feature = "foo", since = "1.0")] struct Foo; // ^^^ error: `rustc_allowed_through_unstable_modules` attribute must be From 501945a22e314bad511e08a187c0aa029df51038 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 25 Mar 2025 18:00:14 +1100 Subject: [PATCH 15/19] Use `sym::dummy` for a dummy arg in `parse_fn_params`. --- compiler/rustc_parse/src/parser/item.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index f4df4044dd2ea..123234fedceb8 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -2871,7 +2871,7 @@ impl<'a> Parser<'a> { // Skip every token until next possible arg or end. p.eat_to_tokens(&[exp!(Comma), exp!(CloseParen)]); // Create a placeholder argument for proper arg count (issue #34264). - Ok(dummy_arg(Ident::new(kw::Empty, lo.to(p.prev_token.span)), guar)) + Ok(dummy_arg(Ident::new(sym::dummy, lo.to(p.prev_token.span)), guar)) }); // ...now that we've parsed the first argument, `self` is no longer allowed. first_param = false; From f756304655b8604327f7a906c9e55b74a5fedfd1 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 10 Mar 2025 18:24:54 +0300 Subject: [PATCH 16/19] privacy: Visit types and traits in impls in type privacy lints --- compiler/rustc_privacy/src/lib.rs | 40 ++++++++++++++----- tests/ui/privacy/pub-priv-dep/pub-priv1.rs | 7 ++-- .../ui/privacy/pub-priv-dep/pub-priv1.stderr | 22 +++++++++- 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index ed00d97c31daa..643b82f475300 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -72,7 +72,9 @@ impl<'tcx> fmt::Display for LazyDefPathStr<'tcx> { pub trait DefIdVisitor<'tcx> { type Result: VisitorResult = (); const SHALLOW: bool = false; - const SKIP_ASSOC_TYS: bool = false; + fn skip_assoc_tys(&self) -> bool { + false + } fn tcx(&self) -> TyCtxt<'tcx>; fn visit_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) @@ -213,7 +215,7 @@ where } } ty::Alias(kind @ (ty::Inherent | ty::Weak | ty::Projection), data) => { - if V::SKIP_ASSOC_TYS { + if self.def_id_visitor.skip_assoc_tys() { // Visitors searching for minimal visibility/reachability want to // conservatively approximate associated types like `Type::Alias` // as visible/reachable even if `Type` is private. @@ -324,7 +326,9 @@ impl<'a, 'tcx, VL: VisibilityLike, const SHALLOW: bool> DefIdVisitor<'tcx> for FindMin<'a, 'tcx, VL, SHALLOW> { const SHALLOW: bool = SHALLOW; - const SKIP_ASSOC_TYS: bool = true; + fn skip_assoc_tys(&self) -> bool { + true + } fn tcx(&self) -> TyCtxt<'tcx> { self.tcx } @@ -342,7 +346,7 @@ trait VisibilityLike: Sized { def_id: LocalDefId, ) -> Self; - // Returns an over-approximation (`SKIP_ASSOC_TYS` = true) of visibility due to + // Returns an over-approximation (`skip_assoc_tys()` = true) of visibility due to // associated types for which we can't determine visibility precisely. fn of_impl( def_id: LocalDefId, @@ -1352,6 +1356,7 @@ struct SearchInterfaceForPrivateItemsVisitor<'tcx> { required_effective_vis: Option, in_assoc_ty: bool, in_primary_interface: bool, + skip_assoc_tys: bool, } impl SearchInterfaceForPrivateItemsVisitor<'_> { @@ -1398,6 +1403,14 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { self } + fn trait_ref(&mut self) -> &mut Self { + self.in_primary_interface = true; + if let Some(trait_ref) = self.tcx.impl_trait_ref(self.item_def_id) { + let _ = self.visit_trait(trait_ref.instantiate_identity()); + } + self + } + fn check_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> bool { if self.leaks_private_dep(def_id) { self.tcx.emit_node_span_lint( @@ -1495,6 +1508,9 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { impl<'tcx> DefIdVisitor<'tcx> for SearchInterfaceForPrivateItemsVisitor<'tcx> { type Result = ControlFlow<()>; + fn skip_assoc_tys(&self) -> bool { + self.skip_assoc_tys + } fn tcx(&self) -> TyCtxt<'tcx> { self.tcx } @@ -1531,6 +1547,7 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'_, 'tcx> { required_effective_vis, in_assoc_ty: false, in_primary_interface: true, + skip_assoc_tys: false, } } @@ -1726,13 +1743,18 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'_, 'tcx> { self.effective_visibilities, ); - // check that private components do not appear in the generics or predicates of inherent impls - // this check is intentionally NOT performed for impls of traits, per #90586 + let mut check = self.check(item.owner_id.def_id, impl_vis, Some(impl_ev)); + // Generics and predicates of trait impls are intentionally not checked + // for private components (#90586). if impl_.of_trait.is_none() { - self.check(item.owner_id.def_id, impl_vis, Some(impl_ev)) - .generics() - .predicates(); + check.generics().predicates(); } + // Skip checking private components in associated types, due to lack of full + // normalization they produce very ridiculous false positives. + // FIXME: Remove this when full normalization is implemented. + check.skip_assoc_tys = true; + check.ty().trait_ref(); + for impl_item_ref in impl_.items { let impl_item_vis = if impl_.of_trait.is_none() { min( diff --git a/tests/ui/privacy/pub-priv-dep/pub-priv1.rs b/tests/ui/privacy/pub-priv-dep/pub-priv1.rs index 112eaf528be27..877029f3de37a 100644 --- a/tests/ui/privacy/pub-priv-dep/pub-priv1.rs +++ b/tests/ui/privacy/pub-priv-dep/pub-priv1.rs @@ -77,15 +77,14 @@ pub type Alias = OtherType; pub struct PublicWithPrivateImpl; -// FIXME: This should trigger. -// See https://github.com/rust-lang/rust/issues/71043 impl OtherTrait for PublicWithPrivateImpl {} +//~^ ERROR trait `OtherTrait` from private dependency 'priv_dep' in public interface pub trait PubTraitOnPrivate {} -// FIXME: This should trigger. -// See https://github.com/rust-lang/rust/issues/71043 impl PubTraitOnPrivate for OtherType {} +//~^ ERROR type `OtherType` from private dependency 'priv_dep' in public interface +//~| ERROR type `OtherType` from private dependency 'priv_dep' in public interface pub struct AllowedPrivType { #[allow(exported_private_dependencies)] diff --git a/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr b/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr index 53d461a5774a0..adfe13424cdf3 100644 --- a/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr +++ b/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr @@ -70,5 +70,25 @@ error: type `OtherType` from private dependency 'priv_dep' in public interface LL | pub type Alias = OtherType; | ^^^^^^^^^^^^^^ -error: aborting due to 11 previous errors +error: trait `OtherTrait` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:80:1 + | +LL | impl OtherTrait for PublicWithPrivateImpl {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: type `OtherType` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:85:1 + | +LL | impl PubTraitOnPrivate for OtherType {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: type `OtherType` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:85:1 + | +LL | impl PubTraitOnPrivate for OtherType {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: aborting due to 14 previous errors From 502d57cb78082efa16ed661ddfb501c189f18321 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 25 Mar 2025 09:02:28 +0000 Subject: [PATCH 17/19] Deduplicate assoc item cfg handling --- compiler/rustc_builtin_macros/src/cfg_eval.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/cfg_eval.rs b/compiler/rustc_builtin_macros/src/cfg_eval.rs index 5788966b6e7b1..b3ba90731184d 100644 --- a/compiler/rustc_builtin_macros/src/cfg_eval.rs +++ b/compiler/rustc_builtin_macros/src/cfg_eval.rs @@ -121,18 +121,11 @@ impl CfgEval<'_> { let item = parser.parse_item(ForceCollect::Yes)?.unwrap(); Annotatable::Item(self.flat_map_item(item).pop().unwrap()) } - Annotatable::AssocItem(_, AssocCtxt::Trait) => { + Annotatable::AssocItem(_, ctxt) => { let item = parser.parse_trait_item(ForceCollect::Yes)?.unwrap().unwrap(); Annotatable::AssocItem( - self.flat_map_assoc_item(item, AssocCtxt::Trait).pop().unwrap(), - AssocCtxt::Trait, - ) - } - Annotatable::AssocItem(_, AssocCtxt::Impl) => { - let item = parser.parse_impl_item(ForceCollect::Yes)?.unwrap().unwrap(); - Annotatable::AssocItem( - self.flat_map_assoc_item(item, AssocCtxt::Impl).pop().unwrap(), - AssocCtxt::Impl, + self.flat_map_assoc_item(item, ctxt).pop().unwrap(), + ctxt, ) } Annotatable::ForeignItem(_) => { From 7cdc456727fb663bf53232ec414a9001410ac843 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 25 Mar 2025 09:00:35 +0000 Subject: [PATCH 18/19] Track whether an assoc item is in a trait impl or an inherent impl --- compiler/rustc_ast/src/mut_visit.rs | 4 +- compiler/rustc_ast/src/visit.rs | 9 +- compiler/rustc_ast_lowering/src/item.rs | 31 ++---- .../rustc_ast_passes/src/ast_validation.rs | 6 +- compiler/rustc_builtin_macros/src/autodiff.rs | 6 +- compiler/rustc_expand/src/base.rs | 15 ++- compiler/rustc_expand/src/expand.rs | 101 +++++++++++++++--- compiler/rustc_expand/src/placeholders.rs | 14 ++- compiler/rustc_lint/src/early.rs | 4 +- .../rustc_resolve/src/build_reduced_graph.rs | 15 ++- compiler/rustc_resolve/src/late.rs | 4 +- compiler/rustc_resolve/src/lib.rs | 5 - compiler/rustc_resolve/src/macros.rs | 2 +- src/tools/rustfmt/src/visitor.rs | 3 +- 14 files changed, 151 insertions(+), 68 deletions(-) diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index 4edd086430000..604555e2df748 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -1318,7 +1318,9 @@ impl WalkItemKind for ItemKind { visit_polarity(vis, polarity); visit_opt(of_trait, |trait_ref| vis.visit_trait_ref(trait_ref)); vis.visit_ty(self_ty); - items.flat_map_in_place(|item| vis.flat_map_assoc_item(item, AssocCtxt::Impl)); + items.flat_map_in_place(|item| { + vis.flat_map_assoc_item(item, AssocCtxt::Impl { of_trait: of_trait.is_some() }) + }); } ItemKind::Trait(box Trait { safety, is_auto: _, generics, bounds, items }) => { visit_safety(vis, safety); diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index ce8d6df75afb2..9f6a53248089d 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -23,7 +23,7 @@ use crate::ptr::P; #[derive(Copy, Clone, Debug, PartialEq)] pub enum AssocCtxt { Trait, - Impl, + Impl { of_trait: bool }, } #[derive(Copy, Clone, Debug, PartialEq)] @@ -422,7 +422,12 @@ impl WalkItemKind for ItemKind { try_visit!(visitor.visit_generics(generics)); visit_opt!(visitor, visit_trait_ref, of_trait); try_visit!(visitor.visit_ty(self_ty)); - walk_list!(visitor, visit_assoc_item, items, AssocCtxt::Impl); + walk_list!( + visitor, + visit_assoc_item, + items, + AssocCtxt::Impl { of_trait: of_trait.is_some() } + ); } ItemKind::Struct(struct_definition, generics) | ItemKind::Union(struct_definition, generics) => { diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 518349343b3a1..cf7ffde836130 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -7,7 +7,6 @@ use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{CRATE_DEF_ID, LocalDefId}; use rustc_hir::{self as hir, HirId, PredicateOrigin}; use rustc_index::{IndexSlice, IndexVec}; -use rustc_middle::span_bug; use rustc_middle::ty::{ResolverAstLowering, TyCtxt}; use rustc_span::edit_distance::find_best_match_for_name; use rustc_span::{DUMMY_SP, DesugaringKind, Ident, Span, Symbol, kw, sym}; @@ -104,10 +103,7 @@ impl<'a, 'hir> ItemLowerer<'a, 'hir> { } fn lower_assoc_item(&mut self, item: &AssocItem, ctxt: AssocCtxt) { - let def_id = self.resolver.node_id_to_def_id[&item.id]; - let parent_id = self.tcx.local_parent(def_id); - let parent_hir = self.lower_node(parent_id).unwrap(); - self.with_lctx(item.id, |lctx| lctx.lower_assoc_item(item, ctxt, parent_hir)) + self.with_lctx(item.id, |lctx| lctx.lower_assoc_item(item, ctxt)) } fn lower_foreign_item(&mut self, item: &ForeignItem) { @@ -631,29 +627,18 @@ impl<'hir> LoweringContext<'_, 'hir> { } } - fn lower_assoc_item( - &mut self, - item: &AssocItem, - ctxt: AssocCtxt, - parent_hir: &'hir hir::OwnerInfo<'hir>, - ) -> hir::OwnerNode<'hir> { - let parent_item = parent_hir.node().expect_item(); - match parent_item.kind { - hir::ItemKind::Impl(impl_) => { - self.is_in_trait_impl = impl_.of_trait.is_some(); - } - hir::ItemKind::Trait(..) => {} - kind => { - span_bug!(item.span, "assoc item has unexpected kind of parent: {}", kind.descr()) - } - } - + fn lower_assoc_item(&mut self, item: &AssocItem, ctxt: AssocCtxt) -> hir::OwnerNode<'hir> { // Evaluate with the lifetimes in `params` in-scope. // This is used to track which lifetimes have already been defined, // and which need to be replicated when lowering an async fn. match ctxt { AssocCtxt::Trait => hir::OwnerNode::TraitItem(self.lower_trait_item(item)), - AssocCtxt::Impl => hir::OwnerNode::ImplItem(self.lower_impl_item(item)), + AssocCtxt::Impl { of_trait } => { + if of_trait { + self.is_in_trait_impl = of_trait; + } + hir::OwnerNode::ImplItem(self.lower_impl_item(item)) + } } } diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 75839e86b8d3a..a1487ca74be87 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -860,7 +860,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { this.visit_trait_ref(t); this.visit_ty(self_ty); - walk_list!(this, visit_assoc_item, items, AssocCtxt::Impl); + walk_list!(this, visit_assoc_item, items, AssocCtxt::Impl { of_trait: true }); }); walk_list!(self, visit_attribute, &item.attrs); return; // Avoid visiting again. @@ -913,7 +913,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { |this| this.visit_generics(generics), ); this.visit_ty(self_ty); - walk_list!(this, visit_assoc_item, items, AssocCtxt::Impl); + walk_list!(this, visit_assoc_item, items, AssocCtxt::Impl { of_trait: false }); }); walk_list!(self, visit_attribute, &item.attrs); return; // Avoid visiting again. @@ -1414,7 +1414,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { self.check_defaultness(item.span, item.kind.defaultness()); } - if ctxt == AssocCtxt::Impl { + if let AssocCtxt::Impl { .. } = ctxt { match &item.kind { AssocItemKind::Const(box ConstItem { expr: None, .. }) => { self.dcx().emit_err(errors::AssocConstWithoutBody { diff --git a/compiler/rustc_builtin_macros/src/autodiff.rs b/compiler/rustc_builtin_macros/src/autodiff.rs index 6591ed151cf6e..5cd653c794566 100644 --- a/compiler/rustc_builtin_macros/src/autodiff.rs +++ b/compiler/rustc_builtin_macros/src/autodiff.rs @@ -157,7 +157,7 @@ mod llvm_enzyme { }; (sig.clone(), false) } - Annotatable::AssocItem(assoc_item, _) => { + Annotatable::AssocItem(assoc_item, Impl { of_trait: false }) => { let sig = match &assoc_item.kind { ast::AssocItemKind::Fn(box ast::Fn { sig, .. }) => sig, _ => { @@ -296,7 +296,7 @@ mod llvm_enzyme { } Annotatable::Item(iitem.clone()) } - Annotatable::AssocItem(ref mut assoc_item, i @ Impl) => { + Annotatable::AssocItem(ref mut assoc_item, i @ Impl { of_trait: false }) => { if !assoc_item.attrs.iter().any(|a| same_attribute(&a.kind, &attr.kind)) { assoc_item.attrs.push(attr); } @@ -327,7 +327,7 @@ mod llvm_enzyme { kind: assoc_item, tokens: None, }); - Annotatable::AssocItem(d_fn, Impl) + Annotatable::AssocItem(d_fn, Impl { of_trait: false }) } else { let mut d_fn = ecx.item(span, d_ident, thin_vec![d_attr, inline_never], ItemKind::Fn(asdf)); diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 1dd152beb475f..990d0f2e028aa 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -153,7 +153,7 @@ impl Annotatable { pub fn expect_impl_item(self) -> P { match self { - Annotatable::AssocItem(i, AssocCtxt::Impl) => i, + Annotatable::AssocItem(i, AssocCtxt::Impl { .. }) => i, _ => panic!("expected Item"), } } @@ -403,6 +403,11 @@ pub trait MacResult { None } + /// Creates zero or more impl items. + fn make_trait_impl_items(self: Box) -> Option; 1]>> { + None + } + /// Creates zero or more trait items. fn make_trait_items(self: Box) -> Option; 1]>> { None @@ -516,6 +521,10 @@ impl MacResult for MacEager { self.impl_items } + fn make_trait_impl_items(self: Box) -> Option; 1]>> { + self.impl_items + } + fn make_trait_items(self: Box) -> Option; 1]>> { self.trait_items } @@ -613,6 +622,10 @@ impl MacResult for DummyResult { Some(SmallVec::new()) } + fn make_trait_impl_items(self: Box) -> Option; 1]>> { + Some(SmallVec::new()) + } + fn make_trait_items(self: Box) -> Option; 1]>> { Some(SmallVec::new()) } diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index e2a557528504a..d0bd8a89d9bdb 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -188,9 +188,15 @@ ast_fragments! { ImplItems(SmallVec<[P; 1]>) { "impl item"; many fn flat_map_assoc_item; - fn visit_assoc_item(AssocCtxt::Impl); + fn visit_assoc_item(AssocCtxt::Impl { of_trait: false }); fn make_impl_items; } + TraitImplItems(SmallVec<[P; 1]>) { + "impl item"; + many fn flat_map_assoc_item; + fn visit_assoc_item(AssocCtxt::Impl { of_trait: true }); + fn make_trait_impl_items; + } ForeignItems(SmallVec<[P; 1]>) { "foreign item"; many fn flat_map_foreign_item; @@ -257,6 +263,7 @@ impl AstFragmentKind { AstFragmentKind::Items | AstFragmentKind::TraitItems | AstFragmentKind::ImplItems + | AstFragmentKind::TraitImplItems | AstFragmentKind::ForeignItems | AstFragmentKind::Crate => SupportsMacroExpansion::Yes { supports_inner_attrs: true }, AstFragmentKind::Arms @@ -306,6 +313,9 @@ impl AstFragmentKind { AstFragmentKind::ImplItems => { AstFragment::ImplItems(items.map(Annotatable::expect_impl_item).collect()) } + AstFragmentKind::TraitImplItems => { + AstFragment::TraitImplItems(items.map(Annotatable::expect_impl_item).collect()) + } AstFragmentKind::TraitItems => { AstFragment::TraitItems(items.map(Annotatable::expect_trait_item).collect()) } @@ -347,10 +357,10 @@ pub enum InvocationKind { }, Attr { attr: ast::Attribute, - // Re-insertion position for inert attributes. + /// Re-insertion position for inert attributes. pos: usize, item: Annotatable, - // Required for resolving derive helper attributes. + /// Required for resolving derive helper attributes. derives: Vec, }, Derive { @@ -360,6 +370,8 @@ pub enum InvocationKind { }, GlobDelegation { item: P, + /// Whether this is a trait impl or an inherent impl + of_trait: bool, }, } @@ -388,7 +400,7 @@ impl Invocation { InvocationKind::Bang { span, .. } => *span, InvocationKind::Attr { attr, .. } => attr.span, InvocationKind::Derive { path, .. } => path.span, - InvocationKind::GlobDelegation { item } => item.span, + InvocationKind::GlobDelegation { item, .. } => item.span, } } @@ -397,7 +409,7 @@ impl Invocation { InvocationKind::Bang { span, .. } => span, InvocationKind::Attr { attr, .. } => &mut attr.span, InvocationKind::Derive { path, .. } => &mut path.span, - InvocationKind::GlobDelegation { item } => &mut item.span, + InvocationKind::GlobDelegation { item, .. } => &mut item.span, } } } @@ -820,7 +832,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { } _ => unreachable!(), }, - InvocationKind::GlobDelegation { item } => { + InvocationKind::GlobDelegation { item, of_trait } => { let AssocItemKind::DelegationMac(deleg) = &item.kind else { unreachable!() }; let suffixes = match ext { SyntaxExtensionKind::GlobDelegation(expander) => match expander.expand(self.cx) @@ -829,7 +841,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { ExpandResult::Retry(()) => { // Reassemble the original invocation for retrying. return ExpandResult::Retry(Invocation { - kind: InvocationKind::GlobDelegation { item }, + kind: InvocationKind::GlobDelegation { item, of_trait }, ..invoc }); } @@ -847,7 +859,8 @@ impl<'a, 'b> MacroExpander<'a, 'b> { self.cx, deleg, &item, &suffixes, item.span, true, ); fragment_kind.expect_from_annotatables( - single_delegations.map(|item| Annotatable::AssocItem(P(item), AssocCtxt::Impl)), + single_delegations + .map(|item| Annotatable::AssocItem(P(item), AssocCtxt::Impl { of_trait })), ) } }) @@ -973,6 +986,13 @@ pub fn parse_ast_fragment<'a>( } AstFragment::ImplItems(items) } + AstFragmentKind::TraitImplItems => { + let mut items = SmallVec::new(); + while let Some(item) = this.parse_impl_item(ForceCollect::No)? { + items.extend(item); + } + AstFragment::TraitImplItems(items) + } AstFragmentKind::ForeignItems => { let mut items = SmallVec::new(); while let Some(item) = this.parse_foreign_item(ForceCollect::No)? { @@ -1355,13 +1375,13 @@ impl InvocationCollectorNode for AstNodeWrapper, ImplItemTag> type ItemKind = AssocItemKind; const KIND: AstFragmentKind = AstFragmentKind::ImplItems; fn to_annotatable(self) -> Annotatable { - Annotatable::AssocItem(self.wrapped, AssocCtxt::Impl) + Annotatable::AssocItem(self.wrapped, AssocCtxt::Impl { of_trait: false }) } fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy { fragment.make_impl_items() } fn walk_flat_map(self, visitor: &mut V) -> Self::OutputTy { - walk_flat_map_assoc_item(visitor, self.wrapped, AssocCtxt::Impl) + walk_flat_map_assoc_item(visitor, self.wrapped, AssocCtxt::Impl { of_trait: false }) } fn is_mac_call(&self) -> bool { matches!(self.wrapped.kind, AssocItemKind::MacCall(..)) @@ -1390,6 +1410,47 @@ impl InvocationCollectorNode for AstNodeWrapper, ImplItemTag> } } +struct TraitImplItemTag; +impl InvocationCollectorNode for AstNodeWrapper, TraitImplItemTag> { + type OutputTy = SmallVec<[P; 1]>; + type ItemKind = AssocItemKind; + const KIND: AstFragmentKind = AstFragmentKind::TraitImplItems; + fn to_annotatable(self) -> Annotatable { + Annotatable::AssocItem(self.wrapped, AssocCtxt::Impl { of_trait: true }) + } + fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy { + fragment.make_trait_impl_items() + } + fn walk_flat_map(self, visitor: &mut V) -> Self::OutputTy { + walk_flat_map_assoc_item(visitor, self.wrapped, AssocCtxt::Impl { of_trait: true }) + } + fn is_mac_call(&self) -> bool { + matches!(self.wrapped.kind, AssocItemKind::MacCall(..)) + } + fn take_mac_call(self) -> (P, Self::AttrsTy, AddSemicolon) { + let item = self.wrapped.into_inner(); + match item.kind { + AssocItemKind::MacCall(mac) => (mac, item.attrs, AddSemicolon::No), + _ => unreachable!(), + } + } + fn delegation(&self) -> Option<(&ast::DelegationMac, &ast::Item)> { + match &self.wrapped.kind { + AssocItemKind::DelegationMac(deleg) => Some((deleg, &self.wrapped)), + _ => None, + } + } + fn delegation_item_kind(deleg: Box) -> Self::ItemKind { + AssocItemKind::Delegation(deleg) + } + fn from_item(item: ast::Item) -> Self { + AstNodeWrapper::new(P(item), TraitImplItemTag) + } + fn flatten_outputs(items: impl Iterator) -> Self::OutputTy { + items.flatten().collect() + } +} + impl InvocationCollectorNode for P { const KIND: AstFragmentKind = AstFragmentKind::ForeignItems; fn to_annotatable(self) -> Annotatable { @@ -1855,9 +1916,10 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { fn collect_glob_delegation( &mut self, item: P, + of_trait: bool, kind: AstFragmentKind, ) -> AstFragment { - self.collect(kind, InvocationKind::GlobDelegation { item }) + self.collect(kind, InvocationKind::GlobDelegation { item, of_trait }) } /// If `item` is an attribute invocation, remove the attribute and return it together with @@ -2030,8 +2092,10 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { let Some(suffixes) = &deleg.suffixes else { let traitless_qself = matches!(&deleg.qself, Some(qself) if qself.position == 0); - let item = match node.to_annotatable() { - Annotatable::AssocItem(item, AssocCtxt::Impl) => item, + let (item, of_trait) = match node.to_annotatable() { + Annotatable::AssocItem(item, AssocCtxt::Impl { of_trait }) => { + (item, of_trait) + } ann @ (Annotatable::Item(_) | Annotatable::AssocItem(..) | Annotatable::Stmt(_)) => { @@ -2046,7 +2110,9 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { self.cx.dcx().emit_err(GlobDelegationTraitlessQpath { span }); return Default::default(); } - return self.collect_glob_delegation(item, Node::KIND).make_ast::(); + return self + .collect_glob_delegation(item, of_trait, Node::KIND) + .make_ast::(); }; let single_delegations = build_single_delegations::( @@ -2126,7 +2192,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { ) -> SmallVec<[P; 1]> { match ctxt { AssocCtxt::Trait => self.flat_map_node(AstNodeWrapper::new(node, TraitItemTag)), - AssocCtxt::Impl => self.flat_map_node(AstNodeWrapper::new(node, ImplItemTag)), + AssocCtxt::Impl { of_trait: false } => { + self.flat_map_node(AstNodeWrapper::new(node, ImplItemTag)) + } + AssocCtxt::Impl { of_trait: true } => { + self.flat_map_node(AstNodeWrapper::new(node, TraitImplItemTag)) + } } } diff --git a/compiler/rustc_expand/src/placeholders.rs b/compiler/rustc_expand/src/placeholders.rs index 3a470924c7f69..a60a87244cc6e 100644 --- a/compiler/rustc_expand/src/placeholders.rs +++ b/compiler/rustc_expand/src/placeholders.rs @@ -86,6 +86,17 @@ pub(crate) fn placeholder( kind: ast::AssocItemKind::MacCall(mac_placeholder()), tokens: None, })]), + AstFragmentKind::TraitImplItems => { + AstFragment::TraitImplItems(smallvec![P(ast::AssocItem { + id, + span, + ident, + vis, + attrs, + kind: ast::AssocItemKind::MacCall(mac_placeholder()), + tokens: None, + })]) + } AstFragmentKind::ForeignItems => { AstFragment::ForeignItems(smallvec![P(ast::ForeignItem { id, @@ -308,7 +319,8 @@ impl MutVisitor for PlaceholderExpander { let it = self.remove(item.id); match ctxt { AssocCtxt::Trait => it.make_trait_items(), - AssocCtxt::Impl => it.make_impl_items(), + AssocCtxt::Impl { of_trait: false } => it.make_impl_items(), + AssocCtxt::Impl { of_trait: true } => it.make_trait_impl_items(), } } _ => walk_flat_map_assoc_item(self, item, ctxt), diff --git a/compiler/rustc_lint/src/early.rs b/compiler/rustc_lint/src/early.rs index 723b894c43bc4..f9601fa5ef1d8 100644 --- a/compiler/rustc_lint/src/early.rs +++ b/compiler/rustc_lint/src/early.rs @@ -241,7 +241,7 @@ impl<'ast, 'ecx, 'tcx, T: EarlyLintPass> ast_visit::Visitor<'ast> ast_visit::AssocCtxt::Trait => { lint_callback!(cx, check_trait_item, item); } - ast_visit::AssocCtxt::Impl => { + ast_visit::AssocCtxt::Impl { .. } => { lint_callback!(cx, check_impl_item, item); } } @@ -250,7 +250,7 @@ impl<'ast, 'ecx, 'tcx, T: EarlyLintPass> ast_visit::Visitor<'ast> ast_visit::AssocCtxt::Trait => { lint_callback!(cx, check_trait_item_post, item); } - ast_visit::AssocCtxt::Impl => { + ast_visit::AssocCtxt::Impl { .. } => { lint_callback!(cx, check_impl_item_post, item); } } diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 763e9207a126b..2e70019ba8ad0 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -884,10 +884,10 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { } // These items do not add names to modules. - ItemKind::Impl(box Impl { of_trait: Some(..), .. }) => { - self.r.trait_impl_items.insert(local_def_id); - } - ItemKind::Impl { .. } | ItemKind::ForeignMod(..) | ItemKind::GlobalAsm(..) => {} + ItemKind::Impl(box Impl { of_trait: Some(..), .. }) + | ItemKind::Impl { .. } + | ItemKind::ForeignMod(..) + | ItemKind::GlobalAsm(..) => {} ItemKind::MacroDef(..) | ItemKind::MacCall(_) | ItemKind::DelegationMac(..) => { unreachable!() @@ -1378,7 +1378,7 @@ impl<'a, 'ra, 'tcx> Visitor<'a> for BuildReducedGraphVisitor<'a, 'ra, 'tcx> { AssocCtxt::Trait => { self.visit_invoc_in_module(item.id); } - AssocCtxt::Impl => { + AssocCtxt::Impl { .. } => { let invoc_id = item.id.placeholder_to_expn_id(); if !self.r.glob_delegation_invoc_ids.contains(&invoc_id) { self.r @@ -1398,9 +1398,8 @@ impl<'a, 'ra, 'tcx> Visitor<'a> for BuildReducedGraphVisitor<'a, 'ra, 'tcx> { let local_def_id = feed.key(); let def_id = local_def_id.to_def_id(); - if !(ctxt == AssocCtxt::Impl - && matches!(item.vis.kind, ast::VisibilityKind::Inherited) - && self.r.trait_impl_items.contains(&self.r.tcx.local_parent(local_def_id))) + if !(matches!(ctxt, AssocCtxt::Impl { of_trait: true }) + && matches!(item.vis.kind, ast::VisibilityKind::Inherited)) { // Trait impl item visibility is inherited from its trait when not specified // explicitly. In that case we cannot determine it here in early resolve, diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index e04d0083548b1..177a2d00ac3cc 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -3376,7 +3376,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { |i, s, c| MethodNotMemberOfTrait(i, s, c), ); - visit::walk_assoc_item(this, item, AssocCtxt::Impl) + visit::walk_assoc_item(this, item, AssocCtxt::Impl { of_trait: true }) }, ); @@ -3410,7 +3410,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> { |i, s, c| TypeNotMemberOfTrait(i, s, c), ); - visit::walk_assoc_item(this, item, AssocCtxt::Impl) + visit::walk_assoc_item(this, item, AssocCtxt::Impl { of_trait: true }) }); }, ); diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index ff31af0025b54..e37acdd1ce721 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1191,10 +1191,6 @@ pub struct Resolver<'ra, 'tcx> { /// and how the `impl Trait` fragments were introduced. invocation_parents: FxHashMap, - /// Some way to know that we are in a *trait* impl in `visit_assoc_item`. - /// FIXME: Replace with a more general AST map (together with some other fields). - trait_impl_items: FxHashSet, - legacy_const_generic_args: FxHashMap>>, /// Amount of lifetime parameters for each item in the crate. item_generics_num_lifetimes: FxHashMap, @@ -1558,7 +1554,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { def_id_to_node_id, placeholder_field_indices: Default::default(), invocation_parents, - trait_impl_items: Default::default(), legacy_const_generic_args: Default::default(), item_generics_num_lifetimes: Default::default(), main_def: Default::default(), diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 34441d313f59f..aef98330e17e0 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -264,7 +264,7 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> { } InvocationKind::Bang { ref mac, .. } => (&mac.path, MacroKind::Bang), InvocationKind::Derive { ref path, .. } => (path, MacroKind::Derive), - InvocationKind::GlobDelegation { ref item } => { + InvocationKind::GlobDelegation { ref item, .. } => { let ast::AssocItemKind::DelegationMac(deleg) = &item.kind else { unreachable!() }; deleg_impl = Some(self.invocation_parent(invoc_id)); // It is sufficient to consider glob delegation a bang macro for now. diff --git a/src/tools/rustfmt/src/visitor.rs b/src/tools/rustfmt/src/visitor.rs index a5cfc542a17c8..5749d8c63faf4 100644 --- a/src/tools/rustfmt/src/visitor.rs +++ b/src/tools/rustfmt/src/visitor.rs @@ -624,7 +624,8 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { // TODO(calebcartwright): Not sure the skip spans are correct let (ai, skip_span, assoc_ctxt) = match visitor_kind { AssocTraitItem(ai) => (*ai, ai.span(), visit::AssocCtxt::Trait), - AssocImplItem(ai) => (*ai, ai.span, visit::AssocCtxt::Impl), + // There is no difference between trait and inherent assoc item formatting + AssocImplItem(ai) => (*ai, ai.span, visit::AssocCtxt::Impl { of_trait: false }), _ => unreachable!(), }; skip_out_of_file_lines_range_visitor!(self, ai.span); From 59e3380744d84c16c23e8d74438c515839306f99 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 25 Mar 2025 10:17:44 +0000 Subject: [PATCH 19/19] Avoid some more global state --- compiler/rustc_ast_lowering/src/delegation.rs | 16 ++++++-- compiler/rustc_ast_lowering/src/item.rs | 39 +++++++++++-------- compiler/rustc_ast_lowering/src/lib.rs | 2 - 3 files changed, 35 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/delegation.rs b/compiler/rustc_ast_lowering/src/delegation.rs index efc1fa05c5f92..f7640c602d6fd 100644 --- a/compiler/rustc_ast_lowering/src/delegation.rs +++ b/compiler/rustc_ast_lowering/src/delegation.rs @@ -61,8 +61,14 @@ pub(crate) struct DelegationResults<'hir> { impl<'hir> LoweringContext<'_, 'hir> { /// Defines whether the delegatee is an associated function whose first parameter is `self`. - pub(crate) fn delegatee_is_method(&self, item_id: NodeId, path_id: NodeId, span: Span) -> bool { - let sig_id = self.get_delegation_sig_id(item_id, path_id, span); + pub(crate) fn delegatee_is_method( + &self, + item_id: NodeId, + path_id: NodeId, + span: Span, + is_in_trait_impl: bool, + ) -> bool { + let sig_id = self.get_delegation_sig_id(item_id, path_id, span, is_in_trait_impl); let Ok(sig_id) = sig_id else { return false; }; @@ -88,9 +94,10 @@ impl<'hir> LoweringContext<'_, 'hir> { &mut self, delegation: &Delegation, item_id: NodeId, + is_in_trait_impl: bool, ) -> DelegationResults<'hir> { let span = self.lower_span(delegation.path.segments.last().unwrap().ident.span); - let sig_id = self.get_delegation_sig_id(item_id, delegation.id, span); + let sig_id = self.get_delegation_sig_id(item_id, delegation.id, span, is_in_trait_impl); match sig_id { Ok(sig_id) => { let (param_count, c_variadic) = self.param_count(sig_id); @@ -110,8 +117,9 @@ impl<'hir> LoweringContext<'_, 'hir> { item_id: NodeId, path_id: NodeId, span: Span, + is_in_trait_impl: bool, ) -> Result { - let sig_id = if self.is_in_trait_impl { item_id } else { path_id }; + let sig_id = if is_in_trait_impl { item_id } else { path_id }; self.get_resolution_id(sig_id, span) } diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index cf7ffde836130..2b4b379525bcf 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -404,10 +404,11 @@ impl<'hir> LoweringContext<'_, 'hir> { (trait_ref, lowered_ty) }); - self.is_in_trait_impl = trait_ref.is_some(); - let new_impl_items = self - .arena - .alloc_from_iter(impl_items.iter().map(|item| self.lower_impl_item_ref(item))); + let new_impl_items = self.arena.alloc_from_iter( + impl_items + .iter() + .map(|item| self.lower_impl_item_ref(item, trait_ref.is_some())), + ); // `defaultness.has_value()` is never called for an `impl`, always `true` in order // to not cause an assertion failure inside the `lower_defaultness` function. @@ -484,7 +485,7 @@ impl<'hir> LoweringContext<'_, 'hir> { ItemKind::Delegation(box delegation) => { debug_assert_ne!(ident.name, kw::Empty); let ident = self.lower_ident(ident); - let delegation_results = self.lower_delegation(delegation, id); + let delegation_results = self.lower_delegation(delegation, id, false); hir::ItemKind::Fn { ident, sig: delegation_results.sig, @@ -634,10 +635,7 @@ impl<'hir> LoweringContext<'_, 'hir> { match ctxt { AssocCtxt::Trait => hir::OwnerNode::TraitItem(self.lower_trait_item(item)), AssocCtxt::Impl { of_trait } => { - if of_trait { - self.is_in_trait_impl = of_trait; - } - hir::OwnerNode::ImplItem(self.lower_impl_item(item)) + hir::OwnerNode::ImplItem(self.lower_impl_item(item, of_trait)) } } } @@ -879,7 +877,7 @@ impl<'hir> LoweringContext<'_, 'hir> { (generics, kind, ty.is_some()) } AssocItemKind::Delegation(box delegation) => { - let delegation_results = self.lower_delegation(delegation, i.id); + let delegation_results = self.lower_delegation(delegation, i.id, false); let item_kind = hir::TraitItemKind::Fn( delegation_results.sig, hir::TraitFn::Provided(delegation_results.body_id), @@ -910,7 +908,7 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::AssocItemKind::Fn { has_self: sig.decl.has_self() } } AssocItemKind::Delegation(box delegation) => hir::AssocItemKind::Fn { - has_self: self.delegatee_is_method(i.id, delegation.id, i.span), + has_self: self.delegatee_is_method(i.id, delegation.id, i.span, false), }, AssocItemKind::MacCall(..) | AssocItemKind::DelegationMac(..) => { panic!("macros should have been expanded by now") @@ -930,7 +928,11 @@ impl<'hir> LoweringContext<'_, 'hir> { self.expr(span, hir::ExprKind::Err(guar)) } - fn lower_impl_item(&mut self, i: &AssocItem) -> &'hir hir::ImplItem<'hir> { + fn lower_impl_item( + &mut self, + i: &AssocItem, + is_in_trait_impl: bool, + ) -> &'hir hir::ImplItem<'hir> { debug_assert_ne!(i.ident.name, kw::Empty); // Since `default impl` is not yet implemented, this is always true in impls. let has_value = true; @@ -966,7 +968,7 @@ impl<'hir> LoweringContext<'_, 'hir> { generics, sig, i.id, - if self.is_in_trait_impl { FnDeclKind::Impl } else { FnDeclKind::Inherent }, + if is_in_trait_impl { FnDeclKind::Impl } else { FnDeclKind::Inherent }, sig.header.coroutine_kind, attrs, ); @@ -1006,7 +1008,7 @@ impl<'hir> LoweringContext<'_, 'hir> { ) } AssocItemKind::Delegation(box delegation) => { - let delegation_results = self.lower_delegation(delegation, i.id); + let delegation_results = self.lower_delegation(delegation, i.id, is_in_trait_impl); ( delegation_results.generics, hir::ImplItemKind::Fn(delegation_results.sig, delegation_results.body_id), @@ -1029,7 +1031,7 @@ impl<'hir> LoweringContext<'_, 'hir> { self.arena.alloc(item) } - fn lower_impl_item_ref(&mut self, i: &AssocItem) -> hir::ImplItemRef { + fn lower_impl_item_ref(&mut self, i: &AssocItem, is_in_trait_impl: bool) -> hir::ImplItemRef { hir::ImplItemRef { id: hir::ImplItemId { owner_id: hir::OwnerId { def_id: self.local_def_id(i.id) } }, ident: self.lower_ident(i.ident), @@ -1041,7 +1043,12 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::AssocItemKind::Fn { has_self: sig.decl.has_self() } } AssocItemKind::Delegation(box delegation) => hir::AssocItemKind::Fn { - has_self: self.delegatee_is_method(i.id, delegation.id, i.span), + has_self: self.delegatee_is_method( + i.id, + delegation.id, + i.span, + is_in_trait_impl, + ), }, AssocItemKind::MacCall(..) | AssocItemKind::DelegationMac(..) => { panic!("macros should have been expanded by now") diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index e08850da4a7a8..32e20f3f6f250 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -121,7 +121,6 @@ struct LoweringContext<'a, 'hir> { catch_scope: Option, loop_scope: Option, is_in_loop_condition: bool, - is_in_trait_impl: bool, is_in_dyn_type: bool, current_hir_id_owner: hir::OwnerId, @@ -173,7 +172,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { catch_scope: None, loop_scope: None, is_in_loop_condition: false, - is_in_trait_impl: false, is_in_dyn_type: false, coroutine_kind: None, task_context: None,