From 34f116101fa5e1f4a3c9597535357fa32db277af Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 7 Jun 2021 19:39:42 -0500 Subject: [PATCH 1/8] Preserve more spans in internal `rustc_queries!` macro We now preserve the span of the various query modifiers, and use the span of the query's name for the commas that we generate to separate the modifiers. This makes debugging issues with the internal query macro infrastructure much nicer - previously, we would get errors messages pointing at the entire call site (the `rustc_queries!` invocation), which isn't very useful. This should have no effect when compilation succeeds. A concrete example of an error message produced after this changed: ``` error: local ambiguity: multiple parsing options: built-in NTs tt ('modifiers') or 1 other option. --> /home/aaron/repos/rust/compiler/rustc_middle/src/query/mod.rs:23:11 | 12 | / rustc_queries! { 13 | | query trigger_delay_span_bug(key: DefId) -> () { 14 | | desc { "trigger a delay span bug" } 15 | | } ... | 23 | | query hir_crate(key: ()) -> &'tcx Crate<'tcx> { | | ^^^^^^^^^ ... | 1715 | | } 1716 | | } | |_- in this expansion of `rustc_query_append!` | ::: compiler/rustc_query_impl/src/lib.rs:51:1 | 51 | rustc_query_append! { [define_queries!][<'tcx>] } | ------------------------------------------------- in this macro invocation ``` The particular bug shown in this error message will be fixed in a separate PR. --- compiler/rustc_macros/src/query.rs | 104 ++++++++++++++++------------- 1 file changed, 56 insertions(+), 48 deletions(-) diff --git a/compiler/rustc_macros/src/query.rs b/compiler/rustc_macros/src/query.rs index 291e7ef045e4f..660ad5ac22ef7 100644 --- a/compiler/rustc_macros/src/query.rs +++ b/compiler/rustc_macros/src/query.rs @@ -1,6 +1,6 @@ use proc_macro::TokenStream; use proc_macro2::{Delimiter, TokenTree}; -use quote::quote; +use quote::{quote, quote_spanned}; use syn::parse::{Parse, ParseStream, Result}; use syn::punctuated::Punctuated; use syn::spanned::Spanned; @@ -42,19 +42,19 @@ enum QueryModifier { LoadCached(Ident, Ident, Block), /// A cycle error for this query aborting the compilation with a fatal error. - FatalCycle, + FatalCycle(Ident), /// A cycle error results in a delay_bug call - CycleDelayBug, + CycleDelayBug(Ident), /// Don't hash the result, instead just mark a query red if it runs - NoHash, + NoHash(Ident), /// Generate a dep node based on the dependencies of the query - Anon, + Anon(Ident), /// Always evaluate the query, ignoring its dependencies - EvalAlways, + EvalAlways(Ident), } impl Parse for QueryModifier { @@ -111,15 +111,15 @@ impl Parse for QueryModifier { let ty = args.parse()?; Ok(QueryModifier::Storage(ty)) } else if modifier == "fatal_cycle" { - Ok(QueryModifier::FatalCycle) + Ok(QueryModifier::FatalCycle(modifier)) } else if modifier == "cycle_delay_bug" { - Ok(QueryModifier::CycleDelayBug) + Ok(QueryModifier::CycleDelayBug(modifier)) } else if modifier == "no_hash" { - Ok(QueryModifier::NoHash) + Ok(QueryModifier::NoHash(modifier)) } else if modifier == "anon" { - Ok(QueryModifier::Anon) + Ok(QueryModifier::Anon(modifier)) } else if modifier == "eval_always" { - Ok(QueryModifier::EvalAlways) + Ok(QueryModifier::EvalAlways(modifier)) } else { Err(Error::new(modifier.span(), "unknown query modifier")) } @@ -203,19 +203,19 @@ struct QueryModifiers { load_cached: Option<(Ident, Ident, Block)>, /// A cycle error for this query aborting the compilation with a fatal error. - fatal_cycle: bool, + fatal_cycle: Option, /// A cycle error results in a delay_bug call - cycle_delay_bug: bool, + cycle_delay_bug: Option, /// Don't hash the result, instead just mark a query red if it runs - no_hash: bool, + no_hash: Option, /// Generate a dep node based on the dependencies of the query - anon: bool, + anon: Option, // Always evaluate the query, ignoring its dependencies - eval_always: bool, + eval_always: Option, } /// Process query modifiers into a struct, erroring on duplicates @@ -224,11 +224,11 @@ fn process_modifiers(query: &mut Query) -> QueryModifiers { let mut storage = None; let mut cache = None; let mut desc = None; - let mut fatal_cycle = false; - let mut cycle_delay_bug = false; - let mut no_hash = false; - let mut anon = false; - let mut eval_always = false; + let mut fatal_cycle = None; + let mut cycle_delay_bug = None; + let mut no_hash = None; + let mut anon = None; + let mut eval_always = None; for modifier in query.modifiers.0.drain(..) { match modifier { QueryModifier::LoadCached(tcx, id, block) => { @@ -289,35 +289,35 @@ fn process_modifiers(query: &mut Query) -> QueryModifiers { } desc = Some((tcx, list)); } - QueryModifier::FatalCycle => { - if fatal_cycle { + QueryModifier::FatalCycle(ident) => { + if fatal_cycle.is_some() { panic!("duplicate modifier `fatal_cycle` for query `{}`", query.name); } - fatal_cycle = true; + fatal_cycle = Some(ident); } - QueryModifier::CycleDelayBug => { - if cycle_delay_bug { + QueryModifier::CycleDelayBug(ident) => { + if cycle_delay_bug.is_some() { panic!("duplicate modifier `cycle_delay_bug` for query `{}`", query.name); } - cycle_delay_bug = true; + cycle_delay_bug = Some(ident); } - QueryModifier::NoHash => { - if no_hash { + QueryModifier::NoHash(ident) => { + if no_hash.is_some() { panic!("duplicate modifier `no_hash` for query `{}`", query.name); } - no_hash = true; + no_hash = Some(ident); } - QueryModifier::Anon => { - if anon { + QueryModifier::Anon(ident) => { + if anon.is_some() { panic!("duplicate modifier `anon` for query `{}`", query.name); } - anon = true; + anon = Some(ident); } - QueryModifier::EvalAlways => { - if eval_always { + QueryModifier::EvalAlways(ident) => { + if eval_always.is_some() { panic!("duplicate modifier `eval_always` for query `{}`", query.name); } - eval_always = true; + eval_always = Some(ident); } } } @@ -454,31 +454,39 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream { let mut attributes = Vec::new(); // Pass on the fatal_cycle modifier - if modifiers.fatal_cycle { - attributes.push(quote! { fatal_cycle }); + if let Some(fatal_cycle) = &modifiers.fatal_cycle { + attributes.push(quote! { #fatal_cycle }); }; // Pass on the storage modifier if let Some(ref ty) = modifiers.storage { - attributes.push(quote! { storage(#ty) }); + let span = ty.span(); + attributes.push(quote_spanned! {span=> storage(#ty) }); }; // Pass on the cycle_delay_bug modifier - if modifiers.cycle_delay_bug { - attributes.push(quote! { cycle_delay_bug }); + if let Some(cycle_delay_bug) = &modifiers.cycle_delay_bug { + attributes.push(quote! { #cycle_delay_bug }); }; // Pass on the no_hash modifier - if modifiers.no_hash { - attributes.push(quote! { no_hash }); + if let Some(no_hash) = &modifiers.no_hash { + attributes.push(quote! { #no_hash }); }; // Pass on the anon modifier - if modifiers.anon { - attributes.push(quote! { anon }); + if let Some(anon) = &modifiers.anon { + attributes.push(quote! { #anon }); }; // Pass on the eval_always modifier - if modifiers.eval_always { - attributes.push(quote! { eval_always }); + if let Some(eval_always) = &modifiers.eval_always { + attributes.push(quote! { #eval_always }); }; - let attribute_stream = quote! {#(#attributes),*}; + // This uses the span of the query definition for the commas, + // which can be important if we later encounter any ambiguity + // errors with any of the numerous macro_rules! macros that + // we use. Using the call-site span would result in a span pointing + // at the entire `rustc_queries!` invocation, which wouldn't + // be very useful. + let span = name.span(); + let attribute_stream = quote_spanned! {span=> #(#attributes),*}; let doc_comments = query.doc_comments.iter(); // Add the query to the group query_stream.extend(quote! { From 2d639ce67cf4d9762e9a1d3b5b6c949e6fb0f83e Mon Sep 17 00:00:00 2001 From: Richard Schneeman Date: Sun, 8 Aug 2021 21:07:09 -0500 Subject: [PATCH 2/8] ## Context While going through the "The Rust Programming Language" book (Klabnik & Nichols), the TCP server example directs us to use TcpListener::incoming. I was curious how I could pass this value to a function (before reading ahead in the book), so I looked up the docs to determine the signature. When I opened the docs, I found https://doc.rust-lang.org/std/net/struct.TcpListener.html#method.incoming, which didn't mention TcpStream anywhere in the example. Eventually, I clicked on https://doc.rust-lang.org/std/net/struct.TcpListener.html#method.accept in the docs (after clicking a few other locations first), and was able to surmise that the value contained TcpStream. ## Opportunity While this type is mentioned several times in this doc, I feel that someone should be able to fully use the results of the TcpListner::incoming iterator based solely on the docs of just this method. ## Implementation I took the code from the top-level TcpListener https://doc.rust-lang.org/std/net/struct.TcpListener.html#method.incoming and blended it with the existing docs for TcpListener::incoming https://doc.rust-lang.org/std/net/struct.TcpListener.html#method.incoming. It does make the example a little longer, and it also introduces a little duplication. It also gives the reader the type signatures they need to move on to the next step. ## Additional considerations I noticed that in this doc, `handle_connection` and `handle_client` are both used to accept a TcpStream in the docs on this page. I want to standardize on one function name convention, so readers don't accidentally think two different concepts are being referenced. I didn't want to cram do too much in one PR, I can update this PR to make that change, or I could send another PR (if you would like). First attempted contribution to Rust (and I'm also still very new, hence reading through the rust book for the first time)! Would you please let me know what you think? --- library/std/src/net/tcp.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/library/std/src/net/tcp.rs b/library/std/src/net/tcp.rs index 325acf0b97931..c8eb4811f7e98 100644 --- a/library/std/src/net/tcp.rs +++ b/library/std/src/net/tcp.rs @@ -767,17 +767,24 @@ impl TcpListener { /// # Examples /// /// ```no_run - /// use std::net::TcpListener; + /// use std::net::{TcpListener, TcpStream}; /// - /// let listener = TcpListener::bind("127.0.0.1:80").unwrap(); + /// fn handle_connection(stream: TcpStream) { + /// //... + /// } /// - /// for stream in listener.incoming() { - /// match stream { - /// Ok(stream) => { - /// println!("new client!"); + /// fn main() -> std::io::Result<()> { + /// let listener = TcpListener::bind("127.0.0.1:80").unwrap(); + /// + /// for stream in listener.incoming() { + /// match stream { + /// Ok(stream) => { + /// handle_connection(stream); + /// } + /// Err(e) => { /* connection failed */ } /// } - /// Err(e) => { /* connection failed */ } /// } + /// Ok(()) /// } /// ``` #[stable(feature = "rust1", since = "1.0.0")] From d8c3a649a6c94c56b4cbd3dd8d8301a58e0424eb Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 15 Aug 2021 10:48:53 -0500 Subject: [PATCH 3/8] Remove `HashStable` impls for `FileName` and `RealFileName` These impls were unused, and incorrectly hashed the local (non-remapped) path for `RealFileName::Remapped` (which would break reproducible builds if these impls were used). --- compiler/rustc_span/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 1c95cc91208d3..9e127577b61a6 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -157,7 +157,7 @@ scoped_tls::scoped_thread_local!(static SESSION_GLOBALS: SessionGlobals); // FIXME: We should use this enum or something like it to get rid of the // use of magic `/rust/1.x/...` paths across the board. #[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd)] -#[derive(HashStable_Generic, Decodable)] +#[derive(Decodable)] pub enum RealFileName { LocalPath(PathBuf), /// For remapped paths (namely paths into libstd that have been mapped @@ -269,7 +269,7 @@ impl RealFileName { /// Differentiates between real files and common virtual files. #[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] -#[derive(HashStable_Generic, Decodable, Encodable)] +#[derive(Decodable, Encodable)] pub enum FileName { Real(RealFileName), /// Call to `quote!`. From 0d9ea425798d6cb412fc2d85bb409eddee1d4eed Mon Sep 17 00:00:00 2001 From: Ellen Date: Wed, 18 Aug 2021 21:15:33 +0100 Subject: [PATCH 4/8] add tests --- .../const-generics/hash-tyvid-regression-1.rs | 15 +++++++ .../hash-tyvid-regression-1.stderr | 35 ++++++++++++++++ .../const-generics/hash-tyvid-regression-2.rs | 18 +++++++++ .../hash-tyvid-regression-2.stderr | 11 +++++ .../const-generics/hash-tyvid-regression-3.rs | 26 ++++++++++++ .../hash-tyvid-regression-3.stderr | 12 ++++++ .../const-generics/hash-tyvid-regression-4.rs | 40 +++++++++++++++++++ .../hash-tyvid-regression-4.stderr | 12 ++++++ 8 files changed, 169 insertions(+) create mode 100644 src/test/incremental/const-generics/hash-tyvid-regression-1.rs create mode 100644 src/test/incremental/const-generics/hash-tyvid-regression-1.stderr create mode 100644 src/test/incremental/const-generics/hash-tyvid-regression-2.rs create mode 100644 src/test/incremental/const-generics/hash-tyvid-regression-2.stderr create mode 100644 src/test/incremental/const-generics/hash-tyvid-regression-3.rs create mode 100644 src/test/incremental/const-generics/hash-tyvid-regression-3.stderr create mode 100644 src/test/incremental/const-generics/hash-tyvid-regression-4.rs create mode 100644 src/test/incremental/const-generics/hash-tyvid-regression-4.stderr diff --git a/src/test/incremental/const-generics/hash-tyvid-regression-1.rs b/src/test/incremental/const-generics/hash-tyvid-regression-1.rs new file mode 100644 index 0000000000000..f98ae59ddfe3c --- /dev/null +++ b/src/test/incremental/const-generics/hash-tyvid-regression-1.rs @@ -0,0 +1,15 @@ +// revisions: cfail +#![feature(const_generics, const_evaluatable_checked)] +#![allow(incomplete_features)] +// regression test for #77650 +fn c() +where + [T; N.get()]: Sized, +{ + use std::convert::TryFrom; + <[T; N.get()]>::try_from(()) + //~^ error: the trait bound + //~^^ error: mismatched types +} + +fn main() {} diff --git a/src/test/incremental/const-generics/hash-tyvid-regression-1.stderr b/src/test/incremental/const-generics/hash-tyvid-regression-1.stderr new file mode 100644 index 0000000000000..cb8ca3abd7f94 --- /dev/null +++ b/src/test/incremental/const-generics/hash-tyvid-regression-1.stderr @@ -0,0 +1,35 @@ +error[E0277]: the trait bound `[T; _]: From<()>` is not satisfied + --> $DIR/hash-tyvid-regression-1.rs:9:5 + | +LL | <[T; N.get()]>::try_from(()) + | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `From<()>` is not implemented for `[T; _]` + | + = note: required because of the requirements on the impl of `Into<[T; _]>` for `()` + = note: required because of the requirements on the impl of `TryFrom<()>` for `[T; _]` +note: required by `try_from` + --> $SRC_DIR/core/src/convert/mod.rs:LL:COL + | +LL | fn try_from(value: T) -> Result; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0308]: mismatched types + --> $DIR/hash-tyvid-regression-1.rs:9:5 + | +LL | <[T; N.get()]>::try_from(()) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found enum `Result` + | + = note: expected unit type `()` + found enum `Result<[T; _], Infallible>` +help: consider using a semicolon here + | +LL | <[T; N.get()]>::try_from(()); + | + +help: try adding a return type + | +LL | -> Result<[T; _], Infallible> where + | +++++++++++++++++++++++++++++ + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0277, E0308. +For more information about an error, try `rustc --explain E0277`. diff --git a/src/test/incremental/const-generics/hash-tyvid-regression-2.rs b/src/test/incremental/const-generics/hash-tyvid-regression-2.rs new file mode 100644 index 0000000000000..22536ff56d7ce --- /dev/null +++ b/src/test/incremental/const-generics/hash-tyvid-regression-2.rs @@ -0,0 +1,18 @@ +// revisions: cfail +#![feature(const_generics, const_evaluatable_checked)] +#![allow(incomplete_features)] +// regression test for #77650 +struct C([T; N.get()]) +where + [T; N.get()]: Sized; +impl<'a, const N: core::num::NonZeroUsize, A, B: PartialEq> PartialEq<&'a [A]> for C +where + [B; N.get()]: Sized, +{ + fn eq(&self, other: &&'a [A]) -> bool { + self.0 == other + //~^ error: can't compare + } +} + +fn main() {} diff --git a/src/test/incremental/const-generics/hash-tyvid-regression-2.stderr b/src/test/incremental/const-generics/hash-tyvid-regression-2.stderr new file mode 100644 index 0000000000000..0e6040ef02e7a --- /dev/null +++ b/src/test/incremental/const-generics/hash-tyvid-regression-2.stderr @@ -0,0 +1,11 @@ +error[E0277]: can't compare `[B; _]` with `&&[A]` + --> $DIR/hash-tyvid-regression-2.rs:12:16 + | +LL | self.0 == other + | ^^ no implementation for `[B; _] == &&[A]` + | + = help: the trait `PartialEq<&&[A]>` is not implemented for `[B; _]` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/incremental/const-generics/hash-tyvid-regression-3.rs b/src/test/incremental/const-generics/hash-tyvid-regression-3.rs new file mode 100644 index 0000000000000..76b1ae11c7d03 --- /dev/null +++ b/src/test/incremental/const-generics/hash-tyvid-regression-3.rs @@ -0,0 +1,26 @@ +// revisions: cfail +#![feature(const_generics, const_evaluatable_checked)] +#![allow(incomplete_features)] +// regression test for #79251 +struct Node +where + SmallVec<{ D * 2 }>: , +{ + keys: SmallVec<{ D * 2 }>, +} + +impl Node +where + SmallVec<{ D * 2 }>: , +{ + fn new() -> Self { + let mut node = Node::new(); + node.keys.some_function(); + //~^ error: no method named + node + } +} + +struct SmallVec {} + +fn main() {} diff --git a/src/test/incremental/const-generics/hash-tyvid-regression-3.stderr b/src/test/incremental/const-generics/hash-tyvid-regression-3.stderr new file mode 100644 index 0000000000000..555d46756dcb9 --- /dev/null +++ b/src/test/incremental/const-generics/hash-tyvid-regression-3.stderr @@ -0,0 +1,12 @@ +error[E0599]: no method named `some_function` found for struct `SmallVec` in the current scope + --> $DIR/hash-tyvid-regression-3.rs:17:19 + | +LL | node.keys.some_function(); + | ^^^^^^^^^^^^^ method not found in `SmallVec<{ D * 2 }>` +... +LL | struct SmallVec {} + | ------------------------------- method `some_function` not found for this + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0599`. diff --git a/src/test/incremental/const-generics/hash-tyvid-regression-4.rs b/src/test/incremental/const-generics/hash-tyvid-regression-4.rs new file mode 100644 index 0000000000000..35a675a2ab4d2 --- /dev/null +++ b/src/test/incremental/const-generics/hash-tyvid-regression-4.rs @@ -0,0 +1,40 @@ +// revisions: cfail +#![feature(const_generics, const_evaluatable_checked)] +#![allow(incomplete_features)] +// regression test for #79251 +#[derive(Debug)] +struct Node +where + SmallVec: , +{ + keys: SmallVec, +} + +impl Node +where + SmallVec: , +{ + fn new() -> Self { + panic!() + } + + #[inline(never)] + fn split(&mut self, i: usize, k: K, right: bool) -> Node { + let mut node = Node::new(); + node.keys.push(k); + //~^ error: no method named + node + } +} + +#[derive(Debug)] +struct SmallVec { + data: [T; D], +} +impl SmallVec { + fn new() -> Self { + panic!() + } +} + +fn main() {} diff --git a/src/test/incremental/const-generics/hash-tyvid-regression-4.stderr b/src/test/incremental/const-generics/hash-tyvid-regression-4.stderr new file mode 100644 index 0000000000000..c9a6715e571c9 --- /dev/null +++ b/src/test/incremental/const-generics/hash-tyvid-regression-4.stderr @@ -0,0 +1,12 @@ +error[E0599]: no method named `push` found for struct `SmallVec` in the current scope + --> $DIR/hash-tyvid-regression-4.rs:23:19 + | +LL | node.keys.push(k); + | ^^^^ method not found in `SmallVec<_, { D * 2 }>` +... +LL | struct SmallVec { + | ---------------------------------- method `push` not found for this + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0599`. From 1fdea560d2b756d1372aa7a8db52aec3a0b94f0b Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 14 Aug 2021 20:51:49 +0300 Subject: [PATCH 5/8] rustc_privacy: Replace `HirId`s and `DefId`s with `LocalDefId`s where possible --- compiler/rustc_privacy/src/lib.rs | 139 ++++++++++++++++-------------- 1 file changed, 76 insertions(+), 63 deletions(-) diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 3df8ade216925..a683cb05e16b6 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -11,7 +11,8 @@ use rustc_data_structures::fx::FxHashSet; use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID, CRATE_DEF_INDEX, LOCAL_CRATE}; +use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdSet}; +use rustc_hir::def_id::{CRATE_DEF_ID, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc_hir::intravisit::{self, DeepVisitor, NestedVisitorMap, Visitor}; use rustc_hir::{AssocItemKind, HirIdSet, Node, PatKind}; use rustc_middle::bug; @@ -354,9 +355,8 @@ trait VisibilityLike: Sized { // Returns an over-approximation (`skip_assoc_tys` = true) of visibility due to // associated types for which we can't determine visibility precisely. - fn of_impl(hir_id: hir::HirId, tcx: TyCtxt<'_>, access_levels: &AccessLevels) -> Self { + fn of_impl(def_id: LocalDefId, tcx: TyCtxt<'_>, access_levels: &AccessLevels) -> Self { let mut find = FindMin { tcx, access_levels, min: Self::MAX }; - let def_id = tcx.hir().local_def_id(hir_id); find.visit(tcx.type_of(def_id)); if let Some(trait_ref) = tcx.impl_trait_ref(def_id) { find.visit_trait(trait_ref); @@ -424,7 +424,7 @@ struct EmbargoVisitor<'tcx> { struct ReachEverythingInTheInterfaceVisitor<'a, 'tcx> { access_level: Option, - item_def_id: DefId, + item_def_id: LocalDefId, ev: &'a mut EmbargoVisitor<'tcx>, } @@ -448,12 +448,12 @@ impl EmbargoVisitor<'tcx> { fn reach( &mut self, - item_id: hir::HirId, + def_id: LocalDefId, access_level: Option, ) -> ReachEverythingInTheInterfaceVisitor<'_, 'tcx> { ReachEverythingInTheInterfaceVisitor { access_level: cmp::min(access_level, Some(AccessLevel::Reachable)), - item_def_id: self.tcx.hir().local_def_id(item_id).to_def_id(), + item_def_id: def_id, ev: self, } } @@ -536,10 +536,10 @@ impl EmbargoVisitor<'tcx> { | hir::ItemKind::Union(ref struct_def, _) = item.kind { for field in struct_def.fields() { - let field_vis = - self.tcx.visibility(self.tcx.hir().local_def_id(field.hir_id)); + let def_id = self.tcx.hir().local_def_id(field.hir_id); + let field_vis = self.tcx.visibility(def_id); if field_vis.is_accessible_from(module.to_def_id(), self.tcx) { - self.reach(field.hir_id, level).ty(); + self.reach(def_id, level).ty(); } } } else { @@ -638,7 +638,7 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> { fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) { let inherited_item_level = match item.kind { hir::ItemKind::Impl { .. } => { - Option::::of_impl(item.hir_id(), self.tcx, &self.access_levels) + Option::::of_impl(item.def_id, self.tcx, &self.access_levels) } // Foreign modules inherit level from parents. hir::ItemKind::ForeignMod { .. } => self.prev_level, @@ -750,7 +750,7 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> { // reachable if they are returned via `impl Trait`, even from private functions. let exist_level = cmp::max(item_level, Some(AccessLevel::ReachableFromImplTrait)); - self.reach(item.hir_id(), exist_level).generics().predicates().ty(); + self.reach(item.def_id, exist_level).generics().predicates().ty(); } } // Visit everything. @@ -759,15 +759,15 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> { | hir::ItemKind::Fn(..) | hir::ItemKind::TyAlias(..) => { if item_level.is_some() { - self.reach(item.hir_id(), item_level).generics().predicates().ty(); + self.reach(item.def_id, item_level).generics().predicates().ty(); } } hir::ItemKind::Trait(.., trait_item_refs) => { if item_level.is_some() { - self.reach(item.hir_id(), item_level).generics().predicates(); + self.reach(item.def_id, item_level).generics().predicates(); for trait_item_ref in trait_item_refs { - let mut reach = self.reach(trait_item_ref.id.hir_id(), item_level); + let mut reach = self.reach(trait_item_ref.id.def_id, item_level); reach.generics().predicates(); if trait_item_ref.kind == AssocItemKind::Type @@ -782,18 +782,18 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> { } hir::ItemKind::TraitAlias(..) => { if item_level.is_some() { - self.reach(item.hir_id(), item_level).generics().predicates(); + self.reach(item.def_id, item_level).generics().predicates(); } } // Visit everything except for private impl items. hir::ItemKind::Impl(ref impl_) => { if item_level.is_some() { - self.reach(item.hir_id(), item_level).generics().predicates().ty().trait_ref(); + self.reach(item.def_id, item_level).generics().predicates().ty().trait_ref(); for impl_item_ref in impl_.items { let impl_item_level = self.get(impl_item_ref.id.def_id); if impl_item_level.is_some() { - self.reach(impl_item_ref.id.hir_id(), impl_item_level) + self.reach(impl_item_ref.id.def_id, impl_item_level) .generics() .predicates() .ty(); @@ -805,13 +805,14 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> { // Visit everything, but enum variants have their own levels. hir::ItemKind::Enum(ref def, _) => { if item_level.is_some() { - self.reach(item.hir_id(), item_level).generics().predicates(); + self.reach(item.def_id, item_level).generics().predicates(); } for variant in def.variants { let variant_level = self.get(self.tcx.hir().local_def_id(variant.id)); if variant_level.is_some() { for field in variant.data.fields() { - self.reach(field.hir_id, variant_level).ty(); + self.reach(self.tcx.hir().local_def_id(field.hir_id), variant_level) + .ty(); } // Corner case: if the variant is reachable, but its // enum is not, make the enum reachable as well. @@ -824,7 +825,7 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> { for foreign_item in items { let foreign_item_level = self.get(foreign_item.id.def_id); if foreign_item_level.is_some() { - self.reach(foreign_item.id.hir_id(), foreign_item_level) + self.reach(foreign_item.id.def_id, foreign_item_level) .generics() .predicates() .ty(); @@ -834,11 +835,12 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> { // Visit everything except for private fields. hir::ItemKind::Struct(ref struct_def, _) | hir::ItemKind::Union(ref struct_def, _) => { if item_level.is_some() { - self.reach(item.hir_id(), item_level).generics().predicates(); + self.reach(item.def_id, item_level).generics().predicates(); for field in struct_def.fields() { - let field_level = self.get(self.tcx.hir().local_def_id(field.hir_id)); + let def_id = self.tcx.hir().local_def_id(field.hir_id); + let field_level = self.get(def_id); if field_level.is_some() { - self.reach(field.hir_id, field_level).ty(); + self.reach(def_id, field_level).ty(); } } } @@ -992,7 +994,7 @@ impl DefIdVisitor<'tcx> for ReachEverythingInTheInterfaceVisitor<'_, 'tcx> { struct NamePrivacyVisitor<'tcx> { tcx: TyCtxt<'tcx>, maybe_typeck_results: Option<&'tcx ty::TypeckResults<'tcx>>, - current_item: Option, + current_item: LocalDefId, } impl<'tcx> NamePrivacyVisitor<'tcx> { @@ -1014,11 +1016,15 @@ impl<'tcx> NamePrivacyVisitor<'tcx> { field: &'tcx ty::FieldDef, in_update_syntax: bool, ) { + if def.is_enum() { + return; + } + // definition of the field let ident = Ident::new(kw::Empty, use_ctxt); - let current_hir = self.current_item.unwrap(); - let def_id = self.tcx.adjust_ident_and_get_scope(ident, def.did, current_hir).1; - if !def.is_enum() && !field.vis.is_accessible_from(def_id, self.tcx) { + let hir_id = self.tcx.hir().local_def_id_to_hir_id(self.current_item); + let def_id = self.tcx.adjust_ident_and_get_scope(ident, def.did, hir_id).1; + if !field.vis.is_accessible_from(def_id, self.tcx) { let label = if in_update_syntax { format!("field `{}` is private", field.ident) } else { @@ -1063,7 +1069,7 @@ impl<'tcx> Visitor<'tcx> for NamePrivacyVisitor<'tcx> { } fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) { - let orig_current_item = self.current_item.replace(item.hir_id()); + let orig_current_item = mem::replace(&mut self.current_item, item.def_id); intravisit::walk_item(self, item); self.current_item = orig_current_item; } @@ -1763,9 +1769,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> { struct SearchInterfaceForPrivateItemsVisitor<'tcx> { tcx: TyCtxt<'tcx>, - item_id: hir::HirId, - item_def_id: DefId, - span: Span, + item_def_id: LocalDefId, /// The visitor checks that each component type is at least this visible. required_visibility: ty::Visibility, has_pub_restricted: bool, @@ -1820,8 +1824,8 @@ impl SearchInterfaceForPrivateItemsVisitor<'tcx> { if self.leaks_private_dep(def_id) { self.tcx.struct_span_lint_hir( lint::builtin::EXPORTED_PRIVATE_DEPENDENCIES, - self.item_id, - self.span, + self.tcx.hir().local_def_id_to_hir_id(self.item_def_id), + self.tcx.def_span(self.item_def_id.to_def_id()), |lint| { lint.build(&format!( "{} `{}` from private dependency '{}' in public \ @@ -1856,15 +1860,16 @@ impl SearchInterfaceForPrivateItemsVisitor<'tcx> { } }; let make_msg = || format!("{} {} `{}` in public interface", vis_descr, kind, descr); + let span = self.tcx.def_span(self.item_def_id.to_def_id()); if self.has_pub_restricted || self.has_old_errors || self.in_assoc_ty { let mut err = if kind == "trait" { - struct_span_err!(self.tcx.sess, self.span, E0445, "{}", make_msg()) + struct_span_err!(self.tcx.sess, span, E0445, "{}", make_msg()) } else { - struct_span_err!(self.tcx.sess, self.span, E0446, "{}", make_msg()) + struct_span_err!(self.tcx.sess, span, E0446, "{}", make_msg()) }; let vis_span = self.tcx.sess.source_map().guess_head_span(self.tcx.def_span(def_id)); - err.span_label(self.span, format!("can't leak {} {}", vis_descr, kind)); + err.span_label(span, format!("can't leak {} {}", vis_descr, kind)); err.span_label(vis_span, format!("`{}` declared as {}", descr, vis_descr)); err.emit(); } else { @@ -1872,7 +1877,7 @@ impl SearchInterfaceForPrivateItemsVisitor<'tcx> { self.tcx.struct_span_lint_hir( lint::builtin::PRIVATE_IN_PUBLIC, hir_id, - self.span, + span, |lint| lint.build(&format!("{} (error {})", make_msg(), err_code)).emit(), ); } @@ -1915,35 +1920,33 @@ impl DefIdVisitor<'tcx> for SearchInterfaceForPrivateItemsVisitor<'tcx> { struct PrivateItemsInPublicInterfacesVisitor<'tcx> { tcx: TyCtxt<'tcx>, has_pub_restricted: bool, - old_error_set_ancestry: HirIdSet, + old_error_set_ancestry: LocalDefIdSet, } impl<'tcx> PrivateItemsInPublicInterfacesVisitor<'tcx> { fn check( &self, - item_id: hir::HirId, + def_id: LocalDefId, required_visibility: ty::Visibility, ) -> SearchInterfaceForPrivateItemsVisitor<'tcx> { SearchInterfaceForPrivateItemsVisitor { tcx: self.tcx, - item_id, - item_def_id: self.tcx.hir().local_def_id(item_id).to_def_id(), - span: self.tcx.hir().span(item_id), + item_def_id: def_id, required_visibility, has_pub_restricted: self.has_pub_restricted, - has_old_errors: self.old_error_set_ancestry.contains(&item_id), + has_old_errors: self.old_error_set_ancestry.contains(&def_id), in_assoc_ty: false, } } fn check_assoc_item( &self, - hir_id: hir::HirId, + def_id: LocalDefId, assoc_item_kind: AssocItemKind, defaultness: hir::Defaultness, vis: ty::Visibility, ) { - let mut check = self.check(hir_id, vis); + let mut check = self.check(def_id, vis); let (check_ty, is_assoc_ty) = match assoc_item_kind { AssocItemKind::Const | AssocItemKind::Fn { .. } => (true, false), @@ -1982,38 +1985,38 @@ impl<'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'tcx> { | hir::ItemKind::Static(..) | hir::ItemKind::Fn(..) | hir::ItemKind::TyAlias(..) => { - self.check(item.hir_id(), item_visibility).generics().predicates().ty(); + self.check(item.def_id, item_visibility).generics().predicates().ty(); } hir::ItemKind::OpaqueTy(..) => { // `ty()` for opaque types is the underlying type, // it's not a part of interface, so we skip it. - self.check(item.hir_id(), item_visibility).generics().bounds(); + self.check(item.def_id, item_visibility).generics().bounds(); } hir::ItemKind::Trait(.., trait_item_refs) => { - self.check(item.hir_id(), item_visibility).generics().predicates(); + self.check(item.def_id, item_visibility).generics().predicates(); for trait_item_ref in trait_item_refs { self.check_assoc_item( - trait_item_ref.id.hir_id(), + trait_item_ref.id.def_id, trait_item_ref.kind, trait_item_ref.defaultness, item_visibility, ); if let AssocItemKind::Type = trait_item_ref.kind { - self.check(trait_item_ref.id.hir_id(), item_visibility).bounds(); + self.check(trait_item_ref.id.def_id, item_visibility).bounds(); } } } hir::ItemKind::TraitAlias(..) => { - self.check(item.hir_id(), item_visibility).generics().predicates(); + self.check(item.def_id, item_visibility).generics().predicates(); } hir::ItemKind::Enum(ref def, _) => { - self.check(item.hir_id(), item_visibility).generics().predicates(); + self.check(item.def_id, item_visibility).generics().predicates(); for variant in def.variants { for field in variant.data.fields() { - self.check(field.hir_id, item_visibility).ty(); + self.check(self.tcx.hir().local_def_id(field.hir_id), item_visibility).ty(); } } } @@ -2021,16 +2024,17 @@ impl<'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'tcx> { hir::ItemKind::ForeignMod { items, .. } => { for foreign_item in items { let vis = tcx.visibility(foreign_item.id.def_id); - self.check(foreign_item.id.hir_id(), vis).generics().predicates().ty(); + self.check(foreign_item.id.def_id, vis).generics().predicates().ty(); } } // Subitems of structs and unions have their own publicity. hir::ItemKind::Struct(ref struct_def, _) | hir::ItemKind::Union(ref struct_def, _) => { - self.check(item.hir_id(), item_visibility).generics().predicates(); + self.check(item.def_id, item_visibility).generics().predicates(); for field in struct_def.fields() { - let field_visibility = tcx.visibility(tcx.hir().local_def_id(field.hir_id)); - self.check(field.hir_id, min(item_visibility, field_visibility, tcx)).ty(); + let def_id = tcx.hir().local_def_id(field.hir_id); + let field_visibility = tcx.visibility(def_id); + self.check(def_id, min(item_visibility, field_visibility, tcx)).ty(); } } // An inherent impl is public when its type is public @@ -2038,8 +2042,8 @@ impl<'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'tcx> { // A trait impl is public when both its type and its trait are public // Subitems of trait impls have inherited publicity. hir::ItemKind::Impl(ref impl_) => { - let impl_vis = ty::Visibility::of_impl(item.hir_id(), tcx, &Default::default()); - self.check(item.hir_id(), impl_vis).generics().predicates(); + let impl_vis = ty::Visibility::of_impl(item.def_id, tcx, &Default::default()); + self.check(item.def_id, impl_vis).generics().predicates(); for impl_item_ref in impl_.items { let impl_item_vis = if impl_.of_trait.is_none() { min(tcx.visibility(impl_item_ref.id.def_id), impl_vis, tcx) @@ -2047,7 +2051,7 @@ impl<'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'tcx> { impl_vis }; self.check_assoc_item( - impl_item_ref.id.hir_id(), + impl_item_ref.id.def_id, impl_item_ref.kind, impl_item_ref.defaultness, impl_item_vis, @@ -2119,7 +2123,8 @@ fn visibility(tcx: TyCtxt<'_>, def_id: DefId) -> ty::Visibility { fn check_mod_privacy(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { // Check privacy of names not checked in previous compilation stages. - let mut visitor = NamePrivacyVisitor { tcx, maybe_typeck_results: None, current_item: None }; + let mut visitor = + NamePrivacyVisitor { tcx, maybe_typeck_results: None, current_item: module_def_id }; let (module, span, hir_id) = tcx.hir().get_module(module_def_id); intravisit::walk_mod(&mut visitor, module, hir_id); @@ -2188,7 +2193,15 @@ fn check_private_in_public(tcx: TyCtxt<'_>, (): ()) { } // Check for private types and traits in public interfaces. - let mut visitor = - PrivateItemsInPublicInterfacesVisitor { tcx, has_pub_restricted, old_error_set_ancestry }; + let mut visitor = PrivateItemsInPublicInterfacesVisitor { + tcx, + has_pub_restricted, + // Only definition IDs are ever searched in `old_error_set_ancestry`, + // so we can filter away all non-definition IDs at this point. + old_error_set_ancestry: old_error_set_ancestry + .into_iter() + .filter_map(|hir_id| tcx.hir().opt_local_def_id(hir_id)) + .collect(), + }; krate.visit_all_item_likes(&mut DeepVisitor::new(&mut visitor)); } From ccd550ee565436b4c9b7f987c19bd3a9111714f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Mon, 16 Aug 2021 23:19:00 +0200 Subject: [PATCH 6/8] [rustdoc] Wrap code blocks in tag --- src/librustdoc/html/highlight.rs | 7 +- src/librustdoc/html/markdown.rs | 2 +- src/librustdoc/html/render/print_item.rs | 550 +++++++++++---------- src/librustdoc/html/static/css/rustdoc.css | 1 - src/test/rustdoc-gui/code-tags.goml | 20 + src/test/rustdoc-gui/source-code-page.goml | 2 +- 6 files changed, 315 insertions(+), 267 deletions(-) create mode 100644 src/test/rustdoc-gui/code-tags.goml diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 3cdb1352bef95..f8fc9243e14b9 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -65,10 +65,11 @@ fn write_header(out: &mut Buffer, class: Option<&str>, extra_content: Option", class); + write!(out, "
", class);
     } else {
-        writeln!(out, "
");
+        write!(out, "
");
     }
+    write!(out, "");
 }
 
 /// Convert the given `src` source code into HTML by adding classes for highlighting.
@@ -101,7 +102,7 @@ fn write_code(
 }
 
 fn write_footer(out: &mut Buffer, playground_button: Option<&str>) {
-    writeln!(out, "
{}", playground_button.unwrap_or_default());
+    writeln!(out, "
{}", playground_button.unwrap_or_default());
 }
 
 /// How a span of text is classified. Mostly corresponds to token kinds.
diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs
index 472323daf3017..7c6d7dff816d5 100644
--- a/src/librustdoc/html/markdown.rs
+++ b/src/librustdoc/html/markdown.rs
@@ -234,7 +234,7 @@ impl<'a, I: Iterator>> Iterator for CodeBlocks<'_, 'a, I> {
                     return Some(Event::Html(
                         format!(
                             "
\
-                                 
{}
\
+                                 
{}
\
                              
",
                             lang,
                             Escape(&text),
diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs
index f31305c76e642..c1d95c8c252ad 100644
--- a/src/librustdoc/html/render/print_item.rs
+++ b/src/librustdoc/html/render/print_item.rs
@@ -455,24 +455,25 @@ fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean::
         + name.as_str().len()
         + generics_len;
 
-    w.write_str("
");
-    render_attributes_in_pre(w, it, "");
-    w.reserve(header_len);
-    write!(
-        w,
-        "{vis}{constness}{asyncness}{unsafety}{abi}fn \
-         {name}{generics}{decl}{notable_traits}{where_clause}
",
-        vis = vis,
-        constness = constness,
-        asyncness = asyncness,
-        unsafety = unsafety,
-        abi = abi,
-        name = name,
-        generics = f.generics.print(cx),
-        where_clause = print_where_clause(&f.generics, cx, 0, true),
-        decl = f.decl.full_print(header_len, 0, f.header.asyncness, cx),
-        notable_traits = notable_traits_decl(&f.decl, cx),
-    );
+    wrap_item(w, "fn", |w| {
+        render_attributes_in_pre(w, it, "");
+        w.reserve(header_len);
+        write!(
+            w,
+            "{vis}{constness}{asyncness}{unsafety}{abi}fn \
+             {name}{generics}{decl}{notable_traits}{where_clause}",
+            vis = vis,
+            constness = constness,
+            asyncness = asyncness,
+            unsafety = unsafety,
+            abi = abi,
+            name = name,
+            generics = f.generics.print(cx),
+            where_clause = print_where_clause(&f.generics, cx, 0, true),
+            decl = f.decl.full_print(header_len, 0, f.header.asyncness, cx),
+            notable_traits = notable_traits_decl(&f.decl, cx),
+        );
+    });
     document(w, cx, it, None)
 }
 
@@ -488,108 +489,111 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
 
     // Output the trait definition
     wrap_into_docblock(w, |w| {
-        w.write_str("
");
-        render_attributes_in_pre(w, it, "");
-        write!(
-            w,
-            "{}{}{}trait {}{}{}",
-            it.visibility.print_with_space(it.def_id, cx),
-            t.unsafety.print_with_space(),
-            if t.is_auto { "auto " } else { "" },
-            it.name.as_ref().unwrap(),
-            t.generics.print(cx),
-            bounds
-        );
-
-        if !t.generics.where_predicates.is_empty() {
-            write!(w, "{}", print_where_clause(&t.generics, cx, 0, true));
-        } else {
-            w.write_str(" ");
-        }
+        wrap_item(w, "trait", |w| {
+            render_attributes_in_pre(w, it, "");
+            write!(
+                w,
+                "{}{}{}trait {}{}{}",
+                it.visibility.print_with_space(it.def_id, cx),
+                t.unsafety.print_with_space(),
+                if t.is_auto { "auto " } else { "" },
+                it.name.as_ref().unwrap(),
+                t.generics.print(cx),
+                bounds
+            );
 
-        if t.items.is_empty() {
-            w.write_str("{ }");
-        } else {
-            // FIXME: we should be using a derived_id for the Anchors here
-            w.write_str("{\n");
-            let mut toggle = false;
-
-            // If there are too many associated types, hide _everything_
-            if should_hide_fields(count_types) {
-                toggle = true;
-                toggle_open(
-                    w,
-                    format_args!("{} associated items", count_types + count_consts + count_methods),
-                );
-            }
-            for t in &types {
-                render_assoc_item(w, t, AssocItemLink::Anchor(None), ItemType::Trait, cx);
-                w.write_str(";\n");
-            }
-            // If there are too many associated constants, hide everything after them
-            // We also do this if the types + consts is large because otherwise we could
-            // render a bunch of types and _then_ a bunch of consts just because both were
-            // _just_ under the limit
-            if !toggle && should_hide_fields(count_types + count_consts) {
-                toggle = true;
-                toggle_open(
-                    w,
-                    format_args!(
-                        "{} associated constant{} and {} method{}",
-                        count_consts,
-                        pluralize(count_consts),
-                        count_methods,
-                        pluralize(count_methods),
-                    ),
-                );
-            }
-            if !types.is_empty() && !consts.is_empty() {
-                w.write_str("\n");
-            }
-            for t in &consts {
-                render_assoc_item(w, t, AssocItemLink::Anchor(None), ItemType::Trait, cx);
-                w.write_str(";\n");
-            }
-            if !toggle && should_hide_fields(count_methods) {
-                toggle = true;
-                toggle_open(w, format_args!("{} methods", count_methods));
-            }
-            if !consts.is_empty() && !required.is_empty() {
-                w.write_str("\n");
+            if !t.generics.where_predicates.is_empty() {
+                write!(w, "{}", print_where_clause(&t.generics, cx, 0, true));
+            } else {
+                w.write_str(" ");
             }
-            for (pos, m) in required.iter().enumerate() {
-                render_assoc_item(w, m, AssocItemLink::Anchor(None), ItemType::Trait, cx);
-                w.write_str(";\n");
 
-                if pos < required.len() - 1 {
-                    w.write_str("
");
+            if t.items.is_empty() {
+                w.write_str("{ }");
+            } else {
+                // FIXME: we should be using a derived_id for the Anchors here
+                w.write_str("{\n");
+                let mut toggle = false;
+
+                // If there are too many associated types, hide _everything_
+                if should_hide_fields(count_types) {
+                    toggle = true;
+                    toggle_open(
+                        w,
+                        format_args!(
+                            "{} associated items",
+                            count_types + count_consts + count_methods
+                        ),
+                    );
                 }
-            }
-            if !required.is_empty() && !provided.is_empty() {
-                w.write_str("\n");
-            }
-            for (pos, m) in provided.iter().enumerate() {
-                render_assoc_item(w, m, AssocItemLink::Anchor(None), ItemType::Trait, cx);
-                match *m.kind {
-                    clean::MethodItem(ref inner, _)
-                        if !inner.generics.where_predicates.is_empty() =>
-                    {
-                        w.write_str(",\n    { ... }\n");
+                for t in &types {
+                    render_assoc_item(w, t, AssocItemLink::Anchor(None), ItemType::Trait, cx);
+                    w.write_str(";\n");
+                }
+                // If there are too many associated constants, hide everything after them
+                // We also do this if the types + consts is large because otherwise we could
+                // render a bunch of types and _then_ a bunch of consts just because both were
+                // _just_ under the limit
+                if !toggle && should_hide_fields(count_types + count_consts) {
+                    toggle = true;
+                    toggle_open(
+                        w,
+                        format_args!(
+                            "{} associated constant{} and {} method{}",
+                            count_consts,
+                            pluralize(count_consts),
+                            count_methods,
+                            pluralize(count_methods),
+                        ),
+                    );
+                }
+                if !types.is_empty() && !consts.is_empty() {
+                    w.write_str("\n");
+                }
+                for t in &consts {
+                    render_assoc_item(w, t, AssocItemLink::Anchor(None), ItemType::Trait, cx);
+                    w.write_str(";\n");
+                }
+                if !toggle && should_hide_fields(count_methods) {
+                    toggle = true;
+                    toggle_open(w, format_args!("{} methods", count_methods));
+                }
+                if !consts.is_empty() && !required.is_empty() {
+                    w.write_str("\n");
+                }
+                for (pos, m) in required.iter().enumerate() {
+                    render_assoc_item(w, m, AssocItemLink::Anchor(None), ItemType::Trait, cx);
+                    w.write_str(";\n");
+
+                    if pos < required.len() - 1 {
+                        w.write_str("
");
                     }
-                    _ => {
-                        w.write_str(" { ... }\n");
+                }
+                if !required.is_empty() && !provided.is_empty() {
+                    w.write_str("\n");
+                }
+                for (pos, m) in provided.iter().enumerate() {
+                    render_assoc_item(w, m, AssocItemLink::Anchor(None), ItemType::Trait, cx);
+                    match *m.kind {
+                        clean::MethodItem(ref inner, _)
+                            if !inner.generics.where_predicates.is_empty() =>
+                        {
+                            w.write_str(",\n    { ... }\n");
+                        }
+                        _ => {
+                            w.write_str(" { ... }\n");
+                        }
+                    }
+                    if pos < provided.len() - 1 {
+                        w.write_str("
");
                     }
                 }
-                if pos < provided.len() - 1 {
-                    w.write_str("
");
+                if toggle {
+                    toggle_close(w);
                 }
+                w.write_str("}");
             }
-            if toggle {
-                toggle_close(w);
-            }
-            w.write_str("}");
-        }
-        w.write_str("
")
+        });
     });
 
     // Trait documentation
@@ -812,16 +816,17 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
 }
 
 fn item_trait_alias(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::TraitAlias) {
-    w.write_str("
");
-    render_attributes_in_pre(w, it, "");
-    write!(
-        w,
-        "trait {}{}{} = {};
",
-        it.name.as_ref().unwrap(),
-        t.generics.print(cx),
-        print_where_clause(&t.generics, cx, 0, true),
-        bounds(&t.bounds, true, cx)
-    );
+    wrap_item(w, "trait-alias", |w| {
+        render_attributes_in_pre(w, it, "");
+        write!(
+            w,
+            "trait {}{}{} = {};",
+            it.name.as_ref().unwrap(),
+            t.generics.print(cx),
+            print_where_clause(&t.generics, cx, 0, true),
+            bounds(&t.bounds, true, cx)
+        );
+    });
 
     document(w, cx, it, None);
 
@@ -833,16 +838,17 @@ fn item_trait_alias(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clea
 }
 
 fn item_opaque_ty(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::OpaqueTy) {
-    w.write_str("
");
-    render_attributes_in_pre(w, it, "");
-    write!(
-        w,
-        "type {}{}{where_clause} = impl {bounds};
",
-        it.name.as_ref().unwrap(),
-        t.generics.print(cx),
-        where_clause = print_where_clause(&t.generics, cx, 0, true),
-        bounds = bounds(&t.bounds, false, cx),
-    );
+    wrap_item(w, "opaque", |w| {
+        render_attributes_in_pre(w, it, "");
+        write!(
+            w,
+            "type {}{}{where_clause} = impl {bounds};",
+            it.name.as_ref().unwrap(),
+            t.generics.print(cx),
+            where_clause = print_where_clause(&t.generics, cx, 0, true),
+            bounds = bounds(&t.bounds, false, cx),
+        );
+    });
 
     document(w, cx, it, None);
 
@@ -860,19 +866,20 @@ fn item_typedef(
     t: &clean::Typedef,
     is_associated: bool,
 ) {
-    w.write_str("
");
-    render_attributes_in_pre(w, it, "");
-    if !is_associated {
-        write!(w, "{}", it.visibility.print_with_space(it.def_id, cx));
-    }
-    write!(
-        w,
-        "type {}{}{where_clause} = {type_};
",
-        it.name.as_ref().unwrap(),
-        t.generics.print(cx),
-        where_clause = print_where_clause(&t.generics, cx, 0, true),
-        type_ = t.type_.print(cx),
-    );
+    wrap_item(w, "typedef", |w| {
+        render_attributes_in_pre(w, it, "");
+        if !is_associated {
+            write!(w, "{}", it.visibility.print_with_space(it.def_id, cx));
+        }
+        write!(
+            w,
+            "type {}{}{where_clause} = {type_};",
+            it.name.as_ref().unwrap(),
+            t.generics.print(cx),
+            where_clause = print_where_clause(&t.generics, cx, 0, true),
+            type_ = t.type_.print(cx),
+        );
+    });
 
     document(w, cx, it, None);
 
@@ -886,10 +893,10 @@ fn item_typedef(
 
 fn item_union(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::Union) {
     wrap_into_docblock(w, |w| {
-        w.write_str("
");
-        render_attributes_in_pre(w, it, "");
-        render_union(w, it, Some(&s.generics), &s.fields, "", cx);
-        w.write_str("
")
+        wrap_item(w, "union", |w| {
+            render_attributes_in_pre(w, it, "");
+            render_union(w, it, Some(&s.generics), &s.fields, "", cx);
+        });
     });
 
     document(w, cx, it, None);
@@ -935,59 +942,68 @@ fn item_union(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::Uni
 
 fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum) {
     wrap_into_docblock(w, |w| {
-        w.write_str("
");
-        render_attributes_in_pre(w, it, "");
-        write!(
-            w,
-            "{}enum {}{}{}",
-            it.visibility.print_with_space(it.def_id, cx),
-            it.name.as_ref().unwrap(),
-            e.generics.print(cx),
-            print_where_clause(&e.generics, cx, 0, true),
-        );
-        if e.variants.is_empty() && !e.variants_stripped {
-            w.write_str(" {}");
-        } else {
-            w.write_str(" {\n");
-            let count_variants = e.variants.len();
-            let toggle = should_hide_fields(count_variants);
-            if toggle {
-                toggle_open(w, format_args!("{} variants", count_variants));
-            }
-            for v in &e.variants {
-                w.write_str("    ");
-                let name = v.name.as_ref().unwrap();
-                match *v.kind {
-                    clean::VariantItem(ref var) => match var {
-                        clean::Variant::CLike => write!(w, "{}", name),
-                        clean::Variant::Tuple(ref tys) => {
-                            write!(w, "{}(", name);
-                            for (i, ty) in tys.iter().enumerate() {
-                                if i > 0 {
-                                    w.write_str(", ")
+        wrap_item(w, "enum", |w| {
+            render_attributes_in_pre(w, it, "");
+            write!(
+                w,
+                "{}enum {}{}{}",
+                it.visibility.print_with_space(it.def_id, cx),
+                it.name.as_ref().unwrap(),
+                e.generics.print(cx),
+                print_where_clause(&e.generics, cx, 0, true),
+            );
+            if e.variants.is_empty() && !e.variants_stripped {
+                w.write_str(" {}");
+            } else {
+                w.write_str(" {\n");
+                let count_variants = e.variants.len();
+                let toggle = should_hide_fields(count_variants);
+                if toggle {
+                    toggle_open(w, format_args!("{} variants", count_variants));
+                }
+                for v in &e.variants {
+                    w.write_str("    ");
+                    let name = v.name.as_ref().unwrap();
+                    match *v.kind {
+                        clean::VariantItem(ref var) => match var {
+                            clean::Variant::CLike => write!(w, "{}", name),
+                            clean::Variant::Tuple(ref tys) => {
+                                write!(w, "{}(", name);
+                                for (i, ty) in tys.iter().enumerate() {
+                                    if i > 0 {
+                                        w.write_str(", ")
+                                    }
+                                    write!(w, "{}", ty.print(cx));
                                 }
-                                write!(w, "{}", ty.print(cx));
+                                w.write_str(")");
                             }
-                            w.write_str(")");
-                        }
-                        clean::Variant::Struct(ref s) => {
-                            render_struct(w, v, None, s.struct_type, &s.fields, "    ", false, cx);
-                        }
-                    },
-                    _ => unreachable!(),
+                            clean::Variant::Struct(ref s) => {
+                                render_struct(
+                                    w,
+                                    v,
+                                    None,
+                                    s.struct_type,
+                                    &s.fields,
+                                    "    ",
+                                    false,
+                                    cx,
+                                );
+                            }
+                        },
+                        _ => unreachable!(),
+                    }
+                    w.write_str(",\n");
                 }
-                w.write_str(",\n");
-            }
 
-            if e.variants_stripped {
-                w.write_str("    // some variants omitted\n");
-            }
-            if toggle {
-                toggle_close(w);
+                if e.variants_stripped {
+                    w.write_str("    // some variants omitted\n");
+                }
+                if toggle {
+                    toggle_close(w);
+                }
+                w.write_str("}");
             }
-            w.write_str("}");
-        }
-        w.write_str("
")
+        });
     });
 
     document(w, cx, it, None);
@@ -1091,27 +1107,27 @@ fn item_proc_macro(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, m: &clean
     let name = it.name.as_ref().expect("proc-macros always have names");
     match m.kind {
         MacroKind::Bang => {
-            w.push_str("
");
-            write!(w, "{}!() {{ /* proc-macro */ }}", name);
-            w.push_str("
");
+            wrap_item(w, "macro", |w| {
+                write!(w, "{}!() {{ /* proc-macro */ }}", name);
+            });
         }
         MacroKind::Attr => {
-            w.push_str("
");
-            write!(w, "#[{}]", name);
-            w.push_str("
");
+            wrap_item(w, "attr", |w| {
+                write!(w, "#[{}]", name);
+            });
         }
         MacroKind::Derive => {
-            w.push_str("
");
-            write!(w, "#[derive({})]", name);
-            if !m.helpers.is_empty() {
-                w.push_str("\n{\n");
-                w.push_str("    // Attributes available to this derive:\n");
-                for attr in &m.helpers {
-                    writeln!(w, "    #[{}]", attr);
+            wrap_item(w, "derive", |w| {
+                write!(w, "#[derive({})]", name);
+                if !m.helpers.is_empty() {
+                    w.push_str("\n{\n");
+                    w.push_str("    // Attributes available to this derive:\n");
+                    for attr in &m.helpers {
+                        writeln!(w, "    #[{}]", attr);
+                    }
+                    w.push_str("}\n");
                 }
-                w.push_str("}\n");
-            }
-            w.push_str("
");
+            });
         }
     }
     document(w, cx, it, None)
@@ -1123,49 +1139,49 @@ fn item_primitive(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item) {
 }
 
 fn item_constant(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, c: &clean::Constant) {
-    w.write_str("
");
-    render_attributes_in_code(w, it);
+    wrap_item(w, "const", |w| {
+        render_attributes_in_code(w, it);
 
-    write!(
-        w,
-        "{vis}const {name}: {typ}",
-        vis = it.visibility.print_with_space(it.def_id, cx),
-        name = it.name.as_ref().unwrap(),
-        typ = c.type_.print(cx),
-    );
+        write!(
+            w,
+            "{vis}const {name}: {typ}",
+            vis = it.visibility.print_with_space(it.def_id, cx),
+            name = it.name.as_ref().unwrap(),
+            typ = c.type_.print(cx),
+        );
 
-    let value = c.value(cx.tcx());
-    let is_literal = c.is_literal(cx.tcx());
-    let expr = c.expr(cx.tcx());
-    if value.is_some() || is_literal {
-        write!(w, " = {expr};", expr = Escape(&expr));
-    } else {
-        w.write_str(";");
-    }
+        let value = c.value(cx.tcx());
+        let is_literal = c.is_literal(cx.tcx());
+        let expr = c.expr(cx.tcx());
+        if value.is_some() || is_literal {
+            write!(w, " = {expr};", expr = Escape(&expr));
+        } else {
+            w.write_str(";");
+        }
 
-    if !is_literal {
-        if let Some(value) = &value {
-            let value_lowercase = value.to_lowercase();
-            let expr_lowercase = expr.to_lowercase();
+        if !is_literal {
+            if let Some(value) = &value {
+                let value_lowercase = value.to_lowercase();
+                let expr_lowercase = expr.to_lowercase();
 
-            if value_lowercase != expr_lowercase
-                && value_lowercase.trim_end_matches("i32") != expr_lowercase
-            {
-                write!(w, " // {value}", value = Escape(value));
+                if value_lowercase != expr_lowercase
+                    && value_lowercase.trim_end_matches("i32") != expr_lowercase
+                {
+                    write!(w, " // {value}", value = Escape(value));
+                }
             }
         }
-    }
+    });
 
-    w.write_str("
");
     document(w, cx, it, None)
 }
 
 fn item_struct(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::Struct) {
     wrap_into_docblock(w, |w| {
-        w.write_str("
");
-        render_attributes_in_code(w, it);
-        render_struct(w, it, Some(&s.generics), s.struct_type, &s.fields, "", true, cx);
-        w.write_str("
")
+        wrap_item(w, "struct", |w| {
+            render_attributes_in_code(w, it);
+            render_struct(w, it, Some(&s.generics), s.struct_type, &s.fields, "", true, cx);
+        });
     });
 
     document(w, cx, it, None);
@@ -1214,28 +1230,31 @@ fn item_struct(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::St
 }
 
 fn item_static(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::Static) {
-    w.write_str("
");
-    render_attributes_in_code(w, it);
-    write!(
-        w,
-        "{vis}static {mutability}{name}: {typ}
",
-        vis = it.visibility.print_with_space(it.def_id, cx),
-        mutability = s.mutability.print_with_space(),
-        name = it.name.as_ref().unwrap(),
-        typ = s.type_.print(cx)
-    );
+    wrap_item(w, "static", |w| {
+        render_attributes_in_code(w, it);
+        write!(
+            w,
+            "{vis}static {mutability}{name}: {typ}",
+            vis = it.visibility.print_with_space(it.def_id, cx),
+            mutability = s.mutability.print_with_space(),
+            name = it.name.as_ref().unwrap(),
+            typ = s.type_.print(cx)
+        );
+    });
     document(w, cx, it, None)
 }
 
 fn item_foreign_type(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item) {
-    w.write_str("
extern {\n");
-    render_attributes_in_code(w, it);
-    write!(
-        w,
-        "    {}type {};\n}}
",
-        it.visibility.print_with_space(it.def_id, cx),
-        it.name.as_ref().unwrap(),
-    );
+    wrap_item(w, "foreigntype", |w| {
+        w.write_str("extern {\n");
+        render_attributes_in_code(w, it);
+        write!(
+            w,
+            "    {}type {};\n}}",
+            it.visibility.print_with_space(it.def_id, cx),
+            it.name.as_ref().unwrap(),
+        );
+    });
 
     document(w, cx, it, None);
 
@@ -1322,6 +1341,15 @@ where
     w.write_str("")
 }
 
+fn wrap_item(w: &mut Buffer, item_name: &str, f: F)
+where
+    F: FnOnce(&mut Buffer),
+{
+    w.write_fmt(format_args!("
", item_name));
+    f(w);
+    w.write_str("
");
+}
+
 fn render_stability_since(
     w: &mut Buffer,
     item: &clean::Item,
diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css
index bbc48f49e63e1..0f95b6aaa898d 100644
--- a/src/librustdoc/html/static/css/rustdoc.css
+++ b/src/librustdoc/html/static/css/rustdoc.css
@@ -249,7 +249,6 @@ code, pre, a.test-arrow, .code-header {
 }
 .docblock pre code, .docblock-short pre code {
 	padding: 0;
-	padding-right: 1ex;
 }
 pre {
 	padding: 14px;
diff --git a/src/test/rustdoc-gui/code-tags.goml b/src/test/rustdoc-gui/code-tags.goml
new file mode 100644
index 0000000000000..200569a28d4a4
--- /dev/null
+++ b/src/test/rustdoc-gui/code-tags.goml
@@ -0,0 +1,20 @@
+// This test ensures that items and documentation code blocks are wrapped in 

+goto: file://|DOC_PATH|/test_docs/fn.foo.html
+size: (1080, 600)
+// There should be three doc codeblocks
+// Check that their content is inside 

+assert-count: (".example-wrap pre > code", 3)
+// Check that function signature is inside 

+assert: "pre.rust.fn > code"
+
+goto: file://|DOC_PATH|/test_docs/struct.Foo.html
+assert: "pre.rust.struct > code"
+
+goto: file://|DOC_PATH|/test_docs/enum.AnEnum.html
+assert: "pre.rust.enum > code"
+
+goto: file://|DOC_PATH|/test_docs/trait.AnotherOne.html
+assert: "pre.rust.trait > code"
+
+goto: file://|DOC_PATH|/test_docs/type.SomeType.html
+assert: "pre.rust.typedef > code"
diff --git a/src/test/rustdoc-gui/source-code-page.goml b/src/test/rustdoc-gui/source-code-page.goml
index d7bae93c211a1..5a49807e180b2 100644
--- a/src/test/rustdoc-gui/source-code-page.goml
+++ b/src/test/rustdoc-gui/source-code-page.goml
@@ -12,4 +12,4 @@ assert-attribute: (".line-numbers > span:nth-child(5)", {"class": "line-highligh
 assert-attribute: (".line-numbers > span:nth-child(6)", {"class": "line-highlighted"})
 assert-attribute-false: (".line-numbers > span:nth-child(7)", {"class": "line-highlighted"})
 // This is to ensure that the content is correctly align with the line numbers.
-compare-elements-position: ("//*[@id='1']", ".rust > span", ("y"))
+compare-elements-position: ("//*[@id='1']", ".rust > code > span", ("y"))

From 8b4e7a47925710db50940cc53638b2b82a308948 Mon Sep 17 00:00:00 2001
From: Hirochika Matsumoto 
Date: Thu, 19 Aug 2021 18:44:43 +0900
Subject: [PATCH 7/8] Update .mailmap

---
 .mailmap | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.mailmap b/.mailmap
index 56ac5296774dd..d1f5ca9371f62 100644
--- a/.mailmap
+++ b/.mailmap
@@ -113,6 +113,7 @@ Hanna Kruppe  
 Heather  
 Heather  
 Herman J. Radtke III  Herman J. Radtke III 
+Hirochika Matsumoto  
 Ian Jackson  
 Ian Jackson  
 Ian Jackson  

From df8a64327ccfcb869867f41cdc6c11ee9357f7d1 Mon Sep 17 00:00:00 2001
From: Santiago Pastorino 
Date: Thu, 19 Aug 2021 10:48:20 -0300
Subject: [PATCH 8/8] Use a trait instead of the now disallowed missing trait
 there

---
 .../type-alias-impl-trait/unused_generic_param.rs  |  9 +++++----
 .../unused_generic_param.stderr                    | 14 --------------
 2 files changed, 5 insertions(+), 18 deletions(-)
 delete mode 100644 src/test/ui/type-alias-impl-trait/unused_generic_param.stderr

diff --git a/src/test/ui/type-alias-impl-trait/unused_generic_param.rs b/src/test/ui/type-alias-impl-trait/unused_generic_param.rs
index 04a5c58cd36e9..ad5e4918ccac5 100644
--- a/src/test/ui/type-alias-impl-trait/unused_generic_param.rs
+++ b/src/test/ui/type-alias-impl-trait/unused_generic_param.rs
@@ -1,16 +1,17 @@
+// check-pass
+
 #![feature(type_alias_impl_trait)]
+#![allow(dead_code)]
 
 fn main() {}
 
-type PartiallyDefined = impl 'static;
-//~^ ERROR: at least one trait must be specified
+type PartiallyDefined = impl Sized;
 
 fn partially_defined(_: T) -> PartiallyDefined {
     4u32
 }
 
-type PartiallyDefined2 = impl 'static;
-//~^ ERROR: at least one trait must be specified
+type PartiallyDefined2 = impl Sized;
 
 fn partially_defined2(_: T) -> PartiallyDefined2 {
     4u32
diff --git a/src/test/ui/type-alias-impl-trait/unused_generic_param.stderr b/src/test/ui/type-alias-impl-trait/unused_generic_param.stderr
deleted file mode 100644
index 4e11854b07189..0000000000000
--- a/src/test/ui/type-alias-impl-trait/unused_generic_param.stderr
+++ /dev/null
@@ -1,14 +0,0 @@
-error: at least one trait must be specified
-  --> $DIR/unused_generic_param.rs:5:28
-   |
-LL | type PartiallyDefined = impl 'static;
-   |                            ^^^^^^^^^^^^
-
-error: at least one trait must be specified
-  --> $DIR/unused_generic_param.rs:12:29
-   |
-LL | type PartiallyDefined2 = impl 'static;
-   |                             ^^^^^^^^^^^^
-
-error: aborting due to 2 previous errors
-