From d5e25d40c969009d0f40cbf6280339f2839e19c4 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 8 Jun 2023 23:12:53 +0000 Subject: [PATCH 1/5] deduplicate identical region constraints --- .../src/solve/eval_ctxt/canonical.rs | 6 +++- tests/ui/traits/new-solver/dedup-regions.rs | 31 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 tests/ui/traits/new-solver/dedup-regions.rs diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs index bca2343e4242..72b3c3d01804 100644 --- a/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs +++ b/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs @@ -11,6 +11,7 @@ use super::{CanonicalInput, Certainty, EvalCtxt, Goal}; use crate::solve::canonicalize::{CanonicalizeMode, Canonicalizer}; use crate::solve::{CanonicalResponse, QueryResult, Response}; +use rustc_data_structures::fx::FxHashSet; use rustc_index::IndexVec; use rustc_infer::infer::canonical::query_response::make_query_region_constraints; use rustc_infer::infer::canonical::CanonicalVarValues; @@ -147,7 +148,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { // Cannot use `take_registered_region_obligations` as we may compute the response // inside of a `probe` whenever we have multiple choices inside of the solver. let region_obligations = self.infcx.inner.borrow().region_obligations().to_owned(); - let region_constraints = self.infcx.with_region_constraints(|region_constraints| { + let mut region_constraints = self.infcx.with_region_constraints(|region_constraints| { make_query_region_constraints( self.tcx(), region_obligations @@ -157,6 +158,9 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { ) }); + let mut seen = FxHashSet::default(); + region_constraints.outlives.retain(|outlives| seen.insert(*outlives)); + let mut opaque_types = self.infcx.clone_opaque_types_for_query_response(); // Only return opaque type keys for newly-defined opaques opaque_types.retain(|(a, _)| { diff --git a/tests/ui/traits/new-solver/dedup-regions.rs b/tests/ui/traits/new-solver/dedup-regions.rs new file mode 100644 index 000000000000..f376f39a5a66 --- /dev/null +++ b/tests/ui/traits/new-solver/dedup-regions.rs @@ -0,0 +1,31 @@ +// compile-flags: -Ztrait-solver=next +// check-pass + +struct A(*mut ()); + +unsafe impl Send for A where A: 'static {} + +macro_rules! mk { + ($name:ident $ty:ty) => { + struct $name($ty, $ty, $ty, $ty, $ty, $ty, $ty, $ty, $ty, $ty); + }; +} + +mk!(B A); +mk!(C B); +mk!(D C); +mk!(E D); +mk!(F E); +mk!(G F); +mk!(H G); +mk!(I H); +mk!(J I); +mk!(K J); +mk!(L K); +mk!(M L); + +fn needs_send() {} + +fn main() { + needs_send::(); +} From 4b1d13d9841c815915433ca2a3088a8e3e97ad96 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 8 Jun 2023 14:30:01 +0200 Subject: [PATCH 2/5] List matching impls on type aliases --- src/librustdoc/html/render/mod.rs | 90 ++++++++++++++++++++---- src/librustdoc/html/render/print_item.rs | 48 ++++++++----- 2 files changed, 110 insertions(+), 28 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index f205ff15ec3d..baffee0964da 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -53,12 +53,15 @@ use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_hir::def_id::{DefId, DefIdSet}; use rustc_hir::Mutability; +use rustc_infer::infer::TyCtxtInferExt; +use rustc_infer::traits::{Obligation, ObligationCause}; use rustc_middle::middle::stability; -use rustc_middle::ty::TyCtxt; +use rustc_middle::ty::{ParamEnv, TyCtxt}; use rustc_span::{ symbol::{sym, Symbol}, BytePos, FileName, RealFileName, }; +use rustc_trait_selection::traits::ObligationCtxt; use serde::ser::{SerializeMap, SerializeSeq}; use serde::{Serialize, Serializer}; @@ -1112,15 +1115,47 @@ fn render_assoc_items<'a, 'cx: 'a>( containing_item: &'a clean::Item, it: DefId, what: AssocItemRender<'a>, + aliased_type: Option, ) -> impl fmt::Display + 'a + Captures<'cx> { let mut derefs = DefIdSet::default(); derefs.insert(it); display_fn(move |f| { - render_assoc_items_inner(f, cx, containing_item, it, what, &mut derefs); + render_assoc_items_inner(f, cx, containing_item, it, what, &mut derefs, aliased_type); Ok(()) }) } +/// Check whether `impl_def_id` may apply to *some instantiation* of `item_def_id`. +fn is_valid_impl_for(tcx: TyCtxt<'_>, item_def_id: DefId, impl_def_id: DefId) -> bool { + let infcx = tcx.infer_ctxt().intercrate(true).build(); + let ocx = ObligationCtxt::new(&infcx); + let param_env = ParamEnv::empty(); + + let alias_substs = infcx.fresh_substs_for_item(rustc_span::DUMMY_SP, item_def_id); + let alias_ty = tcx.type_of(item_def_id).subst(tcx, alias_substs); + let alias_bounds = tcx.predicates_of(item_def_id).instantiate(tcx, alias_substs); + + let impl_substs = infcx.fresh_substs_for_item(rustc_span::DUMMY_SP, impl_def_id); + let impl_self_ty = tcx.type_of(impl_def_id).subst(tcx, impl_substs); + let impl_bounds = tcx.predicates_of(impl_def_id).instantiate(tcx, impl_substs); + + if ocx.eq(&ObligationCause::dummy(), param_env, impl_self_ty, alias_ty).is_err() { + return false; + } + ocx.register_obligations( + alias_bounds + .iter() + .chain(impl_bounds) + .map(|(p, _)| Obligation::new(tcx, ObligationCause::dummy(), param_env, p)), + ); + + let errors = ocx.select_where_possible(); + errors.is_empty() +} + +// If `aliased_type` is `Some`, it means `it` is a type alias and `aliased_type` is the "actual" +// type aliased behind `it`. It is used to check whether or not the implementation of the aliased +// type can be displayed on the alias doc page. fn render_assoc_items_inner( mut w: &mut dyn fmt::Write, cx: &mut Context<'_>, @@ -1128,12 +1163,28 @@ fn render_assoc_items_inner( it: DefId, what: AssocItemRender<'_>, derefs: &mut DefIdSet, + aliased_type: Option, ) { info!("Documenting associated items of {:?}", containing_item.name); let shared = Rc::clone(&cx.shared); let cache = &shared.cache; - let Some(v) = cache.impls.get(&it) else { return }; - let (non_trait, traits): (Vec<_>, _) = v.iter().partition(|i| i.inner_impl().trait_.is_none()); + let empty = Vec::new(); + let v = match cache.impls.get(&it) { + Some(v) => v, + None => &empty, + }; + let v2 = match aliased_type { + Some(aliased_type) => cache.impls.get(&aliased_type).unwrap_or(&empty), + None => &empty, + }; + if v.is_empty() && v2.is_empty() { + return; + } + let mut saw_impls = FxHashSet::default(); + let (non_trait, traits): (Vec<_>, _) = + v.iter().chain(v2).partition(|i| i.inner_impl().trait_.is_none()); + let tcx = cx.tcx(); + let is_alias = aliased_type.is_some(); if !non_trait.is_empty() { let mut tmp_buf = Buffer::html(); let (render_mode, id, class_html) = match what { @@ -1165,6 +1216,12 @@ fn render_assoc_items_inner( }; let mut impls_buf = Buffer::html(); for i in &non_trait { + if !saw_impls.insert(i.def_id()) { + continue; + } + if is_alias && !is_valid_impl_for(tcx, it, i.def_id()) { + continue; + } render_impl( &mut impls_buf, cx, @@ -1193,9 +1250,14 @@ fn render_assoc_items_inner( if !traits.is_empty() { let deref_impl = traits.iter().find(|t| t.trait_did() == cx.tcx().lang_items().deref_trait()); - if let Some(impl_) = deref_impl { + if let Some(impl_) = deref_impl && + (!is_alias || is_valid_impl_for(tcx, it, impl_.def_id())) + { let has_deref_mut = - traits.iter().any(|t| t.trait_did() == cx.tcx().lang_items().deref_mut_trait()); + traits.iter().any(|t| { + t.trait_did() == cx.tcx().lang_items().deref_mut_trait() && + (!is_alias || is_valid_impl_for(tcx, it, t.def_id())) + }); render_deref_methods(&mut w, cx, impl_, containing_item, has_deref_mut, derefs); } @@ -1205,10 +1267,14 @@ fn render_assoc_items_inner( return; } - let (synthetic, concrete): (Vec<&Impl>, Vec<&Impl>) = - traits.into_iter().partition(|t| t.inner_impl().kind.is_auto()); - let (blanket_impl, concrete): (Vec<&Impl>, _) = - concrete.into_iter().partition(|t| t.inner_impl().kind.is_blanket()); + let (synthetic, concrete): (Vec<&Impl>, Vec<&Impl>) = traits + .into_iter() + .filter(|t| saw_impls.insert(t.def_id())) + .partition(|t| t.inner_impl().kind.is_auto()); + let (blanket_impl, concrete): (Vec<&Impl>, _) = concrete + .into_iter() + .filter(|t| !is_alias || is_valid_impl_for(tcx, it, t.def_id())) + .partition(|t| t.inner_impl().kind.is_blanket()); render_all_impls(w, cx, containing_item, &concrete, &synthetic, &blanket_impl); } @@ -1247,10 +1313,10 @@ fn render_deref_methods( return; } } - render_assoc_items_inner(&mut w, cx, container_item, did, what, derefs); + render_assoc_items_inner(&mut w, cx, container_item, did, what, derefs, None); } else if let Some(prim) = target.primitive_type() { if let Some(&did) = cache.primitive_locations.get(&prim) { - render_assoc_items_inner(&mut w, cx, container_item, did, what, derefs); + render_assoc_items_inner(&mut w, cx, container_item, did, what, derefs, None); } } } diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 4be0a7b4a5fb..01089ed348bf 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -260,7 +260,7 @@ fn item_template_render_assoc_items<'a: 'b, 'b, 'cx: 'a>( display_fn(move |f| { let (item, mut cx) = templ.item_and_mut_cx(); let def_id = item.item_id.expect_def_id(); - let v = render_assoc_items(*cx, item, def_id, AssocItemRender::All); + let v = render_assoc_items(*cx, item, def_id, AssocItemRender::All, None); write!(f, "{v}") }) } @@ -893,7 +893,11 @@ fn item_trait(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean: } // If there are methods directly on this trait object, render them here. - write!(w, "{}", render_assoc_items(cx, it, it.item_id.expect_def_id(), AssocItemRender::All)); + write!( + w, + "{}", + render_assoc_items(cx, it, it.item_id.expect_def_id(), AssocItemRender::All, None) + ); let cloned_shared = Rc::clone(&cx.shared); let cache = &cloned_shared.cache; @@ -1125,8 +1129,12 @@ fn item_trait_alias( // won't be visible anywhere in the docs. It would be nice to also show // associated items from the aliased type (see discussion in #32077), but // we need #14072 to make sense of the generics. - write!(w, "{}", render_assoc_items(cx, it, it.item_id.expect_def_id(), AssocItemRender::All)) - .unwrap(); + write!( + w, + "{}", + render_assoc_items(cx, it, it.item_id.expect_def_id(), AssocItemRender::All, None) + ) + .unwrap(); } fn item_opaque_ty( @@ -1154,8 +1162,12 @@ fn item_opaque_ty( // won't be visible anywhere in the docs. It would be nice to also show // associated items from the aliased type (see discussion in #32077), but // we need #14072 to make sense of the generics. - write!(w, "{}", render_assoc_items(cx, it, it.item_id.expect_def_id(), AssocItemRender::All)) - .unwrap(); + write!( + w, + "{}", + render_assoc_items(cx, it, it.item_id.expect_def_id(), AssocItemRender::All, None) + ) + .unwrap(); } fn item_typedef(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean::Typedef) { @@ -1179,11 +1191,11 @@ fn item_typedef(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clea write!(w, "{}", document(cx, it, None, HeadingOffset::H2)); let def_id = it.item_id.expect_def_id(); - // Render any items associated directly to this alias, as otherwise they - // won't be visible anywhere in the docs. It would be nice to also show - // associated items from the aliased type (see discussion in #32077), but - // we need #14072 to make sense of the generics. - write!(w, "{}", render_assoc_items(cx, it, def_id, AssocItemRender::All)); + write!( + w, + "{}", + render_assoc_items(cx, it, def_id, AssocItemRender::All, t.type_.def_id(&cx.cache())) + ); write!(w, "{}", document_type_layout(cx, def_id)); } @@ -1423,7 +1435,7 @@ fn item_enum(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, e: &clean:: write!(w, ""); } let def_id = it.item_id.expect_def_id(); - write!(w, "{}", render_assoc_items(cx, it, def_id, AssocItemRender::All)); + write!(w, "{}", render_assoc_items(cx, it, def_id, AssocItemRender::All, None)); write!(w, "{}", document_type_layout(cx, def_id)); } @@ -1466,7 +1478,7 @@ fn item_primitive(w: &mut impl fmt::Write, cx: &mut Context<'_>, it: &clean::Ite let def_id = it.item_id.expect_def_id(); write!(w, "{}", document(cx, it, None, HeadingOffset::H2)).unwrap(); if it.name.map(|n| n.as_str() != "reference").unwrap_or(false) { - write!(w, "{}", render_assoc_items(cx, it, def_id, AssocItemRender::All)).unwrap(); + write!(w, "{}", render_assoc_items(cx, it, def_id, AssocItemRender::All, None)).unwrap(); } else { // We handle the "reference" primitive type on its own because we only want to list // implementations on generic types. @@ -1571,7 +1583,7 @@ fn item_struct(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, s: &clean } } let def_id = it.item_id.expect_def_id(); - write!(w, "{}", render_assoc_items(cx, it, def_id, AssocItemRender::All)); + write!(w, "{}", render_assoc_items(cx, it, def_id, AssocItemRender::All, None)); write!(w, "{}", document_type_layout(cx, def_id)); } @@ -1606,8 +1618,12 @@ fn item_foreign_type(w: &mut impl fmt::Write, cx: &mut Context<'_>, it: &clean:: }); write!(w, "{}", document(cx, it, None, HeadingOffset::H2)).unwrap(); - write!(w, "{}", render_assoc_items(cx, it, it.item_id.expect_def_id(), AssocItemRender::All)) - .unwrap(); + write!( + w, + "{}", + render_assoc_items(cx, it, it.item_id.expect_def_id(), AssocItemRender::All, None) + ) + .unwrap(); } fn item_keyword(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item) { From 6f552c800b38b3e71c5e33a295e8b490d2018c71 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 8 Jun 2023 19:51:14 +0200 Subject: [PATCH 3/5] Add regression test for #32077 --- tests/rustdoc/issue-32077-type-alias-impls.rs | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 tests/rustdoc/issue-32077-type-alias-impls.rs diff --git a/tests/rustdoc/issue-32077-type-alias-impls.rs b/tests/rustdoc/issue-32077-type-alias-impls.rs new file mode 100644 index 000000000000..555d0579bee7 --- /dev/null +++ b/tests/rustdoc/issue-32077-type-alias-impls.rs @@ -0,0 +1,59 @@ +// Regression test for . + +#![crate_name = "foo"] + +pub struct GenericStruct(T); + +impl GenericStruct { + pub fn on_gen(arg: T) {} +} + +impl GenericStruct { + pub fn on_u32(arg: u32) {} +} + +pub trait Foo {} +pub trait Bar {} + +impl Foo for GenericStruct {} +impl Bar for GenericStruct {} + +// @has 'foo/type.TypedefStruct.html' +// We check that we have the implementation of the type alias itself. +// @has - '//*[@id="impl-TypedefStruct"]/h3' 'impl TypedefStruct' +// @has - '//*[@id="method.on_alias"]/h4' 'pub fn on_alias()' +// @has - '//*[@id="impl-GenericStruct%3CT%3E"]/h3' 'impl GenericStruct' +// @has - '//*[@id="method.on_gen"]/h4' 'pub fn on_gen(arg: T)' +// @has - '//*[@id="impl-Foo-for-GenericStruct%3CT%3E"]/h3' 'impl Foo for GenericStruct' +// This trait implementation doesn't match the type alias parameters so shouldn't appear in docs. +// @!has - '//h3' 'impl Bar for GenericStruct {}' +// Same goes for the `Deref` impl. +// @!has - '//h2' 'Methods from Deref' +pub type TypedefStruct = GenericStruct; + +impl TypedefStruct { + pub fn on_alias() {} +} + +impl std::ops::Deref for GenericStruct { + type Target = u32; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +pub struct Wrap(GenericStruct); + +// @has 'foo/type.Alias.html' +// @has - '//h2' 'Methods from Deref' +// @has - '//*[@id="impl-Deref-for-Wrap%3CT%3E"]/h3' 'impl Deref for Wrap' +pub type Alias = Wrap; + +impl std::ops::Deref for Wrap { + type Target = GenericStruct; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} From 2ce7cd906bde70d8cbd9b07b31c6a7bf1131c345 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 8 Jun 2023 22:48:29 +0200 Subject: [PATCH 4/5] Fix intra-doc links from pointer appearing in windows HANDLE type alias --- library/core/src/ptr/mut_ptr.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index c6f43857887c..e753647ff786 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -106,7 +106,7 @@ impl *mut T { /// with [`cast_mut`] on `*const T` and may have documentation value if used instead of implicit /// coercion. /// - /// [`cast_mut`]: #method.cast_mut + /// [`cast_mut`]: pointer::cast_mut #[stable(feature = "ptr_const_cast", since = "1.65.0")] #[rustc_const_stable(feature = "ptr_const_cast", since = "1.65.0")] #[inline(always)] @@ -117,7 +117,7 @@ impl *mut T { /// Casts a pointer to its raw bits. /// /// This is equivalent to `as usize`, but is more specific to enhance readability. - /// The inverse method is [`from_bits`](#method.from_bits-1). + /// The inverse method is [`from_bits`](pointer#method.from_bits-1). /// /// In particular, `*p as usize` and `p as usize` will both compile for /// pointers to numeric types but do very different things, so using this @@ -153,7 +153,7 @@ impl *mut T { /// Creates a pointer from its raw bits. /// /// This is equivalent to `as *mut T`, but is more specific to enhance readability. - /// The inverse method is [`to_bits`](#method.to_bits-1). + /// The inverse method is [`to_bits`](pointer#method.to_bits-1). /// /// # Examples /// @@ -303,7 +303,7 @@ impl *mut T { /// /// For the mutable counterpart see [`as_mut`]. /// - /// [`as_uninit_ref`]: #method.as_uninit_ref-1 + /// [`as_uninit_ref`]: pointer#method.as_uninit_ref-1 /// [`as_mut`]: #method.as_mut /// /// # Safety @@ -369,7 +369,7 @@ impl *mut T { /// /// For the mutable counterpart see [`as_uninit_mut`]. /// - /// [`as_ref`]: #method.as_ref-1 + /// [`as_ref`]: pointer#method.as_ref-1 /// [`as_uninit_mut`]: #method.as_uninit_mut /// /// # Safety @@ -624,7 +624,7 @@ impl *mut T { /// For the shared counterpart see [`as_ref`]. /// /// [`as_uninit_mut`]: #method.as_uninit_mut - /// [`as_ref`]: #method.as_ref-1 + /// [`as_ref`]: pointer#method.as_ref-1 /// /// # Safety /// @@ -689,7 +689,7 @@ impl *mut T { /// For the shared counterpart see [`as_uninit_ref`]. /// /// [`as_mut`]: #method.as_mut - /// [`as_uninit_ref`]: #method.as_uninit_ref-1 + /// [`as_uninit_ref`]: pointer#method.as_uninit_ref-1 /// /// # Safety /// @@ -779,7 +779,7 @@ impl *mut T { /// /// This function is the inverse of [`offset`]. /// - /// [`offset`]: #method.offset-1 + /// [`offset`]: pointer#method.offset-1 /// /// # Safety /// @@ -2051,7 +2051,7 @@ impl *mut [T] { /// /// For the mutable counterpart see [`as_uninit_slice_mut`]. /// - /// [`as_ref`]: #method.as_ref-1 + /// [`as_ref`]: pointer#method.as_ref-1 /// [`as_uninit_slice_mut`]: #method.as_uninit_slice_mut /// /// # Safety From 30f84c4d1755de683e565a03f269878796f71bdf Mon Sep 17 00:00:00 2001 From: Eval EXEC Date: Fri, 9 Jun 2023 16:11:23 +0800 Subject: [PATCH 5/5] Improve document of `unsafe_code` lint Signed-off-by: Eval EXEC --- compiler/rustc_lint/src/builtin.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 8c0956a618de..ff2989112af1 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -286,7 +286,9 @@ impl<'tcx> LateLintPass<'tcx> for NonShorthandFieldPatterns { } declare_lint! { - /// The `unsafe_code` lint catches usage of `unsafe` code. + /// The `unsafe_code` lint catches usage of `unsafe` code and other + /// potentially unsound constructs like `no_mangle`, `export_name`, + /// and `link_section`. /// /// ### Example /// @@ -297,17 +299,29 @@ declare_lint! { /// /// } /// } + /// + /// #[no_mangle] + /// fn func_0() { } + /// + /// #[export_name = "exported_symbol_name"] + /// pub fn name_in_rust() { } + /// + /// #[no_mangle] + /// #[link_section = ".example_section"] + /// pub static VAR1: u32 = 1; /// ``` /// /// {{produces}} /// /// ### Explanation /// - /// This lint is intended to restrict the usage of `unsafe`, which can be - /// difficult to use correctly. + /// This lint is intended to restrict the usage of `unsafe` blocks and other + /// constructs (including, but not limited to `no_mangle`, `link_section` + /// and `export_name` attributes) wrong usage of which causes undefined + /// behavior. UNSAFE_CODE, Allow, - "usage of `unsafe` code" + "usage of `unsafe` code and other potentially unsound constructs" } declare_lint_pass!(UnsafeCode => [UNSAFE_CODE]);