From 553a69030e5a086eb3841d020db8c9c463948c72 Mon Sep 17 00:00:00 2001 From: Ana Hobden Date: Mon, 24 Jun 2024 12:57:14 -0700 Subject: [PATCH 01/11] Specify target specific linker for riscv64gc-gnu job --- src/ci/docker/host-x86_64/disabled/riscv64gc-gnu/Dockerfile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ci/docker/host-x86_64/disabled/riscv64gc-gnu/Dockerfile b/src/ci/docker/host-x86_64/disabled/riscv64gc-gnu/Dockerfile index a9ffa5918b5..a52c3839196 100644 --- a/src/ci/docker/host-x86_64/disabled/riscv64gc-gnu/Dockerfile +++ b/src/ci/docker/host-x86_64/disabled/riscv64gc-gnu/Dockerfile @@ -91,7 +91,9 @@ RUN sh /scripts/sccache.sh # Avoid "fatal: detected dubious ownership in repository at '/checkout'" error RUN git config --global --add safe.directory /checkout -ENV RUST_CONFIGURE_ARGS --qemu-riscv64-rootfs=/tmp/rootfs +ENV RUST_CONFIGURE_ARGS \ + --qemu-riscv64-rootfs=/tmp/rootfs \ + --set target.riscv64gc-unknown-linux-gnu.linker=riscv64-linux-gnu-gcc ENV SCRIPT python3 ../x.py --stage 2 test --host='' --target riscv64gc-unknown-linux-gnu ENV NO_CHANGE_USER=1 From 8016940ef41b1245aecf9eb19c02b85f45dcd133 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 24 Jun 2024 12:50:04 +1000 Subject: [PATCH 02/11] Tweak a confusing comment in `create_match_candidates` --- compiler/rustc_mir_build/src/build/matches/mod.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 68244136d1a..350b00db7fd 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -358,8 +358,11 @@ fn create_match_candidates<'pat>( where 'a: 'pat, { - // Assemble a list of candidates: there is one candidate per pattern, - // which means there may be more than one candidate *per arm*. + // Assemble the initial list of candidates. These top-level candidates + // are 1:1 with the original match arms, but other parts of match + // lowering also introduce subcandidates (for subpatterns), and will + // also flatten candidates in some cases. So in general a list of + // candidates does _not_ necessarily correspond to a list of arms. arms.iter() .copied() .map(|arm| { From 050595a82619515614674b6e5dfe16a123dab9c2 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 24 Jun 2024 20:24:10 -0700 Subject: [PATCH 03/11] core: VaArgSafe is an unsafe trait `T: VaArgSafe` is relied on for soundness. Safe impls promise nothing. Therefore this must be an unsafe trait. Slightly pedantic, as only core can impl this, but we could choose to unseal the trait. That would allow soundly (but unsafely) implementing this for e.g. a `#[repr(C)] struct` that should be passable by varargs. --- library/core/src/ffi/mod.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/library/core/src/ffi/mod.rs b/library/core/src/ffi/mod.rs index 618897b3aba..6d1f10f5211 100644 --- a/library/core/src/ffi/mod.rs +++ b/library/core/src/ffi/mod.rs @@ -484,7 +484,7 @@ mod sealed_trait { all supported platforms", issue = "44930" )] - pub trait VaArgSafe {} + pub unsafe trait VaArgSafe {} } macro_rules! impl_va_arg_safe { @@ -494,7 +494,7 @@ macro_rules! impl_va_arg_safe { reason = "the `c_variadic` feature has not been properly tested on \ all supported platforms", issue = "44930")] - impl sealed_trait::VaArgSafe for $t {} + unsafe impl sealed_trait::VaArgSafe for $t {} )+ } } @@ -509,14 +509,15 @@ impl sealed_trait::VaArgSafe for $t {} all supported platforms", issue = "44930" )] -impl sealed_trait::VaArgSafe for *mut T {} +unsafe impl sealed_trait::VaArgSafe for *mut T {} + #[unstable( feature = "c_variadic", reason = "the `c_variadic` feature has not been properly tested on \ all supported platforms", issue = "44930" )] -impl sealed_trait::VaArgSafe for *const T {} +unsafe impl sealed_trait::VaArgSafe for *const T {} #[unstable( feature = "c_variadic", From c2f1072e013c3b58febab160b7f8759a21aaaf79 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 25 Jun 2024 16:33:51 +1000 Subject: [PATCH 04/11] Tweak `FlatPat::new` to avoid a temporarily-invalid state It was somewhat confusing that the old constructor would create a `FlatPat` in a (possibly) non-simplified state, and then simplify its contents in-place. So instead we now create its fields as local variables, perform simplification, and then create the struct afterwards. This doesn't affect correctness, but is less confusing. --- .../rustc_mir_build/src/build/matches/mod.rs | 33 ++++++++++++------- .../rustc_mir_build/src/build/matches/util.rs | 2 ++ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 68244136d1a..5ede0dc3ed0 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1031,6 +1031,12 @@ fn is_empty(&self) -> bool { } /// A pattern in a form suitable for generating code. +/// +/// Here, "flat" indicates that the pattern's match pairs have been recursively +/// simplified by [`Builder::simplify_match_pairs`]. They are not necessarily +/// flat in an absolute sense. +/// +/// Will typically be incorporated into a [`Candidate`]. #[derive(Debug, Clone)] struct FlatPat<'pat, 'tcx> { /// To match the pattern, all of these must be satisfied... @@ -1042,23 +1048,25 @@ struct FlatPat<'pat, 'tcx> { } impl<'tcx, 'pat> FlatPat<'pat, 'tcx> { + /// Creates a `FlatPat` containing a simplified [`MatchPair`] list/forest + /// for the given pattern. fn new( place: PlaceBuilder<'tcx>, pattern: &'pat Pat<'tcx>, cx: &mut Builder<'_, 'tcx>, ) -> Self { - let is_never = pattern.is_never_pattern(); - let mut flat_pat = FlatPat { - match_pairs: vec![MatchPair::new(place, pattern, cx)], - extra_data: PatternExtraData { - span: pattern.span, - bindings: Vec::new(), - ascriptions: Vec::new(), - is_never, - }, + // First, recursively build a tree of match pairs for the given pattern. + let mut match_pairs = vec![MatchPair::new(place, pattern, cx)]; + let mut extra_data = PatternExtraData { + span: pattern.span, + bindings: Vec::new(), + ascriptions: Vec::new(), + is_never: pattern.is_never_pattern(), }; - cx.simplify_match_pairs(&mut flat_pat.match_pairs, &mut flat_pat.extra_data); - flat_pat + // Partly-flatten and sort the match pairs, while recording extra data. + cx.simplify_match_pairs(&mut match_pairs, &mut extra_data); + + Self { match_pairs, extra_data } } } @@ -1104,9 +1112,12 @@ fn new( has_guard: bool, cx: &mut Builder<'_, 'tcx>, ) -> Self { + // Use `FlatPat` to build simplified match pairs, then immediately + // incorporate them into a new candidate. Self::from_flat_pat(FlatPat::new(place, pattern, cx), has_guard) } + /// Incorporates an already-simplified [`FlatPat`] into a new candidate. fn from_flat_pat(flat_pat: FlatPat<'pat, 'tcx>, has_guard: bool) -> Self { Candidate { match_pairs: flat_pat.match_pairs, diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index 50f4ca2d819..630d0b9438d 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -95,6 +95,8 @@ pub(crate) fn false_edges( } impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { + /// Recursively builds a `MatchPair` tree for the given pattern and its + /// subpatterns. pub(in crate::build) fn new( mut place_builder: PlaceBuilder<'tcx>, pattern: &'pat Pat<'tcx>, From c7b579a7cb4109f8397a9700523e26269cf6a2ed Mon Sep 17 00:00:00 2001 From: cyrgani <85427285+cyrgani@users.noreply.github.com> Date: Tue, 25 Jun 2024 13:37:22 +0200 Subject: [PATCH 05/11] Add missing slash in const_eval_select doc comment --- library/core/src/intrinsics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 6b5054a9f06..9ba1c6a4154 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2579,7 +2579,7 @@ pub const fn ptr_guaranteed_cmp(ptr: *const T, other: *const T) -> u8 { /// fn runtime() -> i32 { 1 } /// const fn compiletime() -> i32 { 2 } /// -// // ⚠ This code violates the required equivalence of `compiletime` +/// // ⚠ This code violates the required equivalence of `compiletime` /// // and `runtime`. /// const_eval_select((), compiletime, runtime) /// } From 2155c6c47761391673a2794430dc78436c25bbf8 Mon Sep 17 00:00:00 2001 From: ash <97464181+Borgerr@users.noreply.github.com> Date: Sat, 22 Jun 2024 18:53:31 -0600 Subject: [PATCH 06/11] #126333 remove `PathBuf::as_mut_vec` reference at top of `PathBuf::_push` --- library/std/src/path.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/std/src/path.rs b/library/std/src/path.rs index 72073d13280..13947122dca 100644 --- a/library/std/src/path.rs +++ b/library/std/src/path.rs @@ -1290,7 +1290,8 @@ pub fn push>(&mut self, path: P) { fn _push(&mut self, path: &Path) { // in general, a separator is needed if the rightmost byte is not a separator - let mut need_sep = self.as_mut_vec().last().map(|c| !is_sep_byte(*c)).unwrap_or(false); + let buf = self.inner.as_encoded_bytes(); + let mut need_sep = buf.last().map(|c| !is_sep_byte(*c)).unwrap_or(false); // in the special case of `C:` on Windows, do *not* add a separator let comps = self.components(); From b08cd69684e4b121399b3ea0f9ee6cc4a51cad07 Mon Sep 17 00:00:00 2001 From: ash <97464181+Borgerr@users.noreply.github.com> Date: Sun, 23 Jun 2024 08:36:23 -0600 Subject: [PATCH 07/11] inner truncate methods for UEFI platforms --- library/std/src/ffi/os_str.rs | 5 +++++ library/std/src/path.rs | 6 +++--- library/std/src/sys/os_str/bytes.rs | 5 +++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/library/std/src/ffi/os_str.rs b/library/std/src/ffi/os_str.rs index f6b9de26c1c..318fe205e0f 100644 --- a/library/std/src/ffi/os_str.rs +++ b/library/std/src/ffi/os_str.rs @@ -557,6 +557,11 @@ pub fn leak<'a>(self) -> &'a mut OsStr { pub(crate) fn as_mut_vec_for_path_buf(&mut self) -> &mut Vec { self.inner.as_mut_vec_for_path_buf() } + + #[inline] + pub(crate) fn truncate(&mut self, len: usize) { + self.inner.truncate(len); + } } #[stable(feature = "rust1", since = "1.0.0")] diff --git a/library/std/src/path.rs b/library/std/src/path.rs index 13947122dca..fb7476a3b26 100644 --- a/library/std/src/path.rs +++ b/library/std/src/path.rs @@ -1305,7 +1305,7 @@ fn _push(&mut self, path: &Path) { // absolute `path` replaces `self` if path.is_absolute() || path.prefix().is_some() { - self.as_mut_vec().truncate(0); + self.inner.truncate(0); // verbatim paths need . and .. removed } else if comps.prefix_verbatim() && !path.inner.is_empty() { @@ -1350,7 +1350,7 @@ fn _push(&mut self, path: &Path) { // `path` has a root but no prefix, e.g., `\windows` (Windows only) } else if path.has_root() { let prefix_len = self.components().prefix_remaining(); - self.as_mut_vec().truncate(prefix_len); + self.inner.truncate(prefix_len); // `path` is a pure relative path } else if need_sep { @@ -1383,7 +1383,7 @@ fn _push(&mut self, path: &Path) { pub fn pop(&mut self) -> bool { match self.parent().map(|p| p.as_u8_slice().len()) { Some(len) => { - self.as_mut_vec().truncate(len); + self.inner.truncate(len); true } None => false, diff --git a/library/std/src/sys/os_str/bytes.rs b/library/std/src/sys/os_str/bytes.rs index f7c6b0877aa..e494f7d1dea 100644 --- a/library/std/src/sys/os_str/bytes.rs +++ b/library/std/src/sys/os_str/bytes.rs @@ -207,6 +207,11 @@ pub fn into_rc(&self) -> Rc { pub(crate) fn as_mut_vec_for_path_buf(&mut self) -> &mut Vec { &mut self.inner } + + #[inline] + pub(crate) fn truncate(&mut self, len: usize) { + self.inner.truncate(len); + } } impl Slice { From 7e187e8e4b0a12b9adc0d24dc30dffde6ceaba4a Mon Sep 17 00:00:00 2001 From: ash <97464181+Borgerr@users.noreply.github.com> Date: Sun, 23 Jun 2024 17:24:59 -0600 Subject: [PATCH 08/11] remove references to `PathBuf::as_mut_vec` in `PathBuf::_set_extension` --- library/std/src/path.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/library/std/src/path.rs b/library/std/src/path.rs index fb7476a3b26..fb488ae94a1 100644 --- a/library/std/src/path.rs +++ b/library/std/src/path.rs @@ -1511,15 +1511,14 @@ fn _set_extension(&mut self, extension: &OsStr) -> bool { // truncate until right after the file stem let end_file_stem = file_stem[file_stem.len()..].as_ptr().addr(); let start = self.inner.as_encoded_bytes().as_ptr().addr(); - let v = self.as_mut_vec(); - v.truncate(end_file_stem.wrapping_sub(start)); + self.inner.truncate(end_file_stem.wrapping_sub(start)); // add the new extension, if any - let new = extension.as_encoded_bytes(); + let new = extension; if !new.is_empty() { - v.reserve_exact(new.len() + 1); - v.push(b'.'); - v.extend_from_slice(new); + self.inner.reserve_exact(new.len() + 1); + self.inner.push(OsStr::new(".")); + self.inner.push(new); } true From aa46a3368eb017eba41bfab956c7787d46c09935 Mon Sep 17 00:00:00 2001 From: ash <97464181+Borgerr@users.noreply.github.com> Date: Tue, 25 Jun 2024 07:34:35 -0600 Subject: [PATCH 09/11] `PathBuf::as_mut_vec` removed and verified for UEFI and Windows platforms #126333 --- library/std/src/ffi/os_str.rs | 17 +++++++++++------ library/std/src/path.rs | 11 +++-------- library/std/src/sys/os_str/bytes.rs | 17 +++++++++++------ library/std/src/sys/os_str/wtf8.rs | 16 +++++++++++++--- library/std/src/sys_common/wtf8.rs | 12 ++++++------ 5 files changed, 44 insertions(+), 29 deletions(-) diff --git a/library/std/src/ffi/os_str.rs b/library/std/src/ffi/os_str.rs index 318fe205e0f..4a417c84a30 100644 --- a/library/std/src/ffi/os_str.rs +++ b/library/std/src/ffi/os_str.rs @@ -552,16 +552,21 @@ pub fn leak<'a>(self) -> &'a mut OsStr { OsStr::from_inner_mut(self.inner.leak()) } - /// Part of a hack to make PathBuf::push/pop more efficient. - #[inline] - pub(crate) fn as_mut_vec_for_path_buf(&mut self) -> &mut Vec { - self.inner.as_mut_vec_for_path_buf() - } - + /// Provides plumbing to core `Vec::truncate`. + /// More well behaving alternative to allowing outer types + /// full mutable access to the core `Vec`. #[inline] pub(crate) fn truncate(&mut self, len: usize) { self.inner.truncate(len); } + + /// Provides plumbing to core `Vec::extend_from_slice`. + /// More well behaving alternative to allowing outer types + /// full mutable access to the core `Vec`. + #[inline] + pub(crate) fn extend_from_slice(&mut self, other: &[u8]) { + self.inner.extend_from_slice(other); + } } #[stable(feature = "rust1", since = "1.0.0")] diff --git a/library/std/src/path.rs b/library/std/src/path.rs index fb488ae94a1..caae8f924d2 100644 --- a/library/std/src/path.rs +++ b/library/std/src/path.rs @@ -1163,11 +1163,6 @@ pub struct PathBuf { } impl PathBuf { - #[inline] - fn as_mut_vec(&mut self) -> &mut Vec { - self.inner.as_mut_vec_for_path_buf() - } - /// Allocates an empty `PathBuf`. /// /// # Examples @@ -2645,18 +2640,18 @@ fn _with_extension(&self, extension: &OsStr) -> PathBuf { None => { // Enough capacity for the extension and the dot let capacity = self_len + extension.len() + 1; - let whole_path = self_bytes.iter(); + let whole_path = self_bytes; (capacity, whole_path) } Some(previous_extension) => { let capacity = self_len + extension.len() - previous_extension.len(); - let path_till_dot = self_bytes[..self_len - previous_extension.len()].iter(); + let path_till_dot = &self_bytes[..self_len - previous_extension.len()]; (capacity, path_till_dot) } }; let mut new_path = PathBuf::with_capacity(new_capacity); - new_path.as_mut_vec().extend(slice_to_copy); + new_path.inner.extend_from_slice(slice_to_copy); new_path.set_extension(extension); new_path } diff --git a/library/std/src/sys/os_str/bytes.rs b/library/std/src/sys/os_str/bytes.rs index e494f7d1dea..2a7477e3afc 100644 --- a/library/std/src/sys/os_str/bytes.rs +++ b/library/std/src/sys/os_str/bytes.rs @@ -202,16 +202,21 @@ pub fn into_rc(&self) -> Rc { self.as_slice().into_rc() } - /// Part of a hack to make PathBuf::push/pop more efficient. - #[inline] - pub(crate) fn as_mut_vec_for_path_buf(&mut self) -> &mut Vec { - &mut self.inner - } - + /// Provides plumbing to core `Vec::truncate`. + /// More well behaving alternative to allowing outer types + /// full mutable access to the core `Vec`. #[inline] pub(crate) fn truncate(&mut self, len: usize) { self.inner.truncate(len); } + + /// Provides plumbing to core `Vec::extend_from_slice`. + /// More well behaving alternative to allowing outer types + /// full mutable access to the core `Vec`. + #[inline] + pub(crate) fn extend_from_slice(&mut self, other: &[u8]) { + self.inner.extend_from_slice(other); + } } impl Slice { diff --git a/library/std/src/sys/os_str/wtf8.rs b/library/std/src/sys/os_str/wtf8.rs index 96690f8c44e..edb923a4750 100644 --- a/library/std/src/sys/os_str/wtf8.rs +++ b/library/std/src/sys/os_str/wtf8.rs @@ -165,10 +165,20 @@ pub fn into_rc(&self) -> Rc { self.as_slice().into_rc() } - /// Part of a hack to make PathBuf::push/pop more efficient. + /// Provides plumbing to core `Vec::truncate`. + /// More well behaving alternative to allowing outer types + /// full mutable access to the core `Vec`. #[inline] - pub(crate) fn as_mut_vec_for_path_buf(&mut self) -> &mut Vec { - self.inner.as_mut_vec_for_path_buf() + pub(crate) fn truncate(&mut self, len: usize) { + self.inner.truncate(len); + } + + /// Provides plumbing to core `Vec::extend_from_slice`. + /// More well behaving alternative to allowing outer types + /// full mutable access to the core `Vec`. + #[inline] + pub(crate) fn extend_from_slice(&mut self, other: &[u8]) { + self.inner.extend_from_slice(other); } } diff --git a/library/std/src/sys_common/wtf8.rs b/library/std/src/sys_common/wtf8.rs index 84128a4b595..708f62f476e 100644 --- a/library/std/src/sys_common/wtf8.rs +++ b/library/std/src/sys_common/wtf8.rs @@ -474,13 +474,13 @@ pub fn from_box(boxed: Box) -> Wtf8Buf { Wtf8Buf { bytes: bytes.into_vec(), is_known_utf8: false } } - /// Part of a hack to make PathBuf::push/pop more efficient. + /// Provides plumbing to core `Vec::extend_from_slice`. + /// More well behaving alternative to allowing outer types + /// full mutable access to the core `Vec`. #[inline] - pub(crate) fn as_mut_vec_for_path_buf(&mut self) -> &mut Vec { - // FIXME: this function should not even exist, as it implies violating Wtf8Buf invariants - // For now, simply assume that is about to happen. - self.is_known_utf8 = false; - &mut self.bytes + pub(crate) fn extend_from_slice(&mut self, other: &[u8]) { + self.bytes.extend_from_slice(other); + self.is_known_utf8 = self.is_known_utf8 || self.next_surrogate(0).is_none(); } } From d30d85fd9e71e701e9179037d5100b6a04521c50 Mon Sep 17 00:00:00 2001 From: Bryanskiy Date: Tue, 25 Jun 2024 16:32:00 +0300 Subject: [PATCH 10/11] Delegation: ast lowering refactor --- compiler/rustc_ast_lowering/src/delegation.rs | 107 ++++++++---------- tests/ui/delegation/explicit-paths.stderr | 8 +- 2 files changed, 53 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/delegation.rs b/compiler/rustc_ast_lowering/src/delegation.rs index d9dd0b3bca5..678cac210f4 100644 --- a/compiler/rustc_ast_lowering/src/delegation.rs +++ b/compiler/rustc_ast_lowering/src/delegation.rs @@ -66,14 +66,18 @@ pub(crate) fn delegation_has_self(&self, item_id: NodeId, path_id: NodeId, span: let Ok(sig_id) = sig_id else { return false; }; - if let Some(local_sig_id) = sig_id.as_local() { + self.has_self(sig_id, span) + } + + fn has_self(&self, def_id: DefId, span: Span) -> bool { + if let Some(local_sig_id) = def_id.as_local() { // The value may be missing due to recursive delegation. // Error will be emmited later during HIR ty lowering. self.resolver.delegation_fn_sigs.get(&local_sig_id).map_or(false, |sig| sig.has_self) } else { - match self.tcx.def_kind(sig_id) { + match self.tcx.def_kind(def_id) { DefKind::Fn => false, - DefKind::AssocFn => self.tcx.associated_item(sig_id).fn_has_self_parameter, + DefKind::AssocFn => self.tcx.associated_item(def_id).fn_has_self_parameter, _ => span_bug!(span, "unexpected DefKind for delegation item"), } } @@ -107,12 +111,17 @@ fn get_delegation_sig_id( span: Span, ) -> Result { let sig_id = if self.is_in_trait_impl { item_id } else { path_id }; - let sig_id = - self.resolver.get_partial_res(sig_id).and_then(|r| r.expect_full_res().opt_def_id()); - sig_id.ok_or_else(|| { - self.tcx - .dcx() - .span_delayed_bug(span, "LoweringContext: couldn't resolve delegation item") + self.get_resolution_id(sig_id, span) + } + + fn get_resolution_id(&self, node_id: NodeId, span: Span) -> Result { + let def_id = + self.resolver.get_partial_res(node_id).and_then(|r| r.expect_full_res().opt_def_id()); + def_id.ok_or_else(|| { + self.tcx.dcx().span_delayed_bug( + span, + format!("LoweringContext: couldn't resolve node {:?} in delegation item", node_id), + ) }) } @@ -122,7 +131,7 @@ fn lower_delegation_generics(&mut self, span: Span) -> &'hir hir::Generics<'hir> predicates: &[], has_where_clause_predicates: false, where_clause_span: span, - span: span, + span, }) } @@ -222,12 +231,7 @@ fn generate_arg(&mut self, param_id: HirId, span: Span) -> hir::Expr<'hir> { })); let path = self.arena.alloc(hir::Path { span, res: Res::Local(param_id), segments }); - - hir::Expr { - hir_id: self.next_id(), - kind: hir::ExprKind::Path(hir::QPath::Resolved(None, path)), - span, - } + self.mk_expr(hir::ExprKind::Path(hir::QPath::Resolved(None, path)), span) } fn lower_delegation_body( @@ -236,19 +240,11 @@ fn lower_delegation_body( param_count: usize, span: Span, ) -> BodyId { - let path = self.lower_qpath( - delegation.id, - &delegation.qself, - &delegation.path, - ParamMode::Optional, - ImplTraitContext::Disallowed(ImplTraitPosition::Path), - None, - ); let block = delegation.body.as_deref(); self.lower_body(|this| { - let mut parameters: Vec> = Vec::new(); - let mut args: Vec> = Vec::new(); + let mut parameters: Vec> = Vec::with_capacity(param_count); + let mut args: Vec> = Vec::with_capacity(param_count); for idx in 0..param_count { let (param, pat_node_id) = this.generate_param(span); @@ -264,11 +260,7 @@ fn lower_delegation_body( }; self_resolver.visit_block(block); let block = this.lower_block(block, false); - hir::Expr { - hir_id: this.next_id(), - kind: hir::ExprKind::Block(block, None), - span: block.span, - } + this.mk_expr(hir::ExprKind::Block(block, None), block.span) } else { let pat_hir_id = this.lower_node_id(pat_node_id); this.generate_arg(pat_hir_id, span) @@ -276,43 +268,41 @@ fn lower_delegation_body( args.push(arg); } - let args = self.arena.alloc_from_iter(args); - let final_expr = this.generate_call(path, args); + let final_expr = this.finalize_body_lowering(delegation, args, span); (this.arena.alloc_from_iter(parameters), final_expr) }) } - fn generate_call( + // Generates fully qualified call for the resulting body. + fn finalize_body_lowering( &mut self, - path: hir::QPath<'hir>, - args: &'hir [hir::Expr<'hir>], + delegation: &Delegation, + args: Vec>, + span: Span, ) -> hir::Expr<'hir> { - let callee = self.arena.alloc(hir::Expr { - hir_id: self.next_id(), - kind: hir::ExprKind::Path(path), - span: path.span(), - }); + let path = self.lower_qpath( + delegation.id, + &delegation.qself, + &delegation.path, + ParamMode::Optional, + ImplTraitContext::Disallowed(ImplTraitPosition::Path), + None, + ); - let expr = self.arena.alloc(hir::Expr { - hir_id: self.next_id(), - kind: hir::ExprKind::Call(callee, args), - span: path.span(), - }); + let args = self.arena.alloc_from_iter(args); + let path_expr = self.arena.alloc(self.mk_expr(hir::ExprKind::Path(path), span)); + let call = self.arena.alloc(self.mk_expr(hir::ExprKind::Call(path_expr, args), span)); let block = self.arena.alloc(hir::Block { stmts: &[], - expr: Some(expr), + expr: Some(call), hir_id: self.next_id(), rules: hir::BlockCheckMode::DefaultBlock, - span: path.span(), + span, targeted_by_break: false, }); - hir::Expr { - hir_id: self.next_id(), - kind: hir::ExprKind::Block(block, None), - span: path.span(), - } + self.mk_expr(hir::ExprKind::Block(block, None), span) } fn generate_delegation_error( @@ -333,11 +323,7 @@ fn generate_delegation_error( let header = self.generate_header_error(); let sig = hir::FnSig { decl, header, span }; - let body_id = self.lower_body(|this| { - let expr = - hir::Expr { hir_id: this.next_id(), kind: hir::ExprKind::Err(err), span: span }; - (&[], expr) - }); + let body_id = self.lower_body(|this| (&[], this.mk_expr(hir::ExprKind::Err(err), span))); DelegationResults { generics, body_id, sig } } @@ -349,6 +335,11 @@ fn generate_header_error(&self) -> hir::FnHeader { abi: abi::Abi::Rust, } } + + #[inline] + fn mk_expr(&mut self, kind: hir::ExprKind<'hir>, span: Span) -> hir::Expr<'hir> { + hir::Expr { hir_id: self.next_id(), kind, span } + } } struct SelfResolver<'a> { diff --git a/tests/ui/delegation/explicit-paths.stderr b/tests/ui/delegation/explicit-paths.stderr index 30891c94c0e..d33c5da4377 100644 --- a/tests/ui/delegation/explicit-paths.stderr +++ b/tests/ui/delegation/explicit-paths.stderr @@ -110,10 +110,10 @@ error[E0308]: mismatched types --> $DIR/explicit-paths.rs:78:30 | LL | reuse ::foo1; - | ---------------^^^^ - | | | - | | expected `&S2`, found `&S` - | arguments to this function are incorrect + | ^^^^ + | | + | expected `&S2`, found `&S` + | arguments to this function are incorrect | = note: expected reference `&S2` found reference `&S` From 6997b6876d62b7c42faf12af3164c87ab733be13 Mon Sep 17 00:00:00 2001 From: mu001999 Date: Tue, 25 Jun 2024 23:29:44 +0800 Subject: [PATCH 11/11] Detect unused structs which derived Default --- compiler/rustc_passes/src/dead.rs | 25 +++++++++++++++++++ library/alloc/src/sync/tests.rs | 2 +- library/core/src/default.rs | 1 + tests/ui/deriving/deriving-default-enum.rs | 2 +- .../issues/issue-68696-catch-during-unwind.rs | 1 + .../dead-code/unused-struct-derive-default.rs | 25 +++++++++++++++++++ .../unused-struct-derive-default.stderr | 24 ++++++++++++++++++ 7 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 tests/ui/lint/dead-code/unused-struct-derive-default.rs create mode 100644 tests/ui/lint/dead-code/unused-struct-derive-default.stderr diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 2cb3c5d8965..a188a1c2563 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -400,6 +400,31 @@ fn should_ignore_item(&mut self, def_id: DefId) -> bool { return false; } + // don't ignore impls for Enums and pub Structs whose methods don't have self receiver, + // cause external crate may call such methods to construct values of these types + if let Some(local_impl_of) = impl_of.as_local() + && let Some(local_def_id) = def_id.as_local() + && let Some(fn_sig) = + self.tcx.hir().fn_sig_by_hir_id(self.tcx.local_def_id_to_hir_id(local_def_id)) + && matches!(fn_sig.decl.implicit_self, hir::ImplicitSelfKind::None) + && let TyKind::Path(hir::QPath::Resolved(_, path)) = + self.tcx.hir().expect_item(local_impl_of).expect_impl().self_ty.kind + && let Res::Def(def_kind, did) = path.res + { + match def_kind { + // for example, #[derive(Default)] pub struct T(i32); + // external crate can call T::default() to construct T, + // so that don't ignore impl Default for pub Enum and Structs + DefKind::Struct | DefKind::Union if self.tcx.visibility(did).is_public() => { + return false; + } + // don't ignore impl Default for Enums, + // cause we don't know which variant is constructed + DefKind::Enum => return false, + _ => (), + }; + } + if let Some(trait_of) = self.tcx.trait_id_of_impl(impl_of) && self.tcx.has_attr(trait_of, sym::rustc_trivial_field_reads) { diff --git a/library/alloc/src/sync/tests.rs b/library/alloc/src/sync/tests.rs index 49eae718c16..1b123aa58f2 100644 --- a/library/alloc/src/sync/tests.rs +++ b/library/alloc/src/sync/tests.rs @@ -396,7 +396,7 @@ fn show_arc() { // Make sure deriving works with Arc #[derive(Eq, Ord, PartialEq, PartialOrd, Clone, Debug, Default)] -struct Foo { +struct _Foo { inner: Arc, } diff --git a/library/core/src/default.rs b/library/core/src/default.rs index 4524b352ec8..5cacedcb241 100644 --- a/library/core/src/default.rs +++ b/library/core/src/default.rs @@ -103,6 +103,7 @@ /// ``` #[cfg_attr(not(test), rustc_diagnostic_item = "Default")] #[stable(feature = "rust1", since = "1.0.0")] +#[cfg_attr(not(bootstrap), rustc_trivial_field_reads)] pub trait Default: Sized { /// Returns the "default value" for a type. /// diff --git a/tests/ui/deriving/deriving-default-enum.rs b/tests/ui/deriving/deriving-default-enum.rs index 96eba258c97..6b59f39a67d 100644 --- a/tests/ui/deriving/deriving-default-enum.rs +++ b/tests/ui/deriving/deriving-default-enum.rs @@ -22,6 +22,6 @@ enum MyOption { } fn main() { - assert_eq!(Foo::default(), Foo::Alpha); + assert!(matches!(Foo::default(), Foo::Alpha)); assert!(matches!(MyOption::::default(), MyOption::None)); } diff --git a/tests/ui/issues/issue-68696-catch-during-unwind.rs b/tests/ui/issues/issue-68696-catch-during-unwind.rs index 2368cccef0d..80d63b0cde7 100644 --- a/tests/ui/issues/issue-68696-catch-during-unwind.rs +++ b/tests/ui/issues/issue-68696-catch-during-unwind.rs @@ -7,6 +7,7 @@ use std::panic::catch_unwind; +#[allow(dead_code)] #[derive(Default)] struct Guard; diff --git a/tests/ui/lint/dead-code/unused-struct-derive-default.rs b/tests/ui/lint/dead-code/unused-struct-derive-default.rs new file mode 100644 index 00000000000..330ad32dd57 --- /dev/null +++ b/tests/ui/lint/dead-code/unused-struct-derive-default.rs @@ -0,0 +1,25 @@ +#![deny(dead_code)] + +#[derive(Default)] +struct T; //~ ERROR struct `T` is never constructed + +#[derive(Default)] +struct Used; + +#[derive(Default)] +enum E { + #[default] + A, + B, //~ ERROR variant `B` is never constructed +} + +// external crate can call T2::default() to construct T2, +// so that no warnings for pub adts +#[derive(Default)] +pub struct T2 { + _unread: i32, +} + +fn main() { + let _x: Used = Default::default(); +} diff --git a/tests/ui/lint/dead-code/unused-struct-derive-default.stderr b/tests/ui/lint/dead-code/unused-struct-derive-default.stderr new file mode 100644 index 00000000000..bbb0bd7be70 --- /dev/null +++ b/tests/ui/lint/dead-code/unused-struct-derive-default.stderr @@ -0,0 +1,24 @@ +error: struct `T` is never constructed + --> $DIR/unused-struct-derive-default.rs:4:8 + | +LL | struct T; + | ^ + | + = note: `T` has a derived impl for the trait `Default`, but this is intentionally ignored during dead code analysis +note: the lint level is defined here + --> $DIR/unused-struct-derive-default.rs:1:9 + | +LL | #![deny(dead_code)] + | ^^^^^^^^^ + +error: variant `B` is never constructed + --> $DIR/unused-struct-derive-default.rs:13:5 + | +LL | enum E { + | - variant in this enum +... +LL | B, + | ^ + +error: aborting due to 2 previous errors +