From 29b30a9bd291431d4bcae218fc0b1dcb2d35afec Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 16 Oct 2021 14:03:30 +0200 Subject: [PATCH 1/6] Visit type in process_projection_elem. --- compiler/rustc_borrowck/src/renumber.rs | 18 +-------------- compiler/rustc_middle/src/mir/visit.rs | 6 ++++- compiler/rustc_mir_transform/src/dest_prop.rs | 22 ------------------- .../rustc_mir_transform/src/reveal_all.rs | 20 ----------------- 4 files changed, 6 insertions(+), 60 deletions(-) diff --git a/compiler/rustc_borrowck/src/renumber.rs b/compiler/rustc_borrowck/src/renumber.rs index 20567610f6557..4b6cab24cdb70 100644 --- a/compiler/rustc_borrowck/src/renumber.rs +++ b/compiler/rustc_borrowck/src/renumber.rs @@ -1,7 +1,7 @@ use rustc_index::vec::IndexVec; use rustc_infer::infer::{InferCtxt, NllRegionVariableOrigin}; use rustc_middle::mir::visit::{MutVisitor, TyContext}; -use rustc_middle::mir::{Body, Location, PlaceElem, Promoted}; +use rustc_middle::mir::{Body, Location, Promoted}; use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable}; @@ -62,22 +62,6 @@ impl<'a, 'tcx> MutVisitor<'tcx> for NllVisitor<'a, 'tcx> { debug!(?ty); } - fn process_projection_elem( - &mut self, - elem: PlaceElem<'tcx>, - _: Location, - ) -> Option> { - if let PlaceElem::Field(field, ty) = elem { - let new_ty = self.renumber_regions(ty); - - if new_ty != ty { - return Some(PlaceElem::Field(field, new_ty)); - } - } - - None - } - #[instrument(skip(self), level = "debug")] fn visit_substs(&mut self, substs: &mut SubstsRef<'tcx>, location: Location) { *substs = self.renumber_regions(*substs); diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index fda7ebe1a49c1..4c23ab49fa29f 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -1004,8 +1004,12 @@ macro_rules! visit_place_fns { if new_local == local { None } else { Some(PlaceElem::Index(new_local)) } } + PlaceElem::Field(field, ty) => { + let mut new_ty = ty; + self.visit_ty(&mut new_ty, TyContext::Location(location)); + if ty != new_ty { Some(PlaceElem::Field(field, new_ty)) } else { None } + } PlaceElem::Deref - | PlaceElem::Field(..) | PlaceElem::ConstantIndex { .. } | PlaceElem::Subslice { .. } | PlaceElem::Downcast(..) => None, diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index 790d9243fbaec..c45946a9e2a98 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -316,28 +316,6 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'tcx> { } } - fn process_projection_elem( - &mut self, - elem: PlaceElem<'tcx>, - _: Location, - ) -> Option> { - match elem { - PlaceElem::Index(local) => { - if let Some(replacement) = self.replacements.for_src(local) { - bug!( - "cannot replace {:?} with {:?} in index projection {:?}", - local, - replacement, - elem, - ); - } else { - None - } - } - _ => None, - } - } - fn visit_place(&mut self, place: &mut Place<'tcx>, context: PlaceContext, location: Location) { if let Some(replacement) = self.replacements.for_src(place.local) { // Rebase `place`s projections onto `replacement`'s. diff --git a/compiler/rustc_mir_transform/src/reveal_all.rs b/compiler/rustc_mir_transform/src/reveal_all.rs index 6c423a2bb5756..3bcb71b64f455 100644 --- a/compiler/rustc_mir_transform/src/reveal_all.rs +++ b/compiler/rustc_mir_transform/src/reveal_all.rs @@ -35,24 +35,4 @@ impl<'tcx> MutVisitor<'tcx> for RevealAllVisitor<'tcx> { fn visit_ty(&mut self, ty: &mut Ty<'tcx>, _: TyContext) { *ty = self.tcx.normalize_erasing_regions(self.param_env, ty); } - - #[inline] - fn process_projection_elem( - &mut self, - elem: PlaceElem<'tcx>, - _: Location, - ) -> Option> { - match elem { - PlaceElem::Field(field, ty) => { - let new_ty = self.tcx.normalize_erasing_regions(self.param_env, ty); - if ty != new_ty { Some(PlaceElem::Field(field, new_ty)) } else { None } - } - // None of those contain a Ty. - PlaceElem::Index(..) - | PlaceElem::Deref - | PlaceElem::ConstantIndex { .. } - | PlaceElem::Subslice { .. } - | PlaceElem::Downcast(..) => None, - } - } } From e500eb69508a6e65880db3618399c89ed276f34e Mon Sep 17 00:00:00 2001 From: "William D. Jones" Date: Sun, 28 Nov 2021 17:53:50 -0500 Subject: [PATCH 2/6] Bump compiler_builtins to 0.1.55 to bring in fixes for targets lacking atomic support. --- Cargo.lock | 4 ++-- library/std/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b2536c5cda119..2233162be3b6b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -678,9 +678,9 @@ dependencies = [ [[package]] name = "compiler_builtins" -version = "0.1.53" +version = "0.1.55" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2467ff455350a4df7d02f1ed1449d0279605a763de5d586dcf6aa7d732508bcb" +checksum = "c9ac60765140c97aaf531dae151a287646b0805ec725805da9e2a3ee31cd501c" dependencies = [ "cc", "rustc-std-workspace-core", diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index b43445f2eed5c..f711174832142 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -16,7 +16,7 @@ panic_unwind = { path = "../panic_unwind", optional = true } panic_abort = { path = "../panic_abort" } core = { path = "../core" } libc = { version = "0.2.108", default-features = false, features = ['rustc-dep-of-std'] } -compiler_builtins = { version = "0.1.53" } +compiler_builtins = { version = "0.1.55" } profiler_builtins = { path = "../profiler_builtins", optional = true } unwind = { path = "../unwind" } hashbrown = { version = "0.11", default-features = false, features = ['rustc-dep-of-std'] } From 85ba6c7b3489f5b23ca146d6ebde657bf8b52c36 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 29 Nov 2021 12:18:57 -0700 Subject: [PATCH 3/6] Only show notable traits if both types are the same Checking only their DefId doesn't work because all slices have the same fake DefId. Fixes #91347 --- src/librustdoc/clean/types.rs | 39 +++++++++++++++++++++ src/librustdoc/html/render/mod.rs | 9 ++++- src/test/rustdoc/doc-notable_trait-slice.rs | 20 +++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 src/test/rustdoc/doc-notable_trait-slice.rs diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 37acf68defd25..f7762fd2521ff 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1468,6 +1468,45 @@ crate enum Type { rustc_data_structures::static_assert_size!(Type, 72); impl Type { + /// When comparing types for equality, it can help to ignore `&` wrapping. + crate fn without_borrowed_ref(&self) -> &Type { + let mut result = self; + while let Type::BorrowedRef { type_, .. } = result { + result = &*type_; + } + result + } + + /// Check if two types are "potentially the same." + /// This is different from Eq, because it knows that things like + /// `Placeholder` are possible matches for everything. + crate fn is_same(&self, other: &Self, cache: &Cache) -> bool { + match (self, other) { + // Recursive cases. + (Type::Tuple(a), Type::Tuple(b)) => { + a.len() == b.len() && a.iter().zip(b).all(|(a, b)| a.is_same(&b, cache)) + } + (Type::Slice(a), Type::Slice(b)) => a.is_same(&b, cache), + (Type::Array(a, al), Type::Array(b, bl)) => al == bl && a.is_same(&b, cache), + (Type::RawPointer(mutability, type_), Type::RawPointer(b_mutability, b_type_)) => { + mutability == b_mutability && type_.is_same(&b_type_, cache) + } + ( + Type::BorrowedRef { mutability, type_, .. }, + Type::BorrowedRef { mutability: b_mutability, type_: b_type_, .. }, + ) => mutability == b_mutability && type_.is_same(&b_type_, cache), + // Placeholders and generics are equal to all other types. + (Type::Infer, _) | (_, Type::Infer) => true, + (Type::Generic(_), _) | (_, Type::Generic(_)) => true, + // Other cases, such as primitives, just use recursion. + (a, b) => a + .def_id(cache) + .and_then(|a| Some((a, b.def_id(cache)?))) + .map(|(a, b)| a == b) + .unwrap_or(false), + } + } + crate fn primitive_type(&self) -> Option { match *self { Primitive(p) | BorrowedRef { type_: box Primitive(p), .. } => Some(p), diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 08022d526fefb..8bf7d0416dd0a 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1235,10 +1235,17 @@ fn should_render_item(item: &clean::Item, deref_mut_: bool, tcx: TyCtxt<'_>) -> fn notable_traits_decl(decl: &clean::FnDecl, cx: &Context<'_>) -> String { let mut out = Buffer::html(); - if let Some(did) = decl.output.as_return().and_then(|t| t.def_id(cx.cache())) { + if let Some((did, ty)) = decl.output.as_return().and_then(|t| Some((t.def_id(cx.cache())?, t))) + { if let Some(impls) = cx.cache().impls.get(&did) { for i in impls { let impl_ = i.inner_impl(); + if !impl_.for_.without_borrowed_ref().is_same(ty.without_borrowed_ref(), cx.cache()) + { + // Two different types might have the same did, + // without actually being the same. + continue; + } if let Some(trait_) = &impl_.trait_ { let trait_did = trait_.def_id(); diff --git a/src/test/rustdoc/doc-notable_trait-slice.rs b/src/test/rustdoc/doc-notable_trait-slice.rs new file mode 100644 index 0000000000000..b0d414027216a --- /dev/null +++ b/src/test/rustdoc/doc-notable_trait-slice.rs @@ -0,0 +1,20 @@ +#![feature(doc_notable_trait)] + +#[doc(notable_trait)] +pub trait SomeTrait {} + +pub struct SomeStruct; +pub struct OtherStruct; +impl SomeTrait for &[SomeStruct] {} + +// @has doc_notable_trait_slice/fn.bare_fn_matches.html +// @has - '//code[@class="content"]' 'impl SomeTrait for &[SomeStruct]' +pub fn bare_fn_matches() -> &'static [SomeStruct] { + &[] +} + +// @has doc_notable_trait_slice/fn.bare_fn_no_matches.html +// @!has - '//code[@class="content"]' 'impl SomeTrait for &[SomeStruct]' +pub fn bare_fn_no_matches() -> &'static [OtherStruct] { + &[] +} From bd894a08774b1791d8c987d66684f8108f3f336e Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 30 Nov 2021 18:47:01 +0000 Subject: [PATCH 4/6] Emit a warning on generic parameters with doc comments --- compiler/rustc_lint/src/builtin.rs | 4 ++++ .../ui/lint/unused/unused-doc-comments-edge-cases.rs | 3 +++ .../lint/unused/unused-doc-comments-edge-cases.stderr | 10 +++++++++- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index f36b9c82fac4e..077d3e1c82058 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -1079,6 +1079,10 @@ impl EarlyLintPass for UnusedDocComment { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) { warn_if_doc(cx, expr.span, "expressions", &expr.attrs); } + + fn check_generic_param(&mut self, cx: &EarlyContext<'_>, param: &ast::GenericParam) { + warn_if_doc(cx, param.ident.span, "generic parameters", ¶m.attrs); + } } declare_lint! { diff --git a/src/test/ui/lint/unused/unused-doc-comments-edge-cases.rs b/src/test/ui/lint/unused/unused-doc-comments-edge-cases.rs index fd9baf8c6b9a2..258f9e4831f9a 100644 --- a/src/test/ui/lint/unused/unused-doc-comments-edge-cases.rs +++ b/src/test/ui/lint/unused/unused-doc-comments-edge-cases.rs @@ -26,4 +26,7 @@ fn doc_comment_on_expr(num: u8) -> bool { num == 3 } +fn doc_comment_on_generic<#[doc = "x"] T>(val: T) {} +//~^ ERROR: unused doc comment + fn main() {} diff --git a/src/test/ui/lint/unused/unused-doc-comments-edge-cases.stderr b/src/test/ui/lint/unused/unused-doc-comments-edge-cases.stderr index 403367017c6e6..3ce1df71a2ed5 100644 --- a/src/test/ui/lint/unused/unused-doc-comments-edge-cases.stderr +++ b/src/test/ui/lint/unused/unused-doc-comments-edge-cases.stderr @@ -41,6 +41,14 @@ LL | num == 3 | = help: use `//` for a plain comment +error: unused doc comment + --> $DIR/unused-doc-comments-edge-cases.rs:29:27 + | +LL | fn doc_comment_on_generic<#[doc = "x"] T>(val: T) {} + | ^^^^^^^^^^^^ - rustdoc does not generate documentation for generic parameters + | + = help: use `//` for a plain comment + error[E0308]: mismatched types --> $DIR/unused-doc-comments-edge-cases.rs:14:9 | @@ -55,7 +63,7 @@ help: you might have meant to return this value LL | return true; | ++++++ + -error: aborting due to 5 previous errors +error: aborting due to 6 previous errors Some errors have detailed explanations: E0308, E0658. For more information about an error, try `rustc --explain E0308`. From 7bb50d4f309ae0dcf6264e31815777dc5eb3ebaf Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 30 Nov 2021 14:22:03 -0700 Subject: [PATCH 5/6] Update src/librustdoc/clean/types.rs Co-authored-by: Guillaume Gomez --- src/librustdoc/clean/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index f7762fd2521ff..a6552bd010b96 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1477,7 +1477,7 @@ impl Type { result } - /// Check if two types are "potentially the same." + /// Check if two types are "potentially the same". /// This is different from Eq, because it knows that things like /// `Placeholder` are possible matches for everything. crate fn is_same(&self, other: &Self, cache: &Cache) -> bool { From d4f71d8f86ec00c736b121b22284298ecaf9a6e5 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 30 Nov 2021 14:22:15 -0700 Subject: [PATCH 6/6] Update src/librustdoc/clean/types.rs Co-authored-by: Guillaume Gomez --- src/librustdoc/clean/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index a6552bd010b96..11dd140504ae3 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1478,7 +1478,7 @@ impl Type { } /// Check if two types are "potentially the same". - /// This is different from Eq, because it knows that things like + /// This is different from `Eq`, because it knows that things like /// `Placeholder` are possible matches for everything. crate fn is_same(&self, other: &Self, cache: &Cache) -> bool { match (self, other) {