From 9eb6a5446a4e35f48ad22a5b70a74a8badb9fa0d Mon Sep 17 00:00:00 2001 From: Petros Angelatos Date: Thu, 10 Apr 2025 11:09:31 +0300 Subject: [PATCH 01/27] sync::mpsc: add miri reproducer of double free Signed-off-by: Petros Angelatos --- library/std/src/sync/mpmc/list.rs | 5 +++ .../miri/tests/pass/issues/issue-139553.rs | 45 +++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 src/tools/miri/tests/pass/issues/issue-139553.rs diff --git a/library/std/src/sync/mpmc/list.rs b/library/std/src/sync/mpmc/list.rs index d88914f5291..2dd8f41226d 100644 --- a/library/std/src/sync/mpmc/list.rs +++ b/library/std/src/sync/mpmc/list.rs @@ -213,6 +213,11 @@ impl Channel { .compare_exchange(block, new, Ordering::Release, Ordering::Relaxed) .is_ok() { + // This yield point leaves the channel in a half-initialized state where the + // tail.block pointer is set but the head.block is not. This is used to + // facilitate the test in src/tools/miri/tests/pass/issues/issue-139553.rs + #[cfg(miri)] + crate::thread::yield_now(); self.head.block.store(new, Ordering::Release); block = new; } else { diff --git a/src/tools/miri/tests/pass/issues/issue-139553.rs b/src/tools/miri/tests/pass/issues/issue-139553.rs new file mode 100644 index 00000000000..97cb9e1a805 --- /dev/null +++ b/src/tools/miri/tests/pass/issues/issue-139553.rs @@ -0,0 +1,45 @@ +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-compare-exchange-weak-failure-rate=0 +use std::sync::mpsc::channel; +use std::thread; + +/// This test aims to trigger a race condition that causes a double free in the unbounded channel +/// implementation. The test relies on a particular thread scheduling to happen as annotated by the +/// comments below. +fn main() { + let (s1, r) = channel::(); + let s2 = s1.clone(); + + let t1 = thread::spawn(move || { + // 1. The first action executed is an attempt to send the first value in the channel. This + // will begin to initialize the channel but will stop at a critical momement as + // indicated by the `yield_now()` call in the `start_send` method of the implementation. + let _ = s1.send(42); + // 4. The sender is re-scheduled and it finishes the initialization of the channel by + // setting head.block to the same value as tail.block. It then proceeds to publish its + // value but observes that the channel has already disconnected (due to the concurrent + // call of `discard_all_messages`) and aborts the send. + }); + std::thread::yield_now(); + + // 2. A second sender attempts to send a value while the channel is in a half-initialized + // state. Here, half-initialized means that the `tail.block` pointer points to a valid block + // but `head.block` is still null. This condition is ensured by the yield of step 1. When + // this call returns the channel state has tail.index != head.index, tail.block != NULL, and + // head.block = NULL. + s2.send(42).unwrap(); + // 3. This thread continues with dropping the one and only receiver. When all receivers are + // gone `discard_all_messages` will attempt to drop all currently sent values and + // de-allocate all the blocks. If `tail.block != NULL` but `head.block = NULL` the + // implementation waits for the initializing sender to finish by spinning/yielding. + drop(r); + // 5. This thread is rescheduled and `discard_all_messages` observes the head.block pointer set + // by step 4 and proceeds with deallocation. In the problematic version of the code + // `head.block` is simply read via an `Acquire` load and not swapped with NULL. After this + // call returns the channel state has tail.index = head.index, tail.block = NULL, and + // head.block != NULL. + t1.join().unwrap(); + // 6. The last sender (s1) is dropped here which also attempts to cleanup any data in the + // channel. It observes `tail.index = head.index` and so it doesn't attempt to cleanup any + // messages but it also observes that `head.block != NULL` and attempts to deallocate it. + // This is however already deallocated by `discard_all_messages`, leading to a double free. +} From b9e2ac5c7b1d6bb3b6f5fdfe0819eaf7e95bf7ff Mon Sep 17 00:00:00 2001 From: Petros Angelatos Date: Tue, 8 Apr 2025 22:37:25 +0300 Subject: [PATCH 02/27] sync::mpsc: prevent double free on `Drop` This PR is fixing a regression introduced by #121646 that can lead to a double free when dropping the channel. The details of the bug can be found in the corresponding crossbeam PR https://github.com/crossbeam-rs/crossbeam/pull/1187 Signed-off-by: Petros Angelatos --- library/std/src/sync/mpmc/list.rs | 8 +++++++- src/tools/miri/tests/pass/issues/issue-139553.rs | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/library/std/src/sync/mpmc/list.rs b/library/std/src/sync/mpmc/list.rs index 2dd8f41226d..1c6acb29e37 100644 --- a/library/std/src/sync/mpmc/list.rs +++ b/library/std/src/sync/mpmc/list.rs @@ -569,9 +569,15 @@ impl Channel { // In that case, just wait until it gets initialized. while block.is_null() { backoff.spin_heavy(); - block = self.head.block.load(Ordering::Acquire); + block = self.head.block.swap(ptr::null_mut(), Ordering::AcqRel); } } + // After this point `head.block` is not modified again and it will be deallocated if it's + // non-null. The `Drop` code of the channel, which runs after this function, also attempts + // to deallocate `head.block` if it's non-null. Therefore this function must maintain the + // invariant that if a deallocation of head.block is attemped then it must also be set to + // NULL. Failing to do so will lead to the Drop code attempting a double free. For this + // reason both reads above do an atomic swap instead of a simple atomic load. unsafe { // Drop all messages between head and tail and deallocate the heap-allocated blocks. diff --git a/src/tools/miri/tests/pass/issues/issue-139553.rs b/src/tools/miri/tests/pass/issues/issue-139553.rs index 97cb9e1a805..119d589d1ea 100644 --- a/src/tools/miri/tests/pass/issues/issue-139553.rs +++ b/src/tools/miri/tests/pass/issues/issue-139553.rs @@ -38,7 +38,7 @@ fn main() { // call returns the channel state has tail.index = head.index, tail.block = NULL, and // head.block != NULL. t1.join().unwrap(); - // 6. The last sender (s1) is dropped here which also attempts to cleanup any data in the + // 6. The last sender (s2) is dropped here which also attempts to cleanup any data in the // channel. It observes `tail.index = head.index` and so it doesn't attempt to cleanup any // messages but it also observes that `head.block != NULL` and attempts to deallocate it. // This is however already deallocated by `discard_all_messages`, leading to a double free. From f35eae780b492cca74d6ada59dc857959d937cd4 Mon Sep 17 00:00:00 2001 From: dianne Date: Fri, 14 Mar 2025 18:56:15 -0700 Subject: [PATCH 03/27] store the kind of pattern adjustments in `pat_adjustments` This allows us to better distinguish builtin and overloaded implicit dereferences. --- .../rustc_hir_typeck/src/expr_use_visitor.rs | 6 ++--- compiler/rustc_hir_typeck/src/pat.rs | 3 ++- compiler/rustc_middle/src/ty/adjustment.rs | 22 +++++++++++++++++++ .../rustc_middle/src/ty/structural_impls.rs | 6 +++++ .../rustc_middle/src/ty/typeck_results.rs | 10 ++++++--- .../src/thir/pattern/migration.rs | 8 +++---- .../rustc_mir_build/src/thir/pattern/mod.rs | 9 ++++---- .../clippy_lints/src/pattern_type_mismatch.rs | 2 +- 8 files changed, 50 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs index 27df8f0e98a..f43f9b5187b 100644 --- a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs @@ -1227,9 +1227,9 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx // actually this is somewhat "disjoint" from the code below // that aims to account for `ref x`. if let Some(vec) = self.cx.typeck_results().pat_adjustments().get(pat.hir_id) { - if let Some(first_ty) = vec.first() { - debug!("pat_ty(pat={:?}) found adjusted ty `{:?}`", pat, first_ty); - return Ok(*first_ty); + if let Some(first_adjust) = vec.first() { + debug!("pat_ty(pat={:?}) found adjustment `{:?}`", pat, first_adjust); + return Ok(first_adjust.source); } } else if let PatKind::Ref(subpat, _) = pat.kind && self.cx.typeck_results().skipped_ref_pats().contains(pat.hir_id) diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index fbc783c0509..c19d8cf6dc5 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/compiler/rustc_hir_typeck/src/pat.rs @@ -29,6 +29,7 @@ use rustc_trait_selection::infer::InferCtxtExt; use rustc_trait_selection::traits::{ObligationCause, ObligationCauseCode}; use tracing::{debug, instrument, trace}; use ty::VariantDef; +use ty::adjustment::{PatAdjust, PatAdjustment}; use super::report_unexpected_variant_res; use crate::expectation::Expectation; @@ -415,7 +416,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .pat_adjustments_mut() .entry(pat.hir_id) .or_default() - .push(expected); + .push(PatAdjustment { kind: PatAdjust::BuiltinDeref, source: expected }); let mut binding_mode = ByRef::Yes(match pat_info.binding_mode { // If default binding mode is by value, make it `ref` or `ref mut` diff --git a/compiler/rustc_middle/src/ty/adjustment.rs b/compiler/rustc_middle/src/ty/adjustment.rs index f8ab555305f..981d67e58d4 100644 --- a/compiler/rustc_middle/src/ty/adjustment.rs +++ b/compiler/rustc_middle/src/ty/adjustment.rs @@ -214,3 +214,25 @@ pub enum CustomCoerceUnsized { /// Records the index of the field being coerced. Struct(FieldIdx), } + +/// Represents an implicit coercion applied to the scrutinee of a match before testing a pattern +/// against it. Currently, this is used only for implicit dereferences. +#[derive(Clone, Copy, TyEncodable, TyDecodable, HashStable, TypeFoldable, TypeVisitable)] +pub struct PatAdjustment<'tcx> { + pub kind: PatAdjust, + /// The type of the scrutinee before the adjustment is applied, or the "adjusted type" of the + /// pattern. + pub source: Ty<'tcx>, +} + +/// Represents implicit coercions of patterns' types, rather than values' types. +#[derive(Clone, Copy, PartialEq, Debug, TyEncodable, TyDecodable, HashStable)] +#[derive(TypeFoldable, TypeVisitable)] +pub enum PatAdjust { + /// An implicit dereference before matching, such as when matching the pattern `0` against a + /// scrutinee of type `&u8` or `&mut u8`. + BuiltinDeref, + /// An implicit call to `Deref(Mut)::deref(_mut)` before matching, such as when matching the + /// pattern `[..]` against a scrutinee of type `Vec`. + OverloadedDeref, +} diff --git a/compiler/rustc_middle/src/ty/structural_impls.rs b/compiler/rustc_middle/src/ty/structural_impls.rs index 798ef352c40..6434e1a1ad9 100644 --- a/compiler/rustc_middle/src/ty/structural_impls.rs +++ b/compiler/rustc_middle/src/ty/structural_impls.rs @@ -61,6 +61,12 @@ impl<'tcx> fmt::Debug for ty::adjustment::Adjustment<'tcx> { } } +impl<'tcx> fmt::Debug for ty::adjustment::PatAdjustment<'tcx> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{} -> {:?}", self.source, self.kind) + } +} + impl fmt::Debug for ty::BoundRegionKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match *self { diff --git a/compiler/rustc_middle/src/ty/typeck_results.rs b/compiler/rustc_middle/src/ty/typeck_results.rs index 90c6ef67fb8..ee8113b0f76 100644 --- a/compiler/rustc_middle/src/ty/typeck_results.rs +++ b/compiler/rustc_middle/src/ty/typeck_results.rs @@ -90,7 +90,7 @@ pub struct TypeckResults<'tcx> { /// /// See: /// - pat_adjustments: ItemLocalMap>>, + pat_adjustments: ItemLocalMap>>, /// Set of reference patterns that match against a match-ergonomics inserted reference /// (as opposed to against a reference in the scrutinee type). @@ -403,11 +403,15 @@ impl<'tcx> TypeckResults<'tcx> { LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.pat_binding_modes } } - pub fn pat_adjustments(&self) -> LocalTableInContext<'_, Vec>> { + pub fn pat_adjustments( + &self, + ) -> LocalTableInContext<'_, Vec>> { LocalTableInContext { hir_owner: self.hir_owner, data: &self.pat_adjustments } } - pub fn pat_adjustments_mut(&mut self) -> LocalTableInContextMut<'_, Vec>> { + pub fn pat_adjustments_mut( + &mut self, + ) -> LocalTableInContextMut<'_, Vec>> { LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.pat_adjustments } } diff --git a/compiler/rustc_mir_build/src/thir/pattern/migration.rs b/compiler/rustc_mir_build/src/thir/pattern/migration.rs index bd7787b643d..c688b6f2848 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/migration.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/migration.rs @@ -5,7 +5,7 @@ use rustc_errors::MultiSpan; use rustc_hir::{BindingMode, ByRef, HirId, Mutability}; use rustc_lint as lint; use rustc_middle::span_bug; -use rustc_middle::ty::{self, Rust2024IncompatiblePatInfo, Ty, TyCtxt}; +use rustc_middle::ty::{self, Rust2024IncompatiblePatInfo, TyCtxt}; use rustc_span::{Ident, Span}; use crate::errors::{Rust2024IncompatiblePat, Rust2024IncompatiblePatSugg}; @@ -93,10 +93,10 @@ impl<'a> PatMigration<'a> { pub(super) fn visit_implicit_derefs<'tcx>( &mut self, pat_span: Span, - adjustments: &[Ty<'tcx>], + adjustments: &[ty::adjustment::PatAdjustment<'tcx>], ) -> Option<(Span, Mutability)> { - let implicit_deref_mutbls = adjustments.iter().map(|ref_ty| { - let &ty::Ref(_, _, mutbl) = ref_ty.kind() else { + let implicit_deref_mutbls = adjustments.iter().map(|adjust| { + let &ty::Ref(_, _, mutbl) = adjust.source.kind() else { span_bug!(pat_span, "pattern implicitly dereferences a non-ref type"); }; mutbl diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs index 73d60cf4442..f682581d763 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -18,6 +18,7 @@ use rustc_middle::mir::interpret::LitToConstInput; use rustc_middle::thir::{ Ascription, FieldPat, LocalVarId, Pat, PatKind, PatRange, PatRangeBoundary, }; +use rustc_middle::ty::adjustment::PatAdjustment; use rustc_middle::ty::layout::IntegerExt; use rustc_middle::ty::{self, CanonicalUserTypeAnnotation, Ty, TyCtxt, TypingMode}; use rustc_middle::{bug, span_bug}; @@ -63,7 +64,7 @@ pub(super) fn pat_from_hir<'a, 'tcx>( impl<'a, 'tcx> PatCtxt<'a, 'tcx> { fn lower_pattern(&mut self, pat: &'tcx hir::Pat<'tcx>) -> Box> { - let adjustments: &[Ty<'tcx>] = + let adjustments: &[PatAdjustment<'tcx>] = self.typeck_results.pat_adjustments().get(pat.hir_id).map_or(&[], |v| &**v); // Track the default binding mode for the Rust 2024 migration suggestion. @@ -102,11 +103,11 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { _ => self.lower_pattern_unadjusted(pat), }; - let adjusted_pat = adjustments.iter().rev().fold(unadjusted_pat, |thir_pat, ref_ty| { - debug!("{:?}: wrapping pattern with type {:?}", thir_pat, ref_ty); + let adjusted_pat = adjustments.iter().rev().fold(unadjusted_pat, |thir_pat, adjust| { + debug!("{:?}: wrapping pattern with type {:?}", thir_pat, adjust); Box::new(Pat { span: thir_pat.span, - ty: *ref_ty, + ty: adjust.source, kind: PatKind::Deref { subpattern: thir_pat }, }) }); diff --git a/src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs b/src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs index 8f1a1ee76c6..96d3f7196c0 100644 --- a/src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs +++ b/src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs @@ -179,7 +179,7 @@ fn find_first_mismatch(cx: &LateContext<'_>, pat: &Pat<'_>) -> Option<(Span, Mut }; if let Some(adjustments) = cx.typeck_results().pat_adjustments().get(adjust_pat.hir_id) { if let [first, ..] = **adjustments { - if let ty::Ref(.., mutability) = *first.kind() { + if let ty::Ref(.., mutability) = *first.source.kind() { let level = if p.hir_id == pat.hir_id { Level::Top } else { From 2907ab5bf9583bec1acb389637ed300ef333b273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 15 Apr 2025 12:13:11 +0200 Subject: [PATCH 04/27] Move `is_builder_target`, `is_system_llvm` and `is_rust_llvm` from `Builder` to `Config` --- src/bootstrap/src/core/build_steps/compile.rs | 10 +++-- src/bootstrap/src/core/build_steps/dist.rs | 10 +++-- src/bootstrap/src/core/build_steps/llvm.rs | 6 +-- src/bootstrap/src/core/build_steps/test.rs | 4 +- src/bootstrap/src/core/config/config.rs | 36 ++++++++++++++++ src/bootstrap/src/core/sanity.rs | 2 +- src/bootstrap/src/lib.rs | 42 ++----------------- 7 files changed, 57 insertions(+), 53 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index dab58fccf5e..30ab142b53b 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -155,7 +155,7 @@ impl Step for Std { // When using `download-rustc`, we already have artifacts for the host available. Don't // recompile them. - if builder.download_rustc() && builder.is_builder_target(target) + if builder.download_rustc() && builder.config.is_builder_target(target) // NOTE: the beta compiler may generate different artifacts than the downloaded compiler, so // its artifacts can't be reused. && compiler.stage != 0 @@ -229,7 +229,7 @@ impl Step for Std { // The LLD wrappers and `rust-lld` are self-contained linking components that can be // necessary to link the stdlib on some targets. We'll also need to copy these binaries to // the `stage0-sysroot` to ensure the linker is found when bootstrapping on such a target. - if compiler.stage == 0 && builder.is_builder_target(compiler.host) { + if compiler.stage == 0 && builder.config.is_builder_target(compiler.host) { trace!( "(build == host) copying linking components to `stage0-sysroot` for bootstrapping" ); @@ -1374,7 +1374,7 @@ pub fn rustc_cargo_env( /// Pass down configuration from the LLVM build into the build of /// rustc_llvm and rustc_codegen_llvm. fn rustc_llvm_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelection) { - if builder.is_rust_llvm(target) { + if builder.config.is_rust_llvm(target) { cargo.env("LLVM_RUSTLLVM", "1"); } if builder.config.llvm_enzyme { @@ -2532,7 +2532,9 @@ pub fn strip_debug(builder: &Builder<'_>, target: TargetSelection, path: &Path) // FIXME: to make things simpler for now, limit this to the host and target where we know // `strip -g` is both available and will fix the issue, i.e. on a x64 linux host that is not // cross-compiling. Expand this to other appropriate targets in the future. - if target != "x86_64-unknown-linux-gnu" || !builder.is_builder_target(target) || !path.exists() + if target != "x86_64-unknown-linux-gnu" + || !builder.config.is_builder_target(target) + || !path.exists() { return; } diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs index 83f71aeed72..00043cfc204 100644 --- a/src/bootstrap/src/core/build_steps/dist.rs +++ b/src/bootstrap/src/core/build_steps/dist.rs @@ -612,7 +612,7 @@ impl Step for DebuggerScripts { fn skip_host_target_lib(builder: &Builder<'_>, compiler: Compiler) -> bool { // The only true set of target libraries came from the build triple, so // let's reduce redundant work by only producing archives from that host. - if !builder.is_builder_target(compiler.host) { + if !builder.config.is_builder_target(compiler.host) { builder.info("\tskipping, not a build host"); true } else { @@ -671,7 +671,9 @@ fn copy_target_libs( &self_contained_dst.join(path.file_name().unwrap()), FileType::NativeLibrary, ); - } else if dependency_type == DependencyType::Target || builder.is_builder_target(target) { + } else if dependency_type == DependencyType::Target + || builder.config.is_builder_target(target) + { builder.copy_link(&path, &dst.join(path.file_name().unwrap()), FileType::NativeLibrary); } } @@ -824,7 +826,7 @@ impl Step for Analysis { fn run(self, builder: &Builder<'_>) -> Option { let compiler = self.compiler; let target = self.target; - if !builder.is_builder_target(compiler.host) { + if !builder.config.is_builder_target(compiler.host) { return None; } @@ -2118,7 +2120,7 @@ fn maybe_install_llvm( // // If the LLVM is coming from ourselves (just from CI) though, we // still want to install it, as it otherwise won't be available. - if builder.is_system_llvm(target) { + if builder.config.is_system_llvm(target) { trace!("system LLVM requested, no install"); return false; } diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 69a8bd59f16..f29520d25cf 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -485,7 +485,7 @@ impl Step for Llvm { } // https://llvm.org/docs/HowToCrossCompileLLVM.html - if !builder.is_builder_target(target) { + if !builder.config.is_builder_target(target) { let LlvmResult { llvm_config, .. } = builder.ensure(Llvm { target: builder.config.build }); if !builder.config.dry_run() { @@ -637,7 +637,7 @@ fn configure_cmake( } cfg.target(&target.triple).host(&builder.config.build.triple); - if !builder.is_builder_target(target) { + if !builder.config.is_builder_target(target) { cfg.define("CMAKE_CROSSCOMPILING", "True"); // NOTE: Ideally, we wouldn't have to do this, and `cmake-rs` would just handle it for us. @@ -1098,7 +1098,7 @@ impl Step for Lld { .define("LLVM_CMAKE_DIR", llvm_cmake_dir) .define("LLVM_INCLUDE_TESTS", "OFF"); - if !builder.is_builder_target(target) { + if !builder.config.is_builder_target(target) { // Use the host llvm-tblgen binary. cfg.define( "LLVM_TABLEGEN_EXE", diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index b1a3bba0887..ad62651cc69 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -1894,7 +1894,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the .arg(llvm_components.trim()); llvm_components_passed = true; } - if !builder.is_rust_llvm(target) { + if !builder.config.is_rust_llvm(target) { cmd.arg("--system-llvm"); } @@ -2668,7 +2668,7 @@ impl Step for Crate { cargo } else { // Also prepare a sysroot for the target. - if !builder.is_builder_target(target) { + if !builder.config.is_builder_target(target) { builder.ensure(compile::Std::new(compiler, target).force_recompile(true)); builder.ensure(RemoteCopyLibs { compiler, target }); } diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 25ec64f90b5..57015797a62 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -3233,6 +3233,42 @@ impl Config { Some(commit.to_string()) } + + /// Checks if the given target is the same as the builder target. + pub fn is_builder_target(&self, target: TargetSelection) -> bool { + self.build == target + } + + /// Returns `true` if this is an external version of LLVM not managed by bootstrap. + /// In particular, we expect llvm sources to be available when this is false. + /// + /// NOTE: this is not the same as `!is_rust_llvm` when `llvm_has_patches` is set. + pub fn is_system_llvm(&self, target: TargetSelection) -> bool { + match self.target_config.get(&target) { + Some(Target { llvm_config: Some(_), .. }) => { + let ci_llvm = self.llvm_from_ci && self.is_builder_target(target); + !ci_llvm + } + // We're building from the in-tree src/llvm-project sources. + Some(Target { llvm_config: None, .. }) => false, + None => false, + } + } + + /// Returns `true` if this is our custom, patched, version of LLVM. + /// + /// This does not necessarily imply that we're managing the `llvm-project` submodule. + pub fn is_rust_llvm(&self, target: TargetSelection) -> bool { + match self.target_config.get(&target) { + // We're using a user-controlled version of LLVM. The user has explicitly told us whether the version has our patches. + // (They might be wrong, but that's not a supported use-case.) + // In particular, this tries to support `submodules = false` and `patches = false`, for using a newer version of LLVM that's not through `rust-lang/llvm-project`. + Some(Target { llvm_has_rust_patches: Some(patched), .. }) => *patched, + // The user hasn't promised the patches match. + // This only has our patches if it's downloaded from CI or built from source. + _ => !self.is_system_llvm(target), + } + } } /// Compares the current `Llvm` options against those in the CI LLVM builder and detects any incompatible options. diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs index 891340add90..0310937e60c 100644 --- a/src/bootstrap/src/core/sanity.rs +++ b/src/bootstrap/src/core/sanity.rs @@ -325,7 +325,7 @@ than building it. if target.contains("musl") && !target.contains("unikraft") { // If this is a native target (host is also musl) and no musl-root is given, // fall back to the system toolchain in /usr before giving up - if build.musl_root(*target).is_none() && build.is_builder_target(*target) { + if build.musl_root(*target).is_none() && build.config.is_builder_target(*target) { let target = build.config.target_config.entry(*target).or_default(); target.musl_root = Some("/usr".into()); } diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 1a513a240e1..e088cf52073 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -35,7 +35,7 @@ use utils::channel::GitInfo; use crate::core::builder; use crate::core::builder::Kind; -use crate::core::config::{DryRun, LldMode, LlvmLibunwind, Target, TargetSelection, flags}; +use crate::core::config::{DryRun, LldMode, LlvmLibunwind, TargetSelection, flags}; use crate::utils::exec::{BehaviorOnFailure, BootstrapCommand, CommandOutput, OutputMode, command}; use crate::utils::helpers::{ self, dir_is_empty, exe, libdir, output, set_file_times, split_debuginfo, symlink_dir, @@ -803,7 +803,7 @@ impl Build { /// Note that if LLVM is configured externally then the directory returned /// will likely be empty. fn llvm_out(&self, target: TargetSelection) -> PathBuf { - if self.config.llvm_from_ci && self.is_builder_target(target) { + if self.config.llvm_from_ci && self.config.is_builder_target(target) { self.config.ci_llvm_root() } else { self.out.join(target).join("llvm") @@ -851,37 +851,6 @@ impl Build { if self.config.vendor { Some(self.src.join(VENDOR_DIR)) } else { None } } - /// Returns `true` if this is an external version of LLVM not managed by bootstrap. - /// In particular, we expect llvm sources to be available when this is false. - /// - /// NOTE: this is not the same as `!is_rust_llvm` when `llvm_has_patches` is set. - fn is_system_llvm(&self, target: TargetSelection) -> bool { - match self.config.target_config.get(&target) { - Some(Target { llvm_config: Some(_), .. }) => { - let ci_llvm = self.config.llvm_from_ci && self.is_builder_target(target); - !ci_llvm - } - // We're building from the in-tree src/llvm-project sources. - Some(Target { llvm_config: None, .. }) => false, - None => false, - } - } - - /// Returns `true` if this is our custom, patched, version of LLVM. - /// - /// This does not necessarily imply that we're managing the `llvm-project` submodule. - fn is_rust_llvm(&self, target: TargetSelection) -> bool { - match self.config.target_config.get(&target) { - // We're using a user-controlled version of LLVM. The user has explicitly told us whether the version has our patches. - // (They might be wrong, but that's not a supported use-case.) - // In particular, this tries to support `submodules = false` and `patches = false`, for using a newer version of LLVM that's not through `rust-lang/llvm-project`. - Some(Target { llvm_has_rust_patches: Some(patched), .. }) => *patched, - // The user hasn't promised the patches match. - // This only has our patches if it's downloaded from CI or built from source. - _ => !self.is_system_llvm(target), - } - } - /// Returns the path to `FileCheck` binary for the specified target fn llvm_filecheck(&self, target: TargetSelection) -> PathBuf { let target_config = self.config.target_config.get(&target); @@ -1356,7 +1325,7 @@ Executed at: {executed_at}"#, // need to use CXX compiler as linker to resolve the exception functions // that are only existed in CXX libraries Some(self.cxx.borrow()[&target].path().into()) - } else if !self.is_builder_target(target) + } else if !self.config.is_builder_target(target) && helpers::use_host_linker(target) && !target.is_msvc() { @@ -2025,11 +1994,6 @@ to download LLVM rather than building it. stream.reset().unwrap(); result } - - /// Checks if the given target is the same as the builder target. - fn is_builder_target(&self, target: TargetSelection) -> bool { - self.config.build == target - } } #[cfg(unix)] From 3c01dfe2ae09aef9ee42223f492764717e152cde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 15 Apr 2025 12:15:10 +0200 Subject: [PATCH 05/27] Rename `is_builder_target` to `is_host_target` --- src/bootstrap/src/core/build_steps/compile.rs | 6 +++--- src/bootstrap/src/core/build_steps/dist.rs | 7 +++---- src/bootstrap/src/core/build_steps/llvm.rs | 6 +++--- src/bootstrap/src/core/build_steps/test.rs | 2 +- src/bootstrap/src/core/builder/tests.rs | 4 ++-- src/bootstrap/src/core/config/config.rs | 6 +++--- src/bootstrap/src/core/sanity.rs | 2 +- src/bootstrap/src/lib.rs | 4 ++-- 8 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 30ab142b53b..f78539b4781 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -229,7 +229,7 @@ impl Step for Std { // The LLD wrappers and `rust-lld` are self-contained linking components that can be // necessary to link the stdlib on some targets. We'll also need to copy these binaries to // the `stage0-sysroot` to ensure the linker is found when bootstrapping on such a target. - if compiler.stage == 0 && builder.config.is_builder_target(compiler.host) { + if compiler.stage == 0 && builder.config.is_host_target(compiler.host) { trace!( "(build == host) copying linking components to `stage0-sysroot` for bootstrapping" ); @@ -2182,7 +2182,7 @@ impl Step for Assemble { debug!("copying codegen backends to sysroot"); copy_codegen_backends_to_sysroot(builder, build_compiler, target_compiler); - if builder.config.lld_enabled { + if builder.config.lld_enabled && !builder.config.is_system_llvm(target_compiler.host) { builder.ensure(crate::core::build_steps::tool::LldWrapper { build_compiler, target_compiler, @@ -2533,7 +2533,7 @@ pub fn strip_debug(builder: &Builder<'_>, target: TargetSelection, path: &Path) // `strip -g` is both available and will fix the issue, i.e. on a x64 linux host that is not // cross-compiling. Expand this to other appropriate targets in the future. if target != "x86_64-unknown-linux-gnu" - || !builder.config.is_builder_target(target) + || !builder.config.is_host_target(target) || !path.exists() { return; diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs index 00043cfc204..ed90ede7936 100644 --- a/src/bootstrap/src/core/build_steps/dist.rs +++ b/src/bootstrap/src/core/build_steps/dist.rs @@ -612,7 +612,7 @@ impl Step for DebuggerScripts { fn skip_host_target_lib(builder: &Builder<'_>, compiler: Compiler) -> bool { // The only true set of target libraries came from the build triple, so // let's reduce redundant work by only producing archives from that host. - if !builder.config.is_builder_target(compiler.host) { + if !builder.config.is_host_target(compiler.host) { builder.info("\tskipping, not a build host"); true } else { @@ -671,8 +671,7 @@ fn copy_target_libs( &self_contained_dst.join(path.file_name().unwrap()), FileType::NativeLibrary, ); - } else if dependency_type == DependencyType::Target - || builder.config.is_builder_target(target) + } else if dependency_type == DependencyType::Target || builder.config.is_host_target(target) { builder.copy_link(&path, &dst.join(path.file_name().unwrap()), FileType::NativeLibrary); } @@ -826,7 +825,7 @@ impl Step for Analysis { fn run(self, builder: &Builder<'_>) -> Option { let compiler = self.compiler; let target = self.target; - if !builder.config.is_builder_target(compiler.host) { + if !builder.config.is_host_target(compiler.host) { return None; } diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index f29520d25cf..0c82cb16348 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -485,7 +485,7 @@ impl Step for Llvm { } // https://llvm.org/docs/HowToCrossCompileLLVM.html - if !builder.config.is_builder_target(target) { + if !builder.config.is_host_target(target) { let LlvmResult { llvm_config, .. } = builder.ensure(Llvm { target: builder.config.build }); if !builder.config.dry_run() { @@ -637,7 +637,7 @@ fn configure_cmake( } cfg.target(&target.triple).host(&builder.config.build.triple); - if !builder.config.is_builder_target(target) { + if !builder.config.is_host_target(target) { cfg.define("CMAKE_CROSSCOMPILING", "True"); // NOTE: Ideally, we wouldn't have to do this, and `cmake-rs` would just handle it for us. @@ -1098,7 +1098,7 @@ impl Step for Lld { .define("LLVM_CMAKE_DIR", llvm_cmake_dir) .define("LLVM_INCLUDE_TESTS", "OFF"); - if !builder.config.is_builder_target(target) { + if !builder.config.is_host_target(target) { // Use the host llvm-tblgen binary. cfg.define( "LLVM_TABLEGEN_EXE", diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index ad62651cc69..096f7de6597 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2668,7 +2668,7 @@ impl Step for Crate { cargo } else { // Also prepare a sysroot for the target. - if !builder.config.is_builder_target(target) { + if !builder.config.is_host_target(target) { builder.ensure(compile::Std::new(compiler, target).force_recompile(true)); builder.ensure(RemoteCopyLibs { compiler, target }); } diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index fd3b28e4e6a..5de824ebab2 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -1107,8 +1107,8 @@ fn test_is_builder_target() { let build = Build::new(config); let builder = Builder::new(&build); - assert!(builder.is_builder_target(target1)); - assert!(!builder.is_builder_target(target2)); + assert!(builder.config.is_host_target(target1)); + assert!(!builder.config.is_host_target(target2)); } } diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 57015797a62..12e157a34ae 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -3234,8 +3234,8 @@ impl Config { Some(commit.to_string()) } - /// Checks if the given target is the same as the builder target. - pub fn is_builder_target(&self, target: TargetSelection) -> bool { + /// Checks if the given target is the same as the host target. + pub fn is_host_target(&self, target: TargetSelection) -> bool { self.build == target } @@ -3246,7 +3246,7 @@ impl Config { pub fn is_system_llvm(&self, target: TargetSelection) -> bool { match self.target_config.get(&target) { Some(Target { llvm_config: Some(_), .. }) => { - let ci_llvm = self.llvm_from_ci && self.is_builder_target(target); + let ci_llvm = self.llvm_from_ci && self.is_host_target(target); !ci_llvm } // We're building from the in-tree src/llvm-project sources. diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs index 0310937e60c..4b020a7edb7 100644 --- a/src/bootstrap/src/core/sanity.rs +++ b/src/bootstrap/src/core/sanity.rs @@ -325,7 +325,7 @@ than building it. if target.contains("musl") && !target.contains("unikraft") { // If this is a native target (host is also musl) and no musl-root is given, // fall back to the system toolchain in /usr before giving up - if build.musl_root(*target).is_none() && build.config.is_builder_target(*target) { + if build.musl_root(*target).is_none() && build.config.is_host_target(*target) { let target = build.config.target_config.entry(*target).or_default(); target.musl_root = Some("/usr".into()); } diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index e088cf52073..88d181532a7 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -803,7 +803,7 @@ impl Build { /// Note that if LLVM is configured externally then the directory returned /// will likely be empty. fn llvm_out(&self, target: TargetSelection) -> PathBuf { - if self.config.llvm_from_ci && self.config.is_builder_target(target) { + if self.config.llvm_from_ci && self.config.is_host_target(target) { self.config.ci_llvm_root() } else { self.out.join(target).join("llvm") @@ -1325,7 +1325,7 @@ Executed at: {executed_at}"#, // need to use CXX compiler as linker to resolve the exception functions // that are only existed in CXX libraries Some(self.cxx.borrow()[&target].path().into()) - } else if !self.config.is_builder_target(target) + } else if !self.config.is_host_target(target) && helpers::use_host_linker(target) && !target.is_msvc() { From 8c50f95cf088c6ccf882a152bd46c090efa3a1c7 Mon Sep 17 00:00:00 2001 From: Will Glynn Date: Fri, 4 Apr 2025 17:46:35 -0500 Subject: [PATCH 06/27] rustdoc: Output target feature information `#[target_feature]` attributes refer to a target-specific list of features. Enabling certain features can imply enabling other features. Certain features are always enabled on certain targets, since they are required by the target's ABI. Features can also be enabled indirectly based on other compiler flags. Feature information is ultimately known to `rustc`. Rather than force external tools to track it -- which may be wildly impractical due to `-C target-cpu` -- have `rustdoc` output `rustc`'s feature data. --- src/librustdoc/json/mod.rs | 60 +++++++++++++++++++ src/rustdoc-json-types/lib.rs | 58 +++++++++++++++++- src/tools/compiletest/src/directive-list.rs | 3 + src/tools/jsondocck/src/main.rs | 2 +- src/tools/jsondoclint/src/validator/tests.rs | 4 ++ .../targets/aarch64_apple_darwin.rs | 14 +++++ .../aarch64_reflects_compiler_options.rs | 10 ++++ .../targets/aarch64_unknown_linux_gnu.rs | 14 +++++ .../targets/i686_pc_windows_msvc.rs | 14 +++++ .../targets/i686_unknown_linux_gnu.rs | 14 +++++ .../targets/x86_64_apple_darwin.rs | 14 +++++ .../targets/x86_64_pc_windows_gnu.rs | 14 +++++ .../targets/x86_64_pc_windows_msvc.rs | 14 +++++ .../x86_64_reflects_compiler_options.rs | 10 ++++ .../targets/x86_64_unknown_linux_gnu.rs | 14 +++++ 15 files changed, 257 insertions(+), 2 deletions(-) create mode 100644 tests/rustdoc-json/targets/aarch64_apple_darwin.rs create mode 100644 tests/rustdoc-json/targets/aarch64_reflects_compiler_options.rs create mode 100644 tests/rustdoc-json/targets/aarch64_unknown_linux_gnu.rs create mode 100644 tests/rustdoc-json/targets/i686_pc_windows_msvc.rs create mode 100644 tests/rustdoc-json/targets/i686_unknown_linux_gnu.rs create mode 100644 tests/rustdoc-json/targets/x86_64_apple_darwin.rs create mode 100644 tests/rustdoc-json/targets/x86_64_pc_windows_gnu.rs create mode 100644 tests/rustdoc-json/targets/x86_64_pc_windows_msvc.rs create mode 100644 tests/rustdoc-json/targets/x86_64_reflects_compiler_options.rs create mode 100644 tests/rustdoc-json/targets/x86_64_unknown_linux_gnu.rs diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index ba27eed7c11..131a12ce228 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -14,6 +14,7 @@ use std::io::{BufWriter, Write, stdout}; use std::path::PathBuf; use std::rc::Rc; +use rustc_data_structures::fx::FxHashSet; use rustc_hir::def_id::{DefId, DefIdSet}; use rustc_middle::ty::TyCtxt; use rustc_session::Session; @@ -123,6 +124,58 @@ impl<'tcx> JsonRenderer<'tcx> { } } +fn target(sess: &rustc_session::Session) -> types::Target { + // Build a set of which features are enabled on this target + let globally_enabled_features: FxHashSet<&str> = + sess.unstable_target_features.iter().map(|name| name.as_str()).collect(); + + // Build a map of target feature stability by feature name + use rustc_target::target_features::Stability; + let feature_stability: FxHashMap<&str, Stability> = sess + .target + .rust_target_features() + .into_iter() + .copied() + .map(|(name, stability, _)| (name, stability)) + .collect(); + + types::Target { + triple: sess.opts.target_triple.tuple().into(), + target_features: sess + .target + .rust_target_features() + .into_iter() + .copied() + .filter(|(_, stability, _)| { + // Describe only target features which the user can toggle + stability.toggle_allowed().is_ok() + }) + .map(|(name, stability, implied_features)| { + types::TargetFeature { + name: name.into(), + unstable_feature_gate: match stability { + Stability::Unstable(feature_gate) => Some(feature_gate.as_str().into()), + _ => None, + }, + implies_features: implied_features + .into_iter() + .copied() + .filter(|name| { + // Imply only target features which the user can toggle + feature_stability + .get(name) + .map(|stability| stability.toggle_allowed().is_ok()) + .unwrap_or(false) + }) + .map(String::from) + .collect(), + globally_enabled: globally_enabled_features.contains(name), + } + }) + .collect(), + } +} + impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { fn descr() -> &'static str { "json" @@ -248,6 +301,12 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { let e = ExternalCrate { crate_num: LOCAL_CRATE }; let index = (*self.index).clone().into_inner(); + // Note that tcx.rust_target_features is inappropriate here because rustdoc tries to run for + // multiple targets: https://github.com/rust-lang/rust/pull/137632 + // + // We want to describe a single target, so pass tcx.sess rather than tcx. + let target = target(self.tcx.sess); + debug!("Constructing Output"); let output_crate = types::Crate { root: self.id_from_item_default(e.def_id().into()), @@ -288,6 +347,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { ) }) .collect(), + target, format_version: types::FORMAT_VERSION, }; if let Some(ref out_dir) = self.out_dir { diff --git a/src/rustdoc-json-types/lib.rs b/src/rustdoc-json-types/lib.rs index 137fe4c4c35..7247950545a 100644 --- a/src/rustdoc-json-types/lib.rs +++ b/src/rustdoc-json-types/lib.rs @@ -30,7 +30,7 @@ pub type FxHashMap = HashMap; // re-export for use in src/librustdoc /// This integer is incremented with every breaking change to the API, /// and is returned along with the JSON blob as [`Crate::format_version`]. /// Consuming code should assert that this value matches the format version(s) that it supports. -pub const FORMAT_VERSION: u32 = 43; +pub const FORMAT_VERSION: u32 = 44; /// The root of the emitted JSON blob. /// @@ -52,11 +52,67 @@ pub struct Crate { pub paths: HashMap, /// Maps `crate_id` of items to a crate name and html_root_url if it exists. pub external_crates: HashMap, + /// Information about the target for which this documentation was generated + pub target: Target, /// A single version number to be used in the future when making backwards incompatible changes /// to the JSON output. pub format_version: u32, } +/// Information about a target +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct Target { + /// The target triple for which this documentation was generated + pub triple: String, + /// A list of features valid for use in `#[target_feature]` attributes + /// for the target where this rustdoc JSON was generated. + pub target_features: Vec, +} + +/// Information about a target feature. +/// +/// Rust target features are used to influence code generation, especially around selecting +/// instructions which are not universally supported by the target architecture. +/// +/// Target features are commonly enabled by the [`#[target_feature]` attribute][1] to influence code +/// generation for a particular function, and less commonly enabled by compiler options like +/// `-Ctarget-feature` or `-Ctarget-cpu`. Targets themselves automatically enable certain target +/// features by default, for example because the target's ABI specification requires saving specific +/// registers which only exist in an architectural extension. +/// +/// Target features can imply other target features: for example, x86-64 `avx2` implies `avx`, and +/// aarch64 `sve2` implies `sve`, since both of these architectural extensions depend on their +/// predecessors. +/// +/// Target features can be probed at compile time by [`#[cfg(target_feature)]`][2] or `cfg!(…)` +/// conditional compilation to determine whether a target feature is enabled in a particular +/// context. +/// +/// [1]: https://doc.rust-lang.org/stable/reference/attributes/codegen.html#the-target_feature-attribute +/// [2]: https://doc.rust-lang.org/reference/conditional-compilation.html#target_feature +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct TargetFeature { + /// The name of this target feature. + pub name: String, + /// Other target features which are implied by this target feature, if any. + pub implies_features: Vec, + /// If this target feature is unstable, the name of the associated language feature gate. + pub unstable_feature_gate: Option, + /// Whether this feature is globally enabled for this compilation session. + /// + /// Target features can be globally enabled implicitly as a result of the target's definition. + /// For example, x86-64 hardware floating point ABIs require saving x87 and SSE2 registers, + /// which in turn requires globally enabling the `x87` and `sse2` target features so that the + /// generated machine code conforms to the target's ABI. + /// + /// Target features can also be globally enabled explicitly as a result of compiler flags like + /// [`-Ctarget-feature`][1] or [`-Ctarget-cpu`][2]. + /// + /// [1]: https://doc.rust-lang.org/beta/rustc/codegen-options/index.html#target-feature + /// [2]: https://doc.rust-lang.org/beta/rustc/codegen-options/index.html#target-cpu + pub globally_enabled: bool, +} + /// Metadata of a crate, either the same crate on which `rustdoc` was invoked, or its dependency. #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] pub struct ExternalCrate { diff --git a/src/tools/compiletest/src/directive-list.rs b/src/tools/compiletest/src/directive-list.rs index 44d9c0330f7..f8b92f11bde 100644 --- a/src/tools/compiletest/src/directive-list.rs +++ b/src/tools/compiletest/src/directive-list.rs @@ -176,6 +176,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[ "only-32bit", "only-64bit", "only-aarch64", + "only-aarch64-apple-darwin", "only-aarch64-unknown-linux-gnu", "only-apple", "only-arm", @@ -189,6 +190,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[ "only-gnu", "only-i686-pc-windows-gnu", "only-i686-pc-windows-msvc", + "only-i686-unknown-linux-gnu", "only-ios", "only-linux", "only-loongarch64", @@ -220,6 +222,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[ "only-windows-msvc", "only-x86", "only-x86_64", + "only-x86_64-apple-darwin", "only-x86_64-fortanix-unknown-sgx", "only-x86_64-pc-windows-gnu", "only-x86_64-pc-windows-msvc", diff --git a/src/tools/jsondocck/src/main.rs b/src/tools/jsondocck/src/main.rs index 54249fbd9ae..79e419884c6 100644 --- a/src/tools/jsondocck/src/main.rs +++ b/src/tools/jsondocck/src/main.rs @@ -156,7 +156,7 @@ static LINE_PATTERN: LazyLock = LazyLock::new(|| { r#" //@\s+ (?P!?) - (?P[A-Za-z]+(?:-[A-Za-z]+)*) + (?P[A-Za-z0-9]+(?:-[A-Za-z0-9]+)*) (?P.*)$ "#, ) diff --git a/src/tools/jsondoclint/src/validator/tests.rs b/src/tools/jsondoclint/src/validator/tests.rs index 28deb7e7cee..dd0b4ac5601 100644 --- a/src/tools/jsondoclint/src/validator/tests.rs +++ b/src/tools/jsondoclint/src/validator/tests.rs @@ -42,6 +42,7 @@ fn errors_on_missing_links() { )]), paths: FxHashMap::default(), external_crates: FxHashMap::default(), + target: rustdoc_json_types::Target { triple: "".to_string(), target_features: vec![] }, format_version: rustdoc_json_types::FORMAT_VERSION, }; @@ -112,6 +113,7 @@ fn errors_on_local_in_paths_and_not_index() { }, )]), external_crates: FxHashMap::default(), + target: rustdoc_json_types::Target { triple: "".to_string(), target_features: vec![] }, format_version: rustdoc_json_types::FORMAT_VERSION, }; @@ -216,6 +218,7 @@ fn errors_on_missing_path() { ItemSummary { crate_id: 0, path: vec!["foo".to_owned()], kind: ItemKind::Module }, )]), external_crates: FxHashMap::default(), + target: rustdoc_json_types::Target { triple: "".to_string(), target_features: vec![] }, format_version: rustdoc_json_types::FORMAT_VERSION, }; @@ -259,6 +262,7 @@ fn checks_local_crate_id_is_correct() { )]), paths: FxHashMap::default(), external_crates: FxHashMap::default(), + target: rustdoc_json_types::Target { triple: "".to_string(), target_features: vec![] }, format_version: FORMAT_VERSION, }; check(&krate, &[]); diff --git a/tests/rustdoc-json/targets/aarch64_apple_darwin.rs b/tests/rustdoc-json/targets/aarch64_apple_darwin.rs new file mode 100644 index 00000000000..c6ae5517d47 --- /dev/null +++ b/tests/rustdoc-json/targets/aarch64_apple_darwin.rs @@ -0,0 +1,14 @@ +//@ only-aarch64-apple-darwin + +//@ is "$.target.triple" \"aarch64-apple-darwin\" +//@ is "$.target.target_features[?(@.name=='vh')].globally_enabled" true +//@ is "$.target.target_features[?(@.name=='sve')].globally_enabled" false +//@ has "$.target.target_features[?(@.name=='sve2')].implies_features" '["sve"]' +//@ is "$.target.target_features[?(@.name=='sve2')].unstable_feature_gate" null + +// If this breaks due to stabilization, check rustc_target::target_features for a replacement +//@ is "$.target.target_features[?(@.name=='cssc')].unstable_feature_gate" '"aarch64_unstable_target_feature"' +//@ is "$.target.target_features[?(@.name=='v9a')].unstable_feature_gate" '"aarch64_ver_target_feature"' + +// Ensure we don't look like x86-64 +//@ !has "$.target.target_features[?(@.name=='avx2')]" diff --git a/tests/rustdoc-json/targets/aarch64_reflects_compiler_options.rs b/tests/rustdoc-json/targets/aarch64_reflects_compiler_options.rs new file mode 100644 index 00000000000..f91221eb23c --- /dev/null +++ b/tests/rustdoc-json/targets/aarch64_reflects_compiler_options.rs @@ -0,0 +1,10 @@ +//@ only-aarch64 + +// If we enable SVE Bit Permute, we should see that it is enabled +//@ compile-flags: -Ctarget-feature=+sve2-bitperm +//@ is "$.target.target_features[?(@.name=='sve2-bitperm')].globally_enabled" true + +// As well as its dependency chain +//@ is "$.target.target_features[?(@.name=='sve2')].globally_enabled" true +//@ is "$.target.target_features[?(@.name=='sve')].globally_enabled" true +//@ is "$.target.target_features[?(@.name=='neon')].globally_enabled" true diff --git a/tests/rustdoc-json/targets/aarch64_unknown_linux_gnu.rs b/tests/rustdoc-json/targets/aarch64_unknown_linux_gnu.rs new file mode 100644 index 00000000000..9139b00a128 --- /dev/null +++ b/tests/rustdoc-json/targets/aarch64_unknown_linux_gnu.rs @@ -0,0 +1,14 @@ +//@ only-aarch64-unknown-linux-gnu + +//@ is "$.target.triple" \"aarch64-unknown-linux-gnu\" +//@ is "$.target.target_features[?(@.name=='neon')].globally_enabled" true +//@ is "$.target.target_features[?(@.name=='sve')].globally_enabled" false +//@ has "$.target.target_features[?(@.name=='sve2')].implies_features" '["sve"]' +//@ is "$.target.target_features[?(@.name=='sve2')].unstable_feature_gate" null + +// If this breaks due to stabilization, check rustc_target::target_features for a replacement +//@ is "$.target.target_features[?(@.name=='cssc')].unstable_feature_gate" '"aarch64_unstable_target_feature"' +//@ is "$.target.target_features[?(@.name=='v9a')].unstable_feature_gate" '"aarch64_ver_target_feature"' + +// Ensure we don't look like x86-64 +//@ !has "$.target.target_features[?(@.name=='avx2')]" diff --git a/tests/rustdoc-json/targets/i686_pc_windows_msvc.rs b/tests/rustdoc-json/targets/i686_pc_windows_msvc.rs new file mode 100644 index 00000000000..088c741d113 --- /dev/null +++ b/tests/rustdoc-json/targets/i686_pc_windows_msvc.rs @@ -0,0 +1,14 @@ +//@ only-i686-pc-windows-msvc + +//@ is "$.target.triple" \"i686-pc-windows-msvc\" +//@ is "$.target.target_features[?(@.name=='sse2')].globally_enabled" true +//@ is "$.target.target_features[?(@.name=='avx2')].globally_enabled" false +//@ has "$.target.target_features[?(@.name=='avx2')].implies_features" '["avx"]' +//@ is "$.target.target_features[?(@.name=='avx2')].unstable_feature_gate" null + +// If this breaks due to stabilization, check rustc_target::target_features for a replacement +//@ is "$.target.target_features[?(@.name=='amx-tile')].unstable_feature_gate" '"x86_amx_intrinsics"' +//@ is "$.target.target_features[?(@.name=='x87')].unstable_feature_gate" '"x87_target_feature"' + +// Ensure we don't look like aarch64 +//@ !has "$.target.target_features[?(@.name=='sve2')]" diff --git a/tests/rustdoc-json/targets/i686_unknown_linux_gnu.rs b/tests/rustdoc-json/targets/i686_unknown_linux_gnu.rs new file mode 100644 index 00000000000..03788b000f1 --- /dev/null +++ b/tests/rustdoc-json/targets/i686_unknown_linux_gnu.rs @@ -0,0 +1,14 @@ +//@ only-i686-unknown-linux-gnu + +//@ is "$.target.triple" \"i686-unknown-linux-gnu\" +//@ is "$.target.target_features[?(@.name=='sse2')].globally_enabled" true +//@ is "$.target.target_features[?(@.name=='avx2')].globally_enabled" false +//@ has "$.target.target_features[?(@.name=='avx2')].implies_features" '["avx"]' +//@ is "$.target.target_features[?(@.name=='avx2')].unstable_feature_gate" null + +// If this breaks due to stabilization, check rustc_target::target_features for a replacement +//@ is "$.target.target_features[?(@.name=='amx-tile')].unstable_feature_gate" '"x86_amx_intrinsics"' +//@ is "$.target.target_features[?(@.name=='x87')].unstable_feature_gate" '"x87_target_feature"' + +// Ensure we don't look like aarch64 +//@ !has "$.target.target_features[?(@.name=='sve2')]" diff --git a/tests/rustdoc-json/targets/x86_64_apple_darwin.rs b/tests/rustdoc-json/targets/x86_64_apple_darwin.rs new file mode 100644 index 00000000000..a46f9138e86 --- /dev/null +++ b/tests/rustdoc-json/targets/x86_64_apple_darwin.rs @@ -0,0 +1,14 @@ +//@ only-x86_64-apple-darwin + +//@ is "$.target.triple" \"x86_64-apple-darwin\" +//@ is "$.target.target_features[?(@.name=='sse2')].globally_enabled" true +//@ is "$.target.target_features[?(@.name=='avx2')].globally_enabled" false +//@ has "$.target.target_features[?(@.name=='avx2')].implies_features" '["avx"]' +//@ is "$.target.target_features[?(@.name=='avx2')].unstable_feature_gate" null + +// If this breaks due to stabilization, check rustc_target::target_features for a replacement +//@ is "$.target.target_features[?(@.name=='amx-tile')].unstable_feature_gate" '"x86_amx_intrinsics"' +//@ is "$.target.target_features[?(@.name=='x87')].unstable_feature_gate" '"x87_target_feature"' + +// Ensure we don't look like aarch64 +//@ !has "$.target.target_features[?(@.name=='sve2')]" diff --git a/tests/rustdoc-json/targets/x86_64_pc_windows_gnu.rs b/tests/rustdoc-json/targets/x86_64_pc_windows_gnu.rs new file mode 100644 index 00000000000..7da12eb4d58 --- /dev/null +++ b/tests/rustdoc-json/targets/x86_64_pc_windows_gnu.rs @@ -0,0 +1,14 @@ +//@ only-x86_64-pc-windows-gnu + +//@ is "$.target.triple" \"x86_64-pc-windows-gnu\" +//@ is "$.target.target_features[?(@.name=='sse2')].globally_enabled" true +//@ is "$.target.target_features[?(@.name=='avx2')].globally_enabled" false +//@ has "$.target.target_features[?(@.name=='avx2')].implies_features" '["avx"]' +//@ is "$.target.target_features[?(@.name=='avx2')].unstable_feature_gate" null + +// If this breaks due to stabilization, check rustc_target::target_features for a replacement +//@ is "$.target.target_features[?(@.name=='amx-tile')].unstable_feature_gate" '"x86_amx_intrinsics"' +//@ is "$.target.target_features[?(@.name=='x87')].unstable_feature_gate" '"x87_target_feature"' + +// Ensure we don't look like aarch64 +//@ !has "$.target.target_features[?(@.name=='sve2')]" diff --git a/tests/rustdoc-json/targets/x86_64_pc_windows_msvc.rs b/tests/rustdoc-json/targets/x86_64_pc_windows_msvc.rs new file mode 100644 index 00000000000..d55f5776e85 --- /dev/null +++ b/tests/rustdoc-json/targets/x86_64_pc_windows_msvc.rs @@ -0,0 +1,14 @@ +//@ only-x86_64-pc-windows-msvc + +//@ is "$.target.triple" \"x86_64-pc-windows-msvc\" +//@ is "$.target.target_features[?(@.name=='sse2')].globally_enabled" true +//@ is "$.target.target_features[?(@.name=='avx2')].globally_enabled" false +//@ has "$.target.target_features[?(@.name=='avx2')].implies_features" '["avx"]' +//@ is "$.target.target_features[?(@.name=='avx2')].unstable_feature_gate" null + +// If this breaks due to stabilization, check rustc_target::target_features for a replacement +//@ is "$.target.target_features[?(@.name=='amx-tile')].unstable_feature_gate" '"x86_amx_intrinsics"' +//@ is "$.target.target_features[?(@.name=='x87')].unstable_feature_gate" '"x87_target_feature"' + +// Ensure we don't look like aarch64 +//@ !has "$.target.target_features[?(@.name=='sve2')]" diff --git a/tests/rustdoc-json/targets/x86_64_reflects_compiler_options.rs b/tests/rustdoc-json/targets/x86_64_reflects_compiler_options.rs new file mode 100644 index 00000000000..ba029b09996 --- /dev/null +++ b/tests/rustdoc-json/targets/x86_64_reflects_compiler_options.rs @@ -0,0 +1,10 @@ +//@ only-x86_64 + +// If we enable AVX2, we should see that it is enabled +//@ compile-flags: -Ctarget-feature=+avx2 +//@ is "$.target.target_features[?(@.name=='avx2')].globally_enabled" true + +// As well as its dependency chain +//@ is "$.target.target_features[?(@.name=='avx')].globally_enabled" true +//@ is "$.target.target_features[?(@.name=='sse4.2')].globally_enabled" true +//@ is "$.target.target_features[?(@.name=='sse4.1')].globally_enabled" true diff --git a/tests/rustdoc-json/targets/x86_64_unknown_linux_gnu.rs b/tests/rustdoc-json/targets/x86_64_unknown_linux_gnu.rs new file mode 100644 index 00000000000..3372fe7eb9d --- /dev/null +++ b/tests/rustdoc-json/targets/x86_64_unknown_linux_gnu.rs @@ -0,0 +1,14 @@ +//@ only-x86_64-unknown-linux-gnu + +//@ is "$.target.triple" \"x86_64-unknown-linux-gnu\" +//@ is "$.target.target_features[?(@.name=='sse2')].globally_enabled" true +//@ is "$.target.target_features[?(@.name=='avx2')].globally_enabled" false +//@ has "$.target.target_features[?(@.name=='avx2')].implies_features" '["avx"]' +//@ is "$.target.target_features[?(@.name=='avx2')].unstable_feature_gate" null + +// If this breaks due to stabilization, check rustc_target::target_features for a replacement +//@ is "$.target.target_features[?(@.name=='amx-tile')].unstable_feature_gate" '"x86_amx_intrinsics"' +//@ is "$.target.target_features[?(@.name=='x87')].unstable_feature_gate" '"x87_target_feature"' + +// Ensure we don't look like aarch64 +//@ !has "$.target.target_features[?(@.name=='sve2')]" From a870bba0d6b7b304e819ab820900496425ace96b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Wed, 16 Apr 2025 17:38:37 +0200 Subject: [PATCH 07/27] Add a warning when combining LLD with external LLVM config --- src/bootstrap/src/core/build_steps/compile.rs | 2 +- src/bootstrap/src/core/config/config.rs | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index f78539b4781..6a5b38dd504 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -155,7 +155,7 @@ impl Step for Std { // When using `download-rustc`, we already have artifacts for the host available. Don't // recompile them. - if builder.download_rustc() && builder.config.is_builder_target(target) + if builder.download_rustc() && builder.config.is_host_target(target) // NOTE: the beta compiler may generate different artifacts than the downloaded compiler, so // its artifacts can't be reused. && compiler.stage != 0 diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 12e157a34ae..1668443b04d 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -2397,6 +2397,12 @@ impl Config { ); } + if config.lld_enabled && config.is_system_llvm(config.build) { + eprintln!( + "Warning: LLD is enabled when using external llvm-config. LLD will not be built and copied to the sysroot." + ); + } + let default_std_features = BTreeSet::from([String::from("panic-unwind")]); config.rust_std_features = std_features.unwrap_or(default_std_features); From cb6c499bc98a8b6db85856f200ffba2f47b8b73a Mon Sep 17 00:00:00 2001 From: dianne Date: Sun, 23 Feb 2025 19:39:19 -0800 Subject: [PATCH 08/27] lower implicit deref patterns to THIR Since this uses `pat_adjustments`, I've also tweaked the documentation to mention implicit deref patterns and made sure the pattern migration diagnostic logic accounts for it. I'll adjust `ExprUseVisitor` in a later commit and add some tests there for closure capture inference. --- .../rustc_middle/src/ty/typeck_results.rs | 15 ++++++++--- .../src/thir/pattern/migration.rs | 16 +++++------- .../rustc_mir_build/src/thir/pattern/mod.rs | 26 ++++++++++++------- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_middle/src/ty/typeck_results.rs b/compiler/rustc_middle/src/ty/typeck_results.rs index ee8113b0f76..4c5c669771f 100644 --- a/compiler/rustc_middle/src/ty/typeck_results.rs +++ b/compiler/rustc_middle/src/ty/typeck_results.rs @@ -77,8 +77,8 @@ pub struct TypeckResults<'tcx> { /// to a form valid in all Editions, either as a lint diagnostic or hard error. rust_2024_migration_desugared_pats: ItemLocalMap, - /// Stores the types which were implicitly dereferenced in pattern binding modes - /// for later usage in THIR lowering. For example, + /// Stores the types which were implicitly dereferenced in pattern binding modes or deref + /// patterns for later usage in THIR lowering. For example, /// /// ``` /// match &&Some(5i32) { @@ -86,7 +86,16 @@ pub struct TypeckResults<'tcx> { /// _ => {}, /// } /// ``` - /// leads to a `vec![&&Option, &Option]`. Empty vectors are not stored. + /// leads to a `vec![&&Option, &Option]` and + /// + /// ``` + /// #![feature(deref_patterns)] + /// match &Box::new(Some(5i32)) { + /// Some(n) => {}, + /// _ => {}, + /// } + /// ``` + /// leads to a `vec![&Box>, Box>]`. Empty vectors are not stored. /// /// See: /// diff --git a/compiler/rustc_mir_build/src/thir/pattern/migration.rs b/compiler/rustc_mir_build/src/thir/pattern/migration.rs index c688b6f2848..12c457f13fc 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/migration.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/migration.rs @@ -4,7 +4,6 @@ use rustc_data_structures::fx::FxIndexMap; use rustc_errors::MultiSpan; use rustc_hir::{BindingMode, ByRef, HirId, Mutability}; use rustc_lint as lint; -use rustc_middle::span_bug; use rustc_middle::ty::{self, Rust2024IncompatiblePatInfo, TyCtxt}; use rustc_span::{Ident, Span}; @@ -87,19 +86,18 @@ impl<'a> PatMigration<'a> { } /// Tracks when we're lowering a pattern that implicitly dereferences the scrutinee. - /// This should only be called when the pattern type adjustments list `adjustments` is - /// non-empty. Returns the prior default binding mode; this should be followed by a call to - /// [`PatMigration::leave_ref`] to restore it when we leave the pattern. + /// This should only be called when the pattern type adjustments list `adjustments` contains an + /// implicit deref of a reference type. Returns the prior default binding mode; this should be + /// followed by a call to [`PatMigration::leave_ref`] to restore it when we leave the pattern. pub(super) fn visit_implicit_derefs<'tcx>( &mut self, pat_span: Span, adjustments: &[ty::adjustment::PatAdjustment<'tcx>], ) -> Option<(Span, Mutability)> { - let implicit_deref_mutbls = adjustments.iter().map(|adjust| { - let &ty::Ref(_, _, mutbl) = adjust.source.kind() else { - span_bug!(pat_span, "pattern implicitly dereferences a non-ref type"); - }; - mutbl + // Implicitly dereferencing references changes the default binding mode, but implicit derefs + // of smart pointers do not. Thus, we only consider implicit derefs of reference types. + let implicit_deref_mutbls = adjustments.iter().filter_map(|adjust| { + if let &ty::Ref(_, _, mutbl) = adjust.source.kind() { Some(mutbl) } else { None } }); if !self.info.suggest_eliding_modes { diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs index f682581d763..8f058efdfac 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -18,7 +18,7 @@ use rustc_middle::mir::interpret::LitToConstInput; use rustc_middle::thir::{ Ascription, FieldPat, LocalVarId, Pat, PatKind, PatRange, PatRangeBoundary, }; -use rustc_middle::ty::adjustment::PatAdjustment; +use rustc_middle::ty::adjustment::{PatAdjust, PatAdjustment}; use rustc_middle::ty::layout::IntegerExt; use rustc_middle::ty::{self, CanonicalUserTypeAnnotation, Ty, TyCtxt, TypingMode}; use rustc_middle::{bug, span_bug}; @@ -68,9 +68,11 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { self.typeck_results.pat_adjustments().get(pat.hir_id).map_or(&[], |v| &**v); // Track the default binding mode for the Rust 2024 migration suggestion. + // Implicitly dereferencing references changes the default binding mode, but implicit deref + // patterns do not. Only track binding mode changes if a ref type is in the adjustments. let mut opt_old_mode_span = None; if let Some(s) = &mut self.rust_2024_migration - && !adjustments.is_empty() + && adjustments.iter().any(|adjust| adjust.kind == PatAdjust::BuiltinDeref) { opt_old_mode_span = s.visit_implicit_derefs(pat.span, adjustments); } @@ -104,16 +106,22 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { }; let adjusted_pat = adjustments.iter().rev().fold(unadjusted_pat, |thir_pat, adjust| { - debug!("{:?}: wrapping pattern with type {:?}", thir_pat, adjust); - Box::new(Pat { - span: thir_pat.span, - ty: adjust.source, - kind: PatKind::Deref { subpattern: thir_pat }, - }) + debug!("{:?}: wrapping pattern with adjustment {:?}", thir_pat, adjust); + let span = thir_pat.span; + let kind = match adjust.kind { + PatAdjust::BuiltinDeref => PatKind::Deref { subpattern: thir_pat }, + PatAdjust::OverloadedDeref => { + let mutable = self.typeck_results.pat_has_ref_mut_binding(pat); + let mutability = + if mutable { hir::Mutability::Mut } else { hir::Mutability::Not }; + PatKind::DerefPattern { subpattern: thir_pat, mutability } + } + }; + Box::new(Pat { span, ty: adjust.source, kind }) }); if let Some(s) = &mut self.rust_2024_migration - && !adjustments.is_empty() + && adjustments.iter().any(|adjust| adjust.kind == PatAdjust::BuiltinDeref) { s.leave_ref(opt_old_mode_span); } From e4b7b3d820b6db4f4a37a1a09b700bc9a39d80ba Mon Sep 17 00:00:00 2001 From: dianne Date: Sat, 8 Mar 2025 21:07:11 -0800 Subject: [PATCH 09/27] pattern typing for immutable implicit deref patterns --- compiler/rustc_hir_typeck/src/pat.rs | 116 ++++++++++++++---- tests/ui/pattern/deref-patterns/bindings.rs | 27 ++++ tests/ui/pattern/deref-patterns/branch.rs | 22 ++++ .../cant_move_out_of_pattern.rs | 18 +++ .../cant_move_out_of_pattern.stderr | 36 +++++- tests/ui/pattern/deref-patterns/typeck.rs | 6 + .../ui/pattern/deref-patterns/typeck_fail.rs | 11 ++ .../pattern/deref-patterns/typeck_fail.stderr | 36 +++++- .../deref-patterns/unsatisfied-bounds.rs | 21 ++++ .../deref-patterns/unsatisfied-bounds.stderr | 9 ++ 10 files changed, 272 insertions(+), 30 deletions(-) create mode 100644 tests/ui/pattern/deref-patterns/unsatisfied-bounds.rs create mode 100644 tests/ui/pattern/deref-patterns/unsatisfied-bounds.stderr diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index c19d8cf6dc5..11ed7db9eb7 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/compiler/rustc_hir_typeck/src/pat.rs @@ -162,12 +162,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// Mode for adjusting the expected type and binding mode. #[derive(Clone, Copy, Debug, PartialEq, Eq)] enum AdjustMode { - /// Peel off all immediate reference types. - Peel, + /// Peel off all immediate reference types. If the `deref_patterns` feature is enabled, this + /// also peels smart pointer ADTs. + Peel { kind: PeelKind }, /// Pass on the input binding mode and expected type. Pass, } +/// Restrictions on what types to peel when adjusting the expected type and binding mode. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum PeelKind { + /// Only peel reference types. This is used for explicit `deref!(_)` patterns, which dereference + /// any number of `&`/`&mut` references, plus a single smart pointer. + ExplicitDerefPat, + /// Implicitly peel any number of references, and if `deref_patterns` is enabled, smart pointer + /// ADTs. In order to peel only as much as necessary for the pattern to match, the `until_adt` + /// field contains the ADT def that the pattern is a constructor for, if applicable, so that we + /// don't peel it. See [`ResolvedPat`] for more information. + // TODO: add `ResolvedPat` and `until_adt`. + Implicit, +} + /// `ref mut` bindings (explicit or match-ergonomics) are not allowed behind an `&` reference. /// Normally, the borrow checker enforces this, but for (currently experimental) match ergonomics, /// we track this when typing patterns for two purposes: @@ -390,7 +405,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } // Resolve type if needed. - let expected = if let AdjustMode::Peel = adjust_mode + let expected = if let AdjustMode::Peel { .. } = adjust_mode && pat.default_binding_modes { self.try_structurally_resolve_type(pat.span, expected) @@ -403,7 +418,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { match pat.kind { // Peel off a `&` or `&mut` from the scrutinee type. See the examples in // `tests/ui/rfcs/rfc-2005-default-binding-mode`. - _ if let AdjustMode::Peel = adjust_mode + _ if let AdjustMode::Peel { .. } = adjust_mode && pat.default_binding_modes && let ty::Ref(_, inner_ty, inner_mutability) = *expected.kind() => { @@ -443,6 +458,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Recurse with the new expected type. self.check_pat_inner(pat, opt_path_res, adjust_mode, inner_ty, new_pat_info) } + // If `deref_patterns` is enabled, peel a smart pointer from the scrutinee type. See the + // examples in `tests/ui/pattern/deref_patterns/`. + _ if self.tcx.features().deref_patterns() + && let AdjustMode::Peel { kind: PeelKind::Implicit } = adjust_mode + && pat.default_binding_modes + // For simplicity, only apply overloaded derefs if `expected` is a known ADT. + // FIXME(deref_patterns): we'll get better diagnostics for users trying to + // implicitly deref generics if we allow them here, but primitives, tuples, and + // inference vars definitely should be stopped. Figure out what makes most sense. + // TODO: stop peeling if the pattern is a constructor for the scrutinee type + && expected.is_adt() + // At this point, the pattern isn't able to match `expected` without peeling. Check + // that it implements `Deref` before assuming it's a smart pointer, to get a normal + // type error instead of a missing impl error if not. This only checks for `Deref`, + // not `DerefPure`: we require that too, but we want a trait error if it's missing. + && let Some(deref_trait) = self.tcx.lang_items().deref_trait() + && self + .type_implements_trait(deref_trait, [expected], self.param_env) + .may_apply() => + { + debug!("scrutinee ty {expected:?} is a smart pointer, inserting overloaded deref"); + // The scrutinee is a smart pointer; implicitly dereference it. This adds a + // requirement that `expected: DerefPure`. + let inner_ty = self.deref_pat_target(pat.span, expected); + // Once we've checked `pat`, we'll add a `DerefMut` bound if it contains any + // `ref mut` bindings. TODO: implement that, then reference here. + + // Preserve the smart pointer type for THIR lowering and upvar analysis. + self.typeck_results + .borrow_mut() + .pat_adjustments_mut() + .entry(pat.hir_id) + .or_default() + .push(PatAdjustment { kind: PatAdjust::OverloadedDeref, source: expected }); + + // Recurse, using the old pat info to keep `current_depth` to its old value. + // Peeling smart pointers does not update the default binding mode. + self.check_pat_inner(pat, opt_path_res, adjust_mode, inner_ty, old_pat_info) + } PatKind::Missing | PatKind::Wild | PatKind::Err(_) => expected, // We allow any type here; we ensure that the type is uninhabited during match checking. PatKind::Never => expected, @@ -505,12 +559,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { PatKind::Struct(..) | PatKind::TupleStruct(..) | PatKind::Tuple(..) - | PatKind::Box(_) - | PatKind::Deref(_) | PatKind::Range(..) - | PatKind::Slice(..) => AdjustMode::Peel, + | PatKind::Slice(..) => AdjustMode::Peel { kind: PeelKind::Implicit }, + // When checking an explicit deref pattern, only peel reference types. + // FIXME(deref_patterns): If box patterns and deref patterns need to coexist, box + // patterns may want `PeelKind::Implicit`, stopping on encountering a box. + | PatKind::Box(_) + | PatKind::Deref(_) => AdjustMode::Peel { kind: PeelKind::ExplicitDerefPat }, // A never pattern behaves somewhat like a literal or unit variant. - PatKind::Never => AdjustMode::Peel, + PatKind::Never => AdjustMode::Peel { kind: PeelKind::Implicit }, PatKind::Expr(PatExpr { kind: PatExprKind::Path(_), .. }) => match opt_path_res.unwrap() { // These constants can be of a reference type, e.g. `const X: &u8 = &0;`. // Peeling the reference types too early will cause type checking failures. @@ -520,7 +577,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // could successfully compile. The former being `Self` requires a unit struct. // In either case, and unlike constants, the pattern itself cannot be // a reference type wherefore peeling doesn't give up any expressiveness. - _ => AdjustMode::Peel, + _ => AdjustMode::Peel { kind: PeelKind::Implicit }, }, // String and byte-string literals result in types `&str` and `&[u8]` respectively. @@ -530,7 +587,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Call `resolve_vars_if_possible` here for inline const blocks. PatKind::Expr(lt) => match self.resolve_vars_if_possible(self.check_pat_expr_unadjusted(lt)).kind() { ty::Ref(..) => AdjustMode::Pass, - _ => AdjustMode::Peel, + _ => AdjustMode::Peel { kind: PeelKind::Implicit }, }, // Ref patterns are complicated, we handle them in `check_pat_ref`. @@ -2256,31 +2313,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expected: Ty<'tcx>, pat_info: PatInfo<'tcx>, ) -> Ty<'tcx> { - let tcx = self.tcx; - // Register a `DerefPure` bound, which is required by all `deref!()` pats. - self.register_bound( - expected, - tcx.require_lang_item(hir::LangItem::DerefPure, Some(span)), - self.misc(span), - ); - // ::Target - let ty = Ty::new_projection( - tcx, - tcx.require_lang_item(hir::LangItem::DerefTarget, Some(span)), - [expected], - ); - let ty = self.normalize(span, ty); - let ty = self.try_structurally_resolve_type(span, ty); - self.check_pat(inner, ty, pat_info); + let target_ty = self.deref_pat_target(span, expected); + self.check_pat(inner, target_ty, pat_info); // Check if the pattern has any `ref mut` bindings, which would require // `DerefMut` to be emitted in MIR building instead of just `Deref`. // We do this *after* checking the inner pattern, since we want to make // sure to apply any match-ergonomics adjustments. + // TODO: move this to a separate definition to share it with implicit deref pats if self.typeck_results.borrow().pat_has_ref_mut_binding(inner) { self.register_bound( expected, - tcx.require_lang_item(hir::LangItem::DerefMut, Some(span)), + self.tcx.require_lang_item(hir::LangItem::DerefMut, Some(span)), self.misc(span), ); } @@ -2288,6 +2332,24 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expected } + fn deref_pat_target(&self, span: Span, source_ty: Ty<'tcx>) -> Ty<'tcx> { + // Register a `DerefPure` bound, which is required by all `deref!()` pats. + let tcx = self.tcx; + self.register_bound( + source_ty, + tcx.require_lang_item(hir::LangItem::DerefPure, Some(span)), + self.misc(span), + ); + // The expected type for the deref pat's inner pattern is `::Target`. + let target_ty = Ty::new_projection( + tcx, + tcx.require_lang_item(hir::LangItem::DerefTarget, Some(span)), + [source_ty], + ); + let target_ty = self.normalize(span, target_ty); + self.try_structurally_resolve_type(span, target_ty) + } + // Precondition: Pat is Ref(inner) fn check_pat_ref( &self, diff --git a/tests/ui/pattern/deref-patterns/bindings.rs b/tests/ui/pattern/deref-patterns/bindings.rs index 5881e4166a4..ce10cd5439e 100644 --- a/tests/ui/pattern/deref-patterns/bindings.rs +++ b/tests/ui/pattern/deref-patterns/bindings.rs @@ -1,7 +1,9 @@ +//@ revisions: explicit implicit //@ run-pass #![feature(deref_patterns)] #![allow(incomplete_features)] +#[cfg(explicit)] fn simple_vec(vec: Vec) -> u32 { match vec { deref!([]) => 100, @@ -13,6 +15,19 @@ fn simple_vec(vec: Vec) -> u32 { } } +#[cfg(implicit)] +fn simple_vec(vec: Vec) -> u32 { + match vec { + [] => 100, + [x] if x == 4 => x + 4, + [x] => x, + [1, x] => x + 200, + deref!(ref slice) => slice.iter().sum(), + _ => 2000, + } +} + +#[cfg(explicit)] fn nested_vec(vecvec: Vec>) -> u32 { match vecvec { deref!([]) => 0, @@ -24,6 +39,18 @@ fn nested_vec(vecvec: Vec>) -> u32 { } } +#[cfg(implicit)] +fn nested_vec(vecvec: Vec>) -> u32 { + match vecvec { + [] => 0, + [[x]] => x, + [[0, x] | [1, x]] => x, + [ref x] => x.iter().sum(), + [[], [1, x, y]] => y - x, + _ => 2000, + } +} + fn ref_mut(val: u32) -> u32 { let mut b = Box::new(0u32); match &mut b { diff --git a/tests/ui/pattern/deref-patterns/branch.rs b/tests/ui/pattern/deref-patterns/branch.rs index 1bac1006d9d..9d72b35fd2f 100644 --- a/tests/ui/pattern/deref-patterns/branch.rs +++ b/tests/ui/pattern/deref-patterns/branch.rs @@ -1,8 +1,10 @@ +//@ revisions: explicit implicit //@ run-pass // Test the execution of deref patterns. #![feature(deref_patterns)] #![allow(incomplete_features)] +#[cfg(explicit)] fn branch(vec: Vec) -> u32 { match vec { deref!([]) => 0, @@ -12,6 +14,17 @@ fn branch(vec: Vec) -> u32 { } } +#[cfg(implicit)] +fn branch(vec: Vec) -> u32 { + match vec { + [] => 0, + [1, _, 3] => 1, + [2, ..] => 2, + _ => 1000, + } +} + +#[cfg(explicit)] fn nested(vec: Vec>) -> u32 { match vec { deref!([deref!([]), ..]) => 1, @@ -20,6 +33,15 @@ fn nested(vec: Vec>) -> u32 { } } +#[cfg(implicit)] +fn nested(vec: Vec>) -> u32 { + match vec { + [[], ..] => 1, + [[0, ..], [1, ..]] => 2, + _ => 1000, + } +} + fn main() { assert!(matches!(Vec::::new(), deref!([]))); assert!(matches!(vec![1], deref!([1]))); diff --git a/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.rs b/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.rs index 84b5ec09dc7..791776be5ac 100644 --- a/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.rs +++ b/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.rs @@ -21,4 +21,22 @@ fn cant_move_out_rc(rc: Rc) -> Struct { } } +struct Container(Struct); + +fn cant_move_out_box_implicit(b: Box) -> Struct { + match b { + //~^ ERROR: cannot move out of a shared reference + Container(x) => x, + _ => unreachable!(), + } +} + +fn cant_move_out_rc_implicit(rc: Rc) -> Struct { + match rc { + //~^ ERROR: cannot move out of a shared reference + Container(x) => x, + _ => unreachable!(), + } +} + fn main() {} diff --git a/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr b/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr index 2cf435b1179..1887800fc38 100644 --- a/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr +++ b/tests/ui/pattern/deref-patterns/cant_move_out_of_pattern.stderr @@ -32,6 +32,40 @@ help: consider borrowing the pattern binding LL | deref!(ref x) => x, | +++ -error: aborting due to 2 previous errors +error[E0507]: cannot move out of a shared reference + --> $DIR/cant_move_out_of_pattern.rs:27:11 + | +LL | match b { + | ^ +LL | +LL | Container(x) => x, + | - + | | + | data moved here + | move occurs because `x` has type `Struct`, which does not implement the `Copy` trait + | +help: consider borrowing the pattern binding + | +LL | Container(ref x) => x, + | +++ + +error[E0507]: cannot move out of a shared reference + --> $DIR/cant_move_out_of_pattern.rs:35:11 + | +LL | match rc { + | ^^ +LL | +LL | Container(x) => x, + | - + | | + | data moved here + | move occurs because `x` has type `Struct`, which does not implement the `Copy` trait + | +help: consider borrowing the pattern binding + | +LL | Container(ref x) => x, + | +++ + +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0507`. diff --git a/tests/ui/pattern/deref-patterns/typeck.rs b/tests/ui/pattern/deref-patterns/typeck.rs index f23f7042cd8..3a7ce9d1deb 100644 --- a/tests/ui/pattern/deref-patterns/typeck.rs +++ b/tests/ui/pattern/deref-patterns/typeck.rs @@ -10,26 +10,32 @@ fn main() { let vec: Vec = Vec::new(); match vec { deref!([..]) => {} + [..] => {} _ => {} } match Box::new(true) { deref!(true) => {} + true => {} _ => {} } match &Box::new(true) { deref!(true) => {} + true => {} _ => {} } match &Rc::new(0) { deref!(1..) => {} + 1.. => {} _ => {} } let _: &Struct = match &Rc::new(Struct) { deref!(x) => x, + Struct => &Struct, _ => unreachable!(), }; let _: &[Struct] = match &Rc::new(vec![Struct]) { deref!(deref!(x)) => x, + [Struct] => &[Struct], _ => unreachable!(), }; } diff --git a/tests/ui/pattern/deref-patterns/typeck_fail.rs b/tests/ui/pattern/deref-patterns/typeck_fail.rs index 040118449ec..4b9ad7d25f0 100644 --- a/tests/ui/pattern/deref-patterns/typeck_fail.rs +++ b/tests/ui/pattern/deref-patterns/typeck_fail.rs @@ -7,11 +7,22 @@ fn main() { match "foo".to_string() { deref!("foo") => {} //~^ ERROR: mismatched types + "foo" => {} + //~^ ERROR: mismatched types _ => {} } match &"foo".to_string() { deref!("foo") => {} //~^ ERROR: mismatched types + "foo" => {} + //~^ ERROR: mismatched types + _ => {} + } + + // Make sure we don't try implicitly dereferncing any ADT. + match Some(0) { + Ok(0) => {} + //~^ ERROR: mismatched types _ => {} } } diff --git a/tests/ui/pattern/deref-patterns/typeck_fail.stderr b/tests/ui/pattern/deref-patterns/typeck_fail.stderr index 1c14802745a..3e2f3561882 100644 --- a/tests/ui/pattern/deref-patterns/typeck_fail.stderr +++ b/tests/ui/pattern/deref-patterns/typeck_fail.stderr @@ -7,13 +7,45 @@ LL | deref!("foo") => {} | ^^^^^ expected `str`, found `&str` error[E0308]: mismatched types - --> $DIR/typeck_fail.rs:13:16 + --> $DIR/typeck_fail.rs:10:9 + | +LL | match "foo".to_string() { + | ----------------- this expression has type `String` +... +LL | "foo" => {} + | ^^^^^ expected `String`, found `&str` + +error[E0308]: mismatched types + --> $DIR/typeck_fail.rs:15:16 | LL | match &"foo".to_string() { | ------------------ this expression has type `&String` LL | deref!("foo") => {} | ^^^^^ expected `str`, found `&str` -error: aborting due to 2 previous errors +error[E0308]: mismatched types + --> $DIR/typeck_fail.rs:17:9 + | +LL | match &"foo".to_string() { + | ------------------ this expression has type `&String` +... +LL | "foo" => {} + | ^^^^^ expected `&String`, found `&str` + | + = note: expected reference `&String` + found reference `&'static str` + +error[E0308]: mismatched types + --> $DIR/typeck_fail.rs:24:9 + | +LL | match Some(0) { + | ------- this expression has type `Option<{integer}>` +LL | Ok(0) => {} + | ^^^^^ expected `Option<{integer}>`, found `Result<_, _>` + | + = note: expected enum `Option<{integer}>` + found enum `Result<_, _>` + +error: aborting due to 5 previous errors For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/pattern/deref-patterns/unsatisfied-bounds.rs b/tests/ui/pattern/deref-patterns/unsatisfied-bounds.rs new file mode 100644 index 00000000000..00064b2320c --- /dev/null +++ b/tests/ui/pattern/deref-patterns/unsatisfied-bounds.rs @@ -0,0 +1,21 @@ +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +struct MyPointer; + +impl std::ops::Deref for MyPointer { + type Target = (); + fn deref(&self) -> &() { + &() + } +} + +fn main() { + // Test that we get a trait error if a user attempts implicit deref pats on their own impls. + // FIXME(deref_patterns): there should be a special diagnostic for missing `DerefPure`. + match MyPointer { + () => {} + //~^ the trait bound `MyPointer: DerefPure` is not satisfied + _ => {} + } +} diff --git a/tests/ui/pattern/deref-patterns/unsatisfied-bounds.stderr b/tests/ui/pattern/deref-patterns/unsatisfied-bounds.stderr new file mode 100644 index 00000000000..983ce27865c --- /dev/null +++ b/tests/ui/pattern/deref-patterns/unsatisfied-bounds.stderr @@ -0,0 +1,9 @@ +error[E0277]: the trait bound `MyPointer: DerefPure` is not satisfied + --> $DIR/unsatisfied-bounds.rs:17:9 + | +LL | () => {} + | ^^ the trait `DerefPure` is not implemented for `MyPointer` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0277`. From 91d0b579f020964bfe6bcaa86fbfc8fd493ae4db Mon Sep 17 00:00:00 2001 From: dianne Date: Sun, 9 Mar 2025 00:06:00 -0800 Subject: [PATCH 10/27] register `DerefMut` bounds for implicit mutable derefs --- compiler/rustc_hir_typeck/src/pat.rs | 53 +++++++++++++------ tests/ui/pattern/deref-patterns/bindings.rs | 28 ++++++++++ .../ui/pattern/deref-patterns/fake_borrows.rs | 7 +++ .../deref-patterns/fake_borrows.stderr | 11 +++- tests/ui/pattern/deref-patterns/ref-mut.rs | 9 ++++ .../ui/pattern/deref-patterns/ref-mut.stderr | 10 +++- 6 files changed, 100 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index 11ed7db9eb7..f33e270c357 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/compiler/rustc_hir_typeck/src/pat.rs @@ -344,6 +344,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let ty = self.check_pat_inner(pat, opt_path_res, adjust_mode, expected, pat_info); self.write_ty(pat.hir_id, ty); + // If we implicitly inserted overloaded dereferences before matching, check the pattern to + // see if the dereferenced types need `DerefMut` bounds. + if let Some(derefed_tys) = self.typeck_results.borrow().pat_adjustments().get(pat.hir_id) + && derefed_tys.iter().any(|adjust| adjust.kind == PatAdjust::OverloadedDeref) + { + self.register_deref_mut_bounds_if_needed( + pat.span, + pat, + derefed_tys.iter().filter_map(|adjust| match adjust.kind { + PatAdjust::OverloadedDeref => Some(adjust.source), + PatAdjust::BuiltinDeref => None, + }), + ); + } + // (note_1): In most of the cases where (note_1) is referenced // (literals and constants being the exception), we relate types // using strict equality, even though subtyping would be sufficient. @@ -483,7 +498,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // requirement that `expected: DerefPure`. let inner_ty = self.deref_pat_target(pat.span, expected); // Once we've checked `pat`, we'll add a `DerefMut` bound if it contains any - // `ref mut` bindings. TODO: implement that, then reference here. + // `ref mut` bindings. See `Self::register_deref_mut_bounds_if_needed`. // Preserve the smart pointer type for THIR lowering and upvar analysis. self.typeck_results @@ -2315,20 +2330,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) -> Ty<'tcx> { let target_ty = self.deref_pat_target(span, expected); self.check_pat(inner, target_ty, pat_info); - - // Check if the pattern has any `ref mut` bindings, which would require - // `DerefMut` to be emitted in MIR building instead of just `Deref`. - // We do this *after* checking the inner pattern, since we want to make - // sure to apply any match-ergonomics adjustments. - // TODO: move this to a separate definition to share it with implicit deref pats - if self.typeck_results.borrow().pat_has_ref_mut_binding(inner) { - self.register_bound( - expected, - self.tcx.require_lang_item(hir::LangItem::DerefMut, Some(span)), - self.misc(span), - ); - } - + self.register_deref_mut_bounds_if_needed(span, inner, [expected]); expected } @@ -2350,6 +2352,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.try_structurally_resolve_type(span, target_ty) } + /// Check if the interior of a deref pattern (either explicit or implicit) has any `ref mut` + /// bindings, which would require `DerefMut` to be emitted in MIR building instead of just + /// `Deref`. We do this *after* checking the inner pattern, since we want to make sure to + /// account for `ref mut` binding modes inherited from implicitly dereferencing `&mut` refs. + fn register_deref_mut_bounds_if_needed( + &self, + span: Span, + inner: &'tcx Pat<'tcx>, + derefed_tys: impl IntoIterator>, + ) { + if self.typeck_results.borrow().pat_has_ref_mut_binding(inner) { + for mutably_derefed_ty in derefed_tys { + self.register_bound( + mutably_derefed_ty, + self.tcx.require_lang_item(hir::LangItem::DerefMut, Some(span)), + self.misc(span), + ); + } + } + } + // Precondition: Pat is Ref(inner) fn check_pat_ref( &self, diff --git a/tests/ui/pattern/deref-patterns/bindings.rs b/tests/ui/pattern/deref-patterns/bindings.rs index ce10cd5439e..c14d57f3f24 100644 --- a/tests/ui/pattern/deref-patterns/bindings.rs +++ b/tests/ui/pattern/deref-patterns/bindings.rs @@ -51,6 +51,7 @@ fn nested_vec(vecvec: Vec>) -> u32 { } } +#[cfg(explicit)] fn ref_mut(val: u32) -> u32 { let mut b = Box::new(0u32); match &mut b { @@ -64,6 +65,21 @@ fn ref_mut(val: u32) -> u32 { *x } +#[cfg(implicit)] +fn ref_mut(val: u32) -> u32 { + let mut b = Box::new((0u32,)); + match &mut b { + (_x,) if false => unreachable!(), + (x,) => { + *x = val; + } + _ => unreachable!(), + } + let (x,) = &b else { unreachable!() }; + *x +} + +#[cfg(explicit)] #[rustfmt::skip] fn or_and_guard(tuple: (u32, u32)) -> u32 { let mut sum = 0; @@ -75,6 +91,18 @@ fn or_and_guard(tuple: (u32, u32)) -> u32 { sum } +#[cfg(implicit)] +#[rustfmt::skip] +fn or_and_guard(tuple: (u32, u32)) -> u32 { + let mut sum = 0; + let b = Box::new(tuple); + match b { + (x, _) | (_, x) if { sum += x; false } => {}, + _ => {}, + } + sum +} + fn main() { assert_eq!(simple_vec(vec![1]), 1); assert_eq!(simple_vec(vec![1, 2]), 202); diff --git a/tests/ui/pattern/deref-patterns/fake_borrows.rs b/tests/ui/pattern/deref-patterns/fake_borrows.rs index 35fa9cbf7d8..bf614d7d66f 100644 --- a/tests/ui/pattern/deref-patterns/fake_borrows.rs +++ b/tests/ui/pattern/deref-patterns/fake_borrows.rs @@ -11,4 +11,11 @@ fn main() { deref!(false) => {} _ => {}, } + match b { + true => {} + _ if { *b = true; false } => {} + //~^ ERROR cannot assign `*b` in match guard + false => {} + _ => {}, + } } diff --git a/tests/ui/pattern/deref-patterns/fake_borrows.stderr b/tests/ui/pattern/deref-patterns/fake_borrows.stderr index 6a591e6416c..8c060236d0d 100644 --- a/tests/ui/pattern/deref-patterns/fake_borrows.stderr +++ b/tests/ui/pattern/deref-patterns/fake_borrows.stderr @@ -7,6 +7,15 @@ LL | deref!(true) => {} LL | _ if { *b = true; false } => {} | ^^^^^^^^^ cannot assign -error: aborting due to 1 previous error +error[E0510]: cannot assign `*b` in match guard + --> $DIR/fake_borrows.rs:16:16 + | +LL | match b { + | - value is immutable in match guard +LL | true => {} +LL | _ if { *b = true; false } => {} + | ^^^^^^^^^ cannot assign + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0510`. diff --git a/tests/ui/pattern/deref-patterns/ref-mut.rs b/tests/ui/pattern/deref-patterns/ref-mut.rs index 1918008a761..43738671346 100644 --- a/tests/ui/pattern/deref-patterns/ref-mut.rs +++ b/tests/ui/pattern/deref-patterns/ref-mut.rs @@ -8,10 +8,19 @@ fn main() { deref!(x) => {} _ => {} } + match &mut vec![1] { + [x] => {} + _ => {} + } match &mut Rc::new(1) { deref!(x) => {} //~^ ERROR the trait bound `Rc<{integer}>: DerefMut` is not satisfied _ => {} } + match &mut Rc::new((1,)) { + (x,) => {} + //~^ ERROR the trait bound `Rc<({integer},)>: DerefMut` is not satisfied + _ => {} + } } diff --git a/tests/ui/pattern/deref-patterns/ref-mut.stderr b/tests/ui/pattern/deref-patterns/ref-mut.stderr index 41f1c3061ce..24a35b418e9 100644 --- a/tests/ui/pattern/deref-patterns/ref-mut.stderr +++ b/tests/ui/pattern/deref-patterns/ref-mut.stderr @@ -8,13 +8,19 @@ LL | #![feature(deref_patterns)] = note: `#[warn(incomplete_features)]` on by default error[E0277]: the trait bound `Rc<{integer}>: DerefMut` is not satisfied - --> $DIR/ref-mut.rs:13:9 + --> $DIR/ref-mut.rs:17:9 | LL | deref!(x) => {} | ^^^^^^^^^ the trait `DerefMut` is not implemented for `Rc<{integer}>` | = note: this error originates in the macro `deref` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 1 previous error; 1 warning emitted +error[E0277]: the trait bound `Rc<({integer},)>: DerefMut` is not satisfied + --> $DIR/ref-mut.rs:22:9 + | +LL | (x,) => {} + | ^^^^ the trait `DerefMut` is not implemented for `Rc<({integer},)>` + +error: aborting due to 2 previous errors; 1 warning emitted For more information about this error, try `rustc --explain E0277`. From d6df469c3d5d87bd5b853812bddae2ca2cbd1cb8 Mon Sep 17 00:00:00 2001 From: dianne Date: Mon, 7 Apr 2025 12:06:03 -0700 Subject: [PATCH 11/27] refactor path pattern checking to get info for peeling See the doc comment on `ResolvedPat` for more information. This and the next couple commits split resolution apart from checking for path, struct, and tuple struct patterns, in order to find the pattern's type before peeling the scrutinee. This helps us avoid peeling the scrutinee when the pattern could match it. The reason this handles errors from resolution after peeling is for struct and tuple struct patterns: we check their subpatterns even when they fail to resolve, to potentially catch more resolution errors. By doing this after peeling, we're able to use the updated `PatInfo`. I don't know if there's currently any observable difference from using the outdated `PatInfo`, but it could potentially be a source of subtle diagnostic bugs in the future, so I'm opting to peel first. --- compiler/rustc_hir_typeck/src/pat.rs | 137 +++++++++++++++++---------- 1 file changed, 89 insertions(+), 48 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index f33e270c357..327b2736aab 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/compiler/rustc_hir_typeck/src/pat.rs @@ -34,7 +34,7 @@ use ty::adjustment::{PatAdjust, PatAdjustment}; use super::report_unexpected_variant_res; use crate::expectation::Expectation; use crate::gather_locals::DeclOrigin; -use crate::{FnCtxt, LoweredTy, errors}; +use crate::{FnCtxt, errors}; const CANNOT_IMPLICITLY_DEREF_POINTER_TRAIT_OBJ: &str = "\ This error indicates that a pointer to a trait type cannot be implicitly dereferenced by a \ @@ -258,6 +258,44 @@ enum InheritedRefMatchRule { }, } +/// When checking patterns containing paths, we need to know the path's resolution to determine +/// whether to apply match ergonomics and implicitly dereference the scrutinee. For instance, when +/// the `deref_patterns` feature is enabled and we're matching against a scrutinee of type +/// `Cow<'a, Option>`, we insert an implicit dereference to allow the pattern `Some(_)` to type, +/// but we must not dereference it when checking the pattern `Cow::Borrowed(_)`. +/// +/// `ResolvedPat` contains the information from resolution needed to determine match ergonomics +/// adjustments, and to finish checking the pattern once we know its adjusted type. +#[derive(Clone, Copy, Debug)] +struct ResolvedPat<'tcx> { + /// The type of the pattern, to be checked against the type of the scrutinee after peeling. This + /// is also used to avoid peeling the scrutinee's constructors (see the `Cow` example above). + ty: Ty<'tcx>, + kind: ResolvedPatKind<'tcx>, +} + +#[derive(Clone, Copy, Debug)] +enum ResolvedPatKind<'tcx> { + Path { res: Res, pat_res: Res, segments: &'tcx [hir::PathSegment<'tcx>] }, +} + +impl<'tcx> ResolvedPat<'tcx> { + fn adjust_mode(&self) -> AdjustMode { + let ResolvedPatKind::Path { res, .. } = self.kind; + if matches!(res, Res::Def(DefKind::Const | DefKind::AssocConst, _)) { + // These constants can be of a reference type, e.g. `const X: &u8 = &0;`. + // Peeling the reference types too early will cause type checking failures. + // Although it would be possible to *also* peel the types of the constants too. + AdjustMode::Pass + } else { + // The remaining possible resolutions for path, struct, and tuple struct patterns are + // ADT constructors. As such, we may peel references freely, but we must not peel the + // ADT itself from the scrutinee if it's a smart pointer. + AdjustMode::Peel { kind: PeelKind::Implicit } + } + } +} + impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// Experimental pattern feature: after matching against a shared reference, do we limit the /// default binding mode in subpatterns to be `ref` when it would otherwise be `ref mut`? @@ -334,13 +372,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// Conversely, inside this module, `check_pat_top` should never be used. #[instrument(level = "debug", skip(self, pat_info))] fn check_pat(&self, pat: &'tcx Pat<'tcx>, expected: Ty<'tcx>, pat_info: PatInfo<'tcx>) { + // For patterns containing paths, we need the path's resolution to determine whether to + // implicitly dereference the scrutinee before matching. let opt_path_res = match pat.kind { PatKind::Expr(PatExpr { kind: PatExprKind::Path(qpath), hir_id, span }) => { - Some(self.resolve_ty_and_res_fully_qualified_call(qpath, *hir_id, *span)) + Some(self.resolve_pat_path(*hir_id, *span, qpath)) } _ => None, }; - let adjust_mode = self.calc_adjust_mode(pat, opt_path_res.map(|(res, ..)| res)); + let adjust_mode = self.calc_adjust_mode(pat, opt_path_res); let ty = self.check_pat_inner(pat, opt_path_res, adjust_mode, expected, pat_info); self.write_ty(pat.hir_id, ty); @@ -406,7 +446,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn check_pat_inner( &self, pat: &'tcx Pat<'tcx>, - opt_path_res: Option<(Res, Option>, &'tcx [hir::PathSegment<'tcx>])>, + opt_path_res: Option, ErrorGuaranteed>>, adjust_mode: AdjustMode, expected: Ty<'tcx>, pat_info: PatInfo<'tcx>, @@ -515,16 +555,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { PatKind::Missing | PatKind::Wild | PatKind::Err(_) => expected, // We allow any type here; we ensure that the type is uninhabited during match checking. PatKind::Never => expected, - PatKind::Expr(PatExpr { kind: PatExprKind::Path(qpath), hir_id, span }) => { - let ty = self.check_pat_path( - *hir_id, - pat.hir_id, - *span, - qpath, - opt_path_res.unwrap(), - expected, - &pat_info.top_info, - ); + PatKind::Expr(PatExpr { kind: PatExprKind::Path(_), hir_id, .. }) => { + let ty = match opt_path_res.unwrap() { + Ok(ref pr) => { + self.check_pat_path(pat.hir_id, pat.span, pr, expected, &pat_info.top_info) + } + Err(guar) => Ty::new_error(self.tcx, guar), + }; self.write_ty(*hir_id, ty); ty } @@ -566,8 +603,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// How should the binding mode and expected type be adjusted? /// - /// When the pattern is a path pattern, `opt_path_res` must be `Some(res)`. - fn calc_adjust_mode(&self, pat: &'tcx Pat<'tcx>, opt_path_res: Option) -> AdjustMode { + /// When the pattern contains a path, `opt_path_res` must be `Some(path_res)`. + fn calc_adjust_mode( + &self, + pat: &'tcx Pat<'tcx>, + opt_path_res: Option, ErrorGuaranteed>>, + ) -> AdjustMode { match &pat.kind { // Type checking these product-like types successfully always require // that the expected type be of those types and not reference types. @@ -583,17 +624,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | PatKind::Deref(_) => AdjustMode::Peel { kind: PeelKind::ExplicitDerefPat }, // A never pattern behaves somewhat like a literal or unit variant. PatKind::Never => AdjustMode::Peel { kind: PeelKind::Implicit }, - PatKind::Expr(PatExpr { kind: PatExprKind::Path(_), .. }) => match opt_path_res.unwrap() { - // These constants can be of a reference type, e.g. `const X: &u8 = &0;`. - // Peeling the reference types too early will cause type checking failures. - // Although it would be possible to *also* peel the types of the constants too. - Res::Def(DefKind::Const | DefKind::AssocConst, _) => AdjustMode::Pass, - // In the `ValueNS`, we have `SelfCtor(..) | Ctor(_, Const), _)` remaining which - // could successfully compile. The former being `Self` requires a unit struct. - // In either case, and unlike constants, the pattern itself cannot be - // a reference type wherefore peeling doesn't give up any expressiveness. - _ => AdjustMode::Peel { kind: PeelKind::Implicit }, - }, + // For patterns with paths, how we peel the scrutinee depends on the path's resolution. + PatKind::Expr(PatExpr { kind: PatExprKind::Path(_), .. }) => { + // If there was an error resolving the path, default to peeling everything. + opt_path_res.unwrap().map_or(AdjustMode::Peel { kind: PeelKind::Implicit }, |pr| pr.adjust_mode()) + } // String and byte-string literals result in types `&str` and `&[u8]` respectively. // All other literals result in non-reference types. @@ -1216,31 +1251,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - fn check_pat_path( + fn resolve_pat_path( &self, path_id: HirId, - pat_id_for_diag: HirId, span: Span, - qpath: &hir::QPath<'_>, - path_resolution: (Res, Option>, &'tcx [hir::PathSegment<'tcx>]), - expected: Ty<'tcx>, - ti: &TopInfo<'tcx>, - ) -> Ty<'tcx> { + qpath: &'tcx hir::QPath<'_>, + ) -> Result, ErrorGuaranteed> { let tcx = self.tcx; - // We have already resolved the path. - let (res, opt_ty, segments) = path_resolution; + let (res, opt_ty, segments) = + self.resolve_ty_and_res_fully_qualified_call(qpath, path_id, span); match res { Res::Err => { let e = self.dcx().span_delayed_bug(qpath.span(), "`Res::Err` but no error emitted"); self.set_tainted_by_errors(e); - return Ty::new_error(tcx, e); + return Err(e); } Res::Def(DefKind::AssocFn | DefKind::Ctor(_, CtorKind::Fn) | DefKind::Variant, _) => { let expected = "unit struct, unit variant or constant"; let e = report_unexpected_variant_res(tcx, res, None, qpath, span, E0533, expected); - return Ty::new_error(tcx, e); + return Err(e); } Res::SelfCtor(def_id) => { if let ty::Adt(adt_def, _) = *tcx.type_of(def_id).skip_binder().kind() @@ -1258,7 +1289,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { E0533, "unit struct", ); - return Ty::new_error(tcx, e); + return Err(e); } } Res::Def( @@ -1271,15 +1302,26 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { _ => bug!("unexpected pattern resolution: {:?}", res), } - // Type-check the path. + // Find the type of the path pattern, for later checking. let (pat_ty, pat_res) = self.instantiate_value_path(segments, opt_ty, res, span, span, path_id); + Ok(ResolvedPat { ty: pat_ty, kind: ResolvedPatKind::Path { res, pat_res, segments } }) + } + + fn check_pat_path( + &self, + pat_id_for_diag: HirId, + span: Span, + resolved: &ResolvedPat<'tcx>, + expected: Ty<'tcx>, + ti: &TopInfo<'tcx>, + ) -> Ty<'tcx> { if let Err(err) = - self.demand_suptype_with_origin(&self.pattern_cause(ti, span), expected, pat_ty) + self.demand_suptype_with_origin(&self.pattern_cause(ti, span), expected, resolved.ty) { - self.emit_bad_pat_path(err, pat_id_for_diag, span, res, pat_res, pat_ty, segments); + self.emit_bad_pat_path(err, pat_id_for_diag, span, resolved); } - pat_ty + resolved.ty } fn maybe_suggest_range_literal( @@ -1322,11 +1364,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { mut e: Diag<'_>, hir_id: HirId, pat_span: Span, - res: Res, - pat_res: Res, - pat_ty: Ty<'tcx>, - segments: &'tcx [hir::PathSegment<'tcx>], + resolved_pat: &ResolvedPat<'tcx>, ) { + let ResolvedPatKind::Path { res, pat_res, segments } = resolved_pat.kind; + if let Some(span) = self.tcx.hir_res_span(pat_res) { e.span_label(span, format!("{} defined here", res.descr())); if let [hir::PathSegment { ident, .. }] = &*segments { @@ -1349,7 +1390,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } _ => { - let (type_def_id, item_def_id) = match pat_ty.kind() { + let (type_def_id, item_def_id) = match resolved_pat.ty.kind() { ty::Adt(def, _) => match res { Res::Def(DefKind::Const, def_id) => (Some(def.did()), Some(def_id)), _ => (None, None), From 9196048edddd9eb18294f3eab34453c0f7eb25b3 Mon Sep 17 00:00:00 2001 From: dianne Date: Mon, 3 Mar 2025 16:26:20 -0800 Subject: [PATCH 12/27] refactor struct pattern checking to get info for peeling See the previous commit for details. This doesn't yet extract the struct pat's type's ADT def before peeling, but it should now be possible. --- compiler/rustc_hir_typeck/src/pat.rs | 51 +++++++++++++++++----------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index 327b2736aab..c81965967f0 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/compiler/rustc_hir_typeck/src/pat.rs @@ -277,12 +277,14 @@ struct ResolvedPat<'tcx> { #[derive(Clone, Copy, Debug)] enum ResolvedPatKind<'tcx> { Path { res: Res, pat_res: Res, segments: &'tcx [hir::PathSegment<'tcx>] }, + Struct { variant: &'tcx VariantDef }, } impl<'tcx> ResolvedPat<'tcx> { fn adjust_mode(&self) -> AdjustMode { - let ResolvedPatKind::Path { res, .. } = self.kind; - if matches!(res, Res::Def(DefKind::Const | DefKind::AssocConst, _)) { + if let ResolvedPatKind::Path { res, .. } = self.kind + && matches!(res, Res::Def(DefKind::Const | DefKind::AssocConst, _)) + { // These constants can be of a reference type, e.g. `const X: &u8 = &0;`. // Peeling the reference types too early will cause type checking failures. // Although it would be possible to *also* peel the types of the constants too. @@ -378,6 +380,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { PatKind::Expr(PatExpr { kind: PatExprKind::Path(qpath), hir_id, span }) => { Some(self.resolve_pat_path(*hir_id, *span, qpath)) } + PatKind::Struct(ref qpath, ..) => Some(self.resolve_pat_struct(pat, qpath)), _ => None, }; let adjust_mode = self.calc_adjust_mode(pat, opt_path_res); @@ -575,9 +578,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { PatKind::TupleStruct(ref qpath, subpats, ddpos) => { self.check_pat_tuple_struct(pat, qpath, subpats, ddpos, expected, pat_info) } - PatKind::Struct(ref qpath, fields, has_rest_pat) => { - self.check_pat_struct(pat, qpath, fields, has_rest_pat, expected, pat_info) - } + PatKind::Struct(_, fields, has_rest_pat) => match opt_path_res.unwrap() { + Ok(ResolvedPat { ty, kind: ResolvedPatKind::Struct { variant } }) => self + .check_pat_struct(pat, fields, has_rest_pat, ty, variant, expected, pat_info), + Err(guar) => { + let ty_err = Ty::new_error(self.tcx, guar); + for field in fields { + self.check_pat(field.pat, ty_err, pat_info); + } + ty_err + } + Ok(pr) => span_bug!(pat.span, "struct pattern resolved to {pr:?}"), + }, PatKind::Guard(pat, cond) => { self.check_pat(pat, expected, pat_info); self.check_expr_has_type_or_error(cond, self.tcx.types.bool, |_| {}); @@ -1220,27 +1232,26 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Ok(()) } - fn check_pat_struct( + fn resolve_pat_struct( &self, pat: &'tcx Pat<'tcx>, qpath: &hir::QPath<'tcx>, + ) -> Result, ErrorGuaranteed> { + // Resolve the path and check the definition for errors. + let (variant, pat_ty) = self.check_struct_path(qpath, pat.hir_id)?; + Ok(ResolvedPat { ty: pat_ty, kind: ResolvedPatKind::Struct { variant } }) + } + + fn check_pat_struct( + &self, + pat: &'tcx Pat<'tcx>, fields: &'tcx [hir::PatField<'tcx>], has_rest_pat: bool, + pat_ty: Ty<'tcx>, + variant: &'tcx VariantDef, expected: Ty<'tcx>, pat_info: PatInfo<'tcx>, ) -> Ty<'tcx> { - // Resolve the path and check the definition for errors. - let (variant, pat_ty) = match self.check_struct_path(qpath, pat.hir_id) { - Ok(data) => data, - Err(guar) => { - let err = Ty::new_error(self.tcx, guar); - for field in fields { - self.check_pat(field.pat, err, pat_info); - } - return err; - } - }; - // Type-check the path. let _ = self.demand_eqtype_pat(pat.span, expected, pat_ty, &pat_info.top_info); @@ -1366,7 +1377,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pat_span: Span, resolved_pat: &ResolvedPat<'tcx>, ) { - let ResolvedPatKind::Path { res, pat_res, segments } = resolved_pat.kind; + let ResolvedPatKind::Path { res, pat_res, segments } = resolved_pat.kind else { + span_bug!(pat_span, "unexpected resolution for path pattern: {resolved_pat:?}"); + }; if let Some(span) = self.tcx.hir_res_span(pat_res) { e.span_label(span, format!("{} defined here", res.descr())); From 1f40e9aa6a0f0a8e6e4188f15f25d920892bca6a Mon Sep 17 00:00:00 2001 From: dianne Date: Mon, 3 Mar 2025 16:50:53 -0800 Subject: [PATCH 13/27] refactor tuple struct pattern checking to get info for peeling See the previous two commits. --- compiler/rustc_hir_typeck/src/pat.rs | 108 ++++++++++++++++----------- 1 file changed, 65 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index c81965967f0..da5153bed12 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/compiler/rustc_hir_typeck/src/pat.rs @@ -278,6 +278,7 @@ struct ResolvedPat<'tcx> { enum ResolvedPatKind<'tcx> { Path { res: Res, pat_res: Res, segments: &'tcx [hir::PathSegment<'tcx>] }, Struct { variant: &'tcx VariantDef }, + TupleStruct { res: Res, variant: &'tcx VariantDef }, } impl<'tcx> ResolvedPat<'tcx> { @@ -381,6 +382,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Some(self.resolve_pat_path(*hir_id, *span, qpath)) } PatKind::Struct(ref qpath, ..) => Some(self.resolve_pat_struct(pat, qpath)), + PatKind::TupleStruct(ref qpath, ..) => Some(self.resolve_pat_tuple_struct(pat, qpath)), _ => None, }; let adjust_mode = self.calc_adjust_mode(pat, opt_path_res); @@ -575,9 +577,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { PatKind::Binding(ba, var_id, ident, sub) => { self.check_pat_ident(pat, ba, var_id, ident, sub, expected, pat_info) } - PatKind::TupleStruct(ref qpath, subpats, ddpos) => { - self.check_pat_tuple_struct(pat, qpath, subpats, ddpos, expected, pat_info) - } + PatKind::TupleStruct(ref qpath, subpats, ddpos) => match opt_path_res.unwrap() { + Ok(ResolvedPat { ty, kind: ResolvedPatKind::TupleStruct { res, variant } }) => self + .check_pat_tuple_struct( + pat, qpath, subpats, ddpos, res, ty, variant, expected, pat_info, + ), + Err(guar) => { + let ty_err = Ty::new_error(self.tcx, guar); + for subpat in subpats { + self.check_pat(subpat, ty_err, pat_info); + } + ty_err + } + Ok(pr) => span_bug!(pat.span, "tuple struct pattern resolved to {pr:?}"), + }, PatKind::Struct(_, fields, has_rest_pat) => match opt_path_res.unwrap() { Ok(ResolvedPat { ty, kind: ResolvedPatKind::Struct { variant } }) => self .check_pat_struct(pat, fields, has_rest_pat, ty, variant, expected, pat_info), @@ -1443,12 +1456,61 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { e.emit(); } + fn resolve_pat_tuple_struct( + &self, + pat: &'tcx Pat<'tcx>, + qpath: &'tcx hir::QPath<'tcx>, + ) -> Result, ErrorGuaranteed> { + let tcx = self.tcx; + let report_unexpected_res = |res: Res| { + let expected = "tuple struct or tuple variant"; + let e = report_unexpected_variant_res(tcx, res, None, qpath, pat.span, E0164, expected); + Err(e) + }; + + // Resolve the path and check the definition for errors. + let (res, opt_ty, segments) = + self.resolve_ty_and_res_fully_qualified_call(qpath, pat.hir_id, pat.span); + if res == Res::Err { + let e = self.dcx().span_delayed_bug(pat.span, "`Res::Err` but no error emitted"); + self.set_tainted_by_errors(e); + return Err(e); + } + + // Type-check the path. + let (pat_ty, res) = + self.instantiate_value_path(segments, opt_ty, res, pat.span, pat.span, pat.hir_id); + if !pat_ty.is_fn() { + return report_unexpected_res(res); + } + + let variant = match res { + Res::Err => { + self.dcx().span_bug(pat.span, "`Res::Err` but no error emitted"); + } + Res::Def(DefKind::AssocConst | DefKind::AssocFn, _) => { + return report_unexpected_res(res); + } + Res::Def(DefKind::Ctor(_, CtorKind::Fn), _) => tcx.expect_variant_res(res), + _ => bug!("unexpected pattern resolution: {:?}", res), + }; + + // Replace constructor type with constructed type for tuple struct patterns. + let pat_ty = pat_ty.fn_sig(tcx).output(); + let pat_ty = pat_ty.no_bound_vars().expect("expected fn type"); + + Ok(ResolvedPat { ty: pat_ty, kind: ResolvedPatKind::TupleStruct { res, variant } }) + } + fn check_pat_tuple_struct( &self, pat: &'tcx Pat<'tcx>, qpath: &'tcx hir::QPath<'tcx>, subpats: &'tcx [Pat<'tcx>], ddpos: hir::DotDotPos, + res: Res, + pat_ty: Ty<'tcx>, + variant: &'tcx VariantDef, expected: Ty<'tcx>, pat_info: PatInfo<'tcx>, ) -> Ty<'tcx> { @@ -1458,46 +1520,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.check_pat(pat, Ty::new_error(tcx, e), pat_info); } }; - let report_unexpected_res = |res: Res| { - let expected = "tuple struct or tuple variant"; - let e = report_unexpected_variant_res(tcx, res, None, qpath, pat.span, E0164, expected); - on_error(e); - e - }; - - // Resolve the path and check the definition for errors. - let (res, opt_ty, segments) = - self.resolve_ty_and_res_fully_qualified_call(qpath, pat.hir_id, pat.span); - if res == Res::Err { - let e = self.dcx().span_delayed_bug(pat.span, "`Res::Err` but no error emitted"); - self.set_tainted_by_errors(e); - on_error(e); - return Ty::new_error(tcx, e); - } - - // Type-check the path. - let (pat_ty, res) = - self.instantiate_value_path(segments, opt_ty, res, pat.span, pat.span, pat.hir_id); - if !pat_ty.is_fn() { - let e = report_unexpected_res(res); - return Ty::new_error(tcx, e); - } - - let variant = match res { - Res::Err => { - self.dcx().span_bug(pat.span, "`Res::Err` but no error emitted"); - } - Res::Def(DefKind::AssocConst | DefKind::AssocFn, _) => { - let e = report_unexpected_res(res); - return Ty::new_error(tcx, e); - } - Res::Def(DefKind::Ctor(_, CtorKind::Fn), _) => tcx.expect_variant_res(res), - _ => bug!("unexpected pattern resolution: {:?}", res), - }; - - // Replace constructor type with constructed type for tuple struct patterns. - let pat_ty = pat_ty.fn_sig(tcx).output(); - let pat_ty = pat_ty.no_bound_vars().expect("expected fn type"); // Type-check the tuple struct pattern against the expected type. let diag = self.demand_eqtype_pat_diag(pat.span, expected, pat_ty, &pat_info.top_info); From 923d95cc9f318a88cde9166dd44f785c8bcdadfb Mon Sep 17 00:00:00 2001 From: dianne Date: Sat, 8 Mar 2025 21:30:48 -0800 Subject: [PATCH 14/27] don't peel ADTs the pattern could match This is the use for the previous commits' refactors; see the messages there for more information. --- compiler/rustc_hir_typeck/src/pat.rs | 49 +++++++++++++------ .../deref-patterns/implicit-const-deref.rs | 19 +++++++ .../implicit-const-deref.stderr | 16 ++++++ .../deref-patterns/implicit-cow-deref.rs | 44 +++++++++++++++++ 4 files changed, 114 insertions(+), 14 deletions(-) create mode 100644 tests/ui/pattern/deref-patterns/implicit-const-deref.rs create mode 100644 tests/ui/pattern/deref-patterns/implicit-const-deref.stderr create mode 100644 tests/ui/pattern/deref-patterns/implicit-cow-deref.rs diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index da5153bed12..46916f22d0e 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/compiler/rustc_hir_typeck/src/pat.rs @@ -9,6 +9,7 @@ use rustc_errors::{ Applicability, Diag, ErrorGuaranteed, MultiSpan, pluralize, struct_span_code_err, }; use rustc_hir::def::{CtorKind, DefKind, Res}; +use rustc_hir::def_id::DefId; use rustc_hir::pat_util::EnumerateAndAdjustIterator; use rustc_hir::{ self as hir, BindingMode, ByRef, ExprKind, HirId, LangItem, Mutability, Pat, PatExpr, @@ -179,8 +180,16 @@ enum PeelKind { /// ADTs. In order to peel only as much as necessary for the pattern to match, the `until_adt` /// field contains the ADT def that the pattern is a constructor for, if applicable, so that we /// don't peel it. See [`ResolvedPat`] for more information. - // TODO: add `ResolvedPat` and `until_adt`. - Implicit, + Implicit { until_adt: Option }, +} + +impl AdjustMode { + const fn peel_until_adt(opt_adt_def: Option) -> AdjustMode { + AdjustMode::Peel { kind: PeelKind::Implicit { until_adt: opt_adt_def } } + } + const fn peel_all() -> AdjustMode { + AdjustMode::peel_until_adt(None) + } } /// `ref mut` bindings (explicit or match-ergonomics) are not allowed behind an `&` reference. @@ -294,7 +303,7 @@ impl<'tcx> ResolvedPat<'tcx> { // The remaining possible resolutions for path, struct, and tuple struct patterns are // ADT constructors. As such, we may peel references freely, but we must not peel the // ADT itself from the scrutinee if it's a smart pointer. - AdjustMode::Peel { kind: PeelKind::Implicit } + AdjustMode::peel_until_adt(self.ty.ty_adt_def().map(|adt| adt.did())) } } } @@ -521,14 +530,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // If `deref_patterns` is enabled, peel a smart pointer from the scrutinee type. See the // examples in `tests/ui/pattern/deref_patterns/`. _ if self.tcx.features().deref_patterns() - && let AdjustMode::Peel { kind: PeelKind::Implicit } = adjust_mode + && let AdjustMode::Peel { kind: PeelKind::Implicit { until_adt } } = adjust_mode && pat.default_binding_modes // For simplicity, only apply overloaded derefs if `expected` is a known ADT. // FIXME(deref_patterns): we'll get better diagnostics for users trying to // implicitly deref generics if we allow them here, but primitives, tuples, and // inference vars definitely should be stopped. Figure out what makes most sense. - // TODO: stop peeling if the pattern is a constructor for the scrutinee type - && expected.is_adt() + && let ty::Adt(scrutinee_adt, _) = *expected.kind() + // Don't peel if the pattern type already matches the scrutinee. E.g., stop here if + // matching on a `Cow<'a, T>` scrutinee with a `Cow::Owned(_)` pattern. + && until_adt != Some(scrutinee_adt.did()) // At this point, the pattern isn't able to match `expected` without peeling. Check // that it implements `Deref` before assuming it's a smart pointer, to get a normal // type error instead of a missing impl error if not. This only checks for `Deref`, @@ -637,22 +648,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { match &pat.kind { // Type checking these product-like types successfully always require // that the expected type be of those types and not reference types. - PatKind::Struct(..) - | PatKind::TupleStruct(..) - | PatKind::Tuple(..) + PatKind::Tuple(..) | PatKind::Range(..) - | PatKind::Slice(..) => AdjustMode::Peel { kind: PeelKind::Implicit }, + | PatKind::Slice(..) => AdjustMode::peel_all(), // When checking an explicit deref pattern, only peel reference types. // FIXME(deref_patterns): If box patterns and deref patterns need to coexist, box // patterns may want `PeelKind::Implicit`, stopping on encountering a box. | PatKind::Box(_) | PatKind::Deref(_) => AdjustMode::Peel { kind: PeelKind::ExplicitDerefPat }, // A never pattern behaves somewhat like a literal or unit variant. - PatKind::Never => AdjustMode::Peel { kind: PeelKind::Implicit }, + PatKind::Never => AdjustMode::peel_all(), // For patterns with paths, how we peel the scrutinee depends on the path's resolution. - PatKind::Expr(PatExpr { kind: PatExprKind::Path(_), .. }) => { + PatKind::Struct(..) + | PatKind::TupleStruct(..) + | PatKind::Expr(PatExpr { kind: PatExprKind::Path(_), .. }) => { // If there was an error resolving the path, default to peeling everything. - opt_path_res.unwrap().map_or(AdjustMode::Peel { kind: PeelKind::Implicit }, |pr| pr.adjust_mode()) + opt_path_res.unwrap().map_or(AdjustMode::peel_all(), |pr| pr.adjust_mode()) } // String and byte-string literals result in types `&str` and `&[u8]` respectively. @@ -662,7 +673,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Call `resolve_vars_if_possible` here for inline const blocks. PatKind::Expr(lt) => match self.resolve_vars_if_possible(self.check_pat_expr_unadjusted(lt)).kind() { ty::Ref(..) => AdjustMode::Pass, - _ => AdjustMode::Peel { kind: PeelKind::Implicit }, + _ => { + // Path patterns have already been handled, and inline const blocks currently + // aren't possible to write, so any handling for them would be untested. + if cfg!(debug_assertions) + && self.tcx.features().deref_patterns() + && !matches!(lt.kind, PatExprKind::Lit { .. }) + { + span_bug!(lt.span, "FIXME(deref_patterns): adjust mode unimplemented for {:?}", lt.kind); + } + AdjustMode::peel_all() + } }, // Ref patterns are complicated, we handle them in `check_pat_ref`. diff --git a/tests/ui/pattern/deref-patterns/implicit-const-deref.rs b/tests/ui/pattern/deref-patterns/implicit-const-deref.rs new file mode 100644 index 00000000000..70f89629bc2 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/implicit-const-deref.rs @@ -0,0 +1,19 @@ +//! Test that we get an error about structural equality rather than a type error when attempting to +//! use const patterns of library pointer types. Currently there aren't any smart pointers that can +//! be used in constant patterns, but we still need to make sure we don't implicitly dereference the +//! scrutinee and end up with a type error; this would prevent us from reporting that only constants +//! supporting structural equality can be used as patterns. +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +const EMPTY: Vec<()> = Vec::new(); + +fn main() { + // FIXME(inline_const_pat): if `inline_const_pat` is reinstated, there should be a case here for + // inline const block patterns as well; they're checked differently than named constants. + match vec![()] { + EMPTY => {} + //~^ ERROR: constant of non-structural type `Vec<()>` in a pattern + _ => {} + } +} diff --git a/tests/ui/pattern/deref-patterns/implicit-const-deref.stderr b/tests/ui/pattern/deref-patterns/implicit-const-deref.stderr new file mode 100644 index 00000000000..21d09ec44c4 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/implicit-const-deref.stderr @@ -0,0 +1,16 @@ +error: constant of non-structural type `Vec<()>` in a pattern + --> $DIR/implicit-const-deref.rs:15:9 + | +LL | const EMPTY: Vec<()> = Vec::new(); + | -------------------- constant defined here +... +LL | EMPTY => {} + | ^^^^^ constant of non-structural type + --> $SRC_DIR/alloc/src/vec/mod.rs:LL:COL + | + = note: `Vec<()>` must be annotated with `#[derive(PartialEq)]` to be usable in patterns + | + = note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details + +error: aborting due to 1 previous error + diff --git a/tests/ui/pattern/deref-patterns/implicit-cow-deref.rs b/tests/ui/pattern/deref-patterns/implicit-cow-deref.rs new file mode 100644 index 00000000000..6a363c58b63 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/implicit-cow-deref.rs @@ -0,0 +1,44 @@ +//@ run-pass +//! Test that implicit deref patterns interact as expected with `Cow` constructor patterns. +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +use std::borrow::Cow; + +fn main() { + let cow: Cow<'static, [u8]> = Cow::from(&[1, 2, 3]); + + match cow { + [..] => {} + _ => unreachable!(), + } + + match cow { + Cow::Borrowed(_) => {} + Cow::Owned(_) => unreachable!(), + } + + match Box::new(&cow) { + Cow::Borrowed { 0: _ } => {} + Cow::Owned { 0: _ } => unreachable!(), + _ => unreachable!(), + } + + let cow_of_cow: Cow<'_, Cow<'static, [u8]>> = Cow::Owned(cow); + + match cow_of_cow { + [..] => {} + _ => unreachable!(), + } + + match cow_of_cow { + Cow::Borrowed(_) => unreachable!(), + Cow::Owned(_) => {} + } + + match Box::new(&cow_of_cow) { + Cow::Borrowed { 0: _ } => unreachable!(), + Cow::Owned { 0: _ } => {} + _ => unreachable!(), + } +} From 977c9ab7a2ead006e06b1b78cc1b6ac63245560b Mon Sep 17 00:00:00 2001 From: dianne Date: Sun, 9 Mar 2025 17:02:34 -0700 Subject: [PATCH 15/27] respect the tcx's recursion limit when peeling --- compiler/rustc_hir_analysis/src/autoderef.rs | 10 +++++-- compiler/rustc_hir_typeck/src/pat.rs | 28 +++++++++++++------ .../pattern/deref-patterns/recursion-limit.rs | 23 +++++++++++++++ .../deref-patterns/recursion-limit.stderr | 18 ++++++++++++ 4 files changed, 68 insertions(+), 11 deletions(-) create mode 100644 tests/ui/pattern/deref-patterns/recursion-limit.rs create mode 100644 tests/ui/pattern/deref-patterns/recursion-limit.stderr diff --git a/compiler/rustc_hir_analysis/src/autoderef.rs b/compiler/rustc_hir_analysis/src/autoderef.rs index b3eade8c8ae..99e495d9266 100644 --- a/compiler/rustc_hir_analysis/src/autoderef.rs +++ b/compiler/rustc_hir_analysis/src/autoderef.rs @@ -2,8 +2,8 @@ use rustc_infer::infer::InferCtxt; use rustc_infer::traits::PredicateObligations; use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt}; use rustc_session::Limit; -use rustc_span::Span; use rustc_span::def_id::{LOCAL_CRATE, LocalDefId}; +use rustc_span::{ErrorGuaranteed, Span}; use rustc_trait_selection::traits::ObligationCtxt; use tracing::{debug, instrument}; @@ -259,7 +259,11 @@ impl<'a, 'tcx> Autoderef<'a, 'tcx> { } } -pub fn report_autoderef_recursion_limit_error<'tcx>(tcx: TyCtxt<'tcx>, span: Span, ty: Ty<'tcx>) { +pub fn report_autoderef_recursion_limit_error<'tcx>( + tcx: TyCtxt<'tcx>, + span: Span, + ty: Ty<'tcx>, +) -> ErrorGuaranteed { // We've reached the recursion limit, error gracefully. let suggested_limit = match tcx.recursion_limit() { Limit(0) => Limit(2), @@ -270,5 +274,5 @@ pub fn report_autoderef_recursion_limit_error<'tcx>(tcx: TyCtxt<'tcx>, span: Spa ty, suggested_limit, crate_name: tcx.crate_name(LOCAL_CRATE), - }); + }) } diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index 46916f22d0e..e5e4fc7f8b7 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/compiler/rustc_hir_typeck/src/pat.rs @@ -15,6 +15,7 @@ use rustc_hir::{ self as hir, BindingMode, ByRef, ExprKind, HirId, LangItem, Mutability, Pat, PatExpr, PatExprKind, PatKind, expr_needs_parens, }; +use rustc_hir_analysis::autoderef::report_autoderef_recursion_limit_error; use rustc_infer::infer; use rustc_middle::traits::PatternOriginExpr; use rustc_middle::ty::{self, Ty, TypeVisitableExt}; @@ -552,17 +553,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { debug!("scrutinee ty {expected:?} is a smart pointer, inserting overloaded deref"); // The scrutinee is a smart pointer; implicitly dereference it. This adds a // requirement that `expected: DerefPure`. - let inner_ty = self.deref_pat_target(pat.span, expected); + let mut inner_ty = self.deref_pat_target(pat.span, expected); // Once we've checked `pat`, we'll add a `DerefMut` bound if it contains any // `ref mut` bindings. See `Self::register_deref_mut_bounds_if_needed`. - // Preserve the smart pointer type for THIR lowering and upvar analysis. - self.typeck_results - .borrow_mut() - .pat_adjustments_mut() - .entry(pat.hir_id) - .or_default() - .push(PatAdjustment { kind: PatAdjust::OverloadedDeref, source: expected }); + let mut typeck_results = self.typeck_results.borrow_mut(); + let mut pat_adjustments_table = typeck_results.pat_adjustments_mut(); + let pat_adjustments = pat_adjustments_table.entry(pat.hir_id).or_default(); + // We may reach the recursion limit if a user matches on a type `T` satisfying + // `T: Deref`; error gracefully in this case. + // FIXME(deref_patterns): If `deref_patterns` stabilizes, it may make sense to move + // this check out of this branch. Alternatively, this loop could be implemented with + // autoderef and this check removed. For now though, don't break code compiling on + // stable with lots of `&`s and a low recursion limit, if anyone's done that. + if self.tcx.recursion_limit().value_within_limit(pat_adjustments.len()) { + // Preserve the smart pointer type for THIR lowering and closure upvar analysis. + pat_adjustments + .push(PatAdjustment { kind: PatAdjust::OverloadedDeref, source: expected }); + } else { + let guar = report_autoderef_recursion_limit_error(self.tcx, pat.span, expected); + inner_ty = Ty::new_error(self.tcx, guar); + } + drop(typeck_results); // Recurse, using the old pat info to keep `current_depth` to its old value. // Peeling smart pointers does not update the default binding mode. diff --git a/tests/ui/pattern/deref-patterns/recursion-limit.rs b/tests/ui/pattern/deref-patterns/recursion-limit.rs new file mode 100644 index 00000000000..c5fe520f6f1 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/recursion-limit.rs @@ -0,0 +1,23 @@ +//! Test that implicit deref patterns respect the recursion limit +#![feature(deref_patterns)] +#![allow(incomplete_features)] +#![recursion_limit = "8"] + +use std::ops::Deref; + +struct Cyclic; +impl Deref for Cyclic { + type Target = Cyclic; + fn deref(&self) -> &Cyclic { + &Cyclic + } +} + +fn main() { + match &Box::new(Cyclic) { + () => {} + //~^ ERROR: reached the recursion limit while auto-dereferencing `Cyclic` + //~| ERROR: the trait bound `Cyclic: DerefPure` is not satisfied + _ => {} + } +} diff --git a/tests/ui/pattern/deref-patterns/recursion-limit.stderr b/tests/ui/pattern/deref-patterns/recursion-limit.stderr new file mode 100644 index 00000000000..9a83d1eb5a4 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/recursion-limit.stderr @@ -0,0 +1,18 @@ +error[E0055]: reached the recursion limit while auto-dereferencing `Cyclic` + --> $DIR/recursion-limit.rs:18:9 + | +LL | () => {} + | ^^ deref recursion limit reached + | + = help: consider increasing the recursion limit by adding a `#![recursion_limit = "16"]` attribute to your crate (`recursion_limit`) + +error[E0277]: the trait bound `Cyclic: DerefPure` is not satisfied + --> $DIR/recursion-limit.rs:18:9 + | +LL | () => {} + | ^^ the trait `DerefPure` is not implemented for `Cyclic` + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0055, E0277. +For more information about an error, try `rustc --explain E0055`. From ff0d4bc743086111c841812ec9925efb075d1243 Mon Sep 17 00:00:00 2001 From: dianne Date: Thu, 13 Mar 2025 22:34:26 -0700 Subject: [PATCH 16/27] upvar inference for implicit deref patterns --- .../rustc_hir_typeck/src/expr_use_visitor.rs | 54 +++++++++++++++---- .../pattern/deref-patterns/closure_capture.rs | 27 ++++++++++ 2 files changed, 70 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs index f43f9b5187b..c92aa228cc2 100644 --- a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs @@ -1000,6 +1000,8 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx // determines whether to borrow *at the level of the deref pattern* rather than // borrowing the bound place (since that inner place is inside the temporary that // stores the result of calling `deref()`/`deref_mut()` so can't be captured). + // HACK: this could be a fake pattern corresponding to a deref inserted by match + // ergonomics, in which case `pat.hir_id` will be the id of the subpattern. let mutable = self.cx.typeck_results().pat_has_ref_mut_binding(subpattern); let mutability = if mutable { hir::Mutability::Mut } else { hir::Mutability::Not }; @@ -1675,12 +1677,31 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx // Then we see that to get the same result, we must start with // `deref { deref { place_foo }}` instead of `place_foo` since the pattern is now `Some(x,)` // and not `&&Some(x,)`, even though its assigned type is that of `&&Some(x,)`. - for _ in - 0..self.cx.typeck_results().pat_adjustments().get(pat.hir_id).map_or(0, |v| v.len()) - { + let typeck_results = self.cx.typeck_results(); + let adjustments: &[adjustment::PatAdjustment<'tcx>] = + typeck_results.pat_adjustments().get(pat.hir_id).map_or(&[], |v| &**v); + let mut adjusts = adjustments.iter().peekable(); + while let Some(adjust) = adjusts.next() { debug!("applying adjustment to place_with_id={:?}", place_with_id); - place_with_id = self.cat_deref(pat.hir_id, place_with_id)?; + place_with_id = match adjust.kind { + adjustment::PatAdjust::BuiltinDeref => self.cat_deref(pat.hir_id, place_with_id)?, + adjustment::PatAdjust::OverloadedDeref => { + // This adjustment corresponds to an overloaded deref; it borrows the scrutinee to + // call `Deref::deref` or `DerefMut::deref_mut`. Invoke the callback before setting + // `place_with_id` to the temporary storing the result of the deref. + // HACK(dianne): giving the callback a fake deref pattern makes sure it behaves the + // same as it would if this were an explicit deref pattern. + op(&place_with_id, &hir::Pat { kind: PatKind::Deref(pat), ..*pat })?; + let target_ty = match adjusts.peek() { + Some(&&next_adjust) => next_adjust.source, + // At the end of the deref chain, we get `pat`'s scrutinee. + None => self.pat_ty_unadjusted(pat)?, + }; + self.pat_deref_temp(pat.hir_id, pat, target_ty)? + } + }; } + drop(typeck_results); // explicitly release borrow of typeck results, just in case. let place_with_id = place_with_id; // lose mutability debug!("applied adjustment derefs to get place_with_id={:?}", place_with_id); @@ -1783,14 +1804,8 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx self.cat_pattern(subplace, subpat, op)?; } PatKind::Deref(subpat) => { - let mutable = self.cx.typeck_results().pat_has_ref_mut_binding(subpat); - let mutability = if mutable { hir::Mutability::Mut } else { hir::Mutability::Not }; - let re_erased = self.cx.tcx().lifetimes.re_erased; let ty = self.pat_ty_adjusted(subpat)?; - let ty = Ty::new_ref(self.cx.tcx(), re_erased, ty, mutability); - // A deref pattern generates a temporary. - let base = self.cat_rvalue(pat.hir_id, ty); - let place = self.cat_deref(pat.hir_id, base)?; + let place = self.pat_deref_temp(pat.hir_id, subpat, ty)?; self.cat_pattern(place, subpat, op)?; } @@ -1843,6 +1858,23 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx Ok(()) } + /// Represents the place of the temp that stores the scrutinee of a deref pattern's interior. + fn pat_deref_temp( + &self, + hir_id: HirId, + inner: &hir::Pat<'_>, + target_ty: Ty<'tcx>, + ) -> Result, Cx::Error> { + let mutable = self.cx.typeck_results().pat_has_ref_mut_binding(inner); + let mutability = if mutable { hir::Mutability::Mut } else { hir::Mutability::Not }; + let re_erased = self.cx.tcx().lifetimes.re_erased; + let ty = Ty::new_ref(self.cx.tcx(), re_erased, target_ty, mutability); + // A deref pattern stores the result of `Deref::deref` or `DerefMut::deref_mut` ... + let base = self.cat_rvalue(hir_id, ty); + // ... and the inner pattern matches on the place behind that reference. + self.cat_deref(hir_id, base) + } + fn is_multivariant_adt(&self, ty: Ty<'tcx>, span: Span) -> bool { if let ty::Adt(def, _) = self.cx.try_structurally_resolve_type(span, ty).kind() { // Note that if a non-exhaustive SingleVariant is defined in another crate, we need diff --git a/tests/ui/pattern/deref-patterns/closure_capture.rs b/tests/ui/pattern/deref-patterns/closure_capture.rs index fc0ddedac2b..08586b6c7ab 100644 --- a/tests/ui/pattern/deref-patterns/closure_capture.rs +++ b/tests/ui/pattern/deref-patterns/closure_capture.rs @@ -11,6 +11,15 @@ fn main() { assert_eq!(b.len(), 3); f(); + let v = vec![1, 2, 3]; + let f = || { + // this should count as a borrow of `v` as a whole + let [.., x] = v else { unreachable!() }; + assert_eq!(x, 3); + }; + assert_eq!(v, [1, 2, 3]); + f(); + let mut b = Box::new("aaa".to_string()); let mut f = || { let deref!(ref mut s) = b else { unreachable!() }; @@ -18,4 +27,22 @@ fn main() { }; f(); assert_eq!(b.len(), 5); + + let mut v = vec![1, 2, 3]; + let mut f = || { + // this should count as a mutable borrow of `v` as a whole + let [.., ref mut x] = v else { unreachable!() }; + *x = 4; + }; + f(); + assert_eq!(v, [1, 2, 4]); + + let mut v = vec![1, 2, 3]; + let mut f = || { + // here, `[.., x]` is adjusted by both an overloaded deref and a builtin deref + let [.., x] = &mut v else { unreachable!() }; + *x = 4; + }; + f(); + assert_eq!(v, [1, 2, 4]); } From 4c4b61b730ee7b53918043ff403bbc8e2d6d0eb0 Mon Sep 17 00:00:00 2001 From: dianne Date: Fri, 14 Mar 2025 02:51:20 -0700 Subject: [PATCH 17/27] add a feature gate test Implicit deref patterns allow previously ill-typed programs. Make sure they're still ill-typed without the feature gate. I've thrown in a test for `deref!(_)` too, though it seems it refers to `deref_patterns` as a library feature. --- tests/ui/pattern/deref-patterns/needs-gate.rs | 15 ++++++++++ .../pattern/deref-patterns/needs-gate.stderr | 29 +++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 tests/ui/pattern/deref-patterns/needs-gate.rs create mode 100644 tests/ui/pattern/deref-patterns/needs-gate.stderr diff --git a/tests/ui/pattern/deref-patterns/needs-gate.rs b/tests/ui/pattern/deref-patterns/needs-gate.rs new file mode 100644 index 00000000000..2d5ec45217f --- /dev/null +++ b/tests/ui/pattern/deref-patterns/needs-gate.rs @@ -0,0 +1,15 @@ +// gate-test-deref_patterns + +fn main() { + match Box::new(0) { + deref!(0) => {} + //~^ ERROR: use of unstable library feature `deref_patterns`: placeholder syntax for deref patterns + _ => {} + } + + match Box::new(0) { + 0 => {} + //~^ ERROR: mismatched types + _ => {} + } +} diff --git a/tests/ui/pattern/deref-patterns/needs-gate.stderr b/tests/ui/pattern/deref-patterns/needs-gate.stderr new file mode 100644 index 00000000000..8687b5dc977 --- /dev/null +++ b/tests/ui/pattern/deref-patterns/needs-gate.stderr @@ -0,0 +1,29 @@ +error[E0658]: use of unstable library feature `deref_patterns`: placeholder syntax for deref patterns + --> $DIR/needs-gate.rs:5:9 + | +LL | deref!(0) => {} + | ^^^^^ + | + = note: see issue #87121 for more information + = help: add `#![feature(deref_patterns)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + +error[E0308]: mismatched types + --> $DIR/needs-gate.rs:11:9 + | +LL | match Box::new(0) { + | ----------- this expression has type `Box<{integer}>` +LL | 0 => {} + | ^ expected `Box<{integer}>`, found integer + | + = note: expected struct `Box<{integer}>` + found type `{integer}` +help: consider dereferencing to access the inner value using the Deref trait + | +LL | match *Box::new(0) { + | + + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0308, E0658. +For more information about an error, try `rustc --explain E0308`. From 93d13e98069478ab286d28b42333f74b88308a99 Mon Sep 17 00:00:00 2001 From: dianne Date: Fri, 14 Mar 2025 03:51:54 -0700 Subject: [PATCH 18/27] add an unstable book chapter - Clarifies the uses of implicit and explicit deref patterns - Includes motivating examples for both syntaxes - Shows the interaction with match ergonomics for reference types - Cross-links with `string_deref_patterns` and `box_patterns` The examples are contrived, but hopefully the intent comes across. --- .../src/language-features/box-patterns.md | 4 ++ .../src/language-features/deref-patterns.md | 57 +++++++++++++++++++ .../string-deref-patterns.md | 3 + 3 files changed, 64 insertions(+) create mode 100644 src/doc/unstable-book/src/language-features/deref-patterns.md diff --git a/src/doc/unstable-book/src/language-features/box-patterns.md b/src/doc/unstable-book/src/language-features/box-patterns.md index a1ac09633b7..c8a15b8477e 100644 --- a/src/doc/unstable-book/src/language-features/box-patterns.md +++ b/src/doc/unstable-book/src/language-features/box-patterns.md @@ -6,6 +6,8 @@ The tracking issue for this feature is: [#29641] ------------------------ +> **Note**: This feature will be superseded by [`deref_patterns`] in the future. + Box patterns let you match on `Box`s: @@ -28,3 +30,5 @@ fn main() { } } ``` + +[`deref_patterns`]: ./deref-patterns.md diff --git a/src/doc/unstable-book/src/language-features/deref-patterns.md b/src/doc/unstable-book/src/language-features/deref-patterns.md new file mode 100644 index 00000000000..d0102a665b0 --- /dev/null +++ b/src/doc/unstable-book/src/language-features/deref-patterns.md @@ -0,0 +1,57 @@ +# `deref_patterns` + +The tracking issue for this feature is: [#87121] + +[#87121]: https://github.com/rust-lang/rust/issues/87121 + +------------------------ + +> **Note**: This feature is incomplete. In the future, it is meant to supersede +> [`box_patterns`](./box-patterns.md) and [`string_deref_patterns`](./string-deref-patterns.md). + +This feature permits pattern matching on [smart pointers in the standard library] through their +`Deref` target types, either implicitly or with explicit `deref!(_)` patterns (the syntax of which +is currently a placeholder). + +```rust +#![feature(deref_patterns)] +#![allow(incomplete_features)] + +let mut v = vec![Box::new(Some(0))]; + +// Implicit dereferences are inserted when a pattern can match against the +// result of repeatedly dereferencing but can't match against a smart +// pointer itself. This works alongside match ergonomics for references. +if let [Some(x)] = &mut v { + *x += 1; +} + +// Explicit `deref!(_)` patterns may instead be used when finer control is +// needed, e.g. to dereference only a single smart pointer, or to bind the +// the result of dereferencing to a variable. +if let deref!([deref!(opt_x @ Some(1))]) = &mut v { + opt_x.as_mut().map(|x| *x += 1); +} + +assert_eq!(v, [Box::new(Some(2))]); +``` + +Without this feature, it may be necessary to introduce temporaries to represent dereferenced places +when matching on nested structures: + +```rust +let mut v = vec![Box::new(Some(0))]; +if let [b] = &mut *v { + if let Some(x) = &mut **b { + *x += 1; + } +} +if let [b] = &mut *v { + if let opt_x @ Some(1) = &mut **b { + opt_x.as_mut().map(|x| *x += 1); + } +} +assert_eq!(v, [Box::new(Some(2))]); +``` + +[smart pointers in the standard library]: https://doc.rust-lang.org/std/ops/trait.DerefPure.html#implementors diff --git a/src/doc/unstable-book/src/language-features/string-deref-patterns.md b/src/doc/unstable-book/src/language-features/string-deref-patterns.md index 3723830751e..366bb15d4ea 100644 --- a/src/doc/unstable-book/src/language-features/string-deref-patterns.md +++ b/src/doc/unstable-book/src/language-features/string-deref-patterns.md @@ -6,6 +6,8 @@ The tracking issue for this feature is: [#87121] ------------------------ +> **Note**: This feature will be superseded by [`deref_patterns`] in the future. + This feature permits pattern matching `String` to `&str` through [its `Deref` implementation]. ```rust @@ -42,4 +44,5 @@ pub fn is_it_the_answer(value: Value) -> bool { } ``` +[`deref_patterns`]: ./deref-patterns.md [its `Deref` implementation]: https://doc.rust-lang.org/std/string/struct.String.html#impl-Deref-for-String From 3b91b7ac57a534bbb1d4f39f679f86040a588903 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 16 Apr 2025 20:13:42 +0200 Subject: [PATCH 19/27] Make cow_of_cow test a teeny bit more explicit --- tests/ui/pattern/deref-patterns/implicit-cow-deref.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/ui/pattern/deref-patterns/implicit-cow-deref.rs b/tests/ui/pattern/deref-patterns/implicit-cow-deref.rs index 6a363c58b63..a9b8de86010 100644 --- a/tests/ui/pattern/deref-patterns/implicit-cow-deref.rs +++ b/tests/ui/pattern/deref-patterns/implicit-cow-deref.rs @@ -6,7 +6,7 @@ use std::borrow::Cow; fn main() { - let cow: Cow<'static, [u8]> = Cow::from(&[1, 2, 3]); + let cow: Cow<'static, [u8]> = Cow::Borrowed(&[1, 2, 3]); match cow { [..] => {} @@ -31,6 +31,7 @@ fn main() { _ => unreachable!(), } + // This matches on the outer `Cow` (the owned one). match cow_of_cow { Cow::Borrowed(_) => unreachable!(), Cow::Owned(_) => {} From 92ce44f372d081e4c548cdabeec4f580a5eb9642 Mon Sep 17 00:00:00 2001 From: Curtis D'Alves Date: Wed, 16 Apr 2025 19:26:26 -0400 Subject: [PATCH 20/27] ignore aix for tests/ui/erros/pic-linker.rs --- tests/ui/errors/pic-linker.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ui/errors/pic-linker.rs b/tests/ui/errors/pic-linker.rs index d9098990304..36495ca8fe9 100644 --- a/tests/ui/errors/pic-linker.rs +++ b/tests/ui/errors/pic-linker.rs @@ -5,6 +5,7 @@ //@ ignore-windows //@ ignore-macos //@ ignore-cross-compile +//@ ignore-aix //@ compile-flags: -Clink-args=-Wl,-z,text //@ run-pass From 4be670f89b94aaeab04b0e52d0efd9c61f1bef9d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 9 Apr 2025 21:11:12 +1000 Subject: [PATCH 21/27] Warnings-as-errors in `check-builtin-attr-ice.rs`. This adds two new warnings, both of which print the attribute incorrectly as `#[]`. The next commit will fix this. --- tests/ui/attributes/check-builtin-attr-ice.rs | 4 +++ .../attributes/check-builtin-attr-ice.stderr | 26 ++++++++++++++++--- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/tests/ui/attributes/check-builtin-attr-ice.rs b/tests/ui/attributes/check-builtin-attr-ice.rs index 9ef5890601f..e3a2ffc714d 100644 --- a/tests/ui/attributes/check-builtin-attr-ice.rs +++ b/tests/ui/attributes/check-builtin-attr-ice.rs @@ -39,13 +39,17 @@ // Notably, `should_panic` is a `AttributeType::Normal` attribute that is checked separately. +#![deny(unused_attributes)] + struct Foo { #[should_panic::skip] //~^ ERROR failed to resolve + //~| ERROR `#[]` only has an effect on functions pub field: u8, #[should_panic::a::b::c] //~^ ERROR failed to resolve + //~| ERROR `#[]` only has an effect on functions pub field2: u8, } diff --git a/tests/ui/attributes/check-builtin-attr-ice.stderr b/tests/ui/attributes/check-builtin-attr-ice.stderr index 06a4769b2b4..487a0b6d680 100644 --- a/tests/ui/attributes/check-builtin-attr-ice.stderr +++ b/tests/ui/attributes/check-builtin-attr-ice.stderr @@ -1,21 +1,39 @@ error[E0433]: failed to resolve: use of unresolved module or unlinked crate `should_panic` - --> $DIR/check-builtin-attr-ice.rs:43:7 + --> $DIR/check-builtin-attr-ice.rs:45:7 | LL | #[should_panic::skip] | ^^^^^^^^^^^^ use of unresolved module or unlinked crate `should_panic` error[E0433]: failed to resolve: use of unresolved module or unlinked crate `should_panic` - --> $DIR/check-builtin-attr-ice.rs:47:7 + --> $DIR/check-builtin-attr-ice.rs:50:7 | LL | #[should_panic::a::b::c] | ^^^^^^^^^^^^ use of unresolved module or unlinked crate `should_panic` error[E0433]: failed to resolve: use of unresolved module or unlinked crate `deny` - --> $DIR/check-builtin-attr-ice.rs:55:7 + --> $DIR/check-builtin-attr-ice.rs:59:7 | LL | #[deny::skip] | ^^^^ use of unresolved module or unlinked crate `deny` -error: aborting due to 3 previous errors +error: `#[]` only has an effect on functions + --> $DIR/check-builtin-attr-ice.rs:45:5 + | +LL | #[should_panic::skip] + | ^^^^^^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/check-builtin-attr-ice.rs:42:9 + | +LL | #![deny(unused_attributes)] + | ^^^^^^^^^^^^^^^^^ + +error: `#[]` only has an effect on functions + --> $DIR/check-builtin-attr-ice.rs:50:5 + | +LL | #[should_panic::a::b::c] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 5 previous errors For more information about this error, try `rustc --explain E0433`. From 400e8e5dc819b1fa7f994fb60dbe8b5112ec3fd2 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 9 Apr 2025 21:15:45 +1000 Subject: [PATCH 22/27] Fix attribute printing in an error. The current code assumes that the attribute is just an identifier, and so misprints paths. --- compiler/rustc_passes/src/check_attr.rs | 5 ++++- compiler/rustc_passes/src/errors.rs | 2 +- tests/ui/attributes/check-builtin-attr-ice.rs | 4 ++-- tests/ui/attributes/check-builtin-attr-ice.stderr | 4 ++-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 42279258e87..dcfecdc26ea 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -561,12 +561,15 @@ impl<'tcx> CheckAttrVisitor<'tcx> { allowed_target: Target, ) { if target != allowed_target { + let path = attr.path(); + let path: Vec<_> = path.iter().map(|s| s.as_str()).collect(); + let attr_name = path.join("::"); self.tcx.emit_node_span_lint( UNUSED_ATTRIBUTES, hir_id, attr.span(), errors::OnlyHasEffectOn { - attr_name: attr.name_or_empty(), + attr_name, target_name: allowed_target.name().replace(' ', "_"), }, ); diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 85eddafefcd..075927a54f8 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1433,7 +1433,7 @@ pub(crate) struct UselessAssignment<'a> { #[derive(LintDiagnostic)] #[diag(passes_only_has_effect_on)] pub(crate) struct OnlyHasEffectOn { - pub attr_name: Symbol, + pub attr_name: String, pub target_name: String, } diff --git a/tests/ui/attributes/check-builtin-attr-ice.rs b/tests/ui/attributes/check-builtin-attr-ice.rs index e3a2ffc714d..7745849acd0 100644 --- a/tests/ui/attributes/check-builtin-attr-ice.rs +++ b/tests/ui/attributes/check-builtin-attr-ice.rs @@ -44,12 +44,12 @@ struct Foo { #[should_panic::skip] //~^ ERROR failed to resolve - //~| ERROR `#[]` only has an effect on functions + //~| ERROR `#[should_panic::skip]` only has an effect on functions pub field: u8, #[should_panic::a::b::c] //~^ ERROR failed to resolve - //~| ERROR `#[]` only has an effect on functions + //~| ERROR `#[should_panic::a::b::c]` only has an effect on functions pub field2: u8, } diff --git a/tests/ui/attributes/check-builtin-attr-ice.stderr b/tests/ui/attributes/check-builtin-attr-ice.stderr index 487a0b6d680..4f26f71efb7 100644 --- a/tests/ui/attributes/check-builtin-attr-ice.stderr +++ b/tests/ui/attributes/check-builtin-attr-ice.stderr @@ -16,7 +16,7 @@ error[E0433]: failed to resolve: use of unresolved module or unlinked crate `den LL | #[deny::skip] | ^^^^ use of unresolved module or unlinked crate `deny` -error: `#[]` only has an effect on functions +error: `#[should_panic::skip]` only has an effect on functions --> $DIR/check-builtin-attr-ice.rs:45:5 | LL | #[should_panic::skip] @@ -28,7 +28,7 @@ note: the lint level is defined here LL | #![deny(unused_attributes)] | ^^^^^^^^^^^^^^^^^ -error: `#[]` only has an effect on functions +error: `#[should_panic::a::b::c]` only has an effect on functions --> $DIR/check-builtin-attr-ice.rs:50:5 | LL | #[should_panic::a::b::c] From 7e1f2f9c54338cbefddffcce725f89d271d2cbe8 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 10 Apr 2025 13:47:35 +1000 Subject: [PATCH 23/27] Augment some tests involving attributes. This shows places where the use of `name_or_empty` causes problems, i.e. we print empty identifiers in error messages: ``` error: unrecognized field name `` error: `` isn't a valid `#[macro_export]` argument `#[no_sanitize()]` should be applied to a function ``` (The last one is about an attribute `#[no_sanitize("address")]`.) The next commit will fix these. --- tests/ui/abi/debug.rs | 3 +++ tests/ui/abi/debug.stderr | 8 +++++++- .../invalid_macro_export_argument.deny.stderr | 8 +++++++- .../invalid_macro_export_argument.rs | 6 ++++++ tests/ui/attributes/no-sanitize.rs | 5 +++++ tests/ui/attributes/no-sanitize.stderr | 19 ++++++++++++++++++- 6 files changed, 46 insertions(+), 3 deletions(-) diff --git a/tests/ui/abi/debug.rs b/tests/ui/abi/debug.rs index 6dbc3161464..e3260191e5c 100644 --- a/tests/ui/abi/debug.rs +++ b/tests/ui/abi/debug.rs @@ -52,3 +52,6 @@ type TestAbiNeSign = (fn(i32), fn(u32)); //~ ERROR: ABIs are not compatible #[rustc_abi(assert_eq)] type TestAbiEqNonsense = (fn((str, str)), fn((str, str))); //~ ERROR: cannot be known at compilation time + +#[rustc_abi("assert_eq")] //~ ERROR unrecognized field name `` +type Bad = u32; diff --git a/tests/ui/abi/debug.stderr b/tests/ui/abi/debug.stderr index 2239ba0e588..717a95685cb 100644 --- a/tests/ui/abi/debug.stderr +++ b/tests/ui/abi/debug.stderr @@ -906,6 +906,12 @@ LL | type TestAbiEqNonsense = (fn((str, str)), fn((str, str))); = help: the trait `Sized` is not implemented for `str` = note: only the last element of a tuple may have a dynamically sized type +error: unrecognized field name `` + --> $DIR/debug.rs:56:13 + | +LL | #[rustc_abi("assert_eq")] + | ^^^^^^^^^^^ + error: `#[rustc_abi]` can only be applied to function items, type aliases, and associated functions --> $DIR/debug.rs:29:5 | @@ -1004,6 +1010,6 @@ error: fn_abi_of(assoc_test) = FnAbi { LL | fn assoc_test(&self) { } | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 11 previous errors +error: aborting due to 12 previous errors For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/attributes/invalid_macro_export_argument.deny.stderr b/tests/ui/attributes/invalid_macro_export_argument.deny.stderr index 644acc27b58..897bcecc20e 100644 --- a/tests/ui/attributes/invalid_macro_export_argument.deny.stderr +++ b/tests/ui/attributes/invalid_macro_export_argument.deny.stderr @@ -16,5 +16,11 @@ error: `not_local_inner_macros` isn't a valid `#[macro_export]` argument LL | #[macro_export(not_local_inner_macros)] | ^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 2 previous errors +error: `` isn't a valid `#[macro_export]` argument + --> $DIR/invalid_macro_export_argument.rs:33:16 + | +LL | #[macro_export("blah")] + | ^^^^^^ + +error: aborting due to 3 previous errors diff --git a/tests/ui/attributes/invalid_macro_export_argument.rs b/tests/ui/attributes/invalid_macro_export_argument.rs index 96f66991e04..c0aa7df2e87 100644 --- a/tests/ui/attributes/invalid_macro_export_argument.rs +++ b/tests/ui/attributes/invalid_macro_export_argument.rs @@ -30,4 +30,10 @@ macro_rules! e { () => () } +#[macro_export("blah")] +//[deny]~^ ERROR `` isn't a valid `#[macro_export]` argument +macro_rules! f { + () => () +} + fn main() {} diff --git a/tests/ui/attributes/no-sanitize.rs b/tests/ui/attributes/no-sanitize.rs index 8c79866d5aa..a7225efc7b8 100644 --- a/tests/ui/attributes/no-sanitize.rs +++ b/tests/ui/attributes/no-sanitize.rs @@ -38,3 +38,8 @@ fn valid() {} #[no_sanitize(address)] static VALID : i32 = 0; + +#[no_sanitize("address")] +//~^ ERROR `#[no_sanitize()]` should be applied to a function +//~| ERROR invalid argument for `no_sanitize` +static VALID2 : i32 = 0; diff --git a/tests/ui/attributes/no-sanitize.stderr b/tests/ui/attributes/no-sanitize.stderr index 9b0b76e3f4e..1f24aa71af5 100644 --- a/tests/ui/attributes/no-sanitize.stderr +++ b/tests/ui/attributes/no-sanitize.stderr @@ -59,5 +59,22 @@ LL | #[no_sanitize(address, memory)] LL | static INVALID : i32 = 0; | ------------------------- not a function -error: aborting due to 7 previous errors +error: `#[no_sanitize()]` should be applied to a function + --> $DIR/no-sanitize.rs:42:15 + | +LL | #[no_sanitize("address")] + | ^^^^^^^^^ +... +LL | static VALID2 : i32 = 0; + | ------------------------ not a function + +error: invalid argument for `no_sanitize` + --> $DIR/no-sanitize.rs:42:15 + | +LL | #[no_sanitize("address")] + | ^^^^^^^^^ + | + = note: expected one of: `address`, `cfi`, `hwaddress`, `kcfi`, `memory`, `memtag`, `shadow-call-stack`, or `thread` + +error: aborting due to 9 previous errors From 2fef0a30ae6b2687dfb286cb544d2a542f7e2335 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 10 Apr 2025 14:33:59 +1000 Subject: [PATCH 24/27] Replace infallible `name_or_empty` methods with fallible `name` methods. I'm removing empty identifiers everywhere, because in practice they always mean "no identifier" rather than "empty identifier". (An empty identifier is impossible.) It's better to use `Option` to mean "no identifier" because you then can't forget about the "no identifier" possibility. Some specifics: - When testing an attribute for a single name, the commit uses the `has_name` method. - When testing an attribute for multiple names, the commit uses the new `has_any_name` method. - When using `match` on an attribute, the match arms now have `Some` on them. In the tests, we now avoid printing empty identifiers by not printing the identifier in the `error:` line at all, instead letting the carets point out the problem. --- compiler/rustc_ast/src/attr/mod.rs | 33 +++-- compiler/rustc_ast_lowering/src/item.rs | 2 +- .../rustc_ast_passes/src/ast_validation.rs | 5 +- .../rustc_attr_parsing/src/attributes/cfg.rs | 12 +- compiler/rustc_attr_parsing/src/context.rs | 6 +- .../src/deriving/generic/mod.rs | 5 +- .../rustc_codegen_ssa/src/codegen_attrs.rs | 33 ++--- compiler/rustc_expand/src/base.rs | 8 +- compiler/rustc_expand/src/expand.rs | 12 +- compiler/rustc_hir/src/hir.rs | 11 +- compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs | 2 +- compiler/rustc_hir_typeck/src/method/probe.rs | 7 +- compiler/rustc_incremental/messages.ftl | 2 +- compiler/rustc_incremental/src/errors.rs | 5 +- .../src/persist/dirty_clean.rs | 3 +- compiler/rustc_lint_defs/src/lib.rs | 2 +- compiler/rustc_metadata/src/native_libs.rs | 14 +-- compiler/rustc_metadata/src/rmeta/encoder.rs | 4 +- .../rustc_mir_build/src/builder/custom/mod.rs | 3 +- compiler/rustc_mir_build/src/builder/mod.rs | 2 +- compiler/rustc_mir_build/src/thir/cx/mod.rs | 2 +- .../src/framework/graphviz.rs | 5 +- compiler/rustc_passes/messages.ftl | 6 +- compiler/rustc_passes/src/abi_test.rs | 20 +-- compiler/rustc_passes/src/check_attr.rs | 116 +++++++++--------- .../rustc_passes/src/debugger_visualizer.rs | 22 ++-- compiler/rustc_passes/src/errors.rs | 7 +- compiler/rustc_passes/src/layout_test.rs | 18 +-- compiler/rustc_span/src/symbol.rs | 1 + src/librustdoc/clean/types.rs | 2 +- src/librustdoc/core.rs | 4 +- src/librustdoc/doctest/make.rs | 14 +-- src/librustdoc/html/render/context.rs | 12 +- src/tools/clippy/clippy_utils/src/lib.rs | 4 +- tests/ui/abi/debug.rs | 2 +- tests/ui/abi/debug.stderr | 2 +- .../invalid_macro_export_argument.deny.stderr | 4 +- .../invalid_macro_export_argument.rs | 4 +- tests/ui/attributes/no-sanitize.rs | 2 +- tests/ui/attributes/no-sanitize.stderr | 2 +- 40 files changed, 217 insertions(+), 203 deletions(-) diff --git a/compiler/rustc_ast/src/attr/mod.rs b/compiler/rustc_ast/src/attr/mod.rs index d656d9b0b8a..f104400a572 100644 --- a/compiler/rustc_ast/src/attr/mod.rs +++ b/compiler/rustc_ast/src/attr/mod.rs @@ -305,8 +305,8 @@ impl MetaItem { if let [PathSegment { ident, .. }] = self.path.segments[..] { Some(ident) } else { None } } - pub fn name_or_empty(&self) -> Symbol { - self.ident().unwrap_or_else(Ident::empty).name + pub fn name(&self) -> Option { + self.ident().map(|ident| ident.name) } pub fn has_name(&self, name: Symbol) -> bool { @@ -511,13 +511,14 @@ impl MetaItemInner { } } - /// For a single-segment meta item, returns its name; otherwise, returns `None`. + /// For a single-segment meta item, returns its identifier; otherwise, returns `None`. pub fn ident(&self) -> Option { self.meta_item().and_then(|meta_item| meta_item.ident()) } - pub fn name_or_empty(&self) -> Symbol { - self.ident().unwrap_or_else(Ident::empty).name + /// For a single-segment meta item, returns its name; otherwise, returns `None`. + pub fn name(&self) -> Option { + self.ident().map(|ident| ident.name) } /// Returns `true` if this list item is a MetaItem with a name of `name`. @@ -738,9 +739,9 @@ pub trait AttributeExt: Debug { fn id(&self) -> AttrId; /// For a single-segment attribute (i.e., `#[attr]` and not `#[path::atrr]`), - /// return the name of the attribute, else return the empty identifier. - fn name_or_empty(&self) -> Symbol { - self.ident().unwrap_or_else(Ident::empty).name + /// return the name of the attribute; otherwise, returns `None`. + fn name(&self) -> Option { + self.ident().map(|ident| ident.name) } /// Get the meta item list, `#[attr(meta item list)]` @@ -752,7 +753,7 @@ pub trait AttributeExt: Debug { /// Gets the span of the value literal, as string, when using `#[attr = value]` fn value_span(&self) -> Option; - /// For a single-segment attribute, returns its name; otherwise, returns `None`. + /// For a single-segment attribute, returns its ident; otherwise, returns `None`. fn ident(&self) -> Option; /// Checks whether the path of this attribute matches the name. @@ -770,6 +771,11 @@ pub trait AttributeExt: Debug { self.ident().map(|x| x.name == name).unwrap_or(false) } + #[inline] + fn has_any_name(&self, names: &[Symbol]) -> bool { + names.iter().any(|&name| self.has_name(name)) + } + /// get the span of the entire attribute fn span(&self) -> Span; @@ -813,8 +819,8 @@ impl Attribute { AttributeExt::id(self) } - pub fn name_or_empty(&self) -> Symbol { - AttributeExt::name_or_empty(self) + pub fn name(&self) -> Option { + AttributeExt::name(self) } pub fn meta_item_list(&self) -> Option> { @@ -846,6 +852,11 @@ impl Attribute { AttributeExt::has_name(self, name) } + #[inline] + pub fn has_any_name(&self, names: &[Symbol]) -> bool { + AttributeExt::has_any_name(self, names) + } + pub fn span(&self) -> Span { AttributeExt::span(self) } diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index d8b55bea3d7..fc32c4efce5 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -1310,7 +1310,7 @@ impl<'hir> LoweringContext<'_, 'hir> { // create a fake body so that the entire rest of the compiler doesn't have to deal with // this as a special case. return self.lower_fn_body(decl, contract, |this| { - if attrs.iter().any(|a| a.name_or_empty() == sym::rustc_intrinsic) { + if attrs.iter().any(|a| a.has_name(sym::rustc_intrinsic)) { let span = this.lower_span(span); let empty_block = hir::Block { hir_id: this.next_id(), diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index dc77e7b2834..daf8ff5dc65 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -347,7 +347,7 @@ impl<'a> AstValidator<'a> { sym::forbid, sym::warn, ]; - !arr.contains(&attr.name_or_empty()) && rustc_attr_parsing::is_builtin_attr(*attr) + !attr.has_any_name(&arr) && rustc_attr_parsing::is_builtin_attr(*attr) }) .for_each(|attr| { if attr.is_doc_comment() { @@ -945,8 +945,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { self.visit_attrs_vis_ident(&item.attrs, &item.vis, ident); self.check_defaultness(item.span, *defaultness); - let is_intrinsic = - item.attrs.iter().any(|a| a.name_or_empty() == sym::rustc_intrinsic); + let is_intrinsic = item.attrs.iter().any(|a| a.has_name(sym::rustc_intrinsic)); if body.is_none() && !is_intrinsic { self.dcx().emit_err(errors::FnWithoutBody { span: item.span, diff --git a/compiler/rustc_attr_parsing/src/attributes/cfg.rs b/compiler/rustc_attr_parsing/src/attributes/cfg.rs index 0d6d521b40c..a40112e27a2 100644 --- a/compiler/rustc_attr_parsing/src/attributes/cfg.rs +++ b/compiler/rustc_attr_parsing/src/attributes/cfg.rs @@ -117,7 +117,7 @@ pub fn eval_condition( }; match &cfg.kind { - MetaItemKind::List(mis) if cfg.name_or_empty() == sym::version => { + MetaItemKind::List(mis) if cfg.has_name(sym::version) => { try_gate_cfg(sym::version, cfg.span, sess, features); let (min_version, span) = match &mis[..] { [MetaItemInner::Lit(MetaItemLit { kind: LitKind::Str(sym, ..), span, .. })] => { @@ -164,18 +164,18 @@ pub fn eval_condition( // The unwraps below may look dangerous, but we've already asserted // that they won't fail with the loop above. - match cfg.name_or_empty() { - sym::any => mis + match cfg.name() { + Some(sym::any) => mis .iter() // We don't use any() here, because we want to evaluate all cfg condition // as eval_condition can (and does) extra checks .fold(false, |res, mi| res | eval_condition(mi, sess, features, eval)), - sym::all => mis + Some(sym::all) => mis .iter() // We don't use all() here, because we want to evaluate all cfg condition // as eval_condition can (and does) extra checks .fold(true, |res, mi| res & eval_condition(mi, sess, features, eval)), - sym::not => { + Some(sym::not) => { let [mi] = mis.as_slice() else { dcx.emit_err(session_diagnostics::ExpectedOneCfgPattern { span: cfg.span }); return false; @@ -183,7 +183,7 @@ pub fn eval_condition( !eval_condition(mi, sess, features, eval) } - sym::target => { + Some(sym::target) => { if let Some(features) = features && !features.cfg_target_compact() { diff --git a/compiler/rustc_attr_parsing/src/context.rs b/compiler/rustc_attr_parsing/src/context.rs index 3bf03f84ce8..972614a3366 100644 --- a/compiler/rustc_attr_parsing/src/context.rs +++ b/compiler/rustc_attr_parsing/src/context.rs @@ -222,7 +222,7 @@ impl<'sess> AttributeParser<'sess> { // if we're only looking for a single attribute, // skip all the ones we don't care about if let Some(expected) = self.parse_only { - if attr.name_or_empty() != expected { + if !attr.has_name(expected) { continue; } } @@ -232,7 +232,7 @@ impl<'sess> AttributeParser<'sess> { // that's expanded right? But no, sometimes, when parsing attributes on macros, // we already use the lowering logic and these are still there. So, when `omit_doc` // is set we *also* want to ignore these - if omit_doc == OmitDoc::Skip && attr.name_or_empty() == sym::doc { + if omit_doc == OmitDoc::Skip && attr.has_name(sym::doc) { continue; } @@ -250,7 +250,7 @@ impl<'sess> AttributeParser<'sess> { })) } // // FIXME: make doc attributes go through a proper attribute parser - // ast::AttrKind::Normal(n) if n.name_or_empty() == sym::doc => { + // ast::AttrKind::Normal(n) if n.has_name(sym::doc) => { // let p = GenericMetaItemParser::from_attr(&n, self.dcx()); // // attributes.push(Attribute::Parsed(AttributeKind::DocComment { diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index b9197be4442..d9aac54ee73 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -527,15 +527,14 @@ impl<'a> TraitDef<'a> { item.attrs .iter() .filter(|a| { - [ + a.has_any_name(&[ sym::allow, sym::warn, sym::deny, sym::forbid, sym::stable, sym::unstable, - ] - .contains(&a.name_or_empty()) + ]) }) .cloned(), ); diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index ddb61188983..449336424dc 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -345,20 +345,26 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { no_sanitize_span = Some(attr.span()); if let Some(list) = attr.meta_item_list() { for item in list.iter() { - match item.name_or_empty() { - sym::address => { + match item.name() { + Some(sym::address) => { codegen_fn_attrs.no_sanitize |= SanitizerSet::ADDRESS | SanitizerSet::KERNELADDRESS } - sym::cfi => codegen_fn_attrs.no_sanitize |= SanitizerSet::CFI, - sym::kcfi => codegen_fn_attrs.no_sanitize |= SanitizerSet::KCFI, - sym::memory => codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMORY, - sym::memtag => codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMTAG, - sym::shadow_call_stack => { + Some(sym::cfi) => codegen_fn_attrs.no_sanitize |= SanitizerSet::CFI, + Some(sym::kcfi) => codegen_fn_attrs.no_sanitize |= SanitizerSet::KCFI, + Some(sym::memory) => { + codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMORY + } + Some(sym::memtag) => { + codegen_fn_attrs.no_sanitize |= SanitizerSet::MEMTAG + } + Some(sym::shadow_call_stack) => { codegen_fn_attrs.no_sanitize |= SanitizerSet::SHADOWCALLSTACK } - sym::thread => codegen_fn_attrs.no_sanitize |= SanitizerSet::THREAD, - sym::hwaddress => { + Some(sym::thread) => { + codegen_fn_attrs.no_sanitize |= SanitizerSet::THREAD + } + Some(sym::hwaddress) => { codegen_fn_attrs.no_sanitize |= SanitizerSet::HWADDRESS } _ => { @@ -419,9 +425,9 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { continue; }; - let attrib_to_write = match meta_item.name_or_empty() { - sym::prefix_nops => &mut prefix, - sym::entry_nops => &mut entry, + let attrib_to_write = match meta_item.name() { + Some(sym::prefix_nops) => &mut prefix, + Some(sym::entry_nops) => &mut entry, _ => { tcx.dcx().emit_err(errors::UnexpectedParameterName { span: item.span(), @@ -785,8 +791,7 @@ impl<'a> MixedExportNameAndNoMangleState<'a> { fn autodiff_attrs(tcx: TyCtxt<'_>, id: DefId) -> Option { let attrs = tcx.get_attrs(id, sym::rustc_autodiff); - let attrs = - attrs.filter(|attr| attr.name_or_empty() == sym::rustc_autodiff).collect::>(); + let attrs = attrs.filter(|attr| attr.has_name(sym::rustc_autodiff)).collect::>(); // check for exactly one autodiff attribute on placeholder functions. // There should only be one, since we generate a new placeholder per ad macro. diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 49f6d58172f..f5eaf7d616b 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -824,10 +824,10 @@ impl SyntaxExtension { return Err(item.span); } - match item.name_or_empty() { - sym::no => Ok(CollapseMacroDebuginfo::No), - sym::external => Ok(CollapseMacroDebuginfo::External), - sym::yes => Ok(CollapseMacroDebuginfo::Yes), + match item.name() { + Some(sym::no) => Ok(CollapseMacroDebuginfo::No), + Some(sym::external) => Ok(CollapseMacroDebuginfo::External), + Some(sym::yes) => Ok(CollapseMacroDebuginfo::Yes), _ => Err(item.path.span), } } diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 1b539477d51..1e26d668194 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -2053,8 +2053,8 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { ) -> Node::OutputTy { loop { return match self.take_first_attr(&mut node) { - Some((attr, pos, derives)) => match attr.name_or_empty() { - sym::cfg => { + Some((attr, pos, derives)) => match attr.name() { + Some(sym::cfg) => { let (res, meta_item) = self.expand_cfg_true(&mut node, attr, pos); if res { continue; @@ -2071,7 +2071,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { } Default::default() } - sym::cfg_attr => { + Some(sym::cfg_attr) => { self.expand_cfg_attr(&mut node, &attr, pos); continue; } @@ -2144,8 +2144,8 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { ) { loop { return match self.take_first_attr(node) { - Some((attr, pos, derives)) => match attr.name_or_empty() { - sym::cfg => { + Some((attr, pos, derives)) => match attr.name() { + Some(sym::cfg) => { let span = attr.span; if self.expand_cfg_true(node, attr, pos).0 { continue; @@ -2154,7 +2154,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { node.expand_cfg_false(self, pos, span); continue; } - sym::cfg_attr => { + Some(sym::cfg_attr) => { self.expand_cfg_attr(node, &attr, pos); continue; } diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 6455f33b9d1..897831f6276 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1237,7 +1237,7 @@ impl AttributeExt for Attribute { Attribute::Parsed(AttributeKind::DocComment { kind, comment, .. }) => { Some((*comment, *kind)) } - Attribute::Unparsed(_) if self.name_or_empty() == sym::doc => { + Attribute::Unparsed(_) if self.has_name(sym::doc) => { self.value_str().map(|s| (s, CommentKind::Line)) } _ => None, @@ -1262,8 +1262,8 @@ impl Attribute { } #[inline] - pub fn name_or_empty(&self) -> Symbol { - AttributeExt::name_or_empty(self) + pub fn name(&self) -> Option { + AttributeExt::name(self) } #[inline] @@ -1301,6 +1301,11 @@ impl Attribute { AttributeExt::has_name(self, name) } + #[inline] + pub fn has_any_name(&self, names: &[Symbol]) -> bool { + AttributeExt::has_any_name(self, names) + } + #[inline] pub fn span(&self) -> Span { AttributeExt::span(self) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs index 74cc8181418..934820eb4da 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs @@ -488,7 +488,7 @@ fn parse_never_type_options_attr( item.span(), format!( "unknown or duplicate never type option: `{}` (supported: `fallback`, `diverging_block_default`)", - item.name_or_empty() + item.name().unwrap() ), ); } diff --git a/compiler/rustc_hir_typeck/src/method/probe.rs b/compiler/rustc_hir_typeck/src/method/probe.rs index ba4396a5ab3..1d3a081cbb8 100644 --- a/compiler/rustc_hir_typeck/src/method/probe.rs +++ b/compiler/rustc_hir_typeck/src/method/probe.rs @@ -2334,8 +2334,9 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { let hir_id = self.fcx.tcx.local_def_id_to_hir_id(local_def_id); let attrs = self.fcx.tcx.hir_attrs(hir_id); for attr in attrs { - if sym::doc == attr.name_or_empty() { - } else if sym::rustc_confusables == attr.name_or_empty() { + if attr.has_name(sym::doc) { + // do nothing + } else if attr.has_name(sym::rustc_confusables) { let Some(confusables) = attr.meta_item_list() else { continue; }; @@ -2355,7 +2356,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { continue; }; for v in values { - if v.name_or_empty() != sym::alias { + if !v.has_name(sym::alias) { continue; } if let Some(nested) = v.meta_item_list() { diff --git a/compiler/rustc_incremental/messages.ftl b/compiler/rustc_incremental/messages.ftl index 2a65101d360..bbc1fab05df 100644 --- a/compiler/rustc_incremental/messages.ftl +++ b/compiler/rustc_incremental/messages.ftl @@ -93,7 +93,7 @@ incremental_undefined_clean_dirty_assertions = incremental_undefined_clean_dirty_assertions_item = clean/dirty auto-assertions not yet defined for Node::Item.node={$kind} -incremental_unknown_item = unknown item `{$name}` +incremental_unknown_rustc_clean_argument = unknown `rustc_clean` argument incremental_unrecognized_depnode = unrecognized `DepNode` variant: {$name} diff --git a/compiler/rustc_incremental/src/errors.rs b/compiler/rustc_incremental/src/errors.rs index b4a207386dc..dbc72d085be 100644 --- a/compiler/rustc_incremental/src/errors.rs +++ b/compiler/rustc_incremental/src/errors.rs @@ -107,11 +107,10 @@ pub(crate) struct NotLoaded<'a> { } #[derive(Diagnostic)] -#[diag(incremental_unknown_item)] -pub(crate) struct UnknownItem { +#[diag(incremental_unknown_rustc_clean_argument)] +pub(crate) struct UnknownRustcCleanArgument { #[primary_span] pub span: Span, - pub name: Symbol, } #[derive(Diagnostic)] diff --git a/compiler/rustc_incremental/src/persist/dirty_clean.rs b/compiler/rustc_incremental/src/persist/dirty_clean.rs index d40a0d514f6..64166255fa4 100644 --- a/compiler/rustc_incremental/src/persist/dirty_clean.rs +++ b/compiler/rustc_incremental/src/persist/dirty_clean.rs @@ -405,8 +405,7 @@ fn check_config(tcx: TyCtxt<'_>, attr: &Attribute) -> bool { debug!("check_config: searching for cfg {:?}", value); cfg = Some(config.contains(&(value, None))); } else if !(item.has_name(EXCEPT) || item.has_name(LOADED_FROM_DISK)) { - tcx.dcx() - .emit_err(errors::UnknownItem { span: attr.span(), name: item.name_or_empty() }); + tcx.dcx().emit_err(errors::UnknownRustcCleanArgument { span: item.span() }); } } diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index 7fdbae3a59d..b4069b317bf 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -249,7 +249,7 @@ impl Level { /// Converts an `Attribute` to a level. pub fn from_attr(attr: &impl AttributeExt) -> Option<(Self, Option)> { - Self::from_symbol(attr.name_or_empty(), || Some(attr.id())) + attr.name().and_then(|name| Self::from_symbol(name, || Some(attr.id()))) } /// Converts a `Symbol` to a level. diff --git a/compiler/rustc_metadata/src/native_libs.rs b/compiler/rustc_metadata/src/native_libs.rs index cfb0de8475c..cee9cff0775 100644 --- a/compiler/rustc_metadata/src/native_libs.rs +++ b/compiler/rustc_metadata/src/native_libs.rs @@ -226,8 +226,8 @@ impl<'tcx> Collector<'tcx> { let mut wasm_import_module = None; let mut import_name_type = None; for item in items.iter() { - match item.name_or_empty() { - sym::name => { + match item.name() { + Some(sym::name) => { if name.is_some() { sess.dcx().emit_err(errors::MultipleNamesInLink { span: item.span() }); continue; @@ -242,7 +242,7 @@ impl<'tcx> Collector<'tcx> { } name = Some((link_name, span)); } - sym::kind => { + Some(sym::kind) => { if kind.is_some() { sess.dcx().emit_err(errors::MultipleKindsInLink { span: item.span() }); continue; @@ -304,7 +304,7 @@ impl<'tcx> Collector<'tcx> { }; kind = Some(link_kind); } - sym::modifiers => { + Some(sym::modifiers) => { if modifiers.is_some() { sess.dcx() .emit_err(errors::MultipleLinkModifiers { span: item.span() }); @@ -316,7 +316,7 @@ impl<'tcx> Collector<'tcx> { }; modifiers = Some((link_modifiers, item.name_value_literal_span().unwrap())); } - sym::cfg => { + Some(sym::cfg) => { if cfg.is_some() { sess.dcx().emit_err(errors::MultipleCfgs { span: item.span() }); continue; @@ -346,7 +346,7 @@ impl<'tcx> Collector<'tcx> { } cfg = Some(link_cfg.clone()); } - sym::wasm_import_module => { + Some(sym::wasm_import_module) => { if wasm_import_module.is_some() { sess.dcx().emit_err(errors::MultipleWasmImport { span: item.span() }); continue; @@ -357,7 +357,7 @@ impl<'tcx> Collector<'tcx> { }; wasm_import_module = Some((link_wasm_import_module, item.span())); } - sym::import_name_type => { + Some(sym::import_name_type) => { if import_name_type.is_some() { sess.dcx() .emit_err(errors::MultipleImportNameType { span: item.span() }); diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 177318bfe15..3ea61d1b40a 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -821,7 +821,9 @@ struct AnalyzeAttrState<'a> { #[inline] fn analyze_attr(attr: &impl AttributeExt, state: &mut AnalyzeAttrState<'_>) -> bool { let mut should_encode = false; - if !rustc_feature::encode_cross_crate(attr.name_or_empty()) { + if let Some(name) = attr.name() + && !rustc_feature::encode_cross_crate(name) + { // Attributes not marked encode-cross-crate don't need to be encoded for downstream crates. } else if attr.doc_str().is_some() { // We keep all doc comments reachable to rustdoc because they might be "imported" into diff --git a/compiler/rustc_mir_build/src/builder/custom/mod.rs b/compiler/rustc_mir_build/src/builder/custom/mod.rs index bfc16816e2e..902a6e7f115 100644 --- a/compiler/rustc_mir_build/src/builder/custom/mod.rs +++ b/compiler/rustc_mir_build/src/builder/custom/mod.rs @@ -103,8 +103,9 @@ fn parse_attribute(attr: &Attribute) -> MirPhase { let mut dialect: Option = None; let mut phase: Option = None; + // Not handling errors properly for this internal attribute; will just abort on errors. for nested in meta_items { - let name = nested.name_or_empty(); + let name = nested.name().unwrap(); let value = nested.value_str().unwrap().as_str().to_string(); match name.as_str() { "dialect" => { diff --git a/compiler/rustc_mir_build/src/builder/mod.rs b/compiler/rustc_mir_build/src/builder/mod.rs index 8ca9ab58e45..59a52ae67cb 100644 --- a/compiler/rustc_mir_build/src/builder/mod.rs +++ b/compiler/rustc_mir_build/src/builder/mod.rs @@ -485,7 +485,7 @@ fn construct_fn<'tcx>( }; if let Some(custom_mir_attr) = - tcx.hir_attrs(fn_id).iter().find(|attr| attr.name_or_empty() == sym::custom_mir) + tcx.hir_attrs(fn_id).iter().find(|attr| attr.has_name(sym::custom_mir)) { return custom::build_custom_mir( tcx, diff --git a/compiler/rustc_mir_build/src/thir/cx/mod.rs b/compiler/rustc_mir_build/src/thir/cx/mod.rs index b3daed8a7e0..2f593b9a0a7 100644 --- a/compiler/rustc_mir_build/src/thir/cx/mod.rs +++ b/compiler/rustc_mir_build/src/thir/cx/mod.rs @@ -113,7 +113,7 @@ impl<'tcx> ThirBuildCx<'tcx> { apply_adjustments: tcx .hir_attrs(hir_id) .iter() - .all(|attr| attr.name_or_empty() != rustc_span::sym::custom_mir), + .all(|attr| !attr.has_name(rustc_span::sym::custom_mir)), } } diff --git a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs index c436b8c0fb0..144fa8c6fdd 100644 --- a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs +++ b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs @@ -144,9 +144,10 @@ impl RustcMirAttrs { attr: &ast::MetaItemInner, mapper: impl FnOnce(Symbol) -> Result, ) -> Result<(), ()> { + // Unwrapping the name is safe because this is only called when `has_name` has succeeded. if field.is_some() { tcx.dcx() - .emit_err(DuplicateValuesFor { span: attr.span(), name: attr.name_or_empty() }); + .emit_err(DuplicateValuesFor { span: attr.span(), name: attr.name().unwrap() }); return Err(()); } @@ -156,7 +157,7 @@ impl RustcMirAttrs { Ok(()) } else { tcx.dcx() - .emit_err(RequiresAnArgument { span: attr.span(), name: attr.name_or_empty() }); + .emit_err(RequiresAnArgument { span: attr.span(), name: attr.name().unwrap() }); Err(()) } } diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index 6ee5e356435..99789b74488 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -404,7 +404,7 @@ passes_invalid_attr_at_crate_level = passes_invalid_attr_at_crate_level_item = the inner attribute doesn't annotate this {$kind} -passes_invalid_macro_export_arguments = `{$name}` isn't a valid `#[macro_export]` argument +passes_invalid_macro_export_arguments = invalid `#[macro_export]` argument passes_invalid_macro_export_arguments_too_many_items = `#[macro_export]` can only take 1 or 0 arguments @@ -771,8 +771,8 @@ passes_unreachable_due_to_uninhabited = unreachable {$descr} .label_orig = any code following this expression is unreachable .note = this expression has type `{$ty}`, which is uninhabited -passes_unrecognized_field = - unrecognized field name `{$name}` +passes_unrecognized_argument = + unrecognized argument passes_unstable_attr_for_already_stable_feature = can't mark as unstable using an already stable feature diff --git a/compiler/rustc_passes/src/abi_test.rs b/compiler/rustc_passes/src/abi_test.rs index 671b7d7ad76..b139ed6a66c 100644 --- a/compiler/rustc_passes/src/abi_test.rs +++ b/compiler/rustc_passes/src/abi_test.rs @@ -9,7 +9,7 @@ use rustc_span::sym; use rustc_target::callconv::FnAbi; use super::layout_test::ensure_wf; -use crate::errors::{AbiInvalidAttribute, AbiNe, AbiOf, UnrecognizedField}; +use crate::errors::{AbiInvalidAttribute, AbiNe, AbiOf, UnrecognizedArgument}; pub fn test_abi(tcx: TyCtxt<'_>) { if !tcx.features().rustc_attrs() { @@ -77,8 +77,8 @@ fn dump_abi_of_fn_item(tcx: TyCtxt<'_>, item_def_id: LocalDefId, attr: &Attribut // The `..` are the names of fields to dump. let meta_items = attr.meta_item_list().unwrap_or_default(); for meta_item in meta_items { - match meta_item.name_or_empty() { - sym::debug => { + match meta_item.name() { + Some(sym::debug) => { let fn_name = tcx.item_name(item_def_id.into()); tcx.dcx().emit_err(AbiOf { span: tcx.def_span(item_def_id), @@ -88,8 +88,8 @@ fn dump_abi_of_fn_item(tcx: TyCtxt<'_>, item_def_id: LocalDefId, attr: &Attribut }); } - name => { - tcx.dcx().emit_err(UnrecognizedField { span: meta_item.span(), name }); + _ => { + tcx.dcx().emit_err(UnrecognizedArgument { span: meta_item.span() }); } } } @@ -118,8 +118,8 @@ fn dump_abi_of_fn_type(tcx: TyCtxt<'_>, item_def_id: LocalDefId, attr: &Attribut } let meta_items = attr.meta_item_list().unwrap_or_default(); for meta_item in meta_items { - match meta_item.name_or_empty() { - sym::debug => { + match meta_item.name() { + Some(sym::debug) => { let ty::FnPtr(sig_tys, hdr) = ty.kind() else { span_bug!( meta_item.span(), @@ -138,7 +138,7 @@ fn dump_abi_of_fn_type(tcx: TyCtxt<'_>, item_def_id: LocalDefId, attr: &Attribut let fn_name = tcx.item_name(item_def_id.into()); tcx.dcx().emit_err(AbiOf { span, fn_name, fn_abi: format!("{:#?}", abi) }); } - sym::assert_eq => { + Some(sym::assert_eq) => { let ty::Tuple(fields) = ty.kind() else { span_bug!( meta_item.span(), @@ -188,8 +188,8 @@ fn dump_abi_of_fn_type(tcx: TyCtxt<'_>, item_def_id: LocalDefId, attr: &Attribut }); } } - name => { - tcx.dcx().emit_err(UnrecognizedField { span: meta_item.span(), name }); + _ => { + tcx.dcx().emit_err(UnrecognizedArgument { span: meta_item.span() }); } } } diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index dcfecdc26ea..cbe5058b551 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -523,9 +523,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> { fn check_no_sanitize(&self, attr: &Attribute, span: Span, target: Target) { if let Some(list) = attr.meta_item_list() { for item in list.iter() { - let sym = item.name_or_empty(); + let sym = item.name(); match sym { - sym::address | sym::hwaddress => { + Some(s @ sym::address | s @ sym::hwaddress) => { let is_valid = matches!(target, Target::Fn | Target::Method(..) | Target::Static); if !is_valid { @@ -533,7 +533,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { attr_span: item.span(), defn_span: span, accepted_kind: "a function or static", - attr_str: sym.as_str(), + attr_str: s.as_str(), }); } } @@ -544,7 +544,10 @@ impl<'tcx> CheckAttrVisitor<'tcx> { attr_span: item.span(), defn_span: span, accepted_kind: "a function", - attr_str: sym.as_str(), + attr_str: &match sym { + Some(name) => name.to_string(), + None => "...".to_string(), + }, }); } } @@ -592,7 +595,8 @@ impl<'tcx> CheckAttrVisitor<'tcx> { // * `#[track_caller]` // * `#[test]`, `#[ignore]`, `#[should_panic]` // - // NOTE: when making changes to this list, check that `error_codes/E0736.md` remains accurate + // NOTE: when making changes to this list, check that `error_codes/E0736.md` remains + // accurate. const ALLOW_LIST: &[rustc_span::Symbol] = &[ // conditional compilation sym::cfg_trace, @@ -675,11 +679,11 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } } - if !ALLOW_LIST.iter().any(|name| other_attr.has_name(*name)) { + if !other_attr.has_any_name(ALLOW_LIST) { self.dcx().emit_err(errors::NakedFunctionIncompatibleAttribute { span: other_attr.span(), naked_span: attr.span(), - attr: other_attr.name_or_empty(), + attr: other_attr.name().unwrap(), }); return; @@ -1153,7 +1157,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { ) { match target { Target::Use | Target::ExternCrate => { - let do_inline = meta.name_or_empty() == sym::inline; + let do_inline = meta.has_name(sym::inline); if let Some((prev_inline, prev_span)) = *specified_inline { if do_inline != prev_inline { let mut spans = MultiSpan::from_spans(vec![prev_span, meta.span()]); @@ -1263,8 +1267,8 @@ impl<'tcx> CheckAttrVisitor<'tcx> { fn check_test_attr(&self, meta: &MetaItemInner, hir_id: HirId) { if let Some(metas) = meta.meta_item_list() { for i_meta in metas { - match (i_meta.name_or_empty(), i_meta.meta_item()) { - (sym::attr | sym::no_crate_inject, _) => {} + match (i_meta.name(), i_meta.meta_item()) { + (Some(sym::attr | sym::no_crate_inject), _) => {} (_, Some(m)) => { self.tcx.emit_node_span_lint( INVALID_DOC_ATTRIBUTES, @@ -1325,61 +1329,63 @@ impl<'tcx> CheckAttrVisitor<'tcx> { if let Some(list) = attr.meta_item_list() { for meta in &list { if let Some(i_meta) = meta.meta_item() { - match i_meta.name_or_empty() { - sym::alias => { + match i_meta.name() { + Some(sym::alias) => { if self.check_attr_not_crate_level(meta, hir_id, "alias") { self.check_doc_alias(meta, hir_id, target, aliases); } } - sym::keyword => { + Some(sym::keyword) => { if self.check_attr_not_crate_level(meta, hir_id, "keyword") { self.check_doc_keyword(meta, hir_id); } } - sym::fake_variadic => { + Some(sym::fake_variadic) => { if self.check_attr_not_crate_level(meta, hir_id, "fake_variadic") { self.check_doc_fake_variadic(meta, hir_id); } } - sym::search_unbox => { + Some(sym::search_unbox) => { if self.check_attr_not_crate_level(meta, hir_id, "fake_variadic") { self.check_doc_search_unbox(meta, hir_id); } } - sym::test => { + Some(sym::test) => { if self.check_attr_crate_level(attr, meta, hir_id) { self.check_test_attr(meta, hir_id); } } - sym::html_favicon_url - | sym::html_logo_url - | sym::html_playground_url - | sym::issue_tracker_base_url - | sym::html_root_url - | sym::html_no_source => { + Some( + sym::html_favicon_url + | sym::html_logo_url + | sym::html_playground_url + | sym::issue_tracker_base_url + | sym::html_root_url + | sym::html_no_source, + ) => { self.check_attr_crate_level(attr, meta, hir_id); } - sym::cfg_hide => { + Some(sym::cfg_hide) => { if self.check_attr_crate_level(attr, meta, hir_id) { self.check_doc_cfg_hide(meta, hir_id); } } - sym::inline | sym::no_inline => { + Some(sym::inline | sym::no_inline) => { self.check_doc_inline(attr, meta, hir_id, target, specified_inline) } - sym::masked => self.check_doc_masked(attr, meta, hir_id, target), + Some(sym::masked) => self.check_doc_masked(attr, meta, hir_id, target), - sym::cfg | sym::hidden | sym::notable_trait => {} + Some(sym::cfg | sym::hidden | sym::notable_trait) => {} - sym::rust_logo => { + Some(sym::rust_logo) => { if self.check_attr_crate_level(attr, meta, hir_id) && !self.tcx.features().rustdoc_internals() { @@ -2302,7 +2308,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } fn check_macro_use(&self, hir_id: HirId, attr: &Attribute, target: Target) { - let name = attr.name_or_empty(); + let name = attr.name().unwrap(); match target { Target::ExternCrate | Target::Mod => {} _ => { @@ -2334,12 +2340,12 @@ impl<'tcx> CheckAttrVisitor<'tcx> { attr.span(), errors::MacroExport::TooManyItems, ); - } else if meta_item_list[0].name_or_empty() != sym::local_inner_macros { + } else if !meta_item_list[0].has_name(sym::local_inner_macros) { self.tcx.emit_node_span_lint( INVALID_MACRO_EXPORT_ARGUMENTS, hir_id, meta_item_list[0].span(), - errors::MacroExport::UnknownItem { name: meta_item_list[0].name_or_empty() }, + errors::MacroExport::InvalidArgument, ); } } else { @@ -2384,33 +2390,28 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } // Warn on useless empty attributes. - let note = if (matches!( - attr.name_or_empty(), - sym::macro_use - | sym::allow - | sym::expect - | sym::warn - | sym::deny - | sym::forbid - | sym::feature - | sym::target_feature - ) && attr.meta_item_list().is_some_and(|list| list.is_empty())) + let note = if attr.has_any_name(&[ + sym::macro_use, + sym::allow, + sym::expect, + sym::warn, + sym::deny, + sym::forbid, + sym::feature, + sym::target_feature, + ]) && attr.meta_item_list().is_some_and(|list| list.is_empty()) { - errors::UnusedNote::EmptyList { name: attr.name_or_empty() } - } else if matches!( - attr.name_or_empty(), - sym::allow | sym::warn | sym::deny | sym::forbid | sym::expect - ) && let Some(meta) = attr.meta_item_list() + errors::UnusedNote::EmptyList { name: attr.name().unwrap() } + } else if attr.has_any_name(&[sym::allow, sym::warn, sym::deny, sym::forbid, sym::expect]) + && let Some(meta) = attr.meta_item_list() && let [meta] = meta.as_slice() && let Some(item) = meta.meta_item() && let MetaItemKind::NameValue(_) = &item.kind && item.path == sym::reason { - errors::UnusedNote::NoLints { name: attr.name_or_empty() } - } else if matches!( - attr.name_or_empty(), - sym::allow | sym::warn | sym::deny | sym::forbid | sym::expect - ) && let Some(meta) = attr.meta_item_list() + errors::UnusedNote::NoLints { name: attr.name().unwrap() } + } else if attr.has_any_name(&[sym::allow, sym::warn, sym::deny, sym::forbid, sym::expect]) + && let Some(meta) = attr.meta_item_list() && meta.iter().any(|meta| { meta.meta_item().map_or(false, |item| item.path == sym::linker_messages) }) @@ -2443,7 +2444,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { return; } } - } else if attr.name_or_empty() == sym::default_method_body_is_const { + } else if attr.has_name(sym::default_method_body_is_const) { errors::UnusedNote::DefaultMethodBodyConst } else { return; @@ -2900,10 +2901,11 @@ fn check_duplicates( if matches!(duplicates, WarnFollowingWordOnly) && !attr.is_word() { return; } + let attr_name = attr.name().unwrap(); match duplicates { DuplicatesOk => {} WarnFollowing | FutureWarnFollowing | WarnFollowingWordOnly | FutureWarnPreceding => { - match seen.entry(attr.name_or_empty()) { + match seen.entry(attr_name) { Entry::Occupied(mut entry) => { let (this, other) = if matches!(duplicates, FutureWarnPreceding) { let to_remove = entry.insert(attr.span()); @@ -2930,7 +2932,7 @@ fn check_duplicates( } } } - ErrorFollowing | ErrorPreceding => match seen.entry(attr.name_or_empty()) { + ErrorFollowing | ErrorPreceding => match seen.entry(attr_name) { Entry::Occupied(mut entry) => { let (this, other) = if matches!(duplicates, ErrorPreceding) { let to_remove = entry.insert(attr.span()); @@ -2938,11 +2940,7 @@ fn check_duplicates( } else { (attr.span(), *entry.get()) }; - tcx.dcx().emit_err(errors::UnusedMultiple { - this, - other, - name: attr.name_or_empty(), - }); + tcx.dcx().emit_err(errors::UnusedMultiple { this, other, name: attr_name }); } Entry::Vacant(entry) => { entry.insert(attr.span()); diff --git a/compiler/rustc_passes/src/debugger_visualizer.rs b/compiler/rustc_passes/src/debugger_visualizer.rs index 062d56a79a0..7a7a8175e55 100644 --- a/compiler/rustc_passes/src/debugger_visualizer.rs +++ b/compiler/rustc_passes/src/debugger_visualizer.rs @@ -28,17 +28,17 @@ impl DebuggerVisualizerCollector<'_> { return; }; - let (visualizer_type, visualizer_path) = - match (meta_item.name_or_empty(), meta_item.value_str()) { - (sym::natvis_file, Some(value)) => (DebuggerVisualizerType::Natvis, value), - (sym::gdb_script_file, Some(value)) => { - (DebuggerVisualizerType::GdbPrettyPrinter, value) - } - (_, _) => { - self.sess.dcx().emit_err(DebugVisualizerInvalid { span: meta_item.span }); - return; - } - }; + let (visualizer_type, visualizer_path) = match (meta_item.name(), meta_item.value_str()) + { + (Some(sym::natvis_file), Some(value)) => (DebuggerVisualizerType::Natvis, value), + (Some(sym::gdb_script_file), Some(value)) => { + (DebuggerVisualizerType::GdbPrettyPrinter, value) + } + (_, _) => { + self.sess.dcx().emit_err(DebugVisualizerInvalid { span: meta_item.span }); + return; + } + }; let file = match resolve_path(&self.sess, visualizer_path.as_str(), attr.span) { Ok(file) => file, diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 075927a54f8..26ee89a7164 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -756,7 +756,7 @@ pub(crate) enum MacroExport { OnDeclMacro, #[diag(passes_invalid_macro_export_arguments)] - UnknownItem { name: Symbol }, + InvalidArgument, #[diag(passes_invalid_macro_export_arguments_too_many_items)] TooManyItems, @@ -1045,11 +1045,10 @@ pub(crate) struct AbiInvalidAttribute { } #[derive(Diagnostic)] -#[diag(passes_unrecognized_field)] -pub(crate) struct UnrecognizedField { +#[diag(passes_unrecognized_argument)] +pub(crate) struct UnrecognizedArgument { #[primary_span] pub span: Span, - pub name: Symbol, } #[derive(Diagnostic)] diff --git a/compiler/rustc_passes/src/layout_test.rs b/compiler/rustc_passes/src/layout_test.rs index d4512c9417e..a19faf0fa83 100644 --- a/compiler/rustc_passes/src/layout_test.rs +++ b/compiler/rustc_passes/src/layout_test.rs @@ -13,7 +13,7 @@ use rustc_trait_selection::traits; use crate::errors::{ LayoutAbi, LayoutAlign, LayoutHomogeneousAggregate, LayoutInvalidAttribute, LayoutOf, - LayoutSize, UnrecognizedField, + LayoutSize, UnrecognizedArgument, }; pub fn test_layout(tcx: TyCtxt<'_>) { @@ -79,28 +79,28 @@ fn dump_layout_of(tcx: TyCtxt<'_>, item_def_id: LocalDefId, attr: &Attribute) { // The `..` are the names of fields to dump. let meta_items = attr.meta_item_list().unwrap_or_default(); for meta_item in meta_items { - match meta_item.name_or_empty() { + match meta_item.name() { // FIXME: this never was about ABI and now this dump arg is confusing - sym::abi => { + Some(sym::abi) => { tcx.dcx().emit_err(LayoutAbi { span, abi: format!("{:?}", ty_layout.backend_repr), }); } - sym::align => { + Some(sym::align) => { tcx.dcx().emit_err(LayoutAlign { span, align: format!("{:?}", ty_layout.align), }); } - sym::size => { + Some(sym::size) => { tcx.dcx() .emit_err(LayoutSize { span, size: format!("{:?}", ty_layout.size) }); } - sym::homogeneous_aggregate => { + Some(sym::homogeneous_aggregate) => { tcx.dcx().emit_err(LayoutHomogeneousAggregate { span, homogeneous_aggregate: format!( @@ -111,15 +111,15 @@ fn dump_layout_of(tcx: TyCtxt<'_>, item_def_id: LocalDefId, attr: &Attribute) { }); } - sym::debug => { + Some(sym::debug) => { let normalized_ty = tcx.normalize_erasing_regions(typing_env, ty); // FIXME: using the `Debug` impl here isn't ideal. let ty_layout = format!("{:#?}", *ty_layout); tcx.dcx().emit_err(LayoutOf { span, normalized_ty, ty_layout }); } - name => { - tcx.dcx().emit_err(UnrecognizedField { span: meta_item.span(), name }); + _ => { + tcx.dcx().emit_err(UnrecognizedArgument { span: meta_item.span() }); } } } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 2af567f2ec5..6b9ce92b183 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1185,6 +1185,7 @@ symbols! { instruction_set, integer_: "integer", // underscore to avoid clashing with the function `sym::integer` below integral, + internal_features, into_async_iter_into_iter, into_future, into_iter, diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 31aa84535cc..b084eded403 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -788,7 +788,7 @@ impl Item { } _ => Some(rustc_hir_pretty::attribute_to_string(&tcx, attr)), } - } else if ALLOWED_ATTRIBUTES.contains(&attr.name_or_empty()) { + } else if attr.has_any_name(ALLOWED_ATTRIBUTES) { Some( rustc_hir_pretty::attribute_to_string(&tcx, attr) .replace("\\\n", "") diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index c4dea79370d..f7695ac8635 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -412,9 +412,7 @@ pub(crate) fn run_global_ctxt( // Process all of the crate attributes, extracting plugin metadata along // with the passes which we are supposed to run. for attr in krate.module.attrs.lists(sym::doc) { - let name = attr.name_or_empty(); - - if attr.is_word() && name == sym::document_private_items { + if attr.is_word() && attr.has_name(sym::document_private_items) { ctxt.render_options.document_private = true; } } diff --git a/src/librustdoc/doctest/make.rs b/src/librustdoc/doctest/make.rs index 4edd5433de6..d5c965f7053 100644 --- a/src/librustdoc/doctest/make.rs +++ b/src/librustdoc/doctest/make.rs @@ -345,7 +345,7 @@ fn parse_source(source: &str, crate_name: &Option<&str>) -> Result) -> bool { let mut is_extern_crate = false; if !info.has_global_allocator - && item.attrs.iter().any(|attr| attr.name_or_empty() == sym::global_allocator) + && item.attrs.iter().any(|attr| attr.has_name(sym::global_allocator)) { info.has_global_allocator = true; } @@ -377,7 +377,7 @@ fn parse_source(source: &str, crate_name: &Option<&str>) -> Result) -> Result { for attr in &item.attrs { - let attr_name = attr.name_or_empty(); - - if attr.style == AttrStyle::Outer || not_crate_attrs.contains(&attr_name) { + if attr.style == AttrStyle::Outer || attr.has_any_name(not_crate_attrs) { // There is one exception to these attributes: // `#![allow(internal_features)]`. If this attribute is used, we need to // consider it only as a crate-level attribute. - if attr_name == sym::allow + if attr.has_name(sym::allow) && let Some(list) = attr.meta_item_list() - && list.iter().any(|sub_attr| { - sub_attr.name_or_empty().as_str() == "internal_features" - }) + && list.iter().any(|sub_attr| sub_attr.has_name(sym::internal_features)) { push_to_s(&mut info.crate_attrs, source, attr.span, &mut prev_span_hi); } else { diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 596ac665fc3..f22935df96c 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -521,23 +521,23 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { // Crawl the crate attributes looking for attributes which control how we're // going to emit HTML for attr in krate.module.attrs.lists(sym::doc) { - match (attr.name_or_empty(), attr.value_str()) { - (sym::html_favicon_url, Some(s)) => { + match (attr.name(), attr.value_str()) { + (Some(sym::html_favicon_url), Some(s)) => { layout.favicon = s.to_string(); } - (sym::html_logo_url, Some(s)) => { + (Some(sym::html_logo_url), Some(s)) => { layout.logo = s.to_string(); } - (sym::html_playground_url, Some(s)) => { + (Some(sym::html_playground_url), Some(s)) => { playground = Some(markdown::Playground { crate_name: Some(krate.name(tcx)), url: s.to_string(), }); } - (sym::issue_tracker_base_url, Some(s)) => { + (Some(sym::issue_tracker_base_url), Some(s)) => { issue_tracker_base_url = Some(s.to_string()); } - (sym::html_no_source, None) if attr.is_word() => { + (Some(sym::html_no_source), None) if attr.is_word() => { include_sources = false; } _ => {} diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index 9f450d654d5..f715fc86e4e 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -2363,14 +2363,14 @@ pub fn is_no_std_crate(cx: &LateContext<'_>) -> bool { cx.tcx .hir_attrs(hir::CRATE_HIR_ID) .iter() - .any(|attr| attr.name_or_empty() == sym::no_std) + .any(|attr| attr.has_name(sym::no_std)) } pub fn is_no_core_crate(cx: &LateContext<'_>) -> bool { cx.tcx .hir_attrs(hir::CRATE_HIR_ID) .iter() - .any(|attr| attr.name_or_empty() == sym::no_core) + .any(|attr| attr.has_name(sym::no_core)) } /// Check if parent of a hir node is a trait implementation block. diff --git a/tests/ui/abi/debug.rs b/tests/ui/abi/debug.rs index e3260191e5c..c0d8de05fda 100644 --- a/tests/ui/abi/debug.rs +++ b/tests/ui/abi/debug.rs @@ -53,5 +53,5 @@ type TestAbiNeSign = (fn(i32), fn(u32)); //~ ERROR: ABIs are not compatible #[rustc_abi(assert_eq)] type TestAbiEqNonsense = (fn((str, str)), fn((str, str))); //~ ERROR: cannot be known at compilation time -#[rustc_abi("assert_eq")] //~ ERROR unrecognized field name `` +#[rustc_abi("assert_eq")] //~ ERROR unrecognized argument type Bad = u32; diff --git a/tests/ui/abi/debug.stderr b/tests/ui/abi/debug.stderr index 717a95685cb..480f3f04215 100644 --- a/tests/ui/abi/debug.stderr +++ b/tests/ui/abi/debug.stderr @@ -906,7 +906,7 @@ LL | type TestAbiEqNonsense = (fn((str, str)), fn((str, str))); = help: the trait `Sized` is not implemented for `str` = note: only the last element of a tuple may have a dynamically sized type -error: unrecognized field name `` +error: unrecognized argument --> $DIR/debug.rs:56:13 | LL | #[rustc_abi("assert_eq")] diff --git a/tests/ui/attributes/invalid_macro_export_argument.deny.stderr b/tests/ui/attributes/invalid_macro_export_argument.deny.stderr index 897bcecc20e..9d44bd162c7 100644 --- a/tests/ui/attributes/invalid_macro_export_argument.deny.stderr +++ b/tests/ui/attributes/invalid_macro_export_argument.deny.stderr @@ -10,13 +10,13 @@ note: the lint level is defined here LL | #![cfg_attr(deny, deny(invalid_macro_export_arguments))] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: `not_local_inner_macros` isn't a valid `#[macro_export]` argument +error: invalid `#[macro_export]` argument --> $DIR/invalid_macro_export_argument.rs:13:16 | LL | #[macro_export(not_local_inner_macros)] | ^^^^^^^^^^^^^^^^^^^^^^ -error: `` isn't a valid `#[macro_export]` argument +error: invalid `#[macro_export]` argument --> $DIR/invalid_macro_export_argument.rs:33:16 | LL | #[macro_export("blah")] diff --git a/tests/ui/attributes/invalid_macro_export_argument.rs b/tests/ui/attributes/invalid_macro_export_argument.rs index c0aa7df2e87..c5fe39d062a 100644 --- a/tests/ui/attributes/invalid_macro_export_argument.rs +++ b/tests/ui/attributes/invalid_macro_export_argument.rs @@ -11,7 +11,7 @@ macro_rules! a { } #[macro_export(not_local_inner_macros)] -//[deny]~^ ERROR `not_local_inner_macros` isn't a valid `#[macro_export]` argument +//[deny]~^ ERROR invalid `#[macro_export]` argument macro_rules! b { () => () } @@ -31,7 +31,7 @@ macro_rules! e { } #[macro_export("blah")] -//[deny]~^ ERROR `` isn't a valid `#[macro_export]` argument +//[deny]~^ ERROR invalid `#[macro_export]` argument macro_rules! f { () => () } diff --git a/tests/ui/attributes/no-sanitize.rs b/tests/ui/attributes/no-sanitize.rs index a7225efc7b8..ddf909be63a 100644 --- a/tests/ui/attributes/no-sanitize.rs +++ b/tests/ui/attributes/no-sanitize.rs @@ -40,6 +40,6 @@ fn valid() {} static VALID : i32 = 0; #[no_sanitize("address")] -//~^ ERROR `#[no_sanitize()]` should be applied to a function +//~^ ERROR `#[no_sanitize(...)]` should be applied to a function //~| ERROR invalid argument for `no_sanitize` static VALID2 : i32 = 0; diff --git a/tests/ui/attributes/no-sanitize.stderr b/tests/ui/attributes/no-sanitize.stderr index 1f24aa71af5..8d5fbb109ea 100644 --- a/tests/ui/attributes/no-sanitize.stderr +++ b/tests/ui/attributes/no-sanitize.stderr @@ -59,7 +59,7 @@ LL | #[no_sanitize(address, memory)] LL | static INVALID : i32 = 0; | ------------------------- not a function -error: `#[no_sanitize()]` should be applied to a function +error: `#[no_sanitize(...)]` should be applied to a function --> $DIR/no-sanitize.rs:42:15 | LL | #[no_sanitize("address")] From 846c10fecfdbda6e43073af226486ff7ec3f873d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 16 Apr 2025 09:19:52 +1000 Subject: [PATCH 25/27] Avoid an `unwrap` in `RustcMirAttrs::set_field`. --- .../src/framework/graphviz.rs | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs index 144fa8c6fdd..95f488a925b 100644 --- a/compiler/rustc_mir_dataflow/src/framework/graphviz.rs +++ b/compiler/rustc_mir_dataflow/src/framework/graphviz.rs @@ -109,27 +109,29 @@ impl RustcMirAttrs { .flat_map(|attr| attr.meta_item_list().into_iter().flat_map(|v| v.into_iter())); for attr in rustc_mir_attrs { - let attr_result = if attr.has_name(sym::borrowck_graphviz_postflow) { - Self::set_field(&mut ret.basename_and_suffix, tcx, &attr, |s| { - let path = PathBuf::from(s.to_string()); - match path.file_name() { - Some(_) => Ok(path), - None => { - tcx.dcx().emit_err(PathMustEndInFilename { span: attr.span() }); + let attr_result = match attr.name() { + Some(name @ sym::borrowck_graphviz_postflow) => { + Self::set_field(&mut ret.basename_and_suffix, tcx, name, &attr, |s| { + let path = PathBuf::from(s.to_string()); + match path.file_name() { + Some(_) => Ok(path), + None => { + tcx.dcx().emit_err(PathMustEndInFilename { span: attr.span() }); + Err(()) + } + } + }) + } + Some(name @ sym::borrowck_graphviz_format) => { + Self::set_field(&mut ret.formatter, tcx, name, &attr, |s| match s { + sym::two_phase => Ok(s), + _ => { + tcx.dcx().emit_err(UnknownFormatter { span: attr.span() }); Err(()) } - } - }) - } else if attr.has_name(sym::borrowck_graphviz_format) { - Self::set_field(&mut ret.formatter, tcx, &attr, |s| match s { - sym::two_phase => Ok(s), - _ => { - tcx.dcx().emit_err(UnknownFormatter { span: attr.span() }); - Err(()) - } - }) - } else { - Ok(()) + }) + } + _ => Ok(()), }; result = result.and(attr_result); @@ -141,13 +143,12 @@ impl RustcMirAttrs { fn set_field( field: &mut Option, tcx: TyCtxt<'_>, + name: Symbol, attr: &ast::MetaItemInner, mapper: impl FnOnce(Symbol) -> Result, ) -> Result<(), ()> { - // Unwrapping the name is safe because this is only called when `has_name` has succeeded. if field.is_some() { - tcx.dcx() - .emit_err(DuplicateValuesFor { span: attr.span(), name: attr.name().unwrap() }); + tcx.dcx().emit_err(DuplicateValuesFor { span: attr.span(), name }); return Err(()); } From 62146748d8b422fcbeb4b0c6e02a8e7a576f4909 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Tue, 15 Apr 2025 20:15:54 +0200 Subject: [PATCH 26/27] rustdoc/clean: Change terminology of items pertaining to (formal) fn params from "argument" to "parameter" --- src/librustdoc/clean/mod.rs | 150 +++++++++------------ src/librustdoc/clean/types.rs | 16 +-- src/librustdoc/html/format.rs | 54 ++++---- src/librustdoc/html/render/search_index.rs | 12 +- src/librustdoc/json/conversions.rs | 9 +- 5 files changed, 107 insertions(+), 134 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index fe9dc9a9e21..249a822076e 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1052,7 +1052,7 @@ fn clean_fn_or_proc_macro<'tcx>( match macro_kind { Some(kind) => clean_proc_macro(item, name, kind, cx), None => { - let mut func = clean_function(cx, sig, generics, FunctionArgs::Body(body_id)); + let mut func = clean_function(cx, sig, generics, ParamsSrc::Body(body_id)); clean_fn_decl_legacy_const_generics(&mut func, attrs); FunctionItem(func) } @@ -1071,16 +1071,11 @@ fn clean_fn_decl_legacy_const_generics(func: &mut Function, attrs: &[hir::Attrib for (pos, literal) in meta_item_list.iter().filter_map(|meta| meta.lit()).enumerate() { match literal.kind { ast::LitKind::Int(a, _) => { - let param = func.generics.params.remove(0); - if let GenericParamDef { - name, - kind: GenericParamDefKind::Const { ty, .. }, - .. - } = param - { - func.decl.inputs.values.insert( + let GenericParamDef { name, kind, .. } = func.generics.params.remove(0); + if let GenericParamDefKind::Const { ty, .. } = kind { + func.decl.inputs.insert( a.get() as _, - Argument { name: Some(name), type_: *ty, is_const: true }, + Parameter { name: Some(name), type_: *ty, is_const: true }, ); } else { panic!("unexpected non const in position {pos}"); @@ -1092,7 +1087,7 @@ fn clean_fn_decl_legacy_const_generics(func: &mut Function, attrs: &[hir::Attrib } } -enum FunctionArgs<'tcx> { +enum ParamsSrc<'tcx> { Body(hir::BodyId), Idents(&'tcx [Option]), } @@ -1101,30 +1096,26 @@ fn clean_function<'tcx>( cx: &mut DocContext<'tcx>, sig: &hir::FnSig<'tcx>, generics: &hir::Generics<'tcx>, - args: FunctionArgs<'tcx>, + params: ParamsSrc<'tcx>, ) -> Box { let (generics, decl) = enter_impl_trait(cx, |cx| { - // NOTE: generics must be cleaned before args + // NOTE: Generics must be cleaned before params. let generics = clean_generics(generics, cx); - let args = match args { - FunctionArgs::Body(body_id) => { - clean_args_from_types_and_body_id(cx, sig.decl.inputs, body_id) - } - FunctionArgs::Idents(idents) => { - clean_args_from_types_and_names(cx, sig.decl.inputs, idents) - } + let params = match params { + ParamsSrc::Body(body_id) => clean_params_via_body(cx, sig.decl.inputs, body_id), + ParamsSrc::Idents(idents) => clean_params(cx, sig.decl.inputs, idents), }; - let decl = clean_fn_decl_with_args(cx, sig.decl, Some(&sig.header), args); + let decl = clean_fn_decl_with_params(cx, sig.decl, Some(&sig.header), params); (generics, decl) }); Box::new(Function { decl, generics }) } -fn clean_args_from_types_and_names<'tcx>( +fn clean_params<'tcx>( cx: &mut DocContext<'tcx>, types: &[hir::Ty<'tcx>], idents: &[Option], -) -> Arguments { +) -> Vec { fn nonempty_name(ident: &Option) -> Option { if let Some(ident) = ident && ident.name != kw::Underscore @@ -1143,44 +1134,38 @@ fn clean_args_from_types_and_names<'tcx>( None }; - Arguments { - values: types - .iter() - .enumerate() - .map(|(i, ty)| Argument { - type_: clean_ty(ty, cx), - name: idents.get(i).and_then(nonempty_name).or(default_name), - is_const: false, - }) - .collect(), - } + types + .iter() + .enumerate() + .map(|(i, ty)| Parameter { + name: idents.get(i).and_then(nonempty_name).or(default_name), + type_: clean_ty(ty, cx), + is_const: false, + }) + .collect() } -fn clean_args_from_types_and_body_id<'tcx>( +fn clean_params_via_body<'tcx>( cx: &mut DocContext<'tcx>, types: &[hir::Ty<'tcx>], body_id: hir::BodyId, -) -> Arguments { - let body = cx.tcx.hir_body(body_id); - - Arguments { - values: types - .iter() - .zip(body.params) - .map(|(ty, param)| Argument { - name: Some(name_from_pat(param.pat)), - type_: clean_ty(ty, cx), - is_const: false, - }) - .collect(), - } +) -> Vec { + types + .iter() + .zip(cx.tcx.hir_body(body_id).params) + .map(|(ty, param)| Parameter { + name: Some(name_from_pat(param.pat)), + type_: clean_ty(ty, cx), + is_const: false, + }) + .collect() } -fn clean_fn_decl_with_args<'tcx>( +fn clean_fn_decl_with_params<'tcx>( cx: &mut DocContext<'tcx>, decl: &hir::FnDecl<'tcx>, header: Option<&hir::FnHeader>, - args: Arguments, + params: Vec, ) -> FnDecl { let mut output = match decl.output { hir::FnRetTy::Return(typ) => clean_ty(typ, cx), @@ -1191,7 +1176,7 @@ fn clean_fn_decl_with_args<'tcx>( { output = output.sugared_async_return_type(); } - FnDecl { inputs: args, output, c_variadic: decl.c_variadic } + FnDecl { inputs: params, output, c_variadic: decl.c_variadic } } fn clean_poly_fn_sig<'tcx>( @@ -1201,8 +1186,6 @@ fn clean_poly_fn_sig<'tcx>( ) -> FnDecl { let mut names = did.map_or(&[] as &[_], |did| cx.tcx.fn_arg_idents(did)).iter(); - // We assume all empty tuples are default return type. This theoretically can discard `-> ()`, - // but shouldn't change any code meaning. let mut output = clean_middle_ty(sig.output(), cx, None, None); // If the return type isn't an `impl Trait`, we can safely assume that this @@ -1215,25 +1198,21 @@ fn clean_poly_fn_sig<'tcx>( output = output.sugared_async_return_type(); } - FnDecl { - output, - c_variadic: sig.skip_binder().c_variadic, - inputs: Arguments { - values: sig - .inputs() - .iter() - .map(|t| Argument { - type_: clean_middle_ty(t.map_bound(|t| *t), cx, None, None), - name: Some(if let Some(Some(ident)) = names.next() { - ident.name - } else { - kw::Underscore - }), - is_const: false, - }) - .collect(), - }, - } + let params = sig + .inputs() + .iter() + .map(|t| Parameter { + type_: clean_middle_ty(t.map_bound(|t| *t), cx, None, None), + name: Some(if let Some(Some(ident)) = names.next() { + ident.name + } else { + kw::Underscore + }), + is_const: false, + }) + .collect(); + + FnDecl { inputs: params, output, c_variadic: sig.skip_binder().c_variadic } } fn clean_trait_ref<'tcx>(trait_ref: &hir::TraitRef<'tcx>, cx: &mut DocContext<'tcx>) -> Path { @@ -1273,11 +1252,11 @@ fn clean_trait_item<'tcx>(trait_item: &hir::TraitItem<'tcx>, cx: &mut DocContext RequiredAssocConstItem(generics, Box::new(clean_ty(ty, cx))) } hir::TraitItemKind::Fn(ref sig, hir::TraitFn::Provided(body)) => { - let m = clean_function(cx, sig, trait_item.generics, FunctionArgs::Body(body)); + let m = clean_function(cx, sig, trait_item.generics, ParamsSrc::Body(body)); MethodItem(m, None) } hir::TraitItemKind::Fn(ref sig, hir::TraitFn::Required(idents)) => { - let m = clean_function(cx, sig, trait_item.generics, FunctionArgs::Idents(idents)); + let m = clean_function(cx, sig, trait_item.generics, ParamsSrc::Idents(idents)); RequiredMethodItem(m) } hir::TraitItemKind::Type(bounds, Some(default)) => { @@ -1318,7 +1297,7 @@ pub(crate) fn clean_impl_item<'tcx>( type_: clean_ty(ty, cx), })), hir::ImplItemKind::Fn(ref sig, body) => { - let m = clean_function(cx, sig, impl_.generics, FunctionArgs::Body(body)); + let m = clean_function(cx, sig, impl_.generics, ParamsSrc::Body(body)); let defaultness = cx.tcx.defaultness(impl_.owner_id); MethodItem(m, Some(defaultness)) } @@ -1390,14 +1369,14 @@ pub(crate) fn clean_middle_assoc_item(assoc_item: &ty::AssocItem, cx: &mut DocCo } ty::AssocItemContainer::Trait => tcx.types.self_param, }; - let self_arg_ty = + let self_param_ty = tcx.fn_sig(assoc_item.def_id).instantiate_identity().input(0).skip_binder(); - if self_arg_ty == self_ty { - item.decl.inputs.values[0].type_ = SelfTy; - } else if let ty::Ref(_, ty, _) = *self_arg_ty.kind() + if self_param_ty == self_ty { + item.decl.inputs[0].type_ = SelfTy; + } else if let ty::Ref(_, ty, _) = *self_param_ty.kind() && ty == self_ty { - match item.decl.inputs.values[0].type_ { + match item.decl.inputs[0].type_ { BorrowedRef { ref mut type_, .. } => **type_ = SelfTy, _ => unreachable!(), } @@ -2611,15 +2590,15 @@ fn clean_bare_fn_ty<'tcx>( cx: &mut DocContext<'tcx>, ) -> BareFunctionDecl { let (generic_params, decl) = enter_impl_trait(cx, |cx| { - // NOTE: generics must be cleaned before args + // NOTE: Generics must be cleaned before params. let generic_params = bare_fn .generic_params .iter() .filter(|p| !is_elided_lifetime(p)) .map(|x| clean_generic_param(cx, None, x)) .collect(); - let args = clean_args_from_types_and_names(cx, bare_fn.decl.inputs, bare_fn.param_idents); - let decl = clean_fn_decl_with_args(cx, bare_fn.decl, None, args); + let params = clean_params(cx, bare_fn.decl.inputs, bare_fn.param_idents); + let decl = clean_fn_decl_with_params(cx, bare_fn.decl, None, params); (generic_params, decl) }); BareFunctionDecl { safety: bare_fn.safety, abi: bare_fn.abi, decl, generic_params } @@ -2629,7 +2608,6 @@ fn clean_unsafe_binder_ty<'tcx>( unsafe_binder_ty: &hir::UnsafeBinderTy<'tcx>, cx: &mut DocContext<'tcx>, ) -> UnsafeBinderTy { - // NOTE: generics must be cleaned before args let generic_params = unsafe_binder_ty .generic_params .iter() @@ -3155,7 +3133,7 @@ fn clean_maybe_renamed_foreign_item<'tcx>( cx.with_param_env(def_id, |cx| { let kind = match item.kind { hir::ForeignItemKind::Fn(sig, idents, generics) => ForeignFunctionItem( - clean_function(cx, &sig, generics, FunctionArgs::Idents(idents)), + clean_function(cx, &sig, generics, ParamsSrc::Idents(idents)), sig.header.safety(), ), hir::ForeignItemKind::Static(ty, mutability, safety) => ForeignStaticItem( diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index f58cdfc6b5e..cceafcbbb22 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1407,32 +1407,28 @@ pub(crate) struct Function { #[derive(Clone, PartialEq, Eq, Debug, Hash)] pub(crate) struct FnDecl { - pub(crate) inputs: Arguments, + pub(crate) inputs: Vec, pub(crate) output: Type, pub(crate) c_variadic: bool, } impl FnDecl { pub(crate) fn receiver_type(&self) -> Option<&Type> { - self.inputs.values.first().and_then(|v| v.to_receiver()) + self.inputs.first().and_then(|v| v.to_receiver()) } } +/// A function parameter. #[derive(Clone, PartialEq, Eq, Debug, Hash)] -pub(crate) struct Arguments { - pub(crate) values: Vec, -} - -#[derive(Clone, PartialEq, Eq, Debug, Hash)] -pub(crate) struct Argument { - pub(crate) type_: Type, +pub(crate) struct Parameter { pub(crate) name: Option, + pub(crate) type_: Type, /// This field is used to represent "const" arguments from the `rustc_legacy_const_generics` /// feature. More information in . pub(crate) is_const: bool, } -impl Argument { +impl Parameter { pub(crate) fn to_receiver(&self) -> Option<&Type> { if self.name == Some(kw::SelfLower) { Some(&self.type_) } else { None } } diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 9ac328f7495..299fd6b9adb 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -1186,8 +1186,8 @@ impl clean::Impl { { primitive_link(f, PrimitiveType::Array, format_args!("[{name}; N]"), cx)?; } else if let clean::BareFunction(bare_fn) = &type_ - && let [clean::Argument { type_: clean::Type::Generic(name), .. }] = - &bare_fn.decl.inputs.values[..] + && let [clean::Parameter { type_: clean::Type::Generic(name), .. }] = + &bare_fn.decl.inputs[..] && (self.kind.is_fake_variadic() || self.kind.is_auto()) { // Hardcoded anchor library/core/src/primitive_docs.rs @@ -1234,22 +1234,20 @@ impl clean::Impl { } } -impl clean::Arguments { - pub(crate) fn print(&self, cx: &Context<'_>) -> impl Display { - fmt::from_fn(move |f| { - self.values - .iter() - .map(|input| { - fmt::from_fn(|f| { - if let Some(name) = input.name { - write!(f, "{}: ", name)?; - } - input.type_.print(cx).fmt(f) - }) +pub(crate) fn print_params(params: &[clean::Parameter], cx: &Context<'_>) -> impl Display { + fmt::from_fn(move |f| { + params + .iter() + .map(|param| { + fmt::from_fn(|f| { + if let Some(name) = param.name { + write!(f, "{}: ", name)?; + } + param.type_.print(cx).fmt(f) }) - .joined(", ", f) - }) - } + }) + .joined(", ", f) + }) } // Implements Write but only counts the bytes "written". @@ -1281,16 +1279,16 @@ impl clean::FnDecl { if f.alternate() { write!( f, - "({args:#}{ellipsis}){arrow:#}", - args = self.inputs.print(cx), + "({params:#}{ellipsis}){arrow:#}", + params = print_params(&self.inputs, cx), ellipsis = ellipsis, arrow = self.print_output(cx) ) } else { write!( f, - "({args}{ellipsis}){arrow}", - args = self.inputs.print(cx), + "({params}{ellipsis}){arrow}", + params = print_params(&self.inputs, cx), ellipsis = ellipsis, arrow = self.print_output(cx) ) @@ -1336,14 +1334,14 @@ impl clean::FnDecl { write!(f, "(")?; if let Some(n) = line_wrapping_indent - && !self.inputs.values.is_empty() + && !self.inputs.is_empty() { write!(f, "\n{}", Indent(n + 4))?; } - let last_input_index = self.inputs.values.len().checked_sub(1); - for (i, input) in self.inputs.values.iter().enumerate() { - if let Some(selfty) = input.to_receiver() { + let last_input_index = self.inputs.len().checked_sub(1); + for (i, param) in self.inputs.iter().enumerate() { + if let Some(selfty) = param.to_receiver() { match selfty { clean::SelfTy => { write!(f, "self")?; @@ -1361,13 +1359,13 @@ impl clean::FnDecl { } } } else { - if input.is_const { + if param.is_const { write!(f, "const ")?; } - if let Some(name) = input.name { + if let Some(name) = param.name { write!(f, "{}: ", name)?; } - input.type_.print(cx).fmt(f)?; + param.type_.print(cx).fmt(f)?; } match (line_wrapping_indent, last_input_index) { (_, None) => (), diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index 1360ab94cb1..aff8684ee3a 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/src/librustdoc/html/render/search_index.rs @@ -1112,7 +1112,7 @@ fn simplify_fn_type<'a, 'tcx>( } Type::BareFunction(ref bf) => { let mut ty_generics = Vec::new(); - for ty in bf.decl.inputs.values.iter().map(|arg| &arg.type_) { + for ty in bf.decl.inputs.iter().map(|arg| &arg.type_) { simplify_fn_type( self_, generics, @@ -1418,15 +1418,15 @@ fn get_fn_inputs_and_outputs( (None, &func.generics) }; - let mut arg_types = Vec::new(); - for arg in decl.inputs.values.iter() { + let mut param_types = Vec::new(); + for param in decl.inputs.iter() { simplify_fn_type( self_, generics, - &arg.type_, + ¶m.type_, tcx, 0, - &mut arg_types, + &mut param_types, &mut rgen, false, cache, @@ -1439,7 +1439,7 @@ fn get_fn_inputs_and_outputs( let mut simplified_params = rgen.into_iter().collect::>(); simplified_params.sort_by_key(|(_, (idx, _))| -idx); ( - arg_types, + param_types, ret_types, simplified_params .iter() diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs index 5d85a4676b7..dab23f8e42a 100644 --- a/src/librustdoc/json/conversions.rs +++ b/src/librustdoc/json/conversions.rs @@ -609,11 +609,12 @@ impl FromClean for FunctionSignature { let clean::FnDecl { inputs, output, c_variadic } = decl; FunctionSignature { inputs: inputs - .values .into_iter() - // `_` is the most sensible name for missing param names. - .map(|arg| { - (arg.name.unwrap_or(kw::Underscore).to_string(), arg.type_.into_json(renderer)) + .map(|param| { + // `_` is the most sensible name for missing param names. + let name = param.name.unwrap_or(kw::Underscore).to_string(); + let type_ = param.type_.into_json(renderer); + (name, type_) }) .collect(), output: if output.is_unit() { None } else { Some(output.into_json(renderer)) }, From 82ff0a0e6a4be3aab167446ef1529018aa3f1fd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Wed, 16 Apr 2025 12:56:49 +0200 Subject: [PATCH 27/27] rustdoc: Properly clean fn params in all contexts --- src/librustdoc/clean/mod.rs | 58 +++++++++---------- src/librustdoc/clean/utils.rs | 9 ++- tests/rustdoc/anon-fn-params.rs | 25 ++++++++ tests/rustdoc/assoc-fns.rs | 13 +++++ tests/rustdoc/auxiliary/ext-anon-fn-params.rs | 7 +++ tests/rustdoc/ffi.rs | 4 ++ .../auxiliary/{fn-type.rs => fn-ptr-ty.rs} | 0 .../inline_cross/default-generic-args.rs | 8 +-- .../inline_cross/{fn-type.rs => fn-ptr-ty.rs} | 6 +- tests/rustdoc/inline_cross/impl_trait.rs | 2 +- 10 files changed, 89 insertions(+), 43 deletions(-) create mode 100644 tests/rustdoc/anon-fn-params.rs create mode 100644 tests/rustdoc/assoc-fns.rs create mode 100644 tests/rustdoc/auxiliary/ext-anon-fn-params.rs rename tests/rustdoc/inline_cross/auxiliary/{fn-type.rs => fn-ptr-ty.rs} (100%) rename tests/rustdoc/inline_cross/{fn-type.rs => fn-ptr-ty.rs} (71%) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 249a822076e..034ecb2f6c1 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1103,7 +1103,10 @@ fn clean_function<'tcx>( let generics = clean_generics(generics, cx); let params = match params { ParamsSrc::Body(body_id) => clean_params_via_body(cx, sig.decl.inputs, body_id), - ParamsSrc::Idents(idents) => clean_params(cx, sig.decl.inputs, idents), + // Let's not perpetuate anon params from Rust 2015; use `_` for them. + ParamsSrc::Idents(idents) => clean_params(cx, sig.decl.inputs, idents, |ident| { + Some(ident.map_or(kw::Underscore, |ident| ident.name)) + }), }; let decl = clean_fn_decl_with_params(cx, sig.decl, Some(&sig.header), params); (generics, decl) @@ -1115,30 +1118,13 @@ fn clean_params<'tcx>( cx: &mut DocContext<'tcx>, types: &[hir::Ty<'tcx>], idents: &[Option], + postprocess: impl Fn(Option) -> Option, ) -> Vec { - fn nonempty_name(ident: &Option) -> Option { - if let Some(ident) = ident - && ident.name != kw::Underscore - { - Some(ident.name) - } else { - None - } - } - - // If at least one argument has a name, use `_` as the name of unnamed - // arguments. Otherwise omit argument names. - let default_name = if idents.iter().any(|ident| nonempty_name(ident).is_some()) { - Some(kw::Underscore) - } else { - None - }; - types .iter() .enumerate() .map(|(i, ty)| Parameter { - name: idents.get(i).and_then(nonempty_name).or(default_name), + name: postprocess(idents[i]), type_: clean_ty(ty, cx), is_const: false, }) @@ -1184,8 +1170,6 @@ fn clean_poly_fn_sig<'tcx>( did: Option, sig: ty::PolyFnSig<'tcx>, ) -> FnDecl { - let mut names = did.map_or(&[] as &[_], |did| cx.tcx.fn_arg_idents(did)).iter(); - let mut output = clean_middle_ty(sig.output(), cx, None, None); // If the return type isn't an `impl Trait`, we can safely assume that this @@ -1198,16 +1182,20 @@ fn clean_poly_fn_sig<'tcx>( output = output.sugared_async_return_type(); } + let mut idents = did.map(|did| cx.tcx.fn_arg_idents(did)).unwrap_or_default().iter().copied(); + + // If this comes from a fn item, let's not perpetuate anon params from Rust 2015; use `_` for them. + // If this comes from a fn ptr ty, we just keep params unnamed since it's more conventional stylistically. + // Since the param name is not part of the semantic type, these params never bear a name unlike + // in the HIR case, thus we can't peform any fancy fallback logic unlike `clean_bare_fn_ty`. + let fallback = did.map(|_| kw::Underscore); + let params = sig .inputs() .iter() - .map(|t| Parameter { - type_: clean_middle_ty(t.map_bound(|t| *t), cx, None, None), - name: Some(if let Some(Some(ident)) = names.next() { - ident.name - } else { - kw::Underscore - }), + .map(|ty| Parameter { + name: idents.next().flatten().map(|ident| ident.name).or(fallback), + type_: clean_middle_ty(ty.map_bound(|ty| *ty), cx, None, None), is_const: false, }) .collect(); @@ -2597,7 +2585,17 @@ fn clean_bare_fn_ty<'tcx>( .filter(|p| !is_elided_lifetime(p)) .map(|x| clean_generic_param(cx, None, x)) .collect(); - let params = clean_params(cx, bare_fn.decl.inputs, bare_fn.param_idents); + // Since it's more conventional stylistically, elide the name of all params called `_` + // unless there's at least one interestingly named param in which case don't elide any + // name since mixing named and unnamed params is less legible. + let filter = |ident: Option| { + ident.map(|ident| ident.name).filter(|&ident| ident != kw::Underscore) + }; + let fallback = + bare_fn.param_idents.iter().copied().find_map(filter).map(|_| kw::Underscore); + let params = clean_params(cx, bare_fn.decl.inputs, bare_fn.param_idents, |ident| { + filter(ident).or(fallback) + }); let decl = clean_fn_decl_with_params(cx, bare_fn.decl, None, params); (generic_params, decl) }); diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index 8ee08edec19..af7986d030e 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -303,13 +303,12 @@ pub(crate) fn name_from_pat(p: &hir::Pat<'_>) -> Symbol { debug!("trying to get a name from pattern: {p:?}"); Symbol::intern(&match &p.kind { - // FIXME(never_patterns): does this make sense? - PatKind::Missing => unreachable!(), - PatKind::Wild - | PatKind::Err(_) + PatKind::Err(_) + | PatKind::Missing // Let's not perpetuate anon params from Rust 2015; use `_` for them. | PatKind::Never + | PatKind::Range(..) | PatKind::Struct(..) - | PatKind::Range(..) => { + | PatKind::Wild => { return kw::Underscore; } PatKind::Binding(_, _, ident, _) => return ident.name, diff --git a/tests/rustdoc/anon-fn-params.rs b/tests/rustdoc/anon-fn-params.rs new file mode 100644 index 00000000000..9af1af3d3fa --- /dev/null +++ b/tests/rustdoc/anon-fn-params.rs @@ -0,0 +1,25 @@ +// Test that we render the deprecated anonymous trait function parameters from Rust 2015 as +// underscores in order not to perpetuate it and for legibility. + +//@ edition: 2015 +#![expect(anonymous_parameters)] + +// Check the "local case" (HIR cleaning) // + +//@ has anon_fn_params/trait.Trait.html +pub trait Trait { + //@ has - '//*[@id="tymethod.required"]' 'fn required(_: Option, _: impl Fn(&str) -> bool)' + fn required(Option, impl Fn(&str) -> bool); + //@ has - '//*[@id="method.provided"]' 'fn provided(_: [i32; 2])' + fn provided([i32; 2]) {} +} + +// Check the "extern case" (middle cleaning) // + +//@ aux-build: ext-anon-fn-params.rs +extern crate ext_anon_fn_params; + +//@ has anon_fn_params/trait.ExtTrait.html +//@ has - '//*[@id="tymethod.required"]' 'fn required(_: Option, _: impl Fn(&str) -> bool)' +//@ has - '//*[@id="method.provided"]' 'fn provided(_: [i32; 2])' +pub use ext_anon_fn_params::Trait as ExtTrait; diff --git a/tests/rustdoc/assoc-fns.rs b/tests/rustdoc/assoc-fns.rs new file mode 100644 index 00000000000..6ffbebc3d27 --- /dev/null +++ b/tests/rustdoc/assoc-fns.rs @@ -0,0 +1,13 @@ +// Basic testing for associated functions (in traits, trait impls & inherent impls). + +//@ has assoc_fns/trait.Trait.html +pub trait Trait { + //@ has - '//*[@id="tymethod.required"]' 'fn required(first: i32, second: &str)' + fn required(first: i32, second: &str); + + //@ has - '//*[@id="method.provided"]' 'fn provided(only: ())' + fn provided(only: ()) {} + + //@ has - '//*[@id="tymethod.params_are_unnamed"]' 'fn params_are_unnamed(_: i32, _: u32)' + fn params_are_unnamed(_: i32, _: u32); +} diff --git a/tests/rustdoc/auxiliary/ext-anon-fn-params.rs b/tests/rustdoc/auxiliary/ext-anon-fn-params.rs new file mode 100644 index 00000000000..1acb919ca64 --- /dev/null +++ b/tests/rustdoc/auxiliary/ext-anon-fn-params.rs @@ -0,0 +1,7 @@ +//@ edition: 2015 +#![expect(anonymous_parameters)] + +pub trait Trait { + fn required(Option, impl Fn(&str) -> bool); + fn provided([i32; 2]) {} +} diff --git a/tests/rustdoc/ffi.rs b/tests/rustdoc/ffi.rs index 5ba7cdba910..524fb0edefb 100644 --- a/tests/rustdoc/ffi.rs +++ b/tests/rustdoc/ffi.rs @@ -9,4 +9,8 @@ pub use lib::foreigner; extern "C" { //@ has ffi/fn.another.html //pre 'pub unsafe extern "C" fn another(cold_as_ice: u32)' pub fn another(cold_as_ice: u32); + + //@ has ffi/fn.params_are_unnamed.html //pre \ + // 'pub unsafe extern "C" fn params_are_unnamed(_: i32, _: u32)' + pub fn params_are_unnamed(_: i32, _: u32); } diff --git a/tests/rustdoc/inline_cross/auxiliary/fn-type.rs b/tests/rustdoc/inline_cross/auxiliary/fn-ptr-ty.rs similarity index 100% rename from tests/rustdoc/inline_cross/auxiliary/fn-type.rs rename to tests/rustdoc/inline_cross/auxiliary/fn-ptr-ty.rs diff --git a/tests/rustdoc/inline_cross/default-generic-args.rs b/tests/rustdoc/inline_cross/default-generic-args.rs index 0469221b3d8..5124fbdf8da 100644 --- a/tests/rustdoc/inline_cross/default-generic-args.rs +++ b/tests/rustdoc/inline_cross/default-generic-args.rs @@ -53,17 +53,17 @@ pub use default_generic_args::R2; //@ has user/type.H0.html // Check that we handle higher-ranked regions correctly: -//@ has - '//*[@class="rust item-decl"]//code' "fn(_: for<'a> fn(_: Re<'a>))" +//@ has - '//*[@class="rust item-decl"]//code' "fn(for<'a> fn(Re<'a>))" pub use default_generic_args::H0; //@ has user/type.H1.html // Check that we don't conflate distinct universially quantified regions (#1): -//@ has - '//*[@class="rust item-decl"]//code' "for<'b> fn(_: for<'a> fn(_: Re<'a, &'b ()>))" +//@ has - '//*[@class="rust item-decl"]//code' "for<'b> fn(for<'a> fn(Re<'a, &'b ()>))" pub use default_generic_args::H1; //@ has user/type.H2.html // Check that we don't conflate distinct universially quantified regions (#2): -//@ has - '//*[@class="rust item-decl"]//code' "for<'a> fn(_: for<'b> fn(_: Re<'a, &'b ()>))" +//@ has - '//*[@class="rust item-decl"]//code' "for<'a> fn(for<'b> fn(Re<'a, &'b ()>))" pub use default_generic_args::H2; //@ has user/type.P0.html @@ -86,7 +86,7 @@ pub use default_generic_args::A0; // Demonstrates that we currently don't elide generic arguments that are alpha-equivalent to their // respective generic parameter (after instantiation) for perf reasons (it would require us to // create an inference context). -//@ has - '//*[@class="rust item-decl"]//code' "Alpha fn(_: &'arbitrary ())>" +//@ has - '//*[@class="rust item-decl"]//code' "Alpha fn(&'arbitrary ())>" pub use default_generic_args::A1; //@ has user/type.M0.html diff --git a/tests/rustdoc/inline_cross/fn-type.rs b/tests/rustdoc/inline_cross/fn-ptr-ty.rs similarity index 71% rename from tests/rustdoc/inline_cross/fn-type.rs rename to tests/rustdoc/inline_cross/fn-ptr-ty.rs index 8db6f65f421..01059622521 100644 --- a/tests/rustdoc/inline_cross/fn-type.rs +++ b/tests/rustdoc/inline_cross/fn-ptr-ty.rs @@ -2,11 +2,11 @@ // They should be rendered exactly as the user wrote it, i.e., in source order and with unused // parameters present, not stripped. -//@ aux-crate:fn_type=fn-type.rs +//@ aux-crate:fn_ptr_ty=fn-ptr-ty.rs //@ edition: 2021 #![crate_name = "user"] //@ has user/type.F.html //@ has - '//*[@class="rust item-decl"]//code' \ -// "for<'z, 'a, '_unused> fn(_: &'z for<'b> fn(_: &'b str), _: &'a ()) -> &'a ();" -pub use fn_type::F; +// "for<'z, 'a, '_unused> fn(&'z for<'b> fn(&'b str), &'a ()) -> &'a ();" +pub use fn_ptr_ty::F; diff --git a/tests/rustdoc/inline_cross/impl_trait.rs b/tests/rustdoc/inline_cross/impl_trait.rs index e6baf33660a..468ac083061 100644 --- a/tests/rustdoc/inline_cross/impl_trait.rs +++ b/tests/rustdoc/inline_cross/impl_trait.rs @@ -29,7 +29,7 @@ pub use impl_trait_aux::func4; //@ has impl_trait/fn.func5.html //@ has - '//pre[@class="rust item-decl"]' "func5(" //@ has - '//pre[@class="rust item-decl"]' "_f: impl for<'any> Fn(&'any str, &'any str) -> bool + for<'r> Other = ()>," -//@ has - '//pre[@class="rust item-decl"]' "_a: impl for<'beta, 'alpha, '_gamma> Auxiliary<'alpha, Item<'beta> = fn(_: &'beta ())>" +//@ has - '//pre[@class="rust item-decl"]' "_a: impl for<'beta, 'alpha, '_gamma> Auxiliary<'alpha, Item<'beta> = fn(&'beta ())>" //@ !has - '//pre[@class="rust item-decl"]' 'where' pub use impl_trait_aux::func5;