From e2fcdb8d360acd561e2b6425e41009d9fff2d989 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Sun, 12 Jan 2025 20:09:51 -0800 Subject: [PATCH 01/14] rustdoc: Extract `AttributesExt::cfg` trait method as function It's never overridden, so it shouldn't be on the trait. --- src/librustdoc/clean/inline.rs | 17 ++-- src/librustdoc/clean/mod.rs | 2 +- src/librustdoc/clean/types.rs | 147 +++++++++++++++++---------------- src/librustdoc/doctest/rust.rs | 5 +- 4 files changed, 88 insertions(+), 83 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 019a888bd2f62..a17e0b1e4ccb1 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -17,12 +17,12 @@ use rustc_span::symbol::{Symbol, sym}; use thin_vec::{ThinVec, thin_vec}; use tracing::{debug, trace}; -use super::Item; +use super::{Item, extract_cfg_from_attrs}; use crate::clean::{ - self, Attributes, AttributesExt, ImplKind, ItemId, Type, clean_bound_vars, clean_generics, - clean_impl_item, clean_middle_assoc_item, clean_middle_field, clean_middle_ty, - clean_poly_fn_sig, clean_trait_ref_with_constraints, clean_ty, clean_ty_alias_inner_type, - clean_ty_generics, clean_variant_def, utils, + self, Attributes, ImplKind, ItemId, Type, clean_bound_vars, clean_generics, clean_impl_item, + clean_middle_assoc_item, clean_middle_field, clean_middle_ty, clean_poly_fn_sig, + clean_trait_ref_with_constraints, clean_ty, clean_ty_alias_inner_type, clean_ty_generics, + clean_variant_def, utils, }; use crate::core::DocContext; use crate::formats::item_type::ItemType; @@ -408,10 +408,13 @@ pub(crate) fn merge_attrs( } else { Attributes::from_hir(&both) }, - both.cfg(cx.tcx, &cx.cache.hidden_cfg), + extract_cfg_from_attrs(&both[..], cx.tcx, &cx.cache.hidden_cfg), ) } else { - (Attributes::from_hir(old_attrs), old_attrs.cfg(cx.tcx, &cx.cache.hidden_cfg)) + ( + Attributes::from_hir(old_attrs), + extract_cfg_from_attrs(&old_attrs[..], cx.tcx, &cx.cache.hidden_cfg), + ) } } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 2ed2df799dd1c..a36927c47ebab 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -200,7 +200,7 @@ fn generate_item_with_correct_attrs( target_attrs.iter().map(|attr| (Cow::Borrowed(attr), None)).collect() }; - let cfg = attrs.cfg(cx.tcx, &cx.cache.hidden_cfg); + let cfg = extract_cfg_from_attrs(&attrs[..], cx.tcx, &cx.cache.hidden_cfg); let attrs = Attributes::from_hir_iter(attrs.iter().map(|(attr, did)| (&**attr, *did)), false); let name = renamed.or(Some(name)); diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index dcee96978d2b5..704caeb0025cf 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -479,7 +479,7 @@ impl Item { name, kind, Attributes::from_hir(hir_attrs), - hir_attrs.cfg(cx.tcx, &cx.cache.hidden_cfg), + extract_cfg_from_attrs(hir_attrs, cx.tcx, &cx.cache.hidden_cfg), ) } @@ -990,95 +990,98 @@ pub(crate) trait AttributesExt { fn lists(&self, name: Symbol) -> Self::AttributeIterator<'_>; fn iter(&self) -> Self::Attributes<'_>; +} - fn cfg(&self, tcx: TyCtxt<'_>, hidden_cfg: &FxHashSet) -> Option> { - let sess = tcx.sess; - let doc_cfg_active = tcx.features().doc_cfg(); - let doc_auto_cfg_active = tcx.features().doc_auto_cfg(); - - fn single(it: T) -> Option { - let mut iter = it.into_iter(); - let item = iter.next()?; - if iter.next().is_some() { - return None; - } - Some(item) +pub fn extract_cfg_from_attrs( + attrs: &A, + tcx: TyCtxt<'_>, + hidden_cfg: &FxHashSet, +) -> Option> { + let sess = tcx.sess; + let doc_cfg_active = tcx.features().doc_cfg(); + let doc_auto_cfg_active = tcx.features().doc_auto_cfg(); + + fn single(it: T) -> Option { + let mut iter = it.into_iter(); + let item = iter.next()?; + if iter.next().is_some() { + return None; } + Some(item) + } - let mut cfg = if doc_cfg_active || doc_auto_cfg_active { - let mut doc_cfg = self + let mut cfg = if doc_cfg_active || doc_auto_cfg_active { + let mut doc_cfg = attrs + .iter() + .filter(|attr| attr.has_name(sym::doc)) + .flat_map(|attr| attr.meta_item_list().unwrap_or_default()) + .filter(|attr| attr.has_name(sym::cfg)) + .peekable(); + if doc_cfg.peek().is_some() && doc_cfg_active { + doc_cfg + .filter_map(|attr| Cfg::parse(&attr).ok()) + .fold(Cfg::True, |cfg, new_cfg| cfg & new_cfg) + } else if doc_auto_cfg_active { + // If there is no `doc(cfg())`, then we retrieve the `cfg()` attributes (because + // `doc(cfg())` overrides `cfg()`). + attrs .iter() - .filter(|attr| attr.has_name(sym::doc)) - .flat_map(|attr| attr.meta_item_list().unwrap_or_default()) .filter(|attr| attr.has_name(sym::cfg)) - .peekable(); - if doc_cfg.peek().is_some() && doc_cfg_active { - doc_cfg - .filter_map(|attr| Cfg::parse(&attr).ok()) - .fold(Cfg::True, |cfg, new_cfg| cfg & new_cfg) - } else if doc_auto_cfg_active { - // If there is no `doc(cfg())`, then we retrieve the `cfg()` attributes (because - // `doc(cfg())` overrides `cfg()`). - self.iter() - .filter(|attr| attr.has_name(sym::cfg)) - .filter_map(|attr| single(attr.meta_item_list()?)) - .filter_map(|attr| { - Cfg::parse_without(attr.meta_item()?, hidden_cfg).ok().flatten() - }) - .fold(Cfg::True, |cfg, new_cfg| cfg & new_cfg) - } else { - Cfg::True - } + .filter_map(|attr| single(attr.meta_item_list()?)) + .filter_map(|attr| Cfg::parse_without(attr.meta_item()?, hidden_cfg).ok().flatten()) + .fold(Cfg::True, |cfg, new_cfg| cfg & new_cfg) } else { Cfg::True - }; - - for attr in self.iter() { - // #[doc] - if attr.doc_str().is_none() && attr.has_name(sym::doc) { - // #[doc(...)] - if let Some(list) = attr.meta_item_list() { - for item in list { - // #[doc(hidden)] - if !item.has_name(sym::cfg) { - continue; - } - // #[doc(cfg(...))] - if let Some(cfg_mi) = item - .meta_item() - .and_then(|item| rustc_expand::config::parse_cfg(item, sess)) - { - match Cfg::parse(cfg_mi) { - Ok(new_cfg) => cfg &= new_cfg, - Err(e) => { - sess.dcx().span_err(e.span, e.msg); - } + } + } else { + Cfg::True + }; + + for attr in attrs.iter() { + // #[doc] + if attr.doc_str().is_none() && attr.has_name(sym::doc) { + // #[doc(...)] + if let Some(list) = attr.meta_item_list() { + for item in list { + // #[doc(hidden)] + if !item.has_name(sym::cfg) { + continue; + } + // #[doc(cfg(...))] + if let Some(cfg_mi) = item + .meta_item() + .and_then(|item| rustc_expand::config::parse_cfg(item, sess)) + { + match Cfg::parse(cfg_mi) { + Ok(new_cfg) => cfg &= new_cfg, + Err(e) => { + sess.dcx().span_err(e.span, e.msg); } } } } } } + } - // treat #[target_feature(enable = "feat")] attributes as if they were - // #[doc(cfg(target_feature = "feat"))] attributes as well - for attr in self.lists(sym::target_feature) { - if attr.has_name(sym::enable) { - if attr.value_str().is_some() { - // Clone `enable = "feat"`, change to `target_feature = "feat"`. - // Unwrap is safe because `value_str` succeeded above. - let mut meta = attr.meta_item().unwrap().clone(); - meta.path = ast::Path::from_ident(Ident::with_dummy_span(sym::target_feature)); - - if let Ok(feat_cfg) = Cfg::parse(&ast::MetaItemInner::MetaItem(meta)) { - cfg &= feat_cfg; - } + // treat #[target_feature(enable = "feat")] attributes as if they were + // #[doc(cfg(target_feature = "feat"))] attributes as well + for attr in attrs.lists(sym::target_feature) { + if attr.has_name(sym::enable) { + if attr.value_str().is_some() { + // Clone `enable = "feat"`, change to `target_feature = "feat"`. + // Unwrap is safe because `value_str` succeeded above. + let mut meta = attr.meta_item().unwrap().clone(); + meta.path = ast::Path::from_ident(Ident::with_dummy_span(sym::target_feature)); + + if let Ok(feat_cfg) = Cfg::parse(&ast::MetaItemInner::MetaItem(meta)) { + cfg &= feat_cfg; } } } - - if cfg == Cfg::True { None } else { Some(Arc::new(cfg)) } } + + if cfg == Cfg::True { None } else { Some(Arc::new(cfg)) } } impl AttributesExt for [hir::Attribute] { diff --git a/src/librustdoc/doctest/rust.rs b/src/librustdoc/doctest/rust.rs index 0903baddabebe..5986694a25793 100644 --- a/src/librustdoc/doctest/rust.rs +++ b/src/librustdoc/doctest/rust.rs @@ -13,8 +13,7 @@ use rustc_span::source_map::SourceMap; use rustc_span::{BytePos, DUMMY_SP, FileName, Pos, Span}; use super::{DocTestVisitor, ScrapedDocTest}; -use crate::clean::Attributes; -use crate::clean::types::AttributesExt; +use crate::clean::{Attributes, extract_cfg_from_attrs}; use crate::html::markdown::{self, ErrorCodes, LangString, MdRelLine}; struct RustCollector { @@ -97,7 +96,7 @@ impl HirCollector<'_> { nested: F, ) { let ast_attrs = self.tcx.hir().attrs(self.tcx.local_def_id_to_hir_id(def_id)); - if let Some(ref cfg) = ast_attrs.cfg(self.tcx, &FxHashSet::default()) { + if let Some(ref cfg) = extract_cfg_from_attrs(ast_attrs, self.tcx, &FxHashSet::default()) { if !cfg.matches(&self.tcx.sess.psess, Some(self.tcx.features())) { return; } From 5ccd6c04e5b0e81c8a0fdd8c0e9363be49ef053d Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Sun, 12 Jan 2025 20:19:20 -0800 Subject: [PATCH 02/14] rustdoc: Extract `AttributesExt::lists` trait method as function The two implementations were identical, so there's no need to use a trait method. --- src/librustdoc/clean/mod.rs | 10 ++++---- src/librustdoc/clean/types.rs | 43 +++++++++++++---------------------- src/librustdoc/visit_ast.rs | 6 ++--- 3 files changed, 24 insertions(+), 35 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index a36927c47ebab..ed0c566d53df8 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -186,8 +186,7 @@ fn generate_item_with_correct_attrs( // For glob re-exports the item may or may not exist to be re-exported (potentially the cfgs // on the path up until the glob can be removed, and only cfgs on the globbed item itself // matter), for non-inlined re-exports see #85043. - let is_inline = inline::load_attrs(cx, import_id.to_def_id()) - .lists(sym::doc) + let is_inline = hir_attr_lists(inline::load_attrs(cx, import_id.to_def_id()), sym::doc) .get_word_attr(sym::inline) .is_some() || (is_glob_import(cx.tcx, import_id) @@ -979,13 +978,14 @@ fn clean_proc_macro<'tcx>( ) -> ItemKind { let attrs = cx.tcx.hir().attrs(item.hir_id()); if kind == MacroKind::Derive - && let Some(derive_name) = attrs.lists(sym::proc_macro_derive).find_map(|mi| mi.ident()) + && let Some(derive_name) = + hir_attr_lists(attrs, sym::proc_macro_derive).find_map(|mi| mi.ident()) { *name = derive_name.name; } let mut helpers = Vec::new(); - for mi in attrs.lists(sym::proc_macro_derive) { + for mi in hir_attr_lists(attrs, sym::proc_macro_derive) { if !mi.has_name(sym::attributes) { continue; } @@ -2985,7 +2985,7 @@ fn clean_use_statement_inner<'tcx>( let visibility = cx.tcx.visibility(import.owner_id); let attrs = cx.tcx.hir().attrs(import.hir_id()); - let inline_attr = attrs.lists(sym::doc).get_word_attr(sym::inline); + let inline_attr = hir_attr_lists(attrs, sym::doc).get_word_attr(sym::inline); let pub_underscore = visibility.is_public() && name == kw::Underscore; let current_mod = cx.tcx.parent_module_from_def_id(import.owner_id.def_id); let import_def_id = import.owner_id.def_id; diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 704caeb0025cf..70be5742c08c2 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -980,18 +980,24 @@ pub(crate) struct Module { } pub(crate) trait AttributesExt { - type AttributeIterator<'a>: Iterator - where - Self: 'a; type Attributes<'a>: Iterator where Self: 'a; - fn lists(&self, name: Symbol) -> Self::AttributeIterator<'_>; - fn iter(&self) -> Self::Attributes<'_>; } +pub fn hir_attr_lists( + attrs: &A, + name: Symbol, +) -> impl Iterator + use<'_, A> { + attrs + .iter() + .filter(move |attr| attr.has_name(name)) + .filter_map(ast::attr::AttributeExt::meta_item_list) + .flatten() +} + pub fn extract_cfg_from_attrs( attrs: &A, tcx: TyCtxt<'_>, @@ -1066,7 +1072,7 @@ pub fn extract_cfg_from_attrs( // treat #[target_feature(enable = "feat")] attributes as if they were // #[doc(cfg(target_feature = "feat"))] attributes as well - for attr in attrs.lists(sym::target_feature) { + for attr in hir_attr_lists(attrs, sym::target_feature) { if attr.has_name(sym::enable) { if attr.value_str().is_some() { // Clone `enable = "feat"`, change to `target_feature = "feat"`. @@ -1085,38 +1091,19 @@ pub fn extract_cfg_from_attrs( } impl AttributesExt for [hir::Attribute] { - type AttributeIterator<'a> = impl Iterator + 'a; type Attributes<'a> = impl Iterator + 'a; - fn lists(&self, name: Symbol) -> Self::AttributeIterator<'_> { - self.iter() - .filter(move |attr| attr.has_name(name)) - .filter_map(ast::attr::AttributeExt::meta_item_list) - .flatten() - } - fn iter(&self) -> Self::Attributes<'_> { self.iter() } } impl AttributesExt for [(Cow<'_, hir::Attribute>, Option)] { - type AttributeIterator<'a> - = impl Iterator + 'a - where - Self: 'a; type Attributes<'a> = impl Iterator + 'a where Self: 'a; - fn lists(&self, name: Symbol) -> Self::AttributeIterator<'_> { - AttributesExt::iter(self) - .filter(move |attr| attr.has_name(name)) - .filter_map(hir::Attribute::meta_item_list) - .flatten() - } - fn iter(&self) -> Self::Attributes<'_> { self.iter().map(move |(attr, _)| match attr { Cow::Borrowed(attr) => *attr, @@ -1188,7 +1175,7 @@ pub(crate) struct Attributes { impl Attributes { pub(crate) fn lists(&self, name: Symbol) -> impl Iterator + '_ { - self.other_attrs.lists(name) + hir_attr_lists(&self.other_attrs[..], name) } pub(crate) fn has_doc_flag(&self, flag: Symbol) -> bool { @@ -1255,7 +1242,9 @@ impl Attributes { pub(crate) fn get_doc_aliases(&self) -> Box<[Symbol]> { let mut aliases = FxIndexSet::default(); - for attr in self.other_attrs.lists(sym::doc).filter(|a| a.has_name(sym::alias)) { + for attr in + hir_attr_lists(&self.other_attrs[..], sym::doc).filter(|a| a.has_name(sym::alias)) + { if let Some(values) = attr.meta_item_list() { for l in values { if let Some(lit) = l.lit() diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 794ea54b3ef24..d46b0dee36cb2 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -19,7 +19,7 @@ use tracing::debug; use crate::clean::cfg::Cfg; use crate::clean::utils::{inherits_doc_hidden, should_ignore_res}; -use crate::clean::{AttributesExt, NestedAttributesExt, reexport_chain}; +use crate::clean::{NestedAttributesExt, hir_attr_lists, reexport_chain}; use crate::core; /// This module is used to store stuff from Rust's AST in a more convenient @@ -247,8 +247,8 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { let document_hidden = self.cx.render_options.document_hidden; let use_attrs = tcx.hir().attrs(tcx.local_def_id_to_hir_id(def_id)); // Don't inline `doc(hidden)` imports so they can be stripped at a later stage. - let is_no_inline = use_attrs.lists(sym::doc).has_word(sym::no_inline) - || (document_hidden && use_attrs.lists(sym::doc).has_word(sym::hidden)); + let is_no_inline = hir_attr_lists(use_attrs, sym::doc).has_word(sym::no_inline) + || (document_hidden && hir_attr_lists(use_attrs, sym::doc).has_word(sym::hidden)); if is_no_inline { return false; From f92b32c5f64ea900f2cd6936f648f407daee0d25 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Sun, 12 Jan 2025 20:31:53 -0800 Subject: [PATCH 03/14] rustdoc: Eliminate `AttributesExt` The new code is more explicit and avoids trait magic that added needless complexity to this part of rustdoc. --- src/librustdoc/clean/inline.rs | 4 +-- src/librustdoc/clean/mod.rs | 10 +++++-- src/librustdoc/clean/types.rs | 51 +++++++--------------------------- src/librustdoc/doctest/rust.rs | 4 ++- 4 files changed, 23 insertions(+), 46 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index a17e0b1e4ccb1..3d51ab1967d44 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -408,12 +408,12 @@ pub(crate) fn merge_attrs( } else { Attributes::from_hir(&both) }, - extract_cfg_from_attrs(&both[..], cx.tcx, &cx.cache.hidden_cfg), + extract_cfg_from_attrs(both.iter(), cx.tcx, &cx.cache.hidden_cfg), ) } else { ( Attributes::from_hir(old_attrs), - extract_cfg_from_attrs(&old_attrs[..], cx.tcx, &cx.cache.hidden_cfg), + extract_cfg_from_attrs(old_attrs.iter(), cx.tcx, &cx.cache.hidden_cfg), ) } } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index ed0c566d53df8..ed064768c70c6 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -198,8 +198,14 @@ fn generate_item_with_correct_attrs( // We only keep the item's attributes. target_attrs.iter().map(|attr| (Cow::Borrowed(attr), None)).collect() }; - - let cfg = extract_cfg_from_attrs(&attrs[..], cx.tcx, &cx.cache.hidden_cfg); + let cfg = extract_cfg_from_attrs( + attrs.iter().map(move |(attr, _)| match attr { + Cow::Borrowed(attr) => *attr, + Cow::Owned(attr) => attr, + }), + cx.tcx, + &cx.cache.hidden_cfg, + ); let attrs = Attributes::from_hir_iter(attrs.iter().map(|(attr, did)| (&**attr, *did)), false); let name = renamed.or(Some(name)); diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 70be5742c08c2..edec6777c2e18 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1,4 +1,3 @@ -use std::borrow::Cow; use std::hash::Hash; use std::path::PathBuf; use std::sync::{Arc, OnceLock as OnceCell}; @@ -479,7 +478,7 @@ impl Item { name, kind, Attributes::from_hir(hir_attrs), - extract_cfg_from_attrs(hir_attrs, cx.tcx, &cx.cache.hidden_cfg), + extract_cfg_from_attrs(hir_attrs.iter(), cx.tcx, &cx.cache.hidden_cfg), ) } @@ -979,27 +978,19 @@ pub(crate) struct Module { pub(crate) span: Span, } -pub(crate) trait AttributesExt { - type Attributes<'a>: Iterator - where - Self: 'a; - - fn iter(&self) -> Self::Attributes<'_>; -} - -pub fn hir_attr_lists( - attrs: &A, +pub(crate) fn hir_attr_lists<'a, I: IntoIterator>( + attrs: I, name: Symbol, -) -> impl Iterator + use<'_, A> { +) -> impl Iterator + use<'a, I> { attrs - .iter() + .into_iter() .filter(move |attr| attr.has_name(name)) .filter_map(ast::attr::AttributeExt::meta_item_list) .flatten() } -pub fn extract_cfg_from_attrs( - attrs: &A, +pub(crate) fn extract_cfg_from_attrs<'a, I: Iterator + Clone>( + attrs: I, tcx: TyCtxt<'_>, hidden_cfg: &FxHashSet, ) -> Option> { @@ -1018,7 +1009,7 @@ pub fn extract_cfg_from_attrs( let mut cfg = if doc_cfg_active || doc_auto_cfg_active { let mut doc_cfg = attrs - .iter() + .clone() .filter(|attr| attr.has_name(sym::doc)) .flat_map(|attr| attr.meta_item_list().unwrap_or_default()) .filter(|attr| attr.has_name(sym::cfg)) @@ -1031,7 +1022,7 @@ pub fn extract_cfg_from_attrs( // If there is no `doc(cfg())`, then we retrieve the `cfg()` attributes (because // `doc(cfg())` overrides `cfg()`). attrs - .iter() + .clone() .filter(|attr| attr.has_name(sym::cfg)) .filter_map(|attr| single(attr.meta_item_list()?)) .filter_map(|attr| Cfg::parse_without(attr.meta_item()?, hidden_cfg).ok().flatten()) @@ -1043,7 +1034,7 @@ pub fn extract_cfg_from_attrs( Cfg::True }; - for attr in attrs.iter() { + for attr in attrs.clone() { // #[doc] if attr.doc_str().is_none() && attr.has_name(sym::doc) { // #[doc(...)] @@ -1090,28 +1081,6 @@ pub fn extract_cfg_from_attrs( if cfg == Cfg::True { None } else { Some(Arc::new(cfg)) } } -impl AttributesExt for [hir::Attribute] { - type Attributes<'a> = impl Iterator + 'a; - - fn iter(&self) -> Self::Attributes<'_> { - self.iter() - } -} - -impl AttributesExt for [(Cow<'_, hir::Attribute>, Option)] { - type Attributes<'a> - = impl Iterator + 'a - where - Self: 'a; - - fn iter(&self) -> Self::Attributes<'_> { - self.iter().map(move |(attr, _)| match attr { - Cow::Borrowed(attr) => *attr, - Cow::Owned(attr) => attr, - }) - } -} - pub(crate) trait NestedAttributesExt { /// Returns `true` if the attribute list contains a specific `word` fn has_word(self, word: Symbol) -> bool diff --git a/src/librustdoc/doctest/rust.rs b/src/librustdoc/doctest/rust.rs index 5986694a25793..bd292efeb7ea2 100644 --- a/src/librustdoc/doctest/rust.rs +++ b/src/librustdoc/doctest/rust.rs @@ -96,7 +96,9 @@ impl HirCollector<'_> { nested: F, ) { let ast_attrs = self.tcx.hir().attrs(self.tcx.local_def_id_to_hir_id(def_id)); - if let Some(ref cfg) = extract_cfg_from_attrs(ast_attrs, self.tcx, &FxHashSet::default()) { + if let Some(ref cfg) = + extract_cfg_from_attrs(ast_attrs.iter(), self.tcx, &FxHashSet::default()) + { if !cfg.matches(&self.tcx.sess.psess, Some(self.tcx.features())) { return; } From 0e5ee891b2b07321175e398153bfa86c667f3494 Mon Sep 17 00:00:00 2001 From: joboet Date: Mon, 25 Nov 2024 09:50:24 +0100 Subject: [PATCH 04/14] Revert "Remove the Arc rt::init allocation for thread info" This reverts commit 0747f2898e83df7e601189c0f31762e84328becb. --- library/std/src/rt.rs | 2 +- library/std/src/thread/mod.rs | 170 ++++++++------------------ tests/debuginfo/thread.rs | 8 +- tests/rustdoc/demo-allocator-54478.rs | 1 - 4 files changed, 58 insertions(+), 123 deletions(-) diff --git a/library/std/src/rt.rs b/library/std/src/rt.rs index 24a362072ab61..56952aac6c63d 100644 --- a/library/std/src/rt.rs +++ b/library/std/src/rt.rs @@ -110,7 +110,7 @@ unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) { // handle does not match the current ID, we should attempt to use the // current thread ID here instead of unconditionally creating a new // one. Also see #130210. - let thread = unsafe { Thread::new_main(thread::current_id()) }; + let thread = Thread::new_main(thread::current_id()); if let Err(_thread) = thread::set_current(thread) { // `thread::current` will create a new handle if none has been set yet. // Thus, if someone uses it before main, this call will fail. That's a diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index b6ee00a253a95..81d75641ab595 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -158,12 +158,9 @@ #[cfg(all(test, not(any(target_os = "emscripten", target_os = "wasi"))))] mod tests; -use core::cell::SyncUnsafeCell; -use core::ffi::CStr; -use core::mem::MaybeUninit; - use crate::any::Any; use crate::cell::UnsafeCell; +use crate::ffi::CStr; use crate::marker::PhantomData; use crate::mem::{self, ManuallyDrop, forget}; use crate::num::NonZero; @@ -1259,31 +1256,30 @@ impl ThreadId { // Thread //////////////////////////////////////////////////////////////////////////////// +/// The internal representation of a `Thread`'s name. +enum ThreadName { + Main, + Other(ThreadNameString), + Unnamed, +} + // This module ensures private fields are kept private, which is necessary to enforce the safety requirements. mod thread_name_string { use core::str; + use super::ThreadName; use crate::ffi::{CStr, CString}; /// Like a `String` it's guaranteed UTF-8 and like a `CString` it's null terminated. pub(crate) struct ThreadNameString { inner: CString, } - - impl ThreadNameString { - pub fn as_str(&self) -> &str { - // SAFETY: `self.inner` is only initialised via `String`, which upholds the validity invariant of `str`. - unsafe { str::from_utf8_unchecked(self.inner.to_bytes()) } - } - } - impl core::ops::Deref for ThreadNameString { type Target = CStr; fn deref(&self) -> &CStr { &self.inner } } - impl From for ThreadNameString { fn from(s: String) -> Self { Self { @@ -1291,82 +1287,34 @@ mod thread_name_string { } } } + impl ThreadName { + pub fn as_cstr(&self) -> Option<&CStr> { + match self { + ThreadName::Main => Some(c"main"), + ThreadName::Other(other) => Some(other), + ThreadName::Unnamed => None, + } + } + + pub fn as_str(&self) -> Option<&str> { + // SAFETY: `as_cstr` can only return `Some` for a fixed CStr or a `ThreadNameString`, + // which is guaranteed to be UTF-8. + self.as_cstr().map(|s| unsafe { str::from_utf8_unchecked(s.to_bytes()) }) + } + } } pub(crate) use thread_name_string::ThreadNameString; -static MAIN_THREAD_INFO: SyncUnsafeCell<(MaybeUninit, MaybeUninit)> = - SyncUnsafeCell::new((MaybeUninit::uninit(), MaybeUninit::uninit())); - -/// The internal representation of a `Thread` that is not the main thread. -struct OtherInner { - name: Option, +/// The internal representation of a `Thread` handle +struct Inner { + name: ThreadName, // Guaranteed to be UTF-8 id: ThreadId, parker: Parker, } -/// The internal representation of a `Thread` handle. -#[derive(Clone)] -enum Inner { - /// Represents the main thread. May only be constructed by Thread::new_main. - Main(&'static (ThreadId, Parker)), - /// Represents any other thread. - Other(Pin>), -} - impl Inner { - fn id(&self) -> ThreadId { - match self { - Self::Main((thread_id, _)) => *thread_id, - Self::Other(other) => other.id, - } - } - - fn cname(&self) -> Option<&CStr> { - match self { - Self::Main(_) => Some(c"main"), - Self::Other(other) => other.name.as_deref(), - } - } - - fn name(&self) -> Option<&str> { - match self { - Self::Main(_) => Some("main"), - Self::Other(other) => other.name.as_ref().map(ThreadNameString::as_str), - } - } - - fn into_raw(self) -> *const () { - match self { - // Just return the pointer to `MAIN_THREAD_INFO`. - Self::Main(ptr) => crate::ptr::from_ref(ptr).cast(), - Self::Other(arc) => { - // Safety: We only expose an opaque pointer, which maintains the `Pin` invariant. - let inner = unsafe { Pin::into_inner_unchecked(arc) }; - Arc::into_raw(inner) as *const () - } - } - } - - /// # Safety - /// - /// See [`Thread::from_raw`]. - unsafe fn from_raw(ptr: *const ()) -> Self { - // If the pointer is to `MAIN_THREAD_INFO`, we know it is the `Main` variant. - if crate::ptr::eq(ptr.cast(), &MAIN_THREAD_INFO) { - Self::Main(unsafe { &*ptr.cast() }) - } else { - // Safety: Upheld by caller - Self::Other(unsafe { Pin::new_unchecked(Arc::from_raw(ptr as *const OtherInner)) }) - } - } - - fn parker(&self) -> Pin<&Parker> { - match self { - Self::Main((_, parker_ref)) => Pin::static_ref(parker_ref), - Self::Other(inner) => unsafe { - Pin::map_unchecked(inner.as_ref(), |inner| &inner.parker) - }, - } + fn parker(self: Pin<&Self>) -> Pin<&Parker> { + unsafe { Pin::map_unchecked(self, |inner| &inner.parker) } } } @@ -1390,47 +1338,33 @@ impl Inner { /// docs of [`Builder`] and [`spawn`] for more details. /// /// [`thread::current`]: current::current -pub struct Thread(Inner); +pub struct Thread { + inner: Pin>, +} impl Thread { /// Used only internally to construct a thread object without spawning. pub(crate) fn new(id: ThreadId, name: String) -> Thread { - Self::new_inner(id, Some(ThreadNameString::from(name))) + Self::new_inner(id, ThreadName::Other(name.into())) } pub(crate) fn new_unnamed(id: ThreadId) -> Thread { - Self::new_inner(id, None) + Self::new_inner(id, ThreadName::Unnamed) } - /// Used in runtime to construct main thread - /// - /// # Safety - /// - /// This must only ever be called once, and must be called on the main thread. - pub(crate) unsafe fn new_main(thread_id: ThreadId) -> Thread { - // Safety: As this is only called once and on the main thread, nothing else is accessing MAIN_THREAD_INFO - // as the only other read occurs in `main_thread_info` *after* the main thread has been constructed, - // and this function is the only one that constructs the main thread. - // - // Pre-main thread spawning cannot hit this either, as the caller promises that this is only called on the main thread. - let main_thread_info = unsafe { &mut *MAIN_THREAD_INFO.get() }; - - unsafe { Parker::new_in_place((&raw mut main_thread_info.1).cast()) }; - main_thread_info.0.write(thread_id); - - // Store a `'static` ref to the initialised ThreadId and Parker, - // to avoid having to repeatedly prove initialisation. - Self(Inner::Main(unsafe { &*MAIN_THREAD_INFO.get().cast() })) + /// Constructs the thread handle for the main thread. + pub(crate) fn new_main(id: ThreadId) -> Thread { + Self::new_inner(id, ThreadName::Main) } - fn new_inner(id: ThreadId, name: Option) -> Thread { + fn new_inner(id: ThreadId, name: ThreadName) -> Thread { // We have to use `unsafe` here to construct the `Parker` in-place, // which is required for the UNIX implementation. // // SAFETY: We pin the Arc immediately after creation, so its address never // changes. let inner = unsafe { - let mut arc = Arc::::new_uninit(); + let mut arc = Arc::::new_uninit(); let ptr = Arc::get_mut_unchecked(&mut arc).as_mut_ptr(); (&raw mut (*ptr).name).write(name); (&raw mut (*ptr).id).write(id); @@ -1438,7 +1372,7 @@ impl Thread { Pin::new_unchecked(arc.assume_init()) }; - Self(Inner::Other(inner)) + Thread { inner } } /// Like the public [`park`], but callable on any handle. This is used to @@ -1447,7 +1381,7 @@ impl Thread { /// # Safety /// May only be called from the thread to which this handle belongs. pub(crate) unsafe fn park(&self) { - unsafe { self.0.parker().park() } + unsafe { self.inner.as_ref().parker().park() } } /// Like the public [`park_timeout`], but callable on any handle. This is @@ -1456,7 +1390,7 @@ impl Thread { /// # Safety /// May only be called from the thread to which this handle belongs. pub(crate) unsafe fn park_timeout(&self, dur: Duration) { - unsafe { self.0.parker().park_timeout(dur) } + unsafe { self.inner.as_ref().parker().park_timeout(dur) } } /// Atomically makes the handle's token available if it is not already. @@ -1492,7 +1426,7 @@ impl Thread { #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn unpark(&self) { - self.0.parker().unpark(); + self.inner.as_ref().parker().unpark(); } /// Gets the thread's unique identifier. @@ -1512,7 +1446,7 @@ impl Thread { #[stable(feature = "thread_id", since = "1.19.0")] #[must_use] pub fn id(&self) -> ThreadId { - self.0.id() + self.inner.id } /// Gets the thread's name. @@ -1555,11 +1489,7 @@ impl Thread { #[stable(feature = "rust1", since = "1.0.0")] #[must_use] pub fn name(&self) -> Option<&str> { - self.0.name() - } - - fn cname(&self) -> Option<&CStr> { - self.0.cname() + self.inner.name.as_str() } /// Consumes the `Thread`, returning a raw pointer. @@ -1583,7 +1513,9 @@ impl Thread { /// ``` #[unstable(feature = "thread_raw", issue = "97523")] pub fn into_raw(self) -> *const () { - self.0.into_raw() + // Safety: We only expose an opaque pointer, which maintains the `Pin` invariant. + let inner = unsafe { Pin::into_inner_unchecked(self.inner) }; + Arc::into_raw(inner) as *const () } /// Constructs a `Thread` from a raw pointer. @@ -1605,7 +1537,11 @@ impl Thread { #[unstable(feature = "thread_raw", issue = "97523")] pub unsafe fn from_raw(ptr: *const ()) -> Thread { // Safety: Upheld by caller. - unsafe { Thread(Inner::from_raw(ptr)) } + unsafe { Thread { inner: Pin::new_unchecked(Arc::from_raw(ptr as *const Inner)) } } + } + + fn cname(&self) -> Option<&CStr> { + self.inner.name.as_cstr() } } diff --git a/tests/debuginfo/thread.rs b/tests/debuginfo/thread.rs index dc8cb0832192b..0415f586f5d90 100644 --- a/tests/debuginfo/thread.rs +++ b/tests/debuginfo/thread.rs @@ -12,15 +12,15 @@ // cdb-check:join_handle,d [Type: std::thread::JoinHandle >] // cdb-check: [...] __0 [Type: std::thread::JoinInner >] // -// cdb-command:dx -r3 t,d +// cdb-command:dx t,d // cdb-check:t,d : [...] [Type: std::thread::Thread *] -// cdb-check: [...] __0 : Other [Type: enum2$] -// cdb-check: [...] __0 [Type: core::pin::Pin >] +// cdb-check:[...] inner [...][Type: core::pin::Pin >] use std::thread; #[allow(unused_variables)] -fn main() { +fn main() +{ let join_handle = thread::spawn(|| { println!("Initialize a thread"); }); diff --git a/tests/rustdoc/demo-allocator-54478.rs b/tests/rustdoc/demo-allocator-54478.rs index 80acfc0ff58a1..dd98e80f03ade 100644 --- a/tests/rustdoc/demo-allocator-54478.rs +++ b/tests/rustdoc/demo-allocator-54478.rs @@ -40,7 +40,6 @@ //! } //! //! fn main() { -//! drop(String::from("An allocation")); //! assert!(unsafe { HIT }); //! } //! ``` From 14f7f4b7bfcfbfe9a6f778e762cac83420e08007 Mon Sep 17 00:00:00 2001 From: joboet Date: Mon, 25 Nov 2024 13:01:35 +0100 Subject: [PATCH 05/14] std: lazily allocate the main thread handle Thereby, we also allow accessing thread::current before main: as the runtime no longer tries to install its own handle, this will no longer trigger an abort. Rather, the name returned from name will only be "main" after the runtime initialization code has run, but I think that is acceptable. This new approach also requires some changes to the signal handling code, as calling `thread::current` would now allocate when called on the main thread, which is not acceptable. I fixed this by adding a new function (`with_current_name`) that performs all the naming logic without allocation or without initializing the thread ID (which could allocate on some platforms). --- library/std/src/panicking.rs | 39 ++--- library/std/src/rt.rs | 23 +-- .../std/src/sys/pal/unix/stack_overflow.rs | 9 +- .../std/src/sys/pal/windows/stack_overflow.rs | 8 +- library/std/src/thread/current.rs | 36 ++--- library/std/src/thread/mod.rs | 150 ++++++++++++------ 6 files changed, 151 insertions(+), 114 deletions(-) diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index 3b02254548b1c..8e50bf11dd082 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -258,31 +258,34 @@ fn default_hook(info: &PanicHookInfo<'_>) { let location = info.location().unwrap(); let msg = payload_as_str(info.payload()); - let thread = thread::try_current(); - let name = thread.as_ref().and_then(|t| t.name()).unwrap_or(""); let write = #[optimize(size)] |err: &mut dyn crate::io::Write| { // Use a lock to prevent mixed output in multithreading context. // Some platforms also require it when printing a backtrace, like `SymFromAddr` on Windows. let mut lock = backtrace::lock(); - // Try to write the panic message to a buffer first to prevent other concurrent outputs - // interleaving with it. - let mut buffer = [0u8; 512]; - let mut cursor = crate::io::Cursor::new(&mut buffer[..]); - - let write_msg = |dst: &mut dyn crate::io::Write| { - // We add a newline to ensure the panic message appears at the start of a line. - writeln!(dst, "\nthread '{name}' panicked at {location}:\n{msg}") - }; - if write_msg(&mut cursor).is_ok() { - let pos = cursor.position() as usize; - let _ = err.write_all(&buffer[0..pos]); - } else { - // The message did not fit into the buffer, write it directly instead. - let _ = write_msg(err); - }; + thread::with_current_name(|name| { + let name = name.unwrap_or(""); + + // Try to write the panic message to a buffer first to prevent other concurrent outputs + // interleaving with it. + let mut buffer = [0u8; 512]; + let mut cursor = crate::io::Cursor::new(&mut buffer[..]); + + let write_msg = |dst: &mut dyn crate::io::Write| { + // We add a newline to ensure the panic message appears at the start of a line. + writeln!(dst, "\nthread '{name}' panicked at {location}:\n{msg}") + }; + + if write_msg(&mut cursor).is_ok() { + let pos = cursor.position() as usize; + let _ = err.write_all(&buffer[0..pos]); + } else { + // The message did not fit into the buffer, write it directly instead. + let _ = write_msg(err); + }; + }); static FIRST_PANIC: AtomicBool = AtomicBool::new(true); diff --git a/library/std/src/rt.rs b/library/std/src/rt.rs index 56952aac6c63d..384369b0012b5 100644 --- a/library/std/src/rt.rs +++ b/library/std/src/rt.rs @@ -23,7 +23,7 @@ pub use core::panicking::{panic_display, panic_fmt}; #[rustfmt::skip] use crate::any::Any; use crate::sync::Once; -use crate::thread::{self, Thread}; +use crate::thread::{self, main_thread}; use crate::{mem, panic, sys}; // Prints to the "panic output", depending on the platform this may be: @@ -102,24 +102,9 @@ unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) { sys::init(argc, argv, sigpipe) }; - // Set up the current thread handle to give it the right name. - // - // When code running before main uses `ReentrantLock` (for example by - // using `println!`), the thread ID can become initialized before we - // create this handle. Since `set_current` fails when the ID of the - // handle does not match the current ID, we should attempt to use the - // current thread ID here instead of unconditionally creating a new - // one. Also see #130210. - let thread = Thread::new_main(thread::current_id()); - if let Err(_thread) = thread::set_current(thread) { - // `thread::current` will create a new handle if none has been set yet. - // Thus, if someone uses it before main, this call will fail. That's a - // bad idea though, as we then cannot set the main thread name here. - // - // FIXME: detect the main thread in `thread::current` and use the - // correct name there. - rtabort!("code running before main must not use thread::current"); - } + // Remember the main thread ID to give it the correct name. + // SAFETY: this is the only time and place where we call this function. + unsafe { main_thread::set(thread::current_id()) }; } /// Clean up the thread-local runtime state. This *should* be run after all other diff --git a/library/std/src/sys/pal/unix/stack_overflow.rs b/library/std/src/sys/pal/unix/stack_overflow.rs index 69b31da427fcb..db5c6bd3a1c32 100644 --- a/library/std/src/sys/pal/unix/stack_overflow.rs +++ b/library/std/src/sys/pal/unix/stack_overflow.rs @@ -100,10 +100,11 @@ mod imp { // If the faulting address is within the guard page, then we print a // message saying so and abort. if start <= addr && addr < end { - rtprintpanic!( - "\nthread '{}' has overflowed its stack\n", - thread::current().name().unwrap_or("") - ); + thread::with_current_name(|name| { + let name = name.unwrap_or(""); + rtprintpanic!("\nthread '{name}' has overflowed its stack\n"); + }); + rtabort!("stack overflow"); } else { // Unregister ourselves by reverting back to the default behavior. diff --git a/library/std/src/sys/pal/windows/stack_overflow.rs b/library/std/src/sys/pal/windows/stack_overflow.rs index 467e21ab56a28..734cd30bed08f 100644 --- a/library/std/src/sys/pal/windows/stack_overflow.rs +++ b/library/std/src/sys/pal/windows/stack_overflow.rs @@ -18,10 +18,10 @@ unsafe extern "system" fn vectored_handler(ExceptionInfo: *mut c::EXCEPTION_POIN let code = rec.ExceptionCode; if code == c::EXCEPTION_STACK_OVERFLOW { - rtprintpanic!( - "\nthread '{}' has overflowed its stack\n", - thread::current().name().unwrap_or("") - ); + thread::with_current_name(|name| { + let name = name.unwrap_or(""); + rtprintpanic!("\nthread '{name}' has overflowed its stack\n"); + }); } c::EXCEPTION_CONTINUE_SEARCH } diff --git a/library/std/src/thread/current.rs b/library/std/src/thread/current.rs index 3d2c288b36085..fa88059fe64e3 100644 --- a/library/std/src/thread/current.rs +++ b/library/std/src/thread/current.rs @@ -15,7 +15,7 @@ local_pointer! { /// /// We store the thread ID so that it never gets destroyed during the lifetime /// of a thread, either using `#[thread_local]` or multiple `local_pointer!`s. -mod id { +pub(super) mod id { use super::*; cfg_if::cfg_if! { @@ -27,7 +27,7 @@ mod id { pub(super) const CHEAP: bool = true; - pub(super) fn get() -> Option { + pub(crate) fn get() -> Option { ID.get() } @@ -44,7 +44,7 @@ mod id { pub(super) const CHEAP: bool = false; - pub(super) fn get() -> Option { + pub(crate) fn get() -> Option { let id0 = ID0.get().addr() as u64; let id16 = ID16.get().addr() as u64; let id32 = ID32.get().addr() as u64; @@ -67,7 +67,7 @@ mod id { pub(super) const CHEAP: bool = false; - pub(super) fn get() -> Option { + pub(crate) fn get() -> Option { let id0 = ID0.get().addr() as u64; let id32 = ID32.get().addr() as u64; ThreadId::from_u64((id32 << 32) + id0) @@ -85,7 +85,7 @@ mod id { pub(super) const CHEAP: bool = true; - pub(super) fn get() -> Option { + pub(crate) fn get() -> Option { let id = ID.get().addr() as u64; ThreadId::from_u64(id) } @@ -112,7 +112,7 @@ mod id { /// Tries to set the thread handle for the current thread. Fails if a handle was /// already set or if the thread ID of `thread` would change an already-set ID. -pub(crate) fn set_current(thread: Thread) -> Result<(), Thread> { +pub(super) fn set_current(thread: Thread) -> Result<(), Thread> { if CURRENT.get() != NONE { return Err(thread); } @@ -140,28 +140,28 @@ pub(crate) fn current_id() -> ThreadId { // to retrieve it from the current thread handle, which will only take one // TLS access. if !id::CHEAP { - let current = CURRENT.get(); - if current > DESTROYED { - unsafe { - let current = ManuallyDrop::new(Thread::from_raw(current)); - return current.id(); - } + if let Some(id) = try_with_current(|t| t.map(|t| t.id())) { + return id; } } id::get_or_init() } -/// Gets a handle to the thread that invokes it, if the handle has been initialized. -pub(crate) fn try_current() -> Option { +/// Gets a reference to the handle of the thread that invokes it, if the handle +/// has been initialized. +pub(super) fn try_with_current(f: F) -> R +where + F: FnOnce(Option<&Thread>) -> R, +{ let current = CURRENT.get(); if current > DESTROYED { unsafe { let current = ManuallyDrop::new(Thread::from_raw(current)); - Some((*current).clone()) + f(Some(¤t)) } } else { - None + f(None) } } @@ -176,7 +176,7 @@ pub(crate) fn current_or_unnamed() -> Thread { (*current).clone() } } else if current == DESTROYED { - Thread::new_unnamed(id::get_or_init()) + Thread::new(id::get_or_init(), None) } else { init_current(current) } @@ -221,7 +221,7 @@ fn init_current(current: *mut ()) -> Thread { CURRENT.set(BUSY); // If the thread ID was initialized already, use it. let id = id::get_or_init(); - let thread = Thread::new_unnamed(id); + let thread = Thread::new(id, None); // Make sure that `crate::rt::thread_cleanup` will be run, which will // call `drop_current`. diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 81d75641ab595..cf7a951d100ec 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -183,7 +183,8 @@ mod current; #[stable(feature = "rust1", since = "1.0.0")] pub use current::current; -pub(crate) use current::{current_id, current_or_unnamed, drop_current, set_current, try_current}; +pub(crate) use current::{current_id, current_or_unnamed, drop_current}; +use current::{set_current, try_with_current}; mod spawnhook; @@ -498,10 +499,7 @@ impl Builder { }); let id = ThreadId::new(); - let my_thread = match name { - Some(name) => Thread::new(id, name), - None => Thread::new_unnamed(id), - }; + let my_thread = Thread::new(id, name); let hooks = if no_hooks { spawnhook::ChildSpawnHooks::default() @@ -1232,7 +1230,7 @@ impl ThreadId { } } - #[cfg(not(target_thread_local))] + #[cfg(any(not(target_thread_local), target_has_atomic = "64"))] fn from_u64(v: u64) -> Option { NonZero::new(v).map(ThreadId) } @@ -1256,30 +1254,16 @@ impl ThreadId { // Thread //////////////////////////////////////////////////////////////////////////////// -/// The internal representation of a `Thread`'s name. -enum ThreadName { - Main, - Other(ThreadNameString), - Unnamed, -} - // This module ensures private fields are kept private, which is necessary to enforce the safety requirements. mod thread_name_string { - use core::str; - - use super::ThreadName; use crate::ffi::{CStr, CString}; + use crate::str; /// Like a `String` it's guaranteed UTF-8 and like a `CString` it's null terminated. pub(crate) struct ThreadNameString { inner: CString, } - impl core::ops::Deref for ThreadNameString { - type Target = CStr; - fn deref(&self) -> &CStr { - &self.inner - } - } + impl From for ThreadNameString { fn from(s: String) -> Self { Self { @@ -1287,27 +1271,91 @@ mod thread_name_string { } } } - impl ThreadName { - pub fn as_cstr(&self) -> Option<&CStr> { - match self { - ThreadName::Main => Some(c"main"), - ThreadName::Other(other) => Some(other), - ThreadName::Unnamed => None, - } + + impl ThreadNameString { + pub fn as_cstr(&self) -> &CStr { + &self.inner } - pub fn as_str(&self) -> Option<&str> { - // SAFETY: `as_cstr` can only return `Some` for a fixed CStr or a `ThreadNameString`, - // which is guaranteed to be UTF-8. - self.as_cstr().map(|s| unsafe { str::from_utf8_unchecked(s.to_bytes()) }) + pub fn as_str(&self) -> &str { + // SAFETY: `ThreadNameString` is guaranteed to be UTF-8. + unsafe { str::from_utf8_unchecked(self.inner.to_bytes()) } + } + } +} + +use thread_name_string::ThreadNameString; + +pub(crate) mod main_thread { + cfg_if::cfg_if! { + if #[cfg(target_has_atomic = "64")] { + use super::ThreadId; + use crate::sync::atomic::AtomicU64; + use crate::sync::atomic::Ordering::Relaxed; + + static MAIN: AtomicU64 = AtomicU64::new(0); + + pub(super) fn get() -> Option { + ThreadId::from_u64(MAIN.load(Relaxed)) + } + + /// # Safety + /// May only be called once. + pub(crate) unsafe fn set(id: ThreadId) { + MAIN.store(id.as_u64().get(), Relaxed) + } + } else { + use super::ThreadId; + use crate::mem::MaybeUninit; + use crate::sync::atomic::AtomicBool; + use crate::sync::atomic::Ordering::{Acquire, Release}; + + static INIT: AtomicBool = AtomicBool::new(false); + static mut MAIN: MaybeUninit = MaybeUninit::uninit(); + + pub(super) fn get() -> Option { + if INIT.load(Acquire) { + Some(unsafe { MAIN.assume_init() }) + } else { + None + } + } + + /// # Safety + /// May only be called once. + pub(crate) unsafe fn set(id: ThreadId) { + unsafe { MAIN = MaybeUninit::new(id) }; + INIT.store(true, Release); + } } } } -pub(crate) use thread_name_string::ThreadNameString; + +pub(crate) fn with_current_name(f: F) -> R +where + F: FnOnce(Option<&str>) -> R, +{ + try_with_current(|thread| { + if let Some(thread) = thread { + if let Some(name) = &thread.inner.name { + return f(Some(name.as_str())); + } else if Some(thread.inner.id) == main_thread::get() { + return f(Some("main")); + } + } else if let Some(main) = main_thread::get() + && let Some(id) = current::id::get() + && id == main + { + return f(Some("main")); + } + + f(None) + }) +} /// The internal representation of a `Thread` handle struct Inner { - name: ThreadName, // Guaranteed to be UTF-8 + name: Option, id: ThreadId, parker: Parker, } @@ -1343,21 +1391,9 @@ pub struct Thread { } impl Thread { - /// Used only internally to construct a thread object without spawning. - pub(crate) fn new(id: ThreadId, name: String) -> Thread { - Self::new_inner(id, ThreadName::Other(name.into())) - } - - pub(crate) fn new_unnamed(id: ThreadId) -> Thread { - Self::new_inner(id, ThreadName::Unnamed) - } - - /// Constructs the thread handle for the main thread. - pub(crate) fn new_main(id: ThreadId) -> Thread { - Self::new_inner(id, ThreadName::Main) - } + pub(crate) fn new(id: ThreadId, name: Option) -> Thread { + let name = name.map(ThreadNameString::from); - fn new_inner(id: ThreadId, name: ThreadName) -> Thread { // We have to use `unsafe` here to construct the `Parker` in-place, // which is required for the UNIX implementation. // @@ -1489,7 +1525,13 @@ impl Thread { #[stable(feature = "rust1", since = "1.0.0")] #[must_use] pub fn name(&self) -> Option<&str> { - self.inner.name.as_str() + if let Some(name) = &self.inner.name { + Some(name.as_str()) + } else if main_thread::get() == Some(self.inner.id) { + Some("main") + } else { + None + } } /// Consumes the `Thread`, returning a raw pointer. @@ -1541,7 +1583,13 @@ impl Thread { } fn cname(&self) -> Option<&CStr> { - self.inner.name.as_cstr() + if let Some(name) = &self.inner.name { + Some(name.as_cstr()) + } else if main_thread::get() == Some(self.inner.id) { + Some(c"main") + } else { + None + } } } From 1d2525260416349272fc2262818344274d896518 Mon Sep 17 00:00:00 2001 From: joboet Date: Mon, 25 Nov 2024 14:03:02 +0100 Subject: [PATCH 06/14] make sure that the allocator is actually called in allocator test Originally authored by GnomedDev --- tests/rustdoc/demo-allocator-54478.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/rustdoc/demo-allocator-54478.rs b/tests/rustdoc/demo-allocator-54478.rs index dd98e80f03ade..80acfc0ff58a1 100644 --- a/tests/rustdoc/demo-allocator-54478.rs +++ b/tests/rustdoc/demo-allocator-54478.rs @@ -40,6 +40,7 @@ //! } //! //! fn main() { +//! drop(String::from("An allocation")); //! assert!(unsafe { HIT }); //! } //! ``` From 2c28cc45c83961ccfaba790f7dc8e6c1c9d26502 Mon Sep 17 00:00:00 2001 From: joboet Date: Wed, 27 Nov 2024 14:40:09 +0100 Subject: [PATCH 07/14] add comments explaining main thread identification --- library/std/src/thread/current.rs | 3 +++ library/std/src/thread/mod.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/library/std/src/thread/current.rs b/library/std/src/thread/current.rs index fa88059fe64e3..414711298f047 100644 --- a/library/std/src/thread/current.rs +++ b/library/std/src/thread/current.rs @@ -156,6 +156,9 @@ where { let current = CURRENT.get(); if current > DESTROYED { + // SAFETY: `Arc` does not contain interior mutability, so it does not + // matter that the address of the handle might be different depending + // on where this is called. unsafe { let current = ManuallyDrop::new(Thread::from_raw(current)); f(Some(¤t)) diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index cf7a951d100ec..59b395336f2e3 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -1286,6 +1286,18 @@ mod thread_name_string { use thread_name_string::ThreadNameString; +/// Store the ID of the main thread. +/// +/// The thread handle for the main thread is created lazily, and this might even +/// happen pre-main. Since not every platform has a way to identify the main +/// thread when that happens – macOS's `pthread_main_np` function being a notable +/// exception – we cannot assign it the right name right then. Instead, in our +/// runtime startup code, we remember the thread ID of the main thread (through +/// this modules `set` function) and use it to identify the main thread from then +/// on. This works reliably and has the additional advantage that we can report +/// the right thread name on main even after the thread handle has been destroyed. +/// Note however that this also means that the name reported in pre-main functions +/// will be incorrect, but that's just something we have to live with. pub(crate) mod main_thread { cfg_if::cfg_if! { if #[cfg(target_has_atomic = "64")] { @@ -1331,21 +1343,35 @@ pub(crate) mod main_thread { } } +/// Run a function with the current thread's name. +/// +/// Modulo thread local accesses, this function is safe to call from signal +/// handlers and in similar circumstances where allocations are not possible. pub(crate) fn with_current_name(f: F) -> R where F: FnOnce(Option<&str>) -> R, { try_with_current(|thread| { if let Some(thread) = thread { + // If there is a current thread handle, try to use the name stored + // there. if let Some(name) = &thread.inner.name { return f(Some(name.as_str())); } else if Some(thread.inner.id) == main_thread::get() { + // The main thread doesn't store its name in the handle, we must + // identify it through its ID. Since we already have the `Thread`, + // we can retrieve the ID from it instead of going through another + // thread local. return f(Some("main")); } } else if let Some(main) = main_thread::get() && let Some(id) = current::id::get() && id == main { + // The main thread doesn't always have a thread handle, we must + // identify it through its ID instead. The checks are ordered so + // that the current ID is only loaded if it is actually needed, + // since loading it from TLS might need multiple expensive accesses. return f(Some("main")); } From bf545ce2fee9f1fef0fd9e63e46b5aab5de61465 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 14 Jan 2025 17:54:53 +0000 Subject: [PATCH 08/14] Prefer lower TraitUpcasting candidates --- .../src/solve/trait_goals.rs | 8 +++-- .../rustc_trait_selection/src/solve/select.rs | 4 +++ .../src/traits/select/confirmation.rs | 2 +- .../src/traits/select/mod.rs | 12 ++++++++ compiler/rustc_type_ir/src/solve/mod.rs | 5 ++-- .../prefer-lower-candidates.rs | 29 +++++++++++++++++++ 6 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 tests/ui/traits/trait-upcasting/prefer-lower-candidates.rs diff --git a/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs b/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs index d68fca608299a..f4d7c3ce76cf2 100644 --- a/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs +++ b/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs @@ -741,12 +741,14 @@ where a_data.principal(), )); } else if let Some(a_principal) = a_data.principal() { - for new_a_principal in - elaborate::supertraits(self.cx(), a_principal.with_self_ty(cx, a_ty)).skip(1) + for (idx, new_a_principal) in + elaborate::supertraits(self.cx(), a_principal.with_self_ty(cx, a_ty)) + .enumerate() + .skip(1) { responses.extend(self.consider_builtin_upcast_to_principal( goal, - CandidateSource::BuiltinImpl(BuiltinImplSource::TraitUpcasting), + CandidateSource::BuiltinImpl(BuiltinImplSource::TraitUpcasting(idx)), a_data, a_region, b_data, diff --git a/compiler/rustc_trait_selection/src/solve/select.rs b/compiler/rustc_trait_selection/src/solve/select.rs index 1661852903c48..b0b6274907d0c 100644 --- a/compiler/rustc_trait_selection/src/solve/select.rs +++ b/compiler/rustc_trait_selection/src/solve/select.rs @@ -117,6 +117,10 @@ fn candidate_should_be_dropped_in_favor_of<'tcx>( CandidateSource::BuiltinImpl(BuiltinImplSource::Object(a)), CandidateSource::BuiltinImpl(BuiltinImplSource::Object(b)), ) => a >= b, + ( + CandidateSource::BuiltinImpl(BuiltinImplSource::TraitUpcasting(a)), + CandidateSource::BuiltinImpl(BuiltinImplSource::TraitUpcasting(b)), + ) => a >= b, // Prefer dyn candidates over non-dyn candidates. This is necessary to // handle the unsoundness between `impl Any for T` and `dyn Any: Any`. ( diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 3619d16cde248..0ccb0fc0615c0 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -1090,7 +1090,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { )? .expect("did not expect ambiguity during confirmation"); - Ok(ImplSource::Builtin(BuiltinImplSource::TraitUpcasting, nested)) + Ok(ImplSource::Builtin(BuiltinImplSource::TraitUpcasting(idx), nested)) } fn confirm_builtin_unsize_candidate( diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 9e7da5eb3689c..5581ea46882c0 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -1895,6 +1895,18 @@ impl<'tcx> SelectionContext<'_, 'tcx> { Some(None) => {} None => return None, } + // Same for upcasting. + let upcast_bound = candidates + .iter() + .filter_map(|c| { + if let TraitUpcastingUnsizeCandidate(i) = c.candidate { Some(i) } else { None } + }) + .try_reduce(|c1, c2| if has_non_region_infer { None } else { Some(c1.min(c2)) }); + match upcast_bound { + Some(Some(index)) => return Some(TraitUpcastingUnsizeCandidate(index)), + Some(None) => {} + None => return None, + } // Finally, handle overlapping user-written impls. let impls = candidates.iter().filter_map(|c| { diff --git a/compiler/rustc_type_ir/src/solve/mod.rs b/compiler/rustc_type_ir/src/solve/mod.rs index 1ae904d50e066..fbb5c7430eb2a 100644 --- a/compiler/rustc_type_ir/src/solve/mod.rs +++ b/compiler/rustc_type_ir/src/solve/mod.rs @@ -177,8 +177,9 @@ pub enum BuiltinImplSource { /// A built-in implementation of `Upcast` for trait objects to other trait objects. /// /// This can be removed when `feature(dyn_upcasting)` is stabilized, since we only - /// use it to detect when upcasting traits in hir typeck. - TraitUpcasting, + /// use it to detect when upcasting traits in hir typeck. The index is only used + /// for winnowing. + TraitUpcasting(usize), /// Unsizing a tuple like `(A, B, ..., X)` to `(A, B, ..., Y)` if `X` unsizes to `Y`. /// /// This can be removed when `feature(tuple_unsizing)` is stabilized, since we only diff --git a/tests/ui/traits/trait-upcasting/prefer-lower-candidates.rs b/tests/ui/traits/trait-upcasting/prefer-lower-candidates.rs new file mode 100644 index 0000000000000..4bc59b09fb5af --- /dev/null +++ b/tests/ui/traits/trait-upcasting/prefer-lower-candidates.rs @@ -0,0 +1,29 @@ +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) +//@[next] compile-flags: -Znext-solver +//@ check-pass + +// Ensure we don't have ambiguity when upcasting to two supertraits +// that are identical modulo normalization. + +#![feature(trait_upcasting)] + +trait Supertrait { + fn method(&self) {} +} +impl Supertrait for () {} + +trait Identity { + type Selff; +} +impl Identity for Selff { + type Selff = Selff; +} +trait Trait

: Supertrait<()> + Supertrait<

::Selff> {} + +impl

Trait

for () {} + +fn main() { + let x: &dyn Trait<()> = &(); + let x: &dyn Supertrait<()> = x; +} From fe6deb0fa146b762ba809354371518793578e744 Mon Sep 17 00:00:00 2001 From: binarycat Date: Mon, 13 Jan 2025 15:31:15 -0600 Subject: [PATCH 09/14] Treat other items as functions for the purpose of type-based search constants and statics are nullary functions, and struct fields are unary functions. --- .../rustdoc/src/read-documentation/search.md | 14 +++++++++- src/librustdoc/html/render/search_index.rs | 27 +++++++++++++++++++ src/librustdoc/html/static/js/search.js | 11 ++++++++ tests/rustdoc-gui/search-tab.goml | 2 +- tests/rustdoc-js-std/const-is-nullary-func.js | 7 +++++ tests/rustdoc-js-std/field-is-unary-func.js | 7 +++++ 6 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 tests/rustdoc-js-std/const-is-nullary-func.js create mode 100644 tests/rustdoc-js-std/field-is-unary-func.js diff --git a/src/doc/rustdoc/src/read-documentation/search.md b/src/doc/rustdoc/src/read-documentation/search.md index e06dcdb7ed2fb..bace2f5f95390 100644 --- a/src/doc/rustdoc/src/read-documentation/search.md +++ b/src/doc/rustdoc/src/read-documentation/search.md @@ -52,9 +52,10 @@ methods on the allocator or free functions. [`Layout`]: ../../alloc/index.html?search=Layout&filter-crate=alloc -## Searching By Type Signature for functions +## Searching By Type Signature If you know more specifically what the function you want to look at does, +or you want to know how to get from one type to another, Rustdoc can search by more than one type at once in the parameters and return value. Multiple parameters are separated by `,` commas, and the return value is written with after a `->` arrow. @@ -86,6 +87,17 @@ the standard library and functions that are included in the results list: [iterasslice]: ../../std/vec/struct.Vec.html?search=vec%3A%3Aintoiter%20->%20[T]&filter-crate=std [iterreduce]: ../../std/index.html?search=iterator%2C%20fnmut%20->%20T&filter-crate=std +### Non-functions in type-based search +Certain items that are not functions are treated as though they +were a semantically equivelent function. + +For example, struct fields are treated as though they were getter methods. +This means that a search for `CpuidResult -> u32` will show +the `CpuidResult::eax` field in the results. + +Additionally, `const` and `static` items are treated as nullary functions, +so `-> u32` will match `u32::MAX`. + ### How type-based search works In a complex type-based search, Rustdoc always treats every item's name as literal. diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index 66c52bec4bad7..e4a9a2b512e50 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/src/librustdoc/html/render/search_index.rs @@ -840,6 +840,22 @@ pub(crate) fn get_function_type_for_search( | clean::RequiredMethodItem(ref f) => { get_fn_inputs_and_outputs(f, tcx, impl_or_trait_generics, cache) } + clean::ConstantItem(ref c) => make_nullary_fn(&c.type_), + clean::StaticItem(ref s) => make_nullary_fn(&s.type_), + clean::StructFieldItem(ref t) => { + let Some(parent) = parent else { + return None; + }; + let mut rgen: FxIndexMap)> = + Default::default(); + let output = get_index_type(t, vec![], &mut rgen); + let input = RenderType { + id: Some(RenderTypeId::DefId(parent)), + generics: None, + bindings: None, + }; + (vec![input], vec![output], vec![], vec![]) + } _ => return None, }; @@ -1353,6 +1369,17 @@ fn simplify_fn_constraint<'a>( res.push((ty_constrained_assoc, ty_constraints)); } +/// Create a fake nullary function. +/// +/// Used to allow type-based search on constants and statics. +fn make_nullary_fn( + clean_type: &clean::Type, +) -> (Vec, Vec, Vec, Vec>) { + let mut rgen: FxIndexMap)> = Default::default(); + let output = get_index_type(clean_type, vec![], &mut rgen); + (vec![], vec![output], vec![], vec![]) +} + /// Return the full list of types when bounds have been resolved. /// /// i.e. `fn foo>(x: u32, y: B)` will return diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index 5fd5eb1447843..b455fa4b54332 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -63,6 +63,8 @@ const TY_PRIMITIVE = itemTypes.indexOf("primitive"); const TY_GENERIC = itemTypes.indexOf("generic"); const TY_IMPORT = itemTypes.indexOf("import"); const TY_TRAIT = itemTypes.indexOf("trait"); +const TY_FN = itemTypes.indexOf("fn"); +const TY_METHOD = itemTypes.indexOf("method"); const ROOT_PATH = typeof window !== "undefined" ? window.rootPath : "../"; // Hard limit on how deep to recurse into generics when doing type-driven search. @@ -2749,6 +2751,15 @@ class DocSearch { return a - b; } + // in type based search, put functions first + if (parsedQuery.hasReturnArrow) { + a = (aaa.item.ty !== TY_FN && aaa.item.ty !== TY_METHOD); + b = (bbb.item.ty !== TY_FN && bbb.item.ty !== TY_METHOD); + if (a !== b) { + return a - b; + } + } + // Sort by distance in the path part, if specified // (less changes required to match means higher rankings) a = aaa.path_dist; diff --git a/tests/rustdoc-gui/search-tab.goml b/tests/rustdoc-gui/search-tab.goml index 3879c127fd0f8..eea561e0c6760 100644 --- a/tests/rustdoc-gui/search-tab.goml +++ b/tests/rustdoc-gui/search-tab.goml @@ -79,7 +79,7 @@ set-window-size: (851, 600) // Check the size and count in tabs assert-text: ("#search-tabs > button:nth-child(1) > .count", " (26) ") -assert-text: ("#search-tabs > button:nth-child(2) > .count", " (6)  ") +assert-text: ("#search-tabs > button:nth-child(2) > .count", " (7)  ") assert-text: ("#search-tabs > button:nth-child(3) > .count", " (0)  ") store-property: ("#search-tabs > button:nth-child(1)", {"offsetWidth": buttonWidth}) assert-property: ("#search-tabs > button:nth-child(2)", {"offsetWidth": |buttonWidth|}) diff --git a/tests/rustdoc-js-std/const-is-nullary-func.js b/tests/rustdoc-js-std/const-is-nullary-func.js new file mode 100644 index 0000000000000..e929741b038f4 --- /dev/null +++ b/tests/rustdoc-js-std/const-is-nullary-func.js @@ -0,0 +1,7 @@ +const EXPECTED = { + 'query': '-> char', + 'others': [ + { 'path': 'std::char', 'name': 'from_digit' }, + { 'path': 'std::char', 'name': 'MAX' }, + ], +} diff --git a/tests/rustdoc-js-std/field-is-unary-func.js b/tests/rustdoc-js-std/field-is-unary-func.js new file mode 100644 index 0000000000000..09ce8a0dde0a4 --- /dev/null +++ b/tests/rustdoc-js-std/field-is-unary-func.js @@ -0,0 +1,7 @@ +const EXPECTED = { + // one of the only non-generic structs with public fields + 'query': 'CpuidResult -> u32', + 'others': [ + { 'path': 'core::arch::x86::CpuidResult', 'name': 'eax' }, + ], +} From 561a097b653c53d264fb60b417ba07c19ee14f4a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 1 Jan 2025 18:45:41 +0100 Subject: [PATCH 10/14] late_report_deprecation: move fast-path closer to the core logic --- compiler/rustc_middle/src/middle/stability.rs | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_middle/src/middle/stability.rs b/compiler/rustc_middle/src/middle/stability.rs index 84c3c2eb49e27..999908b9bd80e 100644 --- a/compiler/rustc_middle/src/middle/stability.rs +++ b/compiler/rustc_middle/src/middle/stability.rs @@ -232,9 +232,18 @@ fn late_report_deprecation( return; } + let is_in_effect = depr.is_in_effect(); + let lint = deprecation_lint(is_in_effect); + + // Calculating message for lint involves calling `self.def_path_str`, + // which will by default invoke the expensive `visible_parent_map` query. + // Skip all that work if the lint is allowed anyway. + if tcx.lint_level_at_node(lint, hir_id).0 == Level::Allow { + return; + } + let def_path = with_no_trimmed_paths!(tcx.def_path_str(def_id)); let def_kind = tcx.def_descr(def_id); - let is_in_effect = depr.is_in_effect(); let method_span = method_span.unwrap_or(span); let suggestion = @@ -250,7 +259,7 @@ fn late_report_deprecation( note: depr.note, since_kind: deprecated_since_kind(is_in_effect, depr.since), }; - tcx.emit_node_span_lint(deprecation_lint(is_in_effect), hir_id, method_span, diag); + tcx.emit_node_span_lint(lint, hir_id, method_span, diag); } /// Result of `TyCtxt::eval_stability`. @@ -360,13 +369,7 @@ impl<'tcx> TyCtxt<'tcx> { // hierarchy. let depr_attr = &depr_entry.attr; if !skip || depr_attr.is_since_rustc_version() { - // Calculating message for lint involves calling `self.def_path_str`. - // Which by default to calculate visible path will invoke expensive `visible_parent_map` query. - // So we skip message calculation altogether, if lint is allowed. - let lint = deprecation_lint(depr_attr.is_in_effect()); - if self.lint_level_at_node(lint, id).0 != Level::Allow { - late_report_deprecation(self, depr_attr, span, method_span, id, def_id); - } + late_report_deprecation(self, depr_attr, span, method_span, id, def_id); } }; } From cf0ab86251f1ab3e11132332695eee73ff746e27 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 1 Jan 2025 19:09:01 +0100 Subject: [PATCH 11/14] allowed_through_unstable_modules: support showing a deprecation message when the unstable module name is used --- compiler/rustc_ast/src/attr/mod.rs | 2 + .../src/stability.rs | 15 +- .../src/attributes/stability.rs | 21 +-- compiler/rustc_feature/src/builtin_attrs.rs | 2 +- compiler/rustc_passes/src/stability.rs | 136 +++++++++++++----- src/librustdoc/clean/types.rs | 2 +- src/librustdoc/formats/cache.rs | 2 +- src/librustdoc/passes/propagate_stability.rs | 6 +- .../clippy_lints/src/std_instead_of_core.rs | 2 +- .../allowed-through-unstable.rs | 1 + .../allowed-through-unstable.stderr | 12 +- .../allowed-through-unstable-core.rs | 4 + 12 files changed, 146 insertions(+), 59 deletions(-) diff --git a/compiler/rustc_ast/src/attr/mod.rs b/compiler/rustc_ast/src/attr/mod.rs index 97385b2eaab89..51f1858001321 100644 --- a/compiler/rustc_ast/src/attr/mod.rs +++ b/compiler/rustc_ast/src/attr/mod.rs @@ -723,6 +723,8 @@ impl MetaItemLit { 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 } diff --git a/compiler/rustc_attr_data_structures/src/stability.rs b/compiler/rustc_attr_data_structures/src/stability.rs index 3c77d4c766c57..dfda04387ec5a 100644 --- a/compiler/rustc_attr_data_structures/src/stability.rs +++ b/compiler/rustc_attr_data_structures/src/stability.rs @@ -101,6 +101,16 @@ impl PartialConstStability { } } +#[derive(Encodable, Decodable, PartialEq, Copy, Clone, Debug, Eq, Hash)] +#[derive(HashStable_Generic)] +pub enum AllowedThroughUnstableModules { + /// This does not get a deprecation warning. We still generally would prefer people to use the + /// fully stable path, and a warning will likely be emitted in the future. + WithoutDeprecation, + /// Emit the given deprecation warning. + WithDeprecation(Symbol), +} + /// The available stability levels. #[derive(Encodable, Decodable, PartialEq, Copy, Clone, Debug, Eq, Hash)] #[derive(HashStable_Generic)] @@ -137,9 +147,8 @@ pub enum StabilityLevel { Stable { /// Rust release which stabilized this feature. since: StableSince, - /// Is this item allowed to be referred to on stable, despite being contained in unstable - /// modules? - allowed_through_unstable_modules: bool, + /// This is `Some` if this item allowed to be referred to on stable via unstable modules. + allowed_through_unstable_modules: Option, }, } diff --git a/compiler/rustc_attr_parsing/src/attributes/stability.rs b/compiler/rustc_attr_parsing/src/attributes/stability.rs index 89937e1c593d1..bfbe51b27d88d 100644 --- a/compiler/rustc_attr_parsing/src/attributes/stability.rs +++ b/compiler/rustc_attr_parsing/src/attributes/stability.rs @@ -6,8 +6,8 @@ use rustc_ast::MetaItem; use rustc_ast::attr::AttributeExt; use rustc_ast_pretty::pprust; use rustc_attr_data_structures::{ - ConstStability, DefaultBodyStability, Stability, StabilityLevel, StableSince, UnstableReason, - VERSION_PLACEHOLDER, + AllowedThroughUnstableModules, ConstStability, DefaultBodyStability, Stability, StabilityLevel, + StableSince, UnstableReason, VERSION_PLACEHOLDER, }; use rustc_errors::ErrorGuaranteed; use rustc_session::Session; @@ -24,11 +24,16 @@ pub fn find_stability( item_sp: Span, ) -> Option<(Stability, Span)> { let mut stab: Option<(Stability, Span)> = None; - let mut allowed_through_unstable_modules = false; + let mut allowed_through_unstable_modules = None; for attr in attrs { match attr.name_or_empty() { - sym::rustc_allowed_through_unstable_modules => allowed_through_unstable_modules = true, + sym::rustc_allowed_through_unstable_modules => { + allowed_through_unstable_modules = Some(match attr.value_str() { + Some(msg) => AllowedThroughUnstableModules::WithDeprecation(msg), + None => AllowedThroughUnstableModules::WithoutDeprecation, + }) + } sym::unstable => { if stab.is_some() { sess.dcx().emit_err(session_diagnostics::MultipleStabilityLevels { @@ -56,15 +61,15 @@ pub fn find_stability( } } - if allowed_through_unstable_modules { + if let Some(allowed_through_unstable_modules) = allowed_through_unstable_modules { match &mut stab { Some(( Stability { - level: StabilityLevel::Stable { allowed_through_unstable_modules, .. }, + level: StabilityLevel::Stable { allowed_through_unstable_modules: in_stab, .. }, .. }, _, - )) => *allowed_through_unstable_modules = true, + )) => *in_stab = Some(allowed_through_unstable_modules), _ => { sess.dcx() .emit_err(session_diagnostics::RustcAllowedUnstablePairing { span: item_sp }); @@ -283,7 +288,7 @@ fn parse_stability(sess: &Session, attr: &impl AttributeExt) -> Option<(Symbol, match feature { Ok(feature) => { - let level = StabilityLevel::Stable { since, allowed_through_unstable_modules: false }; + let level = StabilityLevel::Stable { since, allowed_through_unstable_modules: None }; Some((feature, level)) } Err(ErrorGuaranteed { .. }) => None, diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index c28a4360f6f91..5510e7e09e50c 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -623,7 +623,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ EncodeCrossCrate::No, "allow_internal_unsafe side-steps the unsafe_code lint", ), rustc_attr!( - rustc_allowed_through_unstable_modules, Normal, template!(Word), + rustc_allowed_through_unstable_modules, Normal, template!(Word, NameValueStr: "deprecation message"), WarnFollowing, EncodeCrossCrate::No, "rustc_allowed_through_unstable_modules special cases accidental stabilizations of stable items \ through unstable paths" diff --git a/compiler/rustc_passes/src/stability.rs b/compiler/rustc_passes/src/stability.rs index 30f9e698521f1..f6c892203f91f 100644 --- a/compiler/rustc_passes/src/stability.rs +++ b/compiler/rustc_passes/src/stability.rs @@ -5,8 +5,8 @@ use std::mem::replace; use std::num::NonZero; use rustc_attr_parsing::{ - self as attr, ConstStability, DeprecatedSince, Stability, StabilityLevel, StableSince, - UnstableReason, VERSION_PLACEHOLDER, + self as attr, AllowedThroughUnstableModules, ConstStability, DeprecatedSince, Stability, + StabilityLevel, StableSince, UnstableReason, VERSION_PLACEHOLDER, }; use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::unord::{ExtendUnord, UnordMap, UnordSet}; @@ -20,11 +20,16 @@ use rustc_hir::{FieldDef, Item, ItemKind, TraitRef, Ty, TyKind, Variant}; use rustc_middle::hir::nested_filter; use rustc_middle::middle::lib_features::{FeatureStability, LibFeatures}; use rustc_middle::middle::privacy::EffectiveVisibilities; -use rustc_middle::middle::stability::{AllowUnstable, DeprecationEntry, Index}; +use rustc_middle::middle::stability::{ + AllowUnstable, Deprecated, DeprecationEntry, EvalResult, Index, +}; use rustc_middle::query::Providers; use rustc_middle::ty::TyCtxt; +use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_session::lint; -use rustc_session::lint::builtin::{INEFFECTIVE_UNSTABLE_TRAIT_IMPL, USELESS_DEPRECATED}; +use rustc_session::lint::builtin::{ + DEPRECATED, INEFFECTIVE_UNSTABLE_TRAIT_IMPL, USELESS_DEPRECATED, +}; use rustc_span::{Span, Symbol, sym}; use tracing::{debug, info}; @@ -844,42 +849,95 @@ impl<'tcx> Visitor<'tcx> for Checker<'tcx> { }, ); - let is_allowed_through_unstable_modules = |def_id| { - self.tcx.lookup_stability(def_id).is_some_and(|stab| match stab.level { - StabilityLevel::Stable { allowed_through_unstable_modules, .. } => { - allowed_through_unstable_modules + if item_is_allowed { + // The item itself is allowed; check whether the path there is also allowed. + let is_allowed_through_unstable_modules: Option = + self.tcx.lookup_stability(def_id).and_then(|stab| match stab.level { + StabilityLevel::Stable { allowed_through_unstable_modules, .. } => { + allowed_through_unstable_modules + } + _ => None, + }); + + if is_allowed_through_unstable_modules.is_none() { + // Check parent modules stability as well if the item the path refers to is itself + // stable. We only emit warnings for unstable path segments if the item is stable + // or allowed because stability is often inherited, so the most common case is that + // both the segments and the item are unstable behind the same feature flag. + // + // We check here rather than in `visit_path_segment` to prevent visiting the last + // path segment twice + // + // We include special cases via #[rustc_allowed_through_unstable_modules] for items + // that were accidentally stabilized through unstable paths before this check was + // added, such as `core::intrinsics::transmute` + let parents = path.segments.iter().rev().skip(1); + for path_segment in parents { + if let Some(def_id) = path_segment.res.opt_def_id() { + // use `None` for id to prevent deprecation check + self.tcx.check_stability_allow_unstable( + def_id, + None, + path.span, + None, + if is_unstable_reexport(self.tcx, id) { + AllowUnstable::Yes + } else { + AllowUnstable::No + }, + ); + } } - _ => false, - }) - }; - - if item_is_allowed && !is_allowed_through_unstable_modules(def_id) { - // Check parent modules stability as well if the item the path refers to is itself - // stable. We only emit warnings for unstable path segments if the item is stable - // or allowed because stability is often inherited, so the most common case is that - // both the segments and the item are unstable behind the same feature flag. - // - // We check here rather than in `visit_path_segment` to prevent visiting the last - // path segment twice - // - // We include special cases via #[rustc_allowed_through_unstable_modules] for items - // that were accidentally stabilized through unstable paths before this check was - // added, such as `core::intrinsics::transmute` - let parents = path.segments.iter().rev().skip(1); - for path_segment in parents { - if let Some(def_id) = path_segment.res.opt_def_id() { - // use `None` for id to prevent deprecation check - self.tcx.check_stability_allow_unstable( - def_id, - None, - path.span, - None, - if is_unstable_reexport(self.tcx, id) { - AllowUnstable::Yes - } else { - AllowUnstable::No - }, - ); + } else if let Some(AllowedThroughUnstableModules::WithDeprecation(deprecation)) = + is_allowed_through_unstable_modules + { + // Similar to above, but we cannot use `check_stability_allow_unstable` as that would + // immediately show the stability error. We just want to know the result and disaplay + // our own kind of error. + let parents = path.segments.iter().rev().skip(1); + for path_segment in parents { + if let Some(def_id) = path_segment.res.opt_def_id() { + // use `None` for id to prevent deprecation check + let eval_result = self.tcx.eval_stability_allow_unstable( + def_id, + None, + path.span, + None, + if is_unstable_reexport(self.tcx, id) { + AllowUnstable::Yes + } else { + AllowUnstable::No + }, + ); + let is_allowed = matches!(eval_result, EvalResult::Allow); + if !is_allowed { + // Calculating message for lint involves calling `self.def_path_str`, + // which will by default invoke the expensive `visible_parent_map` query. + // Skip all that work if the lint is allowed anyway. + if self.tcx.lint_level_at_node(DEPRECATED, id).0 + == lint::Level::Allow + { + return; + } + // Show a deprecation message. + let def_path = + with_no_trimmed_paths!(self.tcx.def_path_str(def_id)); + let def_kind = self.tcx.def_descr(def_id); + let diag = Deprecated { + sub: None, + kind: def_kind.to_owned(), + path: def_path, + note: Some(deprecation), + since_kind: lint::DeprecatedSinceKind::InEffect, + }; + self.tcx.emit_node_span_lint( + DEPRECATED, + id, + method_span.unwrap_or(path.span), + diag, + ); + } + } } } } diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index dcee96978d2b5..5e7c175657955 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -406,7 +406,7 @@ impl Item { // were never supposed to work at all. let stab = self.stability(tcx)?; if let rustc_attr_parsing::StabilityLevel::Stable { - allowed_through_unstable_modules: true, + allowed_through_unstable_modules: Some(_), .. } = stab.level { diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index cbb3ce6abe6a7..4760e579199aa 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -316,7 +316,7 @@ impl DocFolder for CacheBuilder<'_, '_> { let skip_because_unstable = matches!( item.stability.map(|stab| stab.level), - Some(StabilityLevel::Stable { allowed_through_unstable_modules: true, .. }) + Some(StabilityLevel::Stable { allowed_through_unstable_modules: Some(_), .. }) ); if (!self.cache.stripped_mod && !skip_because_unstable) || self.is_json_output { diff --git a/src/librustdoc/passes/propagate_stability.rs b/src/librustdoc/passes/propagate_stability.rs index febb52a3b00a2..9c958710c4200 100644 --- a/src/librustdoc/passes/propagate_stability.rs +++ b/src/librustdoc/passes/propagate_stability.rs @@ -119,7 +119,7 @@ fn merge_stability( parent_stability: Option, ) -> Option { if let Some(own_stab) = own_stability - && let StabilityLevel::Stable { since: own_since, allowed_through_unstable_modules: false } = + && let StabilityLevel::Stable { since: own_since, allowed_through_unstable_modules: None } = own_stab.level && let Some(parent_stab) = parent_stability && (parent_stab.is_unstable() @@ -127,12 +127,12 @@ fn merge_stability( { parent_stability } else if let Some(mut own_stab) = own_stability - && let StabilityLevel::Stable { since, allowed_through_unstable_modules: true } = + && let StabilityLevel::Stable { since, allowed_through_unstable_modules: Some(_) } = own_stab.level && parent_stability.is_some_and(|stab| stab.is_stable()) { // this property does not apply transitively through re-exports - own_stab.level = StabilityLevel::Stable { since, allowed_through_unstable_modules: false }; + own_stab.level = StabilityLevel::Stable { since, allowed_through_unstable_modules: None }; Some(own_stab) } else { own_stability diff --git a/src/tools/clippy/clippy_lints/src/std_instead_of_core.rs b/src/tools/clippy/clippy_lints/src/std_instead_of_core.rs index 82ff13a5aff16..8ec7bfe9edd3f 100644 --- a/src/tools/clippy/clippy_lints/src/std_instead_of_core.rs +++ b/src/tools/clippy/clippy_lints/src/std_instead_of_core.rs @@ -180,7 +180,7 @@ fn is_stable(cx: &LateContext<'_>, mut def_id: DefId, msrv: &Msrv) -> bool { if let Some(stability) = cx.tcx.lookup_stability(def_id) && let StabilityLevel::Stable { since, - allowed_through_unstable_modules: false, + allowed_through_unstable_modules: None, } = stability.level { let stable = match since { diff --git a/tests/ui/stability-attribute/allowed-through-unstable.rs b/tests/ui/stability-attribute/allowed-through-unstable.rs index 29911a70be942..e03417a4dae8d 100644 --- a/tests/ui/stability-attribute/allowed-through-unstable.rs +++ b/tests/ui/stability-attribute/allowed-through-unstable.rs @@ -6,4 +6,5 @@ extern crate allowed_through_unstable_core; use allowed_through_unstable_core::unstable_module::OldStableTraitAllowedThoughUnstable; +use allowed_through_unstable_core::unstable_module::OldStableTraitAllowedThoughUnstableWithDeprecation; //~WARN use of deprecated module `allowed_through_unstable_core::unstable_module`: use the new path instead use allowed_through_unstable_core::unstable_module::NewStableTraitNotAllowedThroughUnstable; //~ ERROR use of unstable library feature `unstable_test_feature` diff --git a/tests/ui/stability-attribute/allowed-through-unstable.stderr b/tests/ui/stability-attribute/allowed-through-unstable.stderr index 00eea9f730d66..8d07b0cf9e8f3 100644 --- a/tests/ui/stability-attribute/allowed-through-unstable.stderr +++ b/tests/ui/stability-attribute/allowed-through-unstable.stderr @@ -1,5 +1,13 @@ +warning: use of deprecated module `allowed_through_unstable_core::unstable_module`: use the new path instead + --> $DIR/allowed-through-unstable.rs:9:53 + | +LL | use allowed_through_unstable_core::unstable_module::OldStableTraitAllowedThoughUnstableWithDeprecation; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(deprecated)]` on by default + error[E0658]: use of unstable library feature `unstable_test_feature` - --> $DIR/allowed-through-unstable.rs:9:5 + --> $DIR/allowed-through-unstable.rs:10:5 | LL | use allowed_through_unstable_core::unstable_module::NewStableTraitNotAllowedThroughUnstable; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -8,6 +16,6 @@ LL | use allowed_through_unstable_core::unstable_module::NewStableTraitNotAllowe = help: add `#![feature(unstable_test_feature)]` 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: aborting due to 1 previous error +error: aborting due to 1 previous error; 1 warning emitted For more information about this error, try `rustc --explain E0658`. diff --git a/tests/ui/stability-attribute/auxiliary/allowed-through-unstable-core.rs b/tests/ui/stability-attribute/auxiliary/allowed-through-unstable-core.rs index b597009a309c9..9dfbb451d04bb 100644 --- a/tests/ui/stability-attribute/auxiliary/allowed-through-unstable-core.rs +++ b/tests/ui/stability-attribute/auxiliary/allowed-through-unstable-core.rs @@ -9,6 +9,10 @@ pub mod unstable_module { #[rustc_allowed_through_unstable_modules] pub trait OldStableTraitAllowedThoughUnstable {} + #[stable(feature = "stable_test_feature", since = "1.2.0")] + #[rustc_allowed_through_unstable_modules = "use the new path instead"] + pub trait OldStableTraitAllowedThoughUnstableWithDeprecation {} + #[stable(feature = "stable_test_feature", since = "1.2.0")] pub trait NewStableTraitNotAllowedThroughUnstable {} } From f1c95c9000804cd84206dfd309aa26c67eb13a4c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 1 Jan 2025 20:22:59 +0100 Subject: [PATCH 12/14] intrinsics: deprecate calling them via the unstable std::intrinsics path --- library/core/src/intrinsics/mod.rs | 24 ++++++++++--- .../clippy/tests/ui/std_instead_of_core.fixed | 2 +- .../clippy/tests/ui/std_instead_of_core.rs | 2 +- src/tools/clippy/tests/ui/transmute.rs | 22 ++++++------ src/tools/clippy/tests/ui/transmute.stderr | 36 +++++++++---------- .../accidental-stable-in-unstable.rs | 1 + .../accidental-stable-in-unstable.stderr | 10 +++++- 7 files changed, 61 insertions(+), 36 deletions(-) diff --git a/library/core/src/intrinsics/mod.rs b/library/core/src/intrinsics/mod.rs index 7b31bbec7547c..41b2ffad6680d 100644 --- a/library/core/src/intrinsics/mod.rs +++ b/library/core/src/intrinsics/mod.rs @@ -1897,7 +1897,11 @@ pub const fn forget(_: T) { /// } /// ``` #[stable(feature = "rust1", since = "1.0.0")] -#[rustc_allowed_through_unstable_modules] +#[cfg_attr(bootstrap, rustc_allowed_through_unstable_modules)] +#[cfg_attr( + not(bootstrap), + rustc_allowed_through_unstable_modules = "import this function via `std::mem` instead" +)] #[rustc_const_stable(feature = "const_transmute", since = "1.56.0")] #[rustc_diagnostic_item = "transmute"] #[rustc_nounwind] @@ -4325,7 +4329,11 @@ pub const fn ptr_metadata + ?Sized, M>(_ptr: *cons /// [`Vec::append`]: ../../std/vec/struct.Vec.html#method.append #[doc(alias = "memcpy")] #[stable(feature = "rust1", since = "1.0.0")] -#[rustc_allowed_through_unstable_modules] +#[cfg_attr(bootstrap, rustc_allowed_through_unstable_modules)] +#[cfg_attr( + not(bootstrap), + rustc_allowed_through_unstable_modules = "import this function via `std::mem` instead" +)] #[rustc_const_stable(feature = "const_intrinsic_copy", since = "1.83.0")] #[inline(always)] #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces @@ -4429,7 +4437,11 @@ pub const unsafe fn copy_nonoverlapping(src: *const T, dst: *mut T, count: us /// ``` #[doc(alias = "memmove")] #[stable(feature = "rust1", since = "1.0.0")] -#[rustc_allowed_through_unstable_modules] +#[cfg_attr(bootstrap, rustc_allowed_through_unstable_modules)] +#[cfg_attr( + not(bootstrap), + rustc_allowed_through_unstable_modules = "import this function via `std::mem` instead" +)] #[rustc_const_stable(feature = "const_intrinsic_copy", since = "1.83.0")] #[inline(always)] #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces @@ -4512,7 +4524,11 @@ pub const unsafe fn copy(src: *const T, dst: *mut T, count: usize) { /// ``` #[doc(alias = "memset")] #[stable(feature = "rust1", since = "1.0.0")] -#[rustc_allowed_through_unstable_modules] +#[cfg_attr(bootstrap, rustc_allowed_through_unstable_modules)] +#[cfg_attr( + not(bootstrap), + rustc_allowed_through_unstable_modules = "import this function via `std::mem` instead" +)] #[rustc_const_stable(feature = "const_ptr_write", since = "1.83.0")] #[inline(always)] #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces diff --git a/src/tools/clippy/tests/ui/std_instead_of_core.fixed b/src/tools/clippy/tests/ui/std_instead_of_core.fixed index 227b98c683e97..ec158ee02de89 100644 --- a/src/tools/clippy/tests/ui/std_instead_of_core.fixed +++ b/src/tools/clippy/tests/ui/std_instead_of_core.fixed @@ -1,7 +1,7 @@ //@aux-build:proc_macro_derive.rs #![warn(clippy::std_instead_of_core)] -#![allow(unused_imports)] +#![allow(unused_imports, deprecated)] extern crate alloc; diff --git a/src/tools/clippy/tests/ui/std_instead_of_core.rs b/src/tools/clippy/tests/ui/std_instead_of_core.rs index 01bb78dd3bf1d..9c3c1658d8fec 100644 --- a/src/tools/clippy/tests/ui/std_instead_of_core.rs +++ b/src/tools/clippy/tests/ui/std_instead_of_core.rs @@ -1,7 +1,7 @@ //@aux-build:proc_macro_derive.rs #![warn(clippy::std_instead_of_core)] -#![allow(unused_imports)] +#![allow(unused_imports, deprecated)] extern crate alloc; diff --git a/src/tools/clippy/tests/ui/transmute.rs b/src/tools/clippy/tests/ui/transmute.rs index eeea3f080b1c9..7f5bdea4acf3c 100644 --- a/src/tools/clippy/tests/ui/transmute.rs +++ b/src/tools/clippy/tests/ui/transmute.rs @@ -24,31 +24,31 @@ fn my_vec() -> MyVec { #[warn(clippy::useless_transmute)] unsafe fn _generic<'a, T, U: 'a>(t: &'a T) { // FIXME: should lint - // let _: &'a T = core::intrinsics::transmute(t); + // let _: &'a T = core::mem::transmute(t); - let _: &'a U = core::intrinsics::transmute(t); + let _: &'a U = core::mem::transmute(t); - let _: *const T = core::intrinsics::transmute(t); + let _: *const T = core::mem::transmute(t); //~^ ERROR: transmute from a reference to a pointer //~| NOTE: `-D clippy::useless-transmute` implied by `-D warnings` - let _: *mut T = core::intrinsics::transmute(t); + let _: *mut T = core::mem::transmute(t); //~^ ERROR: transmute from a reference to a pointer - let _: *const U = core::intrinsics::transmute(t); + let _: *const U = core::mem::transmute(t); //~^ ERROR: transmute from a reference to a pointer } #[warn(clippy::useless_transmute)] fn useless() { unsafe { - let _: Vec = core::intrinsics::transmute(my_vec()); + let _: Vec = core::mem::transmute(my_vec()); //~^ ERROR: transmute from a type (`std::vec::Vec`) to itself let _: Vec = core::mem::transmute(my_vec()); //~^ ERROR: transmute from a type (`std::vec::Vec`) to itself - let _: Vec = std::intrinsics::transmute(my_vec()); + let _: Vec = std::mem::transmute(my_vec()); //~^ ERROR: transmute from a type (`std::vec::Vec`) to itself let _: Vec = std::mem::transmute(my_vec()); @@ -94,17 +94,17 @@ fn crosspointer() { let int_mut_ptr: *mut Usize = &mut int as *mut Usize; unsafe { - let _: Usize = core::intrinsics::transmute(int_const_ptr); + let _: Usize = core::mem::transmute(int_const_ptr); //~^ ERROR: transmute from a type (`*const Usize`) to the type that it points to ( //~| NOTE: `-D clippy::crosspointer-transmute` implied by `-D warnings` - let _: Usize = core::intrinsics::transmute(int_mut_ptr); + let _: Usize = core::mem::transmute(int_mut_ptr); //~^ ERROR: transmute from a type (`*mut Usize`) to the type that it points to (`U - let _: *const Usize = core::intrinsics::transmute(my_int()); + let _: *const Usize = core::mem::transmute(my_int()); //~^ ERROR: transmute from a type (`Usize`) to a pointer to that type (`*const Usi - let _: *mut Usize = core::intrinsics::transmute(my_int()); + let _: *mut Usize = core::mem::transmute(my_int()); //~^ ERROR: transmute from a type (`Usize`) to a pointer to that type (`*mut Usize } } diff --git a/src/tools/clippy/tests/ui/transmute.stderr b/src/tools/clippy/tests/ui/transmute.stderr index 41a10f381dc53..b5032772856ee 100644 --- a/src/tools/clippy/tests/ui/transmute.stderr +++ b/src/tools/clippy/tests/ui/transmute.stderr @@ -1,8 +1,8 @@ error: transmute from a reference to a pointer --> tests/ui/transmute.rs:31:23 | -LL | let _: *const T = core::intrinsics::transmute(t); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `t as *const T` +LL | let _: *const T = core::mem::transmute(t); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `t as *const T` | = note: `-D clippy::useless-transmute` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::useless_transmute)]` @@ -10,20 +10,20 @@ LL | let _: *const T = core::intrinsics::transmute(t); error: transmute from a reference to a pointer --> tests/ui/transmute.rs:35:21 | -LL | let _: *mut T = core::intrinsics::transmute(t); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `t as *const T as *mut T` +LL | let _: *mut T = core::mem::transmute(t); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `t as *const T as *mut T` error: transmute from a reference to a pointer --> tests/ui/transmute.rs:38:23 | -LL | let _: *const U = core::intrinsics::transmute(t); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `t as *const T as *const U` +LL | let _: *const U = core::mem::transmute(t); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `t as *const T as *const U` error: transmute from a type (`std::vec::Vec`) to itself --> tests/ui/transmute.rs:45:27 | -LL | let _: Vec = core::intrinsics::transmute(my_vec()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _: Vec = core::mem::transmute(my_vec()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: transmute from a type (`std::vec::Vec`) to itself --> tests/ui/transmute.rs:48:27 @@ -34,8 +34,8 @@ LL | let _: Vec = core::mem::transmute(my_vec()); error: transmute from a type (`std::vec::Vec`) to itself --> tests/ui/transmute.rs:51:27 | -LL | let _: Vec = std::intrinsics::transmute(my_vec()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _: Vec = std::mem::transmute(my_vec()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: transmute from a type (`std::vec::Vec`) to itself --> tests/ui/transmute.rs:54:27 @@ -64,8 +64,8 @@ LL | let _: *const usize = std::mem::transmute(1 + 1usize); error: transmute from a type (`*const Usize`) to the type that it points to (`Usize`) --> tests/ui/transmute.rs:97:24 | -LL | let _: Usize = core::intrinsics::transmute(int_const_ptr); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _: Usize = core::mem::transmute(int_const_ptr); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::crosspointer-transmute` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::crosspointer_transmute)]` @@ -73,20 +73,20 @@ LL | let _: Usize = core::intrinsics::transmute(int_const_ptr); error: transmute from a type (`*mut Usize`) to the type that it points to (`Usize`) --> tests/ui/transmute.rs:101:24 | -LL | let _: Usize = core::intrinsics::transmute(int_mut_ptr); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _: Usize = core::mem::transmute(int_mut_ptr); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: transmute from a type (`Usize`) to a pointer to that type (`*const Usize`) --> tests/ui/transmute.rs:104:31 | -LL | let _: *const Usize = core::intrinsics::transmute(my_int()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _: *const Usize = core::mem::transmute(my_int()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: transmute from a type (`Usize`) to a pointer to that type (`*mut Usize`) --> tests/ui/transmute.rs:107:29 | -LL | let _: *mut Usize = core::intrinsics::transmute(my_int()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let _: *mut Usize = core::mem::transmute(my_int()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: transmute from a `u8` to a `bool` --> tests/ui/transmute.rs:114:28 diff --git a/tests/ui/stability-attribute/accidental-stable-in-unstable.rs b/tests/ui/stability-attribute/accidental-stable-in-unstable.rs index 86a9d2066eb58..a36a78ee442ce 100644 --- a/tests/ui/stability-attribute/accidental-stable-in-unstable.rs +++ b/tests/ui/stability-attribute/accidental-stable-in-unstable.rs @@ -8,3 +8,4 @@ use core::unicode::UNICODE_VERSION; //~ ERROR use of unstable library feature `u // Known accidental stabilizations with known users // fully stable @ core::mem::transmute use core::intrinsics::transmute; // depended upon by rand_core +//~^WARN deprecated diff --git a/tests/ui/stability-attribute/accidental-stable-in-unstable.stderr b/tests/ui/stability-attribute/accidental-stable-in-unstable.stderr index 9943e6d7ac6ab..16e3676aa6503 100644 --- a/tests/ui/stability-attribute/accidental-stable-in-unstable.stderr +++ b/tests/ui/stability-attribute/accidental-stable-in-unstable.stderr @@ -7,6 +7,14 @@ LL | use core::unicode::UNICODE_VERSION; = help: add `#![feature(unicode_internals)]` 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: aborting due to 1 previous error +warning: use of deprecated module `std::intrinsics`: import this function via `std::mem` instead + --> $DIR/accidental-stable-in-unstable.rs:10:23 + | +LL | use core::intrinsics::transmute; // depended upon by rand_core + | ^^^^^^^^^ + | + = note: `#[warn(deprecated)]` on by default + +error: aborting due to 1 previous error; 1 warning emitted For more information about this error, try `rustc --explain E0658`. From 7ae494abd45bb09549bba796ac0c5848b423a38d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 6 Jan 2025 17:32:57 +0100 Subject: [PATCH 13/14] show deprecation message in rustdoc, too --- src/librustdoc/clean/types.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 5e7c175657955..8088ab4c82c30 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -6,7 +6,9 @@ use std::{fmt, iter}; use arrayvec::ArrayVec; use rustc_abi::{ExternAbi, VariantIdx}; -use rustc_attr_parsing::{ConstStability, Deprecation, Stability, StableSince}; +use rustc_attr_parsing::{ + AllowedThroughUnstableModules, ConstStability, Deprecation, Stability, StableSince, +}; use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet}; use rustc_hir::def::{CtorKind, DefKind, Res}; use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE, LocalDefId}; @@ -406,15 +408,19 @@ impl Item { // were never supposed to work at all. let stab = self.stability(tcx)?; if let rustc_attr_parsing::StabilityLevel::Stable { - allowed_through_unstable_modules: Some(_), + allowed_through_unstable_modules: Some(note), .. } = stab.level { + let note = match note { + AllowedThroughUnstableModules::WithDeprecation(note) => Some(note), + // FIXME: Would be better to say *something* here about the *path* being + // deprecated rather than the item. + AllowedThroughUnstableModules::WithoutDeprecation => None, + }; Some(Deprecation { - // FIXME(#131676, #135003): when a note is added to this stability tag, - // translate it here since: rustc_attr_parsing::DeprecatedSince::Unspecified, - note: None, + note, suggestion: None, }) } else { From 896953aee71be9f175a4b48e3235502999229aed Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 15 Jan 2025 14:19:02 +0300 Subject: [PATCH 14/14] remove outdated FIXME Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/doc.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/doc.rs b/src/bootstrap/src/core/build_steps/doc.rs index dc09d2d109330..0eb4080c053a7 100644 --- a/src/bootstrap/src/core/build_steps/doc.rs +++ b/src/bootstrap/src/core/build_steps/doc.rs @@ -65,8 +65,6 @@ macro_rules! book { // NOTE: When adding a book here, make sure to ALSO build the book by // adding a build step in `src/bootstrap/code/builder/mod.rs`! // NOTE: Make sure to add the corresponding submodule when adding a new book. -// FIXME: Make checking for a submodule automatic somehow (maybe by having a list of all submodules -// and checking against it?). book!( CargoBook, "src/tools/cargo/src/doc", "cargo", &[]; ClippyBook, "src/tools/clippy/book", "clippy", &[];