From 59b6b1101fee97ece51ab7b91036e15b0188463b Mon Sep 17 00:00:00 2001
From: Steven Fackler <sfackler@gmail.com>
Date: Mon, 11 Jan 2021 07:27:03 -0500
Subject: [PATCH 01/24] Fix handling of malicious Readers in read_to_end

---
 library/std/src/io/mod.rs | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs
index dfbf6c3f24443..202889018c595 100644
--- a/library/std/src/io/mod.rs
+++ b/library/std/src/io/mod.rs
@@ -391,7 +391,14 @@ where
                 ret = Ok(g.len - start_len);
                 break;
             }
-            Ok(n) => g.len += n,
+            Ok(n) => {
+                // We can't let g.len overflow which would result in the vec shrinking when the function returns. In
+                // particular, that could break read_to_string if the shortened buffer doesn't end on a UTF-8 boundary.
+                // The minimal check would just be a checked_add, but this assert is a bit more precise and should be
+                // just about the same cost.
+                assert!(n <= g.buf.len() - g.len);
+                g.len += n;
+            }
             Err(ref e) if e.kind() == ErrorKind::Interrupted => {}
             Err(e) => {
                 ret = Err(e);

From 2fe7ddc3f8e7507af5ae0f29bfaf23a43b419004 Mon Sep 17 00:00:00 2001
From: Steven Fackler <sfackler@gmail.com>
Date: Mon, 11 Jan 2021 07:48:24 -0500
Subject: [PATCH 02/24] clean up control flow

---
 library/std/src/io/mod.rs | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs
index 202889018c595..64601cde01df9 100644
--- a/library/std/src/io/mod.rs
+++ b/library/std/src/io/mod.rs
@@ -367,7 +367,6 @@ where
 {
     let start_len = buf.len();
     let mut g = Guard { len: buf.len(), buf };
-    let ret;
     loop {
         if g.len == g.buf.len() {
             unsafe {
@@ -387,10 +386,7 @@ where
         }
 
         match r.read(&mut g.buf[g.len..]) {
-            Ok(0) => {
-                ret = Ok(g.len - start_len);
-                break;
-            }
+            Ok(0) => return Ok(g.len - start_len),
             Ok(n) => {
                 // We can't let g.len overflow which would result in the vec shrinking when the function returns. In
                 // particular, that could break read_to_string if the shortened buffer doesn't end on a UTF-8 boundary.
@@ -400,14 +396,9 @@ where
                 g.len += n;
             }
             Err(ref e) if e.kind() == ErrorKind::Interrupted => {}
-            Err(e) => {
-                ret = Err(e);
-                break;
-            }
+            Err(e) => return Err(e),
         }
     }
-
-    ret
 }
 
 pub(crate) fn default_read_vectored<F>(read: F, bufs: &mut [IoSliceMut<'_>]) -> Result<usize>

From e73125e9e8457747084ca520192a8e7990441269 Mon Sep 17 00:00:00 2001
From: Steven Fackler <sfackler@gmail.com>
Date: Mon, 11 Jan 2021 17:13:50 -0500
Subject: [PATCH 03/24] make check a bit more clear

---
 library/std/src/io/mod.rs | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs
index 64601cde01df9..57b78be618983 100644
--- a/library/std/src/io/mod.rs
+++ b/library/std/src/io/mod.rs
@@ -385,14 +385,15 @@ where
             }
         }
 
-        match r.read(&mut g.buf[g.len..]) {
+        let buf = &mut g.buf[g.len..];
+        match r.read(buf) {
             Ok(0) => return Ok(g.len - start_len),
             Ok(n) => {
                 // We can't let g.len overflow which would result in the vec shrinking when the function returns. In
                 // particular, that could break read_to_string if the shortened buffer doesn't end on a UTF-8 boundary.
                 // The minimal check would just be a checked_add, but this assert is a bit more precise and should be
                 // just about the same cost.
-                assert!(n <= g.buf.len() - g.len);
+                assert!(n <= buf.len());
                 g.len += n;
             }
             Err(ref e) if e.kind() == ErrorKind::Interrupted => {}

From 2eef2e24bac35abd705131dd3f1c2ce23cf41c94 Mon Sep 17 00:00:00 2001
From: Steven Fackler <sfackler@gmail.com>
Date: Mon, 11 Jan 2021 17:16:44 -0500
Subject: [PATCH 04/24] clarify docs a bit

---
 library/std/src/io/mod.rs | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs
index 57b78be618983..30d80b76ab7ca 100644
--- a/library/std/src/io/mod.rs
+++ b/library/std/src/io/mod.rs
@@ -389,10 +389,9 @@ where
         match r.read(buf) {
             Ok(0) => return Ok(g.len - start_len),
             Ok(n) => {
-                // We can't let g.len overflow which would result in the vec shrinking when the function returns. In
-                // particular, that could break read_to_string if the shortened buffer doesn't end on a UTF-8 boundary.
-                // The minimal check would just be a checked_add, but this assert is a bit more precise and should be
-                // just about the same cost.
+                // We can't allow bogus values from read. If it is too large, the returned vec could have its length
+                // set past its capacity, or if it overflows the vec could be shortened which could create an invalid
+                // string if this is called via read_to_string.
                 assert!(n <= buf.len());
                 g.len += n;
             }

From 44a1f09985e9d99bfce1e73e0f1e43066999092d Mon Sep 17 00:00:00 2001
From: Pietro Albini <pietro@pietroalbini.org>
Date: Wed, 30 Dec 2020 16:05:57 +0100
Subject: [PATCH 05/24] bootstrap: extract from any compression algorithm
 during distcheck

---
 src/bootstrap/test.rs | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
index b99692e8ba5b8..1b9523426e735 100644
--- a/src/bootstrap/test.rs
+++ b/src/bootstrap/test.rs
@@ -1968,7 +1968,7 @@ impl Step for Distcheck {
         builder.ensure(dist::Src);
 
         let mut cmd = Command::new("tar");
-        cmd.arg("-xzf")
+        cmd.arg("-xf")
             .arg(builder.ensure(dist::PlainSourceTarball))
             .arg("--strip-components=1")
             .current_dir(&dir);
@@ -1992,10 +1992,7 @@ impl Step for Distcheck {
         t!(fs::create_dir_all(&dir));
 
         let mut cmd = Command::new("tar");
-        cmd.arg("-xzf")
-            .arg(builder.ensure(dist::Src))
-            .arg("--strip-components=1")
-            .current_dir(&dir);
+        cmd.arg("-xf").arg(builder.ensure(dist::Src)).arg("--strip-components=1").current_dir(&dir);
         builder.run(&mut cmd);
 
         let toml = dir.join("rust-src/lib/rustlib/src/rust/library/std/Cargo.toml");

From 4e4636d9d4ee5eff7a758be1dd8d2c4456b043ee Mon Sep 17 00:00:00 2001
From: Pietro Albini <pietro@pietroalbini.org>
Date: Wed, 30 Dec 2020 12:10:31 +0100
Subject: [PATCH 06/24] bootstrap: never delete the tarball temporary directory

Files in the temporary directory are used by ./x.py install.
---
 src/bootstrap/dist.rs    | 4 ++--
 src/bootstrap/tarball.rs | 8 +-------
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs
index 0745260e7e702..997587fe35eba 100644
--- a/src/bootstrap/dist.rs
+++ b/src/bootstrap/dist.rs
@@ -1322,8 +1322,8 @@ impl Step for Extended {
             tarballs.push(mingw_installer.unwrap());
         }
 
-        let mut tarball = Tarball::new(builder, "rust", &target.triple);
-        let work = tarball.persist_work_dir();
+        let tarball = Tarball::new(builder, "rust", &target.triple);
+        let work = tarball.work_dir();
         tarball.combine(&tarballs);
 
         let tmp = tmpdir(builder).join("combined-tarball");
diff --git a/src/bootstrap/tarball.rs b/src/bootstrap/tarball.rs
index 32c8e791bfc6b..688cc0fb8c95a 100644
--- a/src/bootstrap/tarball.rs
+++ b/src/bootstrap/tarball.rs
@@ -97,7 +97,6 @@ pub(crate) struct Tarball<'a> {
 
     include_target_in_component_name: bool,
     is_preview: bool,
-    delete_temp_dir: bool,
 }
 
 impl<'a> Tarball<'a> {
@@ -136,7 +135,6 @@ impl<'a> Tarball<'a> {
 
             include_target_in_component_name: false,
             is_preview: false,
-            delete_temp_dir: true,
         }
     }
 
@@ -198,8 +196,7 @@ impl<'a> Tarball<'a> {
         self.builder.cp_r(src.as_ref(), &dest);
     }
 
-    pub(crate) fn persist_work_dir(&mut self) -> PathBuf {
-        self.delete_temp_dir = false;
+    pub(crate) fn work_dir(&self) -> PathBuf {
         self.temp_dir.clone()
     }
 
@@ -295,9 +292,6 @@ impl<'a> Tarball<'a> {
         build_cli(&self, &mut cmd);
         cmd.arg("--work-dir").arg(&self.temp_dir);
         self.builder.run(&mut cmd);
-        if self.delete_temp_dir {
-            t!(std::fs::remove_dir_all(&self.temp_dir));
-        }
 
         crate::dist::distdir(self.builder).join(format!("{}.tar.gz", package_name))
     }

From 14fa72673435d9591e41f6d42298a8a4b3588baf Mon Sep 17 00:00:00 2001
From: Pietro Albini <pietro@pietroalbini.org>
Date: Wed, 30 Dec 2020 12:20:13 +0100
Subject: [PATCH 07/24] bootstrap: change the dist outputs to GeneratedTarball

The struct will allow to store more context on the generated tarballs.
---
 src/bootstrap/dist.rs    | 76 ++++++++++++++++++++--------------------
 src/bootstrap/tarball.rs | 29 ++++++++++-----
 src/bootstrap/test.rs    |  7 ++--
 3 files changed, 64 insertions(+), 48 deletions(-)

diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs
index 997587fe35eba..947ac6f1ee0eb 100644
--- a/src/bootstrap/dist.rs
+++ b/src/bootstrap/dist.rs
@@ -19,7 +19,7 @@ use crate::builder::{Builder, RunConfig, ShouldRun, Step};
 use crate::cache::{Interned, INTERNER};
 use crate::compile;
 use crate::config::TargetSelection;
-use crate::tarball::{OverlayKind, Tarball};
+use crate::tarball::{GeneratedTarball, OverlayKind, Tarball};
 use crate::tool::{self, Tool};
 use crate::util::{exe, is_dylib, timeit};
 use crate::{Compiler, DependencyType, Mode, LLVM_TOOLS};
@@ -51,7 +51,7 @@ pub struct Docs {
 }
 
 impl Step for Docs {
-    type Output = Option<PathBuf>;
+    type Output = Option<GeneratedTarball>;
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -63,7 +63,7 @@ impl Step for Docs {
     }
 
     /// Builds the `rust-docs` installer component.
-    fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
+    fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
         let host = self.host;
         if !builder.config.docs {
             return None;
@@ -86,7 +86,7 @@ pub struct RustcDocs {
 }
 
 impl Step for RustcDocs {
-    type Output = Option<PathBuf>;
+    type Output = Option<GeneratedTarball>;
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -98,7 +98,7 @@ impl Step for RustcDocs {
     }
 
     /// Builds the `rustc-docs` installer component.
-    fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
+    fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
         let host = self.host;
         if !builder.config.compiler_docs {
             return None;
@@ -267,7 +267,7 @@ pub struct Mingw {
 }
 
 impl Step for Mingw {
-    type Output = Option<PathBuf>;
+    type Output = Option<GeneratedTarball>;
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -282,7 +282,7 @@ impl Step for Mingw {
     ///
     /// This contains all the bits and pieces to run the MinGW Windows targets
     /// without any extra installed software (e.g., we bundle gcc, libraries, etc).
-    fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
+    fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
         let host = self.host;
         if !host.contains("pc-windows-gnu") {
             return None;
@@ -307,7 +307,7 @@ pub struct Rustc {
 }
 
 impl Step for Rustc {
-    type Output = PathBuf;
+    type Output = GeneratedTarball;
     const DEFAULT: bool = true;
     const ONLY_HOSTS: bool = true;
 
@@ -321,7 +321,7 @@ impl Step for Rustc {
     }
 
     /// Creates the `rustc` installer component.
-    fn run(self, builder: &Builder<'_>) -> PathBuf {
+    fn run(self, builder: &Builder<'_>) -> GeneratedTarball {
         let compiler = self.compiler;
         let host = self.compiler.host;
 
@@ -555,7 +555,7 @@ pub struct Std {
 }
 
 impl Step for Std {
-    type Output = Option<PathBuf>;
+    type Output = Option<GeneratedTarball>;
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -573,7 +573,7 @@ impl Step for Std {
         });
     }
 
-    fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
+    fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
         let compiler = self.compiler;
         let target = self.target;
 
@@ -601,7 +601,7 @@ pub struct RustcDev {
 }
 
 impl Step for RustcDev {
-    type Output = Option<PathBuf>;
+    type Output = Option<GeneratedTarball>;
     const DEFAULT: bool = true;
     const ONLY_HOSTS: bool = true;
 
@@ -620,7 +620,7 @@ impl Step for RustcDev {
         });
     }
 
-    fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
+    fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
         let compiler = self.compiler;
         let target = self.target;
         if skip_host_target_lib(builder, compiler) {
@@ -660,7 +660,7 @@ pub struct Analysis {
 }
 
 impl Step for Analysis {
-    type Output = Option<PathBuf>;
+    type Output = Option<GeneratedTarball>;
     const DEFAULT: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -683,7 +683,7 @@ impl Step for Analysis {
     }
 
     /// Creates a tarball of save-analysis metadata, if available.
-    fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
+    fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
         let compiler = self.compiler;
         let target = self.target;
         assert!(builder.config.extended);
@@ -796,7 +796,7 @@ pub struct Src;
 
 impl Step for Src {
     /// The output path of the src installer tarball
-    type Output = PathBuf;
+    type Output = GeneratedTarball;
     const DEFAULT: bool = true;
     const ONLY_HOSTS: bool = true;
 
@@ -809,7 +809,7 @@ impl Step for Src {
     }
 
     /// Creates the `rust-src` installer component
-    fn run(self, builder: &Builder<'_>) -> PathBuf {
+    fn run(self, builder: &Builder<'_>) -> GeneratedTarball {
         let tarball = Tarball::new_targetless(builder, "rust-src");
 
         // A lot of tools expect the rust-src component to be entirely in this directory, so if you
@@ -848,7 +848,7 @@ pub struct PlainSourceTarball;
 
 impl Step for PlainSourceTarball {
     /// Produces the location of the tarball generated
-    type Output = PathBuf;
+    type Output = GeneratedTarball;
     const DEFAULT: bool = true;
     const ONLY_HOSTS: bool = true;
 
@@ -862,7 +862,7 @@ impl Step for PlainSourceTarball {
     }
 
     /// Creates the plain source tarball
-    fn run(self, builder: &Builder<'_>) -> PathBuf {
+    fn run(self, builder: &Builder<'_>) -> GeneratedTarball {
         let tarball = Tarball::new(builder, "rustc", "src");
         let plain_dst_src = tarball.image_dir();
 
@@ -941,7 +941,7 @@ pub struct Cargo {
 }
 
 impl Step for Cargo {
-    type Output = PathBuf;
+    type Output = GeneratedTarball;
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -959,7 +959,7 @@ impl Step for Cargo {
         });
     }
 
-    fn run(self, builder: &Builder<'_>) -> PathBuf {
+    fn run(self, builder: &Builder<'_>) -> GeneratedTarball {
         let compiler = self.compiler;
         let target = self.target;
 
@@ -995,7 +995,7 @@ pub struct Rls {
 }
 
 impl Step for Rls {
-    type Output = Option<PathBuf>;
+    type Output = Option<GeneratedTarball>;
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -1013,7 +1013,7 @@ impl Step for Rls {
         });
     }
 
-    fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
+    fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
         let compiler = self.compiler;
         let target = self.target;
         assert!(builder.config.extended);
@@ -1047,7 +1047,7 @@ pub struct RustAnalyzer {
 }
 
 impl Step for RustAnalyzer {
-    type Output = Option<PathBuf>;
+    type Output = Option<GeneratedTarball>;
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -1065,7 +1065,7 @@ impl Step for RustAnalyzer {
         });
     }
 
-    fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
+    fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
         let compiler = self.compiler;
         let target = self.target;
         assert!(builder.config.extended);
@@ -1096,7 +1096,7 @@ pub struct Clippy {
 }
 
 impl Step for Clippy {
-    type Output = PathBuf;
+    type Output = GeneratedTarball;
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -1114,7 +1114,7 @@ impl Step for Clippy {
         });
     }
 
-    fn run(self, builder: &Builder<'_>) -> PathBuf {
+    fn run(self, builder: &Builder<'_>) -> GeneratedTarball {
         let compiler = self.compiler;
         let target = self.target;
         assert!(builder.config.extended);
@@ -1146,7 +1146,7 @@ pub struct Miri {
 }
 
 impl Step for Miri {
-    type Output = Option<PathBuf>;
+    type Output = Option<GeneratedTarball>;
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -1164,7 +1164,7 @@ impl Step for Miri {
         });
     }
 
-    fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
+    fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
         let compiler = self.compiler;
         let target = self.target;
         assert!(builder.config.extended);
@@ -1199,7 +1199,7 @@ pub struct Rustfmt {
 }
 
 impl Step for Rustfmt {
-    type Output = Option<PathBuf>;
+    type Output = Option<GeneratedTarball>;
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -1217,7 +1217,7 @@ impl Step for Rustfmt {
         });
     }
 
-    fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
+    fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
         let compiler = self.compiler;
         let target = self.target;
 
@@ -1876,7 +1876,7 @@ pub struct LlvmTools {
 }
 
 impl Step for LlvmTools {
-    type Output = Option<PathBuf>;
+    type Output = Option<GeneratedTarball>;
     const ONLY_HOSTS: bool = true;
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -1887,7 +1887,7 @@ impl Step for LlvmTools {
         run.builder.ensure(LlvmTools { target: run.target });
     }
 
-    fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
+    fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
         let target = self.target;
         assert!(builder.config.extended);
 
@@ -1930,7 +1930,7 @@ pub struct RustDev {
 }
 
 impl Step for RustDev {
-    type Output = Option<PathBuf>;
+    type Output = Option<GeneratedTarball>;
     const DEFAULT: bool = true;
     const ONLY_HOSTS: bool = true;
 
@@ -1942,7 +1942,7 @@ impl Step for RustDev {
         run.builder.ensure(RustDev { target: run.target });
     }
 
-    fn run(self, builder: &Builder<'_>) -> Option<PathBuf> {
+    fn run(self, builder: &Builder<'_>) -> Option<GeneratedTarball> {
         let target = self.target;
 
         /* run only if llvm-config isn't used */
@@ -1995,7 +1995,7 @@ pub struct BuildManifest {
 }
 
 impl Step for BuildManifest {
-    type Output = PathBuf;
+    type Output = GeneratedTarball;
     const DEFAULT: bool = false;
     const ONLY_HOSTS: bool = true;
 
@@ -2007,7 +2007,7 @@ impl Step for BuildManifest {
         run.builder.ensure(BuildManifest { target: run.target });
     }
 
-    fn run(self, builder: &Builder<'_>) -> PathBuf {
+    fn run(self, builder: &Builder<'_>) -> GeneratedTarball {
         let build_manifest = builder.tool_exe(Tool::BuildManifest);
 
         let tarball = Tarball::new(builder, "build-manifest", &self.target.triple);
@@ -2027,7 +2027,7 @@ pub struct ReproducibleArtifacts {
 }
 
 impl Step for ReproducibleArtifacts {
-    type Output = Option<PathBuf>;
+    type Output = Option<GeneratedTarball>;
     const DEFAULT: bool = true;
     const ONLY_HOSTS: bool = true;
 
diff --git a/src/bootstrap/tarball.rs b/src/bootstrap/tarball.rs
index 688cc0fb8c95a..482ca40c5c1ff 100644
--- a/src/bootstrap/tarball.rs
+++ b/src/bootstrap/tarball.rs
@@ -200,7 +200,7 @@ impl<'a> Tarball<'a> {
         self.temp_dir.clone()
     }
 
-    pub(crate) fn generate(self) -> PathBuf {
+    pub(crate) fn generate(self) -> GeneratedTarball {
         let mut component_name = self.component.clone();
         if self.is_preview {
             component_name.push_str("-preview");
@@ -224,20 +224,20 @@ impl<'a> Tarball<'a> {
         })
     }
 
-    pub(crate) fn combine(self, tarballs: &[PathBuf]) {
-        let mut input_tarballs = tarballs[0].as_os_str().to_os_string();
+    pub(crate) fn combine(self, tarballs: &[GeneratedTarball]) -> GeneratedTarball {
+        let mut input_tarballs = tarballs[0].path.as_os_str().to_os_string();
         for tarball in &tarballs[1..] {
             input_tarballs.push(",");
-            input_tarballs.push(tarball);
+            input_tarballs.push(&tarball.path);
         }
 
         self.run(|this, cmd| {
             cmd.arg("combine").arg("--input-tarballs").arg(input_tarballs);
             this.non_bare_args(cmd);
-        });
+        })
     }
 
-    pub(crate) fn bare(self) -> PathBuf {
+    pub(crate) fn bare(self) -> GeneratedTarball {
         // Bare tarballs should have the top level directory match the package
         // name, not "image". We rename the image directory just before passing
         // into rust-installer.
@@ -273,7 +273,7 @@ impl<'a> Tarball<'a> {
             .arg(crate::dist::distdir(self.builder));
     }
 
-    fn run(self, build_cli: impl FnOnce(&Tarball<'a>, &mut Command)) -> PathBuf {
+    fn run(self, build_cli: impl FnOnce(&Tarball<'a>, &mut Command)) -> GeneratedTarball {
         t!(std::fs::create_dir_all(&self.overlay_dir));
         self.builder.create(&self.overlay_dir.join("version"), &self.overlay.version(self.builder));
         if let Some(sha) = self.builder.rust_sha() {
@@ -293,6 +293,19 @@ impl<'a> Tarball<'a> {
         cmd.arg("--work-dir").arg(&self.temp_dir);
         self.builder.run(&mut cmd);
 
-        crate::dist::distdir(self.builder).join(format!("{}.tar.gz", package_name))
+        GeneratedTarball {
+            path: crate::dist::distdir(self.builder).join(format!("{}.tar.gz", package_name)),
+        }
+    }
+}
+
+#[derive(Debug, Clone)]
+pub struct GeneratedTarball {
+    path: PathBuf,
+}
+
+impl GeneratedTarball {
+    pub(crate) fn tarball(&self) -> &Path {
+        &self.path
     }
 }
diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
index 1b9523426e735..ce4274fa22e0d 100644
--- a/src/bootstrap/test.rs
+++ b/src/bootstrap/test.rs
@@ -1969,7 +1969,7 @@ impl Step for Distcheck {
 
         let mut cmd = Command::new("tar");
         cmd.arg("-xf")
-            .arg(builder.ensure(dist::PlainSourceTarball))
+            .arg(builder.ensure(dist::PlainSourceTarball).tarball())
             .arg("--strip-components=1")
             .current_dir(&dir);
         builder.run(&mut cmd);
@@ -1992,7 +1992,10 @@ impl Step for Distcheck {
         t!(fs::create_dir_all(&dir));
 
         let mut cmd = Command::new("tar");
-        cmd.arg("-xf").arg(builder.ensure(dist::Src)).arg("--strip-components=1").current_dir(&dir);
+        cmd.arg("-xf")
+            .arg(builder.ensure(dist::Src).tarball())
+            .arg("--strip-components=1")
+            .current_dir(&dir);
         builder.run(&mut cmd);
 
         let toml = dir.join("rust-src/lib/rustlib/src/rust/library/std/Cargo.toml");

From 13cd7680eccb3fb485e1018b9cb10b60866819a1 Mon Sep 17 00:00:00 2001
From: Pietro Albini <pietro@pietroalbini.org>
Date: Wed, 30 Dec 2020 14:00:24 +0100
Subject: [PATCH 08/24] bootstrap: use the correct paths during ./x.py install

---
 src/bootstrap/dist.rs    |   4 +-
 src/bootstrap/install.rs | 106 ++++++++++++---------------------------
 src/bootstrap/tarball.rs |  16 ++++--
 3 files changed, 45 insertions(+), 81 deletions(-)

diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs
index 947ac6f1ee0eb..871a175091319 100644
--- a/src/bootstrap/dist.rs
+++ b/src/bootstrap/dist.rs
@@ -1323,10 +1323,10 @@ impl Step for Extended {
         }
 
         let tarball = Tarball::new(builder, "rust", &target.triple);
-        let work = tarball.work_dir();
-        tarball.combine(&tarballs);
+        let generated = tarball.combine(&tarballs);
 
         let tmp = tmpdir(builder).join("combined-tarball");
+        let work = generated.work_dir();
 
         let mut license = String::new();
         license += &builder.read(&builder.src.join("COPYRIGHT"));
diff --git a/src/bootstrap/install.rs b/src/bootstrap/install.rs
index 8f2b128b3688e..96164947943ba 100644
--- a/src/bootstrap/install.rs
+++ b/src/bootstrap/install.rs
@@ -10,60 +10,19 @@ use std::process::Command;
 
 use build_helper::t;
 
-use crate::dist::{self, pkgname, sanitize_sh, tmpdir};
+use crate::dist::{self, sanitize_sh};
+use crate::tarball::GeneratedTarball;
 use crate::Compiler;
 
 use crate::builder::{Builder, RunConfig, ShouldRun, Step};
 use crate::config::{Config, TargetSelection};
 
-pub fn install_docs(builder: &Builder<'_>, stage: u32, host: TargetSelection) {
-    install_sh(builder, "docs", "rust-docs", stage, Some(host));
-}
-
-pub fn install_std(builder: &Builder<'_>, stage: u32, target: TargetSelection) {
-    install_sh(builder, "std", "rust-std", stage, Some(target));
-}
-
-pub fn install_cargo(builder: &Builder<'_>, stage: u32, host: TargetSelection) {
-    install_sh(builder, "cargo", "cargo", stage, Some(host));
-}
-
-pub fn install_rls(builder: &Builder<'_>, stage: u32, host: TargetSelection) {
-    install_sh(builder, "rls", "rls", stage, Some(host));
-}
-
-pub fn install_rust_analyzer(builder: &Builder<'_>, stage: u32, host: TargetSelection) {
-    install_sh(builder, "rust-analyzer", "rust-analyzer", stage, Some(host));
-}
-
-pub fn install_clippy(builder: &Builder<'_>, stage: u32, host: TargetSelection) {
-    install_sh(builder, "clippy", "clippy", stage, Some(host));
-}
-pub fn install_miri(builder: &Builder<'_>, stage: u32, host: TargetSelection) {
-    install_sh(builder, "miri", "miri", stage, Some(host));
-}
-
-pub fn install_rustfmt(builder: &Builder<'_>, stage: u32, host: TargetSelection) {
-    install_sh(builder, "rustfmt", "rustfmt", stage, Some(host));
-}
-
-pub fn install_analysis(builder: &Builder<'_>, stage: u32, host: TargetSelection) {
-    install_sh(builder, "analysis", "rust-analysis", stage, Some(host));
-}
-
-pub fn install_src(builder: &Builder<'_>, stage: u32) {
-    install_sh(builder, "src", "rust-src", stage, None);
-}
-pub fn install_rustc(builder: &Builder<'_>, stage: u32, host: TargetSelection) {
-    install_sh(builder, "rustc", "rustc", stage, Some(host));
-}
-
 fn install_sh(
     builder: &Builder<'_>,
     package: &str,
-    name: &str,
     stage: u32,
     host: Option<TargetSelection>,
+    tarball: &GeneratedTarball,
 ) {
     builder.info(&format!("Install {} stage{} ({:?})", package, stage, host));
 
@@ -108,15 +67,10 @@ fn install_sh(
     let empty_dir = builder.out.join("tmp/empty_dir");
 
     t!(fs::create_dir_all(&empty_dir));
-    let package_name = if let Some(host) = host {
-        format!("{}-{}", pkgname(builder, name), host.triple)
-    } else {
-        pkgname(builder, name)
-    };
 
     let mut cmd = Command::new("sh");
     cmd.current_dir(&empty_dir)
-        .arg(sanitize_sh(&tmpdir(builder).join(&package_name).join("install.sh")))
+        .arg(sanitize_sh(&tarball.decompressed_output().join("install.sh")))
         .arg(format!("--prefix={}", sanitize_sh(&prefix)))
         .arg(format!("--sysconfdir={}", sanitize_sh(&sysconfdir)))
         .arg(format!("--datadir={}", sanitize_sh(&datadir)))
@@ -191,25 +145,25 @@ macro_rules! install {
 
 install!((self, builder, _config),
     Docs, "src/doc", _config.docs, only_hosts: false, {
-        builder.ensure(dist::Docs { host: self.target });
-        install_docs(builder, self.compiler.stage, self.target);
+        let tarball = builder.ensure(dist::Docs { host: self.target }).expect("missing docs");
+        install_sh(builder, "docs", self.compiler.stage, Some(self.target), &tarball);
     };
     Std, "library/std", true, only_hosts: false, {
         for target in &builder.targets {
-            builder.ensure(dist::Std {
+            let tarball = builder.ensure(dist::Std {
                 compiler: self.compiler,
                 target: *target
-            });
-            install_std(builder, self.compiler.stage, *target);
+            }).expect("missing std");
+            install_sh(builder, "std", self.compiler.stage, Some(*target), &tarball);
         }
     };
     Cargo, "cargo", Self::should_build(_config), only_hosts: true, {
-        builder.ensure(dist::Cargo { compiler: self.compiler, target: self.target });
-        install_cargo(builder, self.compiler.stage, self.target);
+        let tarball = builder.ensure(dist::Cargo { compiler: self.compiler, target: self.target });
+        install_sh(builder, "cargo", self.compiler.stage, Some(self.target), &tarball);
     };
     Rls, "rls", Self::should_build(_config), only_hosts: true, {
-        if builder.ensure(dist::Rls { compiler: self.compiler, target: self.target }).is_some() {
-            install_rls(builder, self.compiler.stage, self.target);
+        if let Some(tarball) = builder.ensure(dist::Rls { compiler: self.compiler, target: self.target }) {
+            install_sh(builder, "rls", self.compiler.stage, Some(self.target), &tarball);
         } else {
             builder.info(
                 &format!("skipping Install RLS stage{} ({})", self.compiler.stage, self.target),
@@ -217,16 +171,18 @@ install!((self, builder, _config),
         }
     };
     RustAnalyzer, "rust-analyzer", Self::should_build(_config), only_hosts: true, {
-        builder.ensure(dist::RustAnalyzer { compiler: self.compiler, target: self.target });
-        install_rust_analyzer(builder, self.compiler.stage, self.target);
+        let tarball = builder
+            .ensure(dist::RustAnalyzer { compiler: self.compiler, target: self.target })
+            .expect("missing rust-analyzer");
+        install_sh(builder, "rust-analyzer", self.compiler.stage, Some(self.target), &tarball);
     };
     Clippy, "clippy", Self::should_build(_config), only_hosts: true, {
-        builder.ensure(dist::Clippy { compiler: self.compiler, target: self.target });
-        install_clippy(builder, self.compiler.stage, self.target);
+        let tarball = builder.ensure(dist::Clippy { compiler: self.compiler, target: self.target });
+        install_sh(builder, "clippy", self.compiler.stage, Some(self.target), &tarball);
     };
     Miri, "miri", Self::should_build(_config), only_hosts: true, {
-        if builder.ensure(dist::Miri { compiler: self.compiler, target: self.target }).is_some() {
-            install_miri(builder, self.compiler.stage, self.target);
+        if let Some(tarball) = builder.ensure(dist::Miri { compiler: self.compiler, target: self.target }) {
+            install_sh(builder, "miri", self.compiler.stage, Some(self.target), &tarball);
         } else {
             builder.info(
                 &format!("skipping Install miri stage{} ({})", self.compiler.stage, self.target),
@@ -234,11 +190,11 @@ install!((self, builder, _config),
         }
     };
     Rustfmt, "rustfmt", Self::should_build(_config), only_hosts: true, {
-        if builder.ensure(dist::Rustfmt {
+        if let Some(tarball) = builder.ensure(dist::Rustfmt {
             compiler: self.compiler,
             target: self.target
-        }).is_some() {
-            install_rustfmt(builder, self.compiler.stage, self.target);
+        }) {
+            install_sh(builder, "rustfmt", self.compiler.stage, Some(self.target), &tarball);
         } else {
             builder.info(
                 &format!("skipping Install Rustfmt stage{} ({})", self.compiler.stage, self.target),
@@ -246,20 +202,20 @@ install!((self, builder, _config),
         }
     };
     Analysis, "analysis", Self::should_build(_config), only_hosts: false, {
-        builder.ensure(dist::Analysis {
+        let tarball = builder.ensure(dist::Analysis {
             // Find the actual compiler (handling the full bootstrap option) which
             // produced the save-analysis data because that data isn't copied
             // through the sysroot uplifting.
             compiler: builder.compiler_for(builder.top_stage, builder.config.build, self.target),
             target: self.target
-        });
-        install_analysis(builder, self.compiler.stage, self.target);
+        }).expect("missing analysis");
+        install_sh(builder, "analysis", self.compiler.stage, Some(self.target), &tarball);
     };
     Rustc, "src/librustc", true, only_hosts: true, {
-        builder.ensure(dist::Rustc {
+        let tarball = builder.ensure(dist::Rustc {
             compiler: builder.compiler(builder.top_stage, self.target),
         });
-        install_rustc(builder, self.compiler.stage, self.target);
+        install_sh(builder, "rustc", self.compiler.stage, Some(self.target), &tarball);
     };
 );
 
@@ -284,7 +240,7 @@ impl Step for Src {
     }
 
     fn run(self, builder: &Builder<'_>) {
-        builder.ensure(dist::Src);
-        install_src(builder, self.stage);
+        let tarball = builder.ensure(dist::Src);
+        install_sh(builder, "src", self.stage, None, &tarball);
     }
 }
diff --git a/src/bootstrap/tarball.rs b/src/bootstrap/tarball.rs
index 482ca40c5c1ff..9cfe12359dc25 100644
--- a/src/bootstrap/tarball.rs
+++ b/src/bootstrap/tarball.rs
@@ -196,10 +196,6 @@ impl<'a> Tarball<'a> {
         self.builder.cp_r(src.as_ref(), &dest);
     }
 
-    pub(crate) fn work_dir(&self) -> PathBuf {
-        self.temp_dir.clone()
-    }
-
     pub(crate) fn generate(self) -> GeneratedTarball {
         let mut component_name = self.component.clone();
         if self.is_preview {
@@ -295,6 +291,8 @@ impl<'a> Tarball<'a> {
 
         GeneratedTarball {
             path: crate::dist::distdir(self.builder).join(format!("{}.tar.gz", package_name)),
+            decompressed_output: self.temp_dir.join(package_name),
+            work: self.temp_dir,
         }
     }
 }
@@ -302,10 +300,20 @@ impl<'a> Tarball<'a> {
 #[derive(Debug, Clone)]
 pub struct GeneratedTarball {
     path: PathBuf,
+    decompressed_output: PathBuf,
+    work: PathBuf,
 }
 
 impl GeneratedTarball {
     pub(crate) fn tarball(&self) -> &Path {
         &self.path
     }
+
+    pub(crate) fn decompressed_output(&self) -> &Path {
+        &self.decompressed_output
+    }
+
+    pub(crate) fn work_dir(&self) -> &Path {
+        &self.work
+    }
 }

From 4050e2796e1cf3ba53dfd2c2fd095d29e43652b6 Mon Sep 17 00:00:00 2001
From: Pietro Albini <pietro@pietroalbini.org>
Date: Thu, 7 Jan 2021 19:59:24 +0100
Subject: [PATCH 09/24] bootstrap: fix x.py install not working with relative
 prefix

---
 src/bootstrap/install.rs | 99 ++++++++++++++++++----------------------
 1 file changed, 44 insertions(+), 55 deletions(-)

diff --git a/src/bootstrap/install.rs b/src/bootstrap/install.rs
index 96164947943ba..fd0acc3a919b0 100644
--- a/src/bootstrap/install.rs
+++ b/src/bootstrap/install.rs
@@ -5,7 +5,7 @@
 
 use std::env;
 use std::fs;
-use std::path::{Component, Path, PathBuf};
+use std::path::{Component, PathBuf};
 use std::process::Command;
 
 use build_helper::t;
@@ -26,74 +26,63 @@ fn install_sh(
 ) {
     builder.info(&format!("Install {} stage{} ({:?})", package, stage, host));
 
-    let prefix_default = PathBuf::from("/usr/local");
-    let sysconfdir_default = PathBuf::from("/etc");
-    let datadir_default = PathBuf::from("share");
-    let docdir_default = datadir_default.join("doc/rust");
-    let libdir_default = PathBuf::from("lib");
-    let mandir_default = datadir_default.join("man");
-    let prefix = builder.config.prefix.as_ref().unwrap_or(&prefix_default);
-    let sysconfdir = builder.config.sysconfdir.as_ref().unwrap_or(&sysconfdir_default);
-    let datadir = builder.config.datadir.as_ref().unwrap_or(&datadir_default);
-    let docdir = builder.config.docdir.as_ref().unwrap_or(&docdir_default);
-    let bindir = &builder.config.bindir;
-    let libdir = builder.config.libdir.as_ref().unwrap_or(&libdir_default);
-    let mandir = builder.config.mandir.as_ref().unwrap_or(&mandir_default);
-
-    let sysconfdir = prefix.join(sysconfdir);
-    let datadir = prefix.join(datadir);
-    let docdir = prefix.join(docdir);
-    let bindir = prefix.join(bindir);
-    let libdir = prefix.join(libdir);
-    let mandir = prefix.join(mandir);
-
-    let destdir = env::var_os("DESTDIR").map(PathBuf::from);
-
-    let prefix = add_destdir(&prefix, &destdir);
-    let sysconfdir = add_destdir(&sysconfdir, &destdir);
-    let datadir = add_destdir(&datadir, &destdir);
-    let docdir = add_destdir(&docdir, &destdir);
-    let bindir = add_destdir(&bindir, &destdir);
-    let libdir = add_destdir(&libdir, &destdir);
-    let mandir = add_destdir(&mandir, &destdir);
-
-    let prefix = {
-        fs::create_dir_all(&prefix)
-            .unwrap_or_else(|err| panic!("could not create {}: {}", prefix.display(), err));
-        fs::canonicalize(&prefix)
-            .unwrap_or_else(|err| panic!("could not canonicalize {}: {}", prefix.display(), err))
-    };
+    let prefix = default_path(&builder.config.prefix, "/usr/local");
+    let sysconfdir = prefix.join(default_path(&builder.config.sysconfdir, "/etc"));
+    let datadir = prefix.join(default_path(&builder.config.datadir, "share"));
+    let docdir = prefix.join(default_path(&builder.config.docdir, "share/doc"));
+    let mandir = prefix.join(default_path(&builder.config.mandir, "share/man"));
+    let libdir = prefix.join(default_path(&builder.config.libdir, "lib"));
+    let bindir = prefix.join(&builder.config.bindir); // Default in config.rs
 
     let empty_dir = builder.out.join("tmp/empty_dir");
-
     t!(fs::create_dir_all(&empty_dir));
 
     let mut cmd = Command::new("sh");
     cmd.current_dir(&empty_dir)
         .arg(sanitize_sh(&tarball.decompressed_output().join("install.sh")))
-        .arg(format!("--prefix={}", sanitize_sh(&prefix)))
-        .arg(format!("--sysconfdir={}", sanitize_sh(&sysconfdir)))
-        .arg(format!("--datadir={}", sanitize_sh(&datadir)))
-        .arg(format!("--docdir={}", sanitize_sh(&docdir)))
-        .arg(format!("--bindir={}", sanitize_sh(&bindir)))
-        .arg(format!("--libdir={}", sanitize_sh(&libdir)))
-        .arg(format!("--mandir={}", sanitize_sh(&mandir)))
+        .arg(format!("--prefix={}", prepare_dir(prefix)))
+        .arg(format!("--sysconfdir={}", prepare_dir(sysconfdir)))
+        .arg(format!("--datadir={}", prepare_dir(datadir)))
+        .arg(format!("--docdir={}", prepare_dir(docdir)))
+        .arg(format!("--bindir={}", prepare_dir(bindir)))
+        .arg(format!("--libdir={}", prepare_dir(libdir)))
+        .arg(format!("--mandir={}", prepare_dir(mandir)))
         .arg("--disable-ldconfig");
     builder.run(&mut cmd);
     t!(fs::remove_dir_all(&empty_dir));
 }
 
-fn add_destdir(path: &Path, destdir: &Option<PathBuf>) -> PathBuf {
-    let mut ret = match *destdir {
-        Some(ref dest) => dest.clone(),
-        None => return path.to_path_buf(),
-    };
-    for part in path.components() {
-        if let Component::Normal(s) = part {
-            ret.push(s)
+fn default_path(config: &Option<PathBuf>, default: &str) -> PathBuf {
+    PathBuf::from(config.as_ref().cloned().unwrap_or_else(|| PathBuf::from(default)))
+}
+
+fn prepare_dir(mut path: PathBuf) -> String {
+    // The DESTDIR environment variable is a standard way to install software in a subdirectory
+    // while keeping the original directory structure, even if the prefix or other directories
+    // contain absolute paths.
+    //
+    // More information on the environment variable is available here:
+    // https://www.gnu.org/prep/standards/html_node/DESTDIR.html
+    if let Some(destdir) = env::var_os("DESTDIR").map(PathBuf::from) {
+        let without_destdir = path.clone();
+        path = destdir;
+        // Custom .join() which ignores disk roots.
+        for part in without_destdir.components() {
+            if let Component::Normal(s) = part {
+                path.push(s)
+            }
         }
     }
-    ret
+
+    // The installation command is not executed from the current directory, but from a temporary
+    // directory. To prevent relative paths from breaking this converts relative paths to absolute
+    // paths. std::fs::canonicalize is not used as that requires the path to actually be present.
+    if path.is_relative() {
+        path = std::env::current_dir().expect("failed to get the current directory").join(path);
+        assert!(path.is_absolute(), "could not make the path relative");
+    }
+
+    sanitize_sh(&path)
 }
 
 macro_rules! install {

From 96a00a96bdbac954ea3bd39cbade4b02b30c1986 Mon Sep 17 00:00:00 2001
From: Eric Huss <eric@huss.org>
Date: Mon, 4 Jan 2021 07:18:37 -0800
Subject: [PATCH 10/24] Update mdbook

---
 Cargo.lock                    | 4 ++--
 src/tools/rustbook/Cargo.toml | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 4676e4127e83c..89ac858a61bf6 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1973,9 +1973,9 @@ dependencies = [
 
 [[package]]
 name = "mdbook"
-version = "0.4.3"
+version = "0.4.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "29be448fcafb00c5a8966c4020c2a5ffbbc333e5b96d0bb5ef54b5bd0524d9ff"
+checksum = "21251d3eb9ca5e8ac5b73384ddaa483a9bbc7d7dcd656b1fa8f266634810334a"
 dependencies = [
  "ammonia",
  "anyhow",
diff --git a/src/tools/rustbook/Cargo.toml b/src/tools/rustbook/Cargo.toml
index 99310157a64fe..8e17f2919082b 100644
--- a/src/tools/rustbook/Cargo.toml
+++ b/src/tools/rustbook/Cargo.toml
@@ -10,6 +10,6 @@ clap = "2.25.0"
 env_logger = "0.7.1"
 
 [dependencies.mdbook]
-version = "0.4.3"
+version = "0.4.5"
 default-features = false
 features = ["search"]

From 5a4852a113b234cb682ade697745320a77288363 Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Fri, 25 Dec 2020 11:48:12 -0800
Subject: [PATCH 11/24] rustdoc: Render visibilities succinctly

---
 src/librustdoc/clean/mod.rs       |  4 +--
 src/librustdoc/html/format.rs     | 13 ++++++++--
 src/librustdoc/html/render/mod.rs | 43 ++++++++++++++++++-------------
 3 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index dd96178cdb700..24c689012b031 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -2315,14 +2315,14 @@ impl Clean<Item> for (&hir::MacroDef<'_>, Option<Symbol>) {
             if matchers.len() <= 1 {
                 format!(
                     "{}macro {}{} {{\n    ...\n}}",
-                    vis.print_with_space(cx.tcx),
+                    vis.print_with_space(cx.tcx, item.hir_id.owner),
                     name,
                     matchers.iter().map(|span| span.to_src(cx)).collect::<String>(),
                 )
             } else {
                 format!(
                     "{}macro {} {{\n{}}}",
-                    vis.print_with_space(cx.tcx),
+                    vis.print_with_space(cx.tcx, item.hir_id.owner),
                     name,
                     matchers
                         .iter()
diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs
index 7b0b219570b98..bd5f5a3c6cc88 100644
--- a/src/librustdoc/html/format.rs
+++ b/src/librustdoc/html/format.rs
@@ -12,7 +12,7 @@ use std::fmt;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
 use rustc_middle::ty::TyCtxt;
-use rustc_span::def_id::{DefId, CRATE_DEF_INDEX};
+use rustc_span::def_id::{DefId, LocalDefId, CRATE_DEF_INDEX};
 use rustc_target::spec::abi::Abi;
 
 use crate::clean::{self, PrimitiveType};
@@ -1085,12 +1085,21 @@ impl Function<'_> {
 }
 
 impl clean::Visibility {
-    crate fn print_with_space<'tcx>(self, tcx: TyCtxt<'tcx>) -> impl fmt::Display + 'tcx {
+    crate fn print_with_space<'tcx>(
+        self,
+        tcx: TyCtxt<'tcx>,
+        item_did: LocalDefId,
+    ) -> impl fmt::Display + 'tcx {
         use rustc_span::symbol::kw;
 
         display_fn(move |f| match self {
             clean::Public => f.write_str("pub "),
             clean::Inherited => Ok(()),
+            clean::Visibility::Restricted(did)
+                if did.index == tcx.parent_module_from_def_id(item_did).local_def_index =>
+            {
+                Ok(())
+            }
             clean::Visibility::Restricted(did) if did.index == CRATE_DEF_INDEX => {
                 write!(f, "pub(crate) ")
             }
diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs
index 185df60735fd4..0ae32f4d27d20 100644
--- a/src/librustdoc/html/render/mod.rs
+++ b/src/librustdoc/html/render/mod.rs
@@ -2157,14 +2157,14 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl
                     Some(ref src) => write!(
                         w,
                         "<tr><td><code>{}extern crate {} as {};",
-                        myitem.visibility.print_with_space(cx.tcx()),
+                        myitem.visibility.print_with_space(cx.tcx(), myitem.def_id.expect_local()),
                         anchor(myitem.def_id, &*src.as_str()),
                         name
                     ),
                     None => write!(
                         w,
                         "<tr><td><code>{}extern crate {};",
-                        myitem.visibility.print_with_space(cx.tcx()),
+                        myitem.visibility.print_with_space(cx.tcx(), myitem.def_id.expect_local()),
                         anchor(myitem.def_id, &*name.as_str())
                     ),
                 }
@@ -2175,7 +2175,7 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl
                 write!(
                     w,
                     "<tr><td><code>{}{}</code></td></tr>",
-                    myitem.visibility.print_with_space(cx.tcx()),
+                    myitem.visibility.print_with_space(cx.tcx(), myitem.def_id.expect_local()),
                     import.print()
                 );
             }
@@ -2392,7 +2392,7 @@ fn item_constant(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, c: &clean::
     write!(
         w,
         "{vis}const {name}: {typ}",
-        vis = it.visibility.print_with_space(cx.tcx()),
+        vis = it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
         name = it.name.as_ref().unwrap(),
         typ = c.type_.print(),
     );
@@ -2426,7 +2426,7 @@ fn item_static(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::St
     write!(
         w,
         "{vis}static {mutability}{name}: {typ}</pre>",
-        vis = it.visibility.print_with_space(cx.tcx()),
+        vis = it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
         mutability = s.mutability.print_with_space(),
         name = it.name.as_ref().unwrap(),
         typ = s.type_.print()
@@ -2437,7 +2437,7 @@ fn item_static(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::St
 fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean::Function) {
     let header_len = format!(
         "{}{}{}{}{:#}fn {}{:#}",
-        it.visibility.print_with_space(cx.tcx()),
+        it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
         f.header.constness.print_with_space(),
         f.header.asyncness.print_with_space(),
         f.header.unsafety.print_with_space(),
@@ -2452,7 +2452,7 @@ fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean::
         w,
         "{vis}{constness}{asyncness}{unsafety}{abi}fn \
          {name}{generics}{decl}{spotlight}{where_clause}</pre>",
-        vis = it.visibility.print_with_space(cx.tcx()),
+        vis = it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
         constness = f.header.constness.print_with_space(),
         asyncness = f.header.asyncness.print_with_space(),
         unsafety = f.header.unsafety.print_with_space(),
@@ -2578,7 +2578,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
         write!(
             w,
             "{}{}{}trait {}{}{}",
-            it.visibility.print_with_space(cx.tcx()),
+            it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
             t.unsafety.print_with_space(),
             if t.is_auto { "auto " } else { "" },
             it.name.as_ref().unwrap(),
@@ -2896,7 +2896,7 @@ fn assoc_const(
         w,
         "{}{}const <a href=\"{}\" class=\"constant\"><b>{}</b></a>: {}",
         extra,
-        it.visibility.print_with_space(cx.tcx()),
+        it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
         naive_assoc_href(it, link),
         it.name.as_ref().unwrap(),
         ty.print()
@@ -3015,7 +3015,7 @@ fn render_assoc_item(
         };
         let mut header_len = format!(
             "{}{}{}{}{}{:#}fn {}{:#}",
-            meth.visibility.print_with_space(cx.tcx()),
+            meth.visibility.print_with_space(cx.tcx(), meth.def_id.expect_local()),
             header.constness.print_with_space(),
             header.asyncness.print_with_space(),
             header.unsafety.print_with_space(),
@@ -3037,7 +3037,7 @@ fn render_assoc_item(
             "{}{}{}{}{}{}{}fn <a href=\"{href}\" class=\"fnname\">{name}</a>\
              {generics}{decl}{spotlight}{where_clause}",
             if parent == ItemType::Trait { "    " } else { "" },
-            meth.visibility.print_with_space(cx.tcx()),
+            meth.visibility.print_with_space(cx.tcx(), meth.def_id.expect_local()),
             header.constness.print_with_space(),
             header.asyncness.print_with_space(),
             header.unsafety.print_with_space(),
@@ -3189,7 +3189,7 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum
         write!(
             w,
             "{}enum {}{}{}",
-            it.visibility.print_with_space(cx.tcx()),
+            it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
             it.name.as_ref().unwrap(),
             e.generics.print(),
             WhereClause { gens: &e.generics, indent: 0, end_newline: true }
@@ -3364,7 +3364,7 @@ fn render_struct(
     write!(
         w,
         "{}{}{}",
-        it.visibility.print_with_space(cx.tcx()),
+        it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
         if structhead { "struct " } else { "" },
         it.name.as_ref().unwrap()
     );
@@ -3384,7 +3384,7 @@ fn render_struct(
                         w,
                         "\n{}    {}{}: {},",
                         tab,
-                        field.visibility.print_with_space(cx.tcx()),
+                        field.visibility.print_with_space(cx.tcx(), field.def_id.expect_local()),
                         field.name.as_ref().unwrap(),
                         ty.print()
                     );
@@ -3413,7 +3413,14 @@ fn render_struct(
                 match field.kind {
                     clean::StrippedItem(box clean::StructFieldItem(..)) => write!(w, "_"),
                     clean::StructFieldItem(ref ty) => {
-                        write!(w, "{}{}", field.visibility.print_with_space(cx.tcx()), ty.print())
+                        write!(
+                            w,
+                            "{}{}",
+                            field
+                                .visibility
+                                .print_with_space(cx.tcx(), field.def_id.expect_local()),
+                            ty.print()
+                        )
                     }
                     _ => unreachable!(),
                 }
@@ -3446,7 +3453,7 @@ fn render_union(
     write!(
         w,
         "{}{}{}",
-        it.visibility.print_with_space(cx.tcx()),
+        it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
         if structhead { "union " } else { "" },
         it.name.as_ref().unwrap()
     );
@@ -3461,7 +3468,7 @@ fn render_union(
             write!(
                 w,
                 "    {}{}: {},\n{}",
-                field.visibility.print_with_space(cx.tcx()),
+                field.visibility.print_with_space(cx.tcx(), field.def_id.expect_local()),
                 field.name.as_ref().unwrap(),
                 ty.print(),
                 tab
@@ -4100,7 +4107,7 @@ fn item_foreign_type(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, cache:
     write!(
         w,
         "    {}type {};\n}}</pre>",
-        it.visibility.print_with_space(cx.tcx()),
+        it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
         it.name.as_ref().unwrap(),
     );
 

From 9c4494c60d247c12a064ae55775317f702b6a72c Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Fri, 25 Dec 2020 15:38:46 -0800
Subject: [PATCH 12/24] Fix bugs; fix and add tests

---
 src/librustdoc/clean/mod.rs                   | 10 +++-
 src/librustdoc/clean/utils.rs                 | 24 +++++++-
 src/librustdoc/html/format.rs                 | 56 ++++++++++---------
 src/librustdoc/html/render/mod.rs             | 38 ++++++-------
 .../passes/collect_intra_doc_links.rs         | 22 +-------
 src/test/rustdoc/decl_macro_priv.rs           |  2 +-
 src/test/rustdoc/pub-restricted.rs            | 32 +++++------
 src/test/rustdoc/visibility.rs                | 13 +++++
 8 files changed, 110 insertions(+), 87 deletions(-)
 create mode 100644 src/test/rustdoc/visibility.rs

diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index 24c689012b031..33a891da7464e 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -2315,14 +2315,20 @@ impl Clean<Item> for (&hir::MacroDef<'_>, Option<Symbol>) {
             if matchers.len() <= 1 {
                 format!(
                     "{}macro {}{} {{\n    ...\n}}",
-                    vis.print_with_space(cx.tcx, item.hir_id.owner),
+                    vis.print_with_space(
+                        cx.tcx,
+                        cx.tcx.hir().local_def_id(item.hir_id).to_def_id()
+                    ),
                     name,
                     matchers.iter().map(|span| span.to_src(cx)).collect::<String>(),
                 )
             } else {
                 format!(
                     "{}macro {} {{\n{}}}",
-                    vis.print_with_space(cx.tcx, item.hir_id.owner),
+                    vis.print_with_space(
+                        cx.tcx,
+                        cx.tcx.hir().local_def_id(item.hir_id).to_def_id()
+                    ),
                     name,
                     matchers
                         .iter()
diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs
index 270321aaa1188..fe1b58164474a 100644
--- a/src/librustdoc/clean/utils.rs
+++ b/src/librustdoc/clean/utils.rs
@@ -15,7 +15,7 @@ use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{DefId, LOCAL_CRATE};
 use rustc_middle::mir::interpret::ConstValue;
 use rustc_middle::ty::subst::{GenericArgKind, SubstsRef};
-use rustc_middle::ty::{self, DefIdTree, Ty};
+use rustc_middle::ty::{self, DefIdTree, Ty, TyCtxt};
 use rustc_span::symbol::{kw, sym, Symbol};
 use std::mem;
 
@@ -624,3 +624,25 @@ where
     *cx.impl_trait_bounds.borrow_mut() = old_bounds;
     r
 }
+
+crate fn find_closest_parent_module(tcx: TyCtxt<'_>, def_id: DefId) -> Option<DefId> {
+    let mut current = def_id;
+    // The immediate parent might not always be a module.
+    // Find the first parent which is.
+    loop {
+        if let Some(parent) = tcx.parent(current) {
+            if tcx.def_kind(parent) == DefKind::Mod {
+                break Some(parent);
+            }
+            current = parent;
+        } else {
+            debug!(
+                "{:?} has no parent (kind={:?}, original was {:?})",
+                current,
+                tcx.def_kind(current),
+                def_id
+            );
+            break None;
+        }
+    }
+}
diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs
index bd5f5a3c6cc88..6a611ccf58ec0 100644
--- a/src/librustdoc/html/format.rs
+++ b/src/librustdoc/html/format.rs
@@ -12,10 +12,10 @@ use std::fmt;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
 use rustc_middle::ty::TyCtxt;
-use rustc_span::def_id::{DefId, LocalDefId, CRATE_DEF_INDEX};
+use rustc_span::def_id::{DefId, CRATE_DEF_INDEX};
 use rustc_target::spec::abi::Abi;
 
-use crate::clean::{self, PrimitiveType};
+use crate::clean::{self, utils::find_closest_parent_module, PrimitiveType};
 use crate::formats::cache::cache;
 use crate::formats::item_type::ItemType;
 use crate::html::escape::Escape;
@@ -1088,38 +1088,40 @@ impl clean::Visibility {
     crate fn print_with_space<'tcx>(
         self,
         tcx: TyCtxt<'tcx>,
-        item_did: LocalDefId,
+        item_did: DefId,
     ) -> impl fmt::Display + 'tcx {
         use rustc_span::symbol::kw;
 
         display_fn(move |f| match self {
             clean::Public => f.write_str("pub "),
             clean::Inherited => Ok(()),
-            clean::Visibility::Restricted(did)
-                if did.index == tcx.parent_module_from_def_id(item_did).local_def_index =>
-            {
-                Ok(())
-            }
-            clean::Visibility::Restricted(did) if did.index == CRATE_DEF_INDEX => {
-                write!(f, "pub(crate) ")
-            }
-            clean::Visibility::Restricted(did) => {
-                f.write_str("pub(")?;
-                let path = tcx.def_path(did);
-                debug!("path={:?}", path);
-                let first_name =
-                    path.data[0].data.get_opt_name().expect("modules are always named");
-                if path.data.len() != 1 || (first_name != kw::SelfLower && first_name != kw::Super)
-                {
-                    f.write_str("in ")?;
-                }
-                // modified from `resolved_path()` to work with `DefPathData`
-                let last_name = path.data.last().unwrap().data.get_opt_name().unwrap();
-                for seg in &path.data[..path.data.len() - 1] {
-                    write!(f, "{}::", seg.data.get_opt_name().unwrap())?;
+
+            clean::Visibility::Restricted(vis_did) => {
+                if find_closest_parent_module(tcx, item_did) == Some(vis_did) {
+                    // `pub(in foo)` where `foo` is the parent module
+                    // is the same as no visibility modifier
+                    Ok(())
+                } else if vis_did.index == CRATE_DEF_INDEX {
+                    write!(f, "pub(crate) ")
+                } else {
+                    f.write_str("pub(")?;
+                    let path = tcx.def_path(vis_did);
+                    debug!("path={:?}", path);
+                    let first_name =
+                        path.data[0].data.get_opt_name().expect("modules are always named");
+                    if path.data.len() != 1
+                        || (first_name != kw::SelfLower && first_name != kw::Super)
+                    {
+                        f.write_str("in ")?;
+                    }
+                    // modified from `resolved_path()` to work with `DefPathData`
+                    let last_name = path.data.last().unwrap().data.get_opt_name().unwrap();
+                    for seg in &path.data[..path.data.len() - 1] {
+                        write!(f, "{}::", seg.data.get_opt_name().unwrap())?;
+                    }
+                    let path = anchor(vis_did, &last_name.as_str()).to_string();
+                    write!(f, "{}) ", path)
                 }
-                let path = anchor(did, &last_name.as_str()).to_string();
-                write!(f, "{}) ", path)
             }
         })
     }
diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs
index 0ae32f4d27d20..be2275aa7a7a4 100644
--- a/src/librustdoc/html/render/mod.rs
+++ b/src/librustdoc/html/render/mod.rs
@@ -2157,14 +2157,14 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl
                     Some(ref src) => write!(
                         w,
                         "<tr><td><code>{}extern crate {} as {};",
-                        myitem.visibility.print_with_space(cx.tcx(), myitem.def_id.expect_local()),
+                        myitem.visibility.print_with_space(cx.tcx(), myitem.def_id),
                         anchor(myitem.def_id, &*src.as_str()),
                         name
                     ),
                     None => write!(
                         w,
                         "<tr><td><code>{}extern crate {};",
-                        myitem.visibility.print_with_space(cx.tcx(), myitem.def_id.expect_local()),
+                        myitem.visibility.print_with_space(cx.tcx(), myitem.def_id),
                         anchor(myitem.def_id, &*name.as_str())
                     ),
                 }
@@ -2175,7 +2175,7 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl
                 write!(
                     w,
                     "<tr><td><code>{}{}</code></td></tr>",
-                    myitem.visibility.print_with_space(cx.tcx(), myitem.def_id.expect_local()),
+                    myitem.visibility.print_with_space(cx.tcx(), myitem.def_id),
                     import.print()
                 );
             }
@@ -2392,7 +2392,7 @@ fn item_constant(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, c: &clean::
     write!(
         w,
         "{vis}const {name}: {typ}",
-        vis = it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
+        vis = it.visibility.print_with_space(cx.tcx(), it.def_id),
         name = it.name.as_ref().unwrap(),
         typ = c.type_.print(),
     );
@@ -2426,7 +2426,7 @@ fn item_static(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::St
     write!(
         w,
         "{vis}static {mutability}{name}: {typ}</pre>",
-        vis = it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
+        vis = it.visibility.print_with_space(cx.tcx(), it.def_id),
         mutability = s.mutability.print_with_space(),
         name = it.name.as_ref().unwrap(),
         typ = s.type_.print()
@@ -2437,7 +2437,7 @@ fn item_static(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::St
 fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean::Function) {
     let header_len = format!(
         "{}{}{}{}{:#}fn {}{:#}",
-        it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
+        it.visibility.print_with_space(cx.tcx(), it.def_id),
         f.header.constness.print_with_space(),
         f.header.asyncness.print_with_space(),
         f.header.unsafety.print_with_space(),
@@ -2452,7 +2452,7 @@ fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean::
         w,
         "{vis}{constness}{asyncness}{unsafety}{abi}fn \
          {name}{generics}{decl}{spotlight}{where_clause}</pre>",
-        vis = it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
+        vis = it.visibility.print_with_space(cx.tcx(), it.def_id),
         constness = f.header.constness.print_with_space(),
         asyncness = f.header.asyncness.print_with_space(),
         unsafety = f.header.unsafety.print_with_space(),
@@ -2578,7 +2578,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
         write!(
             w,
             "{}{}{}trait {}{}{}",
-            it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
+            it.visibility.print_with_space(cx.tcx(), it.def_id),
             t.unsafety.print_with_space(),
             if t.is_auto { "auto " } else { "" },
             it.name.as_ref().unwrap(),
@@ -2896,7 +2896,7 @@ fn assoc_const(
         w,
         "{}{}const <a href=\"{}\" class=\"constant\"><b>{}</b></a>: {}",
         extra,
-        it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
+        it.visibility.print_with_space(cx.tcx(), it.def_id),
         naive_assoc_href(it, link),
         it.name.as_ref().unwrap(),
         ty.print()
@@ -3015,7 +3015,7 @@ fn render_assoc_item(
         };
         let mut header_len = format!(
             "{}{}{}{}{}{:#}fn {}{:#}",
-            meth.visibility.print_with_space(cx.tcx(), meth.def_id.expect_local()),
+            meth.visibility.print_with_space(cx.tcx(), meth.def_id),
             header.constness.print_with_space(),
             header.asyncness.print_with_space(),
             header.unsafety.print_with_space(),
@@ -3037,7 +3037,7 @@ fn render_assoc_item(
             "{}{}{}{}{}{}{}fn <a href=\"{href}\" class=\"fnname\">{name}</a>\
              {generics}{decl}{spotlight}{where_clause}",
             if parent == ItemType::Trait { "    " } else { "" },
-            meth.visibility.print_with_space(cx.tcx(), meth.def_id.expect_local()),
+            meth.visibility.print_with_space(cx.tcx(), meth.def_id),
             header.constness.print_with_space(),
             header.asyncness.print_with_space(),
             header.unsafety.print_with_space(),
@@ -3189,7 +3189,7 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum
         write!(
             w,
             "{}enum {}{}{}",
-            it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
+            it.visibility.print_with_space(cx.tcx(), it.def_id),
             it.name.as_ref().unwrap(),
             e.generics.print(),
             WhereClause { gens: &e.generics, indent: 0, end_newline: true }
@@ -3364,7 +3364,7 @@ fn render_struct(
     write!(
         w,
         "{}{}{}",
-        it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
+        it.visibility.print_with_space(cx.tcx(), it.def_id),
         if structhead { "struct " } else { "" },
         it.name.as_ref().unwrap()
     );
@@ -3384,7 +3384,7 @@ fn render_struct(
                         w,
                         "\n{}    {}{}: {},",
                         tab,
-                        field.visibility.print_with_space(cx.tcx(), field.def_id.expect_local()),
+                        field.visibility.print_with_space(cx.tcx(), field.def_id),
                         field.name.as_ref().unwrap(),
                         ty.print()
                     );
@@ -3416,9 +3416,7 @@ fn render_struct(
                         write!(
                             w,
                             "{}{}",
-                            field
-                                .visibility
-                                .print_with_space(cx.tcx(), field.def_id.expect_local()),
+                            field.visibility.print_with_space(cx.tcx(), field.def_id),
                             ty.print()
                         )
                     }
@@ -3453,7 +3451,7 @@ fn render_union(
     write!(
         w,
         "{}{}{}",
-        it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
+        it.visibility.print_with_space(cx.tcx(), it.def_id),
         if structhead { "union " } else { "" },
         it.name.as_ref().unwrap()
     );
@@ -3468,7 +3466,7 @@ fn render_union(
             write!(
                 w,
                 "    {}{}: {},\n{}",
-                field.visibility.print_with_space(cx.tcx(), field.def_id.expect_local()),
+                field.visibility.print_with_space(cx.tcx(), field.def_id),
                 field.name.as_ref().unwrap(),
                 ty.print(),
                 tab
@@ -4107,7 +4105,7 @@ fn item_foreign_type(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, cache:
     write!(
         w,
         "    {}type {};\n}}</pre>",
-        it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()),
+        it.visibility.print_with_space(cx.tcx(), it.def_id),
         it.name.as_ref().unwrap(),
     );
 
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index a8adfe08b2561..f392f321fbfc3 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -31,7 +31,7 @@ use std::cell::Cell;
 use std::mem;
 use std::ops::Range;
 
-use crate::clean::{self, Crate, Item, ItemLink, PrimitiveType};
+use crate::clean::{self, utils::find_closest_parent_module, Crate, Item, ItemLink, PrimitiveType};
 use crate::core::DocContext;
 use crate::fold::DocFolder;
 use crate::html::markdown::markdown_links;
@@ -774,25 +774,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
         } else if item.def_id.is_top_level_module() {
             Some(item.def_id)
         } else {
-            let mut current = item.def_id;
-            // The immediate parent might not always be a module.
-            // Find the first parent which is.
-            loop {
-                if let Some(parent) = self.cx.tcx.parent(current) {
-                    if self.cx.tcx.def_kind(parent) == DefKind::Mod {
-                        break Some(parent);
-                    }
-                    current = parent;
-                } else {
-                    debug!(
-                        "{:?} has no parent (kind={:?}, original was {:?})",
-                        current,
-                        self.cx.tcx.def_kind(current),
-                        item.def_id
-                    );
-                    break None;
-                }
-            }
+            find_closest_parent_module(self.cx.tcx, item.def_id)
         };
 
         if parent_node.is_some() {
diff --git a/src/test/rustdoc/decl_macro_priv.rs b/src/test/rustdoc/decl_macro_priv.rs
index 4e1279e34d933..cb71bca9cf201 100644
--- a/src/test/rustdoc/decl_macro_priv.rs
+++ b/src/test/rustdoc/decl_macro_priv.rs
@@ -2,7 +2,7 @@
 
 #![feature(decl_macro)]
 
-// @has decl_macro_priv/macro.crate_macro.html //pre 'pub(crate) macro crate_macro() {'
+// @has decl_macro_priv/macro.crate_macro.html //pre 'macro crate_macro() {'
 // @has - //pre '...'
 // @has - //pre '}'
 pub(crate) macro crate_macro() {}
diff --git a/src/test/rustdoc/pub-restricted.rs b/src/test/rustdoc/pub-restricted.rs
index 6720d848ac3b5..9ff3cad070d82 100644
--- a/src/test/rustdoc/pub-restricted.rs
+++ b/src/test/rustdoc/pub-restricted.rs
@@ -6,27 +6,27 @@
 
 // @has 'foo/struct.FooPublic.html' '//pre' 'pub struct FooPublic'
 pub struct FooPublic;
-// @has 'foo/struct.FooJustCrate.html' '//pre' 'pub(crate) struct FooJustCrate'
+// @has 'foo/struct.FooJustCrate.html' '//pre' 'struct FooJustCrate'
 crate struct FooJustCrate;
-// @has 'foo/struct.FooPubCrate.html' '//pre' 'pub(crate) struct FooPubCrate'
+// @has 'foo/struct.FooPubCrate.html' '//pre' 'struct FooPubCrate'
 pub(crate) struct FooPubCrate;
-// @has 'foo/struct.FooSelf.html' '//pre' 'pub(crate) struct FooSelf'
+// @has 'foo/struct.FooSelf.html' '//pre' 'struct FooSelf'
 pub(self) struct FooSelf;
-// @has 'foo/struct.FooInSelf.html' '//pre' 'pub(crate) struct FooInSelf'
+// @has 'foo/struct.FooInSelf.html' '//pre' 'struct FooInSelf'
 pub(in self) struct FooInSelf;
 mod a {
-    // @has 'foo/a/struct.FooSuper.html' '//pre' 'pub(crate) struct FooSuper'
-    pub(super) struct FooSuper;
-    // @has 'foo/a/struct.FooInSuper.html' '//pre' 'pub(crate) struct FooInSuper'
-    pub(in super) struct FooInSuper;
-    // @has 'foo/a/struct.FooInA.html' '//pre' 'pub(in a) struct FooInA'
-    pub(in a) struct FooInA;
+    // @has 'foo/a/struct.FooASuper.html' '//pre' 'pub(crate) struct FooASuper'
+    pub(super) struct FooASuper;
+    // @has 'foo/a/struct.FooAInSuper.html' '//pre' 'pub(crate) struct FooAInSuper'
+    pub(in super) struct FooAInSuper;
+    // @has 'foo/a/struct.FooAInA.html' '//pre' 'struct FooAInA'
+    pub(in a) struct FooAInA;
     mod b {
-        // @has 'foo/a/b/struct.FooInSelfSuperB.html' '//pre' 'pub(in a::b) struct FooInSelfSuperB'
-        pub(in a::b) struct FooInSelfSuperB;
-        // @has 'foo/a/b/struct.FooInSuperSuper.html' '//pre' 'pub(crate) struct FooInSuperSuper'
-        pub(in super::super) struct FooInSuperSuper;
-        // @has 'foo/a/b/struct.FooInAB.html' '//pre' 'pub(in a::b) struct FooInAB'
-        pub(in a::b) struct FooInAB;
+        // @has 'foo/a/b/struct.FooBSuper.html' '//pre' 'pub(super) struct FooBSuper'
+        pub(super) struct FooBSuper;
+        // @has 'foo/a/b/struct.FooBInSuperSuper.html' '//pre' 'pub(crate) struct FooBInSuperSuper'
+        pub(in super::super) struct FooBInSuperSuper;
+        // @has 'foo/a/b/struct.FooBInAB.html' '//pre' 'struct FooBInAB'
+        pub(in a::b) struct FooBInAB;
     }
 }
diff --git a/src/test/rustdoc/visibility.rs b/src/test/rustdoc/visibility.rs
new file mode 100644
index 0000000000000..9dd0b68b1d95e
--- /dev/null
+++ b/src/test/rustdoc/visibility.rs
@@ -0,0 +1,13 @@
+// compile-flags: --document-private-items
+
+#![crate_name = "foo"]
+
+// @has 'foo/fn.foo.html' '//pre' 'fn foo'
+// !@has 'foo/fn.foo.html' '//pre' 'pub'
+fn foo() {}
+
+mod bar {
+    // @has 'foo/bar/fn.baz.html' '//pre' 'fn baz'
+    // !@has 'foo/bar/fn.baz.html' '//pre' 'pub'
+    fn baz() {}
+}

From 2ea0fa53a74050994a153f169ff31cdb36160676 Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Fri, 25 Dec 2020 15:54:04 -0800
Subject: [PATCH 13/24] Handle `pub(super)`

---
 src/librustdoc/html/format.rs | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs
index 6a611ccf58ec0..a8eae52fc565e 100644
--- a/src/librustdoc/html/format.rs
+++ b/src/librustdoc/html/format.rs
@@ -1097,12 +1097,20 @@ impl clean::Visibility {
             clean::Inherited => Ok(()),
 
             clean::Visibility::Restricted(vis_did) => {
-                if find_closest_parent_module(tcx, item_did) == Some(vis_did) {
+                let parent_module = find_closest_parent_module(tcx, item_did);
+
+                if parent_module == Some(vis_did) {
                     // `pub(in foo)` where `foo` is the parent module
                     // is the same as no visibility modifier
                     Ok(())
                 } else if vis_did.index == CRATE_DEF_INDEX {
                     write!(f, "pub(crate) ")
+                } else if parent_module
+                    .map(|parent| find_closest_parent_module(tcx, parent))
+                    .flatten()
+                    == Some(vis_did)
+                {
+                    write!(f, "pub(super) ")
                 } else {
                     f.write_str("pub(")?;
                     let path = tcx.def_path(vis_did);

From 76d26036ae72239f8a497503d03f072ad3ce911f Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Fri, 25 Dec 2020 16:16:40 -0800
Subject: [PATCH 14/24] Prefer `pub(crate)` over no modifier

---
 src/librustdoc/html/format.rs       | 6 +++---
 src/test/rustdoc/decl_macro_priv.rs | 2 +-
 src/test/rustdoc/pub-restricted.rs  | 8 ++++----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs
index a8eae52fc565e..90f4aaefc9bc1 100644
--- a/src/librustdoc/html/format.rs
+++ b/src/librustdoc/html/format.rs
@@ -1099,12 +1099,12 @@ impl clean::Visibility {
             clean::Visibility::Restricted(vis_did) => {
                 let parent_module = find_closest_parent_module(tcx, item_did);
 
-                if parent_module == Some(vis_did) {
+                if vis_did.index == CRATE_DEF_INDEX {
+                    write!(f, "pub(crate) ")
+                } else if parent_module == Some(vis_did) {
                     // `pub(in foo)` where `foo` is the parent module
                     // is the same as no visibility modifier
                     Ok(())
-                } else if vis_did.index == CRATE_DEF_INDEX {
-                    write!(f, "pub(crate) ")
                 } else if parent_module
                     .map(|parent| find_closest_parent_module(tcx, parent))
                     .flatten()
diff --git a/src/test/rustdoc/decl_macro_priv.rs b/src/test/rustdoc/decl_macro_priv.rs
index cb71bca9cf201..4e1279e34d933 100644
--- a/src/test/rustdoc/decl_macro_priv.rs
+++ b/src/test/rustdoc/decl_macro_priv.rs
@@ -2,7 +2,7 @@
 
 #![feature(decl_macro)]
 
-// @has decl_macro_priv/macro.crate_macro.html //pre 'macro crate_macro() {'
+// @has decl_macro_priv/macro.crate_macro.html //pre 'pub(crate) macro crate_macro() {'
 // @has - //pre '...'
 // @has - //pre '}'
 pub(crate) macro crate_macro() {}
diff --git a/src/test/rustdoc/pub-restricted.rs b/src/test/rustdoc/pub-restricted.rs
index 9ff3cad070d82..f828e642abdca 100644
--- a/src/test/rustdoc/pub-restricted.rs
+++ b/src/test/rustdoc/pub-restricted.rs
@@ -6,13 +6,13 @@
 
 // @has 'foo/struct.FooPublic.html' '//pre' 'pub struct FooPublic'
 pub struct FooPublic;
-// @has 'foo/struct.FooJustCrate.html' '//pre' 'struct FooJustCrate'
+// @has 'foo/struct.FooJustCrate.html' '//pre' 'pub(crate) struct FooJustCrate'
 crate struct FooJustCrate;
-// @has 'foo/struct.FooPubCrate.html' '//pre' 'struct FooPubCrate'
+// @has 'foo/struct.FooPubCrate.html' '//pre' 'pub(crate) struct FooPubCrate'
 pub(crate) struct FooPubCrate;
-// @has 'foo/struct.FooSelf.html' '//pre' 'struct FooSelf'
+// @has 'foo/struct.FooSelf.html' '//pre' 'pub(crate) struct FooSelf'
 pub(self) struct FooSelf;
-// @has 'foo/struct.FooInSelf.html' '//pre' 'struct FooInSelf'
+// @has 'foo/struct.FooInSelf.html' '//pre' 'pub(crate) struct FooInSelf'
 pub(in self) struct FooInSelf;
 mod a {
     // @has 'foo/a/struct.FooASuper.html' '//pre' 'pub(crate) struct FooASuper'

From c73576a22a0c9dc777709e9a753a5c2883339398 Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Fri, 25 Dec 2020 16:27:56 -0800
Subject: [PATCH 15/24] Merge `pub-restricted` and `visibility` test

---
 src/test/rustdoc/pub-restricted.rs | 32 -----------------------
 src/test/rustdoc/visibility.rs     | 41 +++++++++++++++++++++++++-----
 2 files changed, 34 insertions(+), 39 deletions(-)
 delete mode 100644 src/test/rustdoc/pub-restricted.rs

diff --git a/src/test/rustdoc/pub-restricted.rs b/src/test/rustdoc/pub-restricted.rs
deleted file mode 100644
index f828e642abdca..0000000000000
--- a/src/test/rustdoc/pub-restricted.rs
+++ /dev/null
@@ -1,32 +0,0 @@
-// compile-flags: --document-private-items
-
-#![feature(crate_visibility_modifier)]
-
-#![crate_name = "foo"]
-
-// @has 'foo/struct.FooPublic.html' '//pre' 'pub struct FooPublic'
-pub struct FooPublic;
-// @has 'foo/struct.FooJustCrate.html' '//pre' 'pub(crate) struct FooJustCrate'
-crate struct FooJustCrate;
-// @has 'foo/struct.FooPubCrate.html' '//pre' 'pub(crate) struct FooPubCrate'
-pub(crate) struct FooPubCrate;
-// @has 'foo/struct.FooSelf.html' '//pre' 'pub(crate) struct FooSelf'
-pub(self) struct FooSelf;
-// @has 'foo/struct.FooInSelf.html' '//pre' 'pub(crate) struct FooInSelf'
-pub(in self) struct FooInSelf;
-mod a {
-    // @has 'foo/a/struct.FooASuper.html' '//pre' 'pub(crate) struct FooASuper'
-    pub(super) struct FooASuper;
-    // @has 'foo/a/struct.FooAInSuper.html' '//pre' 'pub(crate) struct FooAInSuper'
-    pub(in super) struct FooAInSuper;
-    // @has 'foo/a/struct.FooAInA.html' '//pre' 'struct FooAInA'
-    pub(in a) struct FooAInA;
-    mod b {
-        // @has 'foo/a/b/struct.FooBSuper.html' '//pre' 'pub(super) struct FooBSuper'
-        pub(super) struct FooBSuper;
-        // @has 'foo/a/b/struct.FooBInSuperSuper.html' '//pre' 'pub(crate) struct FooBInSuperSuper'
-        pub(in super::super) struct FooBInSuperSuper;
-        // @has 'foo/a/b/struct.FooBInAB.html' '//pre' 'struct FooBInAB'
-        pub(in a::b) struct FooBInAB;
-    }
-}
diff --git a/src/test/rustdoc/visibility.rs b/src/test/rustdoc/visibility.rs
index 9dd0b68b1d95e..ebb314a7941b6 100644
--- a/src/test/rustdoc/visibility.rs
+++ b/src/test/rustdoc/visibility.rs
@@ -1,13 +1,40 @@
 // compile-flags: --document-private-items
 
+#![feature(crate_visibility_modifier)]
+
 #![crate_name = "foo"]
 
-// @has 'foo/fn.foo.html' '//pre' 'fn foo'
-// !@has 'foo/fn.foo.html' '//pre' 'pub'
-fn foo() {}
+// @has 'foo/struct.FooPublic.html' '//pre' 'pub struct FooPublic'
+pub struct FooPublic;
+// @has 'foo/struct.FooJustCrate.html' '//pre' 'pub(crate) struct FooJustCrate'
+crate struct FooJustCrate;
+// @has 'foo/struct.FooPubCrate.html' '//pre' 'pub(crate) struct FooPubCrate'
+pub(crate) struct FooPubCrate;
+// @has 'foo/struct.FooSelf.html' '//pre' 'pub(crate) struct FooSelf'
+pub(self) struct FooSelf;
+// @has 'foo/struct.FooInSelf.html' '//pre' 'pub(crate) struct FooInSelf'
+pub(in self) struct FooInSelf;
+// @has 'foo/struct.FooPriv.html' '//pre' 'pub(crate) struct FooPriv'
+struct FooPriv;
+
+mod a {
+    // @has 'foo/a/struct.FooASuper.html' '//pre' 'pub(crate) struct FooASuper'
+    pub(super) struct FooASuper;
+    // @has 'foo/a/struct.FooAInSuper.html' '//pre' 'pub(crate) struct FooAInSuper'
+    pub(in super) struct FooAInSuper;
+    // @has 'foo/a/struct.FooAInA.html' '//pre' 'struct FooAInA'
+    pub(in a) struct FooAInA;
+    // @has 'foo/a/struct.FooAPriv.html' '//pre' 'struct FooAPriv'
+    struct FooAPriv;
 
-mod bar {
-    // @has 'foo/bar/fn.baz.html' '//pre' 'fn baz'
-    // !@has 'foo/bar/fn.baz.html' '//pre' 'pub'
-    fn baz() {}
+    mod b {
+        // @has 'foo/a/b/struct.FooBSuper.html' '//pre' 'pub(super) struct FooBSuper'
+        pub(super) struct FooBSuper;
+        // @has 'foo/a/b/struct.FooBInSuperSuper.html' '//pre' 'pub(crate) struct FooBInSuperSuper'
+        pub(in super::super) struct FooBInSuperSuper;
+        // @has 'foo/a/b/struct.FooBInAB.html' '//pre' 'struct FooBInAB'
+        pub(in a::b) struct FooBInAB;
+        // @has 'foo/a/b/struct.FooBPriv.html' '//pre' 'struct FooBPriv'
+        struct FooBPriv;
+    }
 }

From c7a85ca05d0de86e48417209de46c3b2815a60ef Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Fri, 25 Dec 2020 16:33:15 -0800
Subject: [PATCH 16/24] Add missing code to `find_closest_parent_module`

---
 src/librustdoc/clean/utils.rs                 | 40 +++++++++++--------
 .../passes/collect_intra_doc_links.rs         | 10 +----
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs
index fe1b58164474a..4b9541b7e1421 100644
--- a/src/librustdoc/clean/utils.rs
+++ b/src/librustdoc/clean/utils.rs
@@ -626,23 +626,31 @@ where
 }
 
 crate fn find_closest_parent_module(tcx: TyCtxt<'_>, def_id: DefId) -> Option<DefId> {
-    let mut current = def_id;
-    // The immediate parent might not always be a module.
-    // Find the first parent which is.
-    loop {
-        if let Some(parent) = tcx.parent(current) {
-            if tcx.def_kind(parent) == DefKind::Mod {
-                break Some(parent);
+    if item.is_fake() {
+        // FIXME: is this correct?
+        None
+    // If we're documenting the crate root itself, it has no parent. Use the root instead.
+    } else if item.def_id.is_top_level_module() {
+        Some(item.def_id)
+    } else {
+        let mut current = def_id;
+        // The immediate parent might not always be a module.
+        // Find the first parent which is.
+        loop {
+            if let Some(parent) = tcx.parent(current) {
+                if tcx.def_kind(parent) == DefKind::Mod {
+                    break Some(parent);
+                }
+                current = parent;
+            } else {
+                debug!(
+                    "{:?} has no parent (kind={:?}, original was {:?})",
+                    current,
+                    tcx.def_kind(current),
+                    def_id
+                );
+                break None;
             }
-            current = parent;
-        } else {
-            debug!(
-                "{:?} has no parent (kind={:?}, original was {:?})",
-                current,
-                tcx.def_kind(current),
-                def_id
-            );
-            break None;
         }
     }
 }
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index f392f321fbfc3..4e261c3fd1934 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -767,15 +767,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
     fn fold_item(&mut self, mut item: Item) -> Option<Item> {
         use rustc_middle::ty::DefIdTree;
 
-        let parent_node = if item.is_fake() {
-            // FIXME: is this correct?
-            None
-        // If we're documenting the crate root itself, it has no parent. Use the root instead.
-        } else if item.def_id.is_top_level_module() {
-            Some(item.def_id)
-        } else {
-            find_closest_parent_module(self.cx.tcx, item.def_id)
-        };
+        let parent_node = find_closest_parent_module(self.cx.tcx, item.def_id);
 
         if parent_node.is_some() {
             trace!("got parent node for {:?} {:?}, id {:?}", item.type_(), item.name, item.def_id);

From 0c127b3d4341691681531831bbfe8b1fb59ec354 Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Fri, 25 Dec 2020 16:35:28 -0800
Subject: [PATCH 17/24] Simplify loop and remove old debugging code

Co-authored-by: Joshua Nelson <jyn514@gmail.com>
---
 src/librustdoc/clean/utils.rs | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs
index 4b9541b7e1421..b49ed07f8e83f 100644
--- a/src/librustdoc/clean/utils.rs
+++ b/src/librustdoc/clean/utils.rs
@@ -636,21 +636,12 @@ crate fn find_closest_parent_module(tcx: TyCtxt<'_>, def_id: DefId) -> Option<De
         let mut current = def_id;
         // The immediate parent might not always be a module.
         // Find the first parent which is.
-        loop {
-            if let Some(parent) = tcx.parent(current) {
-                if tcx.def_kind(parent) == DefKind::Mod {
-                    break Some(parent);
-                }
-                current = parent;
-            } else {
-                debug!(
-                    "{:?} has no parent (kind={:?}, original was {:?})",
-                    current,
-                    tcx.def_kind(current),
-                    def_id
-                );
-                break None;
+        while let Some(parent) = tcx.parent(current) {
+            if tcx.def_kind(parent) == DefKind::Mod {
+                return Some(parent);
             }
+            current = parent;
         }
+        None
     }
 }

From 69e17227539c73e733d140eed3e9d7ccde72117f Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Fri, 25 Dec 2020 16:37:09 -0800
Subject: [PATCH 18/24] Extract local variable

---
 src/librustdoc/clean/mod.rs | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index 33a891da7464e..e6ba3b54ab67d 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -2311,24 +2311,20 @@ impl Clean<Item> for (&hir::MacroDef<'_>, Option<Symbol>) {
             )
         } else {
             let vis = item.vis.clean(cx);
+            let vis_printed_with_space =
+                vis.print_with_space(cx.tcx, cx.tcx.hir().local_def_id(item.hir_id).to_def_id());
 
             if matchers.len() <= 1 {
                 format!(
                     "{}macro {}{} {{\n    ...\n}}",
-                    vis.print_with_space(
-                        cx.tcx,
-                        cx.tcx.hir().local_def_id(item.hir_id).to_def_id()
-                    ),
+                    vis_printed_with_space,
                     name,
                     matchers.iter().map(|span| span.to_src(cx)).collect::<String>(),
                 )
             } else {
                 format!(
                     "{}macro {} {{\n{}}}",
-                    vis.print_with_space(
-                        cx.tcx,
-                        cx.tcx.hir().local_def_id(item.hir_id).to_def_id()
-                    ),
+                    vis_printed_with_space,
                     name,
                     matchers
                         .iter()

From e9ae18deb7d0ea4b872d1c01cb855a8aa292dbed Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Wed, 30 Dec 2020 16:38:25 -0800
Subject: [PATCH 19/24] Update `find_nearest_parent_module`

---
 src/librustdoc/clean/utils.rs                    | 12 +++++-------
 src/librustdoc/html/format.rs                    |  6 +++---
 src/librustdoc/passes/collect_intra_doc_links.rs |  9 +++++++--
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs
index b49ed07f8e83f..4009a42955f87 100644
--- a/src/librustdoc/clean/utils.rs
+++ b/src/librustdoc/clean/utils.rs
@@ -625,13 +625,11 @@ where
     r
 }
 
-crate fn find_closest_parent_module(tcx: TyCtxt<'_>, def_id: DefId) -> Option<DefId> {
-    if item.is_fake() {
-        // FIXME: is this correct?
-        None
-    // If we're documenting the crate root itself, it has no parent. Use the root instead.
-    } else if item.def_id.is_top_level_module() {
-        Some(item.def_id)
+/// Find the nearest parent module of a [`DefId`].
+crate fn find_nearest_parent_module(tcx: TyCtxt<'_>, def_id: DefId) -> Option<DefId> {
+    if def_id.is_top_level_module() {
+        // The crate root has no parent. Use it as the root instead.
+        Some(def_id)
     } else {
         let mut current = def_id;
         // The immediate parent might not always be a module.
diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs
index 90f4aaefc9bc1..9fca005c34ee0 100644
--- a/src/librustdoc/html/format.rs
+++ b/src/librustdoc/html/format.rs
@@ -15,7 +15,7 @@ use rustc_middle::ty::TyCtxt;
 use rustc_span::def_id::{DefId, CRATE_DEF_INDEX};
 use rustc_target::spec::abi::Abi;
 
-use crate::clean::{self, utils::find_closest_parent_module, PrimitiveType};
+use crate::clean::{self, utils::find_nearest_parent_module, PrimitiveType};
 use crate::formats::cache::cache;
 use crate::formats::item_type::ItemType;
 use crate::html::escape::Escape;
@@ -1097,7 +1097,7 @@ impl clean::Visibility {
             clean::Inherited => Ok(()),
 
             clean::Visibility::Restricted(vis_did) => {
-                let parent_module = find_closest_parent_module(tcx, item_did);
+                let parent_module = find_nearest_parent_module(tcx, item_did);
 
                 if vis_did.index == CRATE_DEF_INDEX {
                     write!(f, "pub(crate) ")
@@ -1106,7 +1106,7 @@ impl clean::Visibility {
                     // is the same as no visibility modifier
                     Ok(())
                 } else if parent_module
-                    .map(|parent| find_closest_parent_module(tcx, parent))
+                    .map(|parent| find_nearest_parent_module(tcx, parent))
                     .flatten()
                     == Some(vis_did)
                 {
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index 4e261c3fd1934..e225eb47b12be 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -31,7 +31,7 @@ use std::cell::Cell;
 use std::mem;
 use std::ops::Range;
 
-use crate::clean::{self, utils::find_closest_parent_module, Crate, Item, ItemLink, PrimitiveType};
+use crate::clean::{self, utils::find_nearest_parent_module, Crate, Item, ItemLink, PrimitiveType};
 use crate::core::DocContext;
 use crate::fold::DocFolder;
 use crate::html::markdown::markdown_links;
@@ -767,7 +767,12 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
     fn fold_item(&mut self, mut item: Item) -> Option<Item> {
         use rustc_middle::ty::DefIdTree;
 
-        let parent_node = find_closest_parent_module(self.cx.tcx, item.def_id);
+        let parent_node = if item.is_fake() {
+            // FIXME: is this correct?
+            None
+        } else {
+            find_nearest_parent_module(self.cx.tcx, item.def_id)
+        };
 
         if parent_node.is_some() {
             trace!("got parent node for {:?} {:?}, id {:?}", item.type_(), item.name, item.def_id);

From 8f0ce5d9f8818bb26a7d9c2612fafa8c674353ea Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Wed, 30 Dec 2020 16:29:47 -0800
Subject: [PATCH 20/24] Remove FIXME

Co-authored-by: Joshua Nelson <jyn514@gmail.com>
---
 src/librustdoc/passes/collect_intra_doc_links.rs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index e225eb47b12be..2e116da6ff5dd 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -768,7 +768,6 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
         use rustc_middle::ty::DefIdTree;
 
         let parent_node = if item.is_fake() {
-            // FIXME: is this correct?
             None
         } else {
             find_nearest_parent_module(self.cx.tcx, item.def_id)

From dc7eb419b274297fa31cfeb2106f9a730045cc23 Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Wed, 30 Dec 2020 16:41:18 -0800
Subject: [PATCH 21/24] Small refactor

---
 src/librustdoc/clean/mod.rs | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index e6ba3b54ab67d..cd59e7d32dde8 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -2311,20 +2311,19 @@ impl Clean<Item> for (&hir::MacroDef<'_>, Option<Symbol>) {
             )
         } else {
             let vis = item.vis.clean(cx);
-            let vis_printed_with_space =
-                vis.print_with_space(cx.tcx, cx.tcx.hir().local_def_id(item.hir_id).to_def_id());
+            let def_id = cx.tcx.hir().local_def_id(item.hir_id).to_def_id();
 
             if matchers.len() <= 1 {
                 format!(
                     "{}macro {}{} {{\n    ...\n}}",
-                    vis_printed_with_space,
+                    vis.print_with_space(cx.tcx, def_id),
                     name,
                     matchers.iter().map(|span| span.to_src(cx)).collect::<String>(),
                 )
             } else {
                 format!(
                     "{}macro {} {{\n{}}}",
-                    vis_printed_with_space,
+                    vis.print_with_space(cx.tcx, def_id),
                     name,
                     matchers
                         .iter()

From 0a3048d95f42fa92d7e4eaa50a418c78cc55322c Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Wed, 30 Dec 2020 17:39:03 -0800
Subject: [PATCH 22/24] Add note on panic behavior

---
 src/librustdoc/clean/utils.rs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs
index 4009a42955f87..17f12d0a82f2e 100644
--- a/src/librustdoc/clean/utils.rs
+++ b/src/librustdoc/clean/utils.rs
@@ -626,6 +626,8 @@ where
 }
 
 /// Find the nearest parent module of a [`DefId`].
+///
+/// **Panics if the item it belongs to [is fake][Item::is_fake].**
 crate fn find_nearest_parent_module(tcx: TyCtxt<'_>, def_id: DefId) -> Option<DefId> {
     if def_id.is_top_level_module() {
         // The crate root has no parent. Use it as the root instead.

From 6ec4c71c5632e28ebce8d4f3122236a3e9ed040a Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Thu, 31 Dec 2020 12:00:23 -0800
Subject: [PATCH 23/24] Add FIXME for visibility of a module

---
 src/librustdoc/html/format.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs
index 9fca005c34ee0..9b2fb8582f54f 100644
--- a/src/librustdoc/html/format.rs
+++ b/src/librustdoc/html/format.rs
@@ -1097,6 +1097,9 @@ impl clean::Visibility {
             clean::Inherited => Ok(()),
 
             clean::Visibility::Restricted(vis_did) => {
+                // FIXME(camelid): This may not work correctly if `item_did` is a module.
+                //                 However, rustdoc currently never displays a module's
+                //                 visibility, so it shouldn't matter.
                 let parent_module = find_nearest_parent_module(tcx, item_did);
 
                 if vis_did.index == CRATE_DEF_INDEX {

From d239ea6a094925de15f3c0b98d835001870b1b11 Mon Sep 17 00:00:00 2001
From: Camelid <camelidcamel@gmail.com>
Date: Thu, 31 Dec 2020 12:04:13 -0800
Subject: [PATCH 24/24] Add `@!has` checks to ensure private items don't have
 `pub`

---
 src/test/rustdoc/visibility.rs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/test/rustdoc/visibility.rs b/src/test/rustdoc/visibility.rs
index ebb314a7941b6..59427693c5a5d 100644
--- a/src/test/rustdoc/visibility.rs
+++ b/src/test/rustdoc/visibility.rs
@@ -23,8 +23,10 @@ mod a {
     // @has 'foo/a/struct.FooAInSuper.html' '//pre' 'pub(crate) struct FooAInSuper'
     pub(in super) struct FooAInSuper;
     // @has 'foo/a/struct.FooAInA.html' '//pre' 'struct FooAInA'
+    // @!has 'foo/a/struct.FooAInA.html' '//pre' 'pub'
     pub(in a) struct FooAInA;
     // @has 'foo/a/struct.FooAPriv.html' '//pre' 'struct FooAPriv'
+    // @!has 'foo/a/struct.FooAPriv.html' '//pre' 'pub'
     struct FooAPriv;
 
     mod b {
@@ -33,8 +35,10 @@ mod a {
         // @has 'foo/a/b/struct.FooBInSuperSuper.html' '//pre' 'pub(crate) struct FooBInSuperSuper'
         pub(in super::super) struct FooBInSuperSuper;
         // @has 'foo/a/b/struct.FooBInAB.html' '//pre' 'struct FooBInAB'
+        // @!has 'foo/a/b/struct.FooBInAB.html' '//pre' 'pub'
         pub(in a::b) struct FooBInAB;
         // @has 'foo/a/b/struct.FooBPriv.html' '//pre' 'struct FooBPriv'
+        // @!has 'foo/a/b/struct.FooBPriv.html' '//pre' 'pub'
         struct FooBPriv;
     }
 }