Rollup merge of rust-lang#111076 - notriddle:notriddle/silence-privat…

…e-dep-trait-impl-suggestions, r=cjgillot

diagnostics: exclude indirect private deps from trait impl suggest

Fixes rust-lang#88696

compiler/rustc_data_structures/src/sync.rs

@@ -143,6 +143,11 @@ cfg_if! {
                 self.0.set(val);
                 result
             }
+            pub fn fetch_and(&self, val: bool, _: Ordering) -> bool {
+                let result = self.0.get() & val;
+                self.0.set(val);
+                result
+            }
         }
 
         impl<T: Copy + PartialEq> Atomic<T> {

compiler/rustc_metadata/src/creader.rs

@@ -361,15 +361,21 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
         lib: Library,
         dep_kind: CrateDepKind,
         name: Symbol,
+        private_dep: Option<bool>,
     ) -> Result<CrateNum, CrateError> {
         let _prof_timer = self.sess.prof.generic_activity("metadata_register_crate");
 
         let Library { source, metadata } = lib;
         let crate_root = metadata.get_root();
         let host_hash = host_lib.as_ref().map(|lib| lib.metadata.get_root().hash());
 
-        let private_dep =
-            self.sess.opts.externs.get(name.as_str()).map_or(false, |e| e.is_private_dep);
+        let private_dep = self
+            .sess
+            .opts
+            .externs
+            .get(name.as_str())
+            .map_or(private_dep.unwrap_or(false), |e| e.is_private_dep)
+            && private_dep.unwrap_or(true);
 
         // Claim this crate number and cache it
         let cnum = self.cstore.intern_stable_crate_id(&crate_root)?;
@@ -514,15 +520,16 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
         if !name.as_str().is_ascii() {
             return Err(CrateError::NonAsciiName(name));
         }
-        let (root, hash, host_hash, extra_filename, path_kind) = match dep {
+        let (root, hash, host_hash, extra_filename, path_kind, private_dep) = match dep {
             Some((root, dep)) => (
                 Some(root),
                 Some(dep.hash),
                 dep.host_hash,
                 Some(&dep.extra_filename[..]),
                 PathKind::Dependency,
+                Some(dep.is_private),
             ),
-            None => (None, None, None, None, PathKind::Crate),
+            None => (None, None, None, None, PathKind::Crate, None),
         };
         let result = if let Some(cnum) = self.existing_match(name, hash, path_kind) {
             (LoadResult::Previous(cnum), None)
@@ -558,10 +565,13 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
                     dep_kind = CrateDepKind::MacrosOnly;
                 }
                 data.update_dep_kind(|data_dep_kind| cmp::max(data_dep_kind, dep_kind));
+                if let Some(private_dep) = private_dep {
+                    data.update_and_private_dep(private_dep);
+                }
                 Ok(cnum)
             }
             (LoadResult::Loaded(library), host_library) => {
-                self.register_crate(host_library, root, library, dep_kind, name)
+                self.register_crate(host_library, root, library, dep_kind, name, private_dep)
             }
             _ => panic!(),
         }

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -8,7 +8,7 @@ use rustc_ast as ast;
 use rustc_data_structures::captures::Captures;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::svh::Svh;
-use rustc_data_structures::sync::{AppendOnlyVec, Lock, Lrc, OnceCell};
+use rustc_data_structures::sync::{AppendOnlyVec, AtomicBool, Lock, Lrc, OnceCell};
 use rustc_data_structures::unhash::UnhashMap;
 use rustc_expand::base::{SyntaxExtension, SyntaxExtensionKind};
 use rustc_expand::proc_macro::{AttrProcMacro, BangProcMacro, DeriveProcMacro};
@@ -38,6 +38,7 @@ use proc_macro::bridge::client::ProcMacro;
 use std::iter::TrustedLen;
 use std::num::NonZeroUsize;
 use std::path::Path;
+use std::sync::atomic::Ordering;
 use std::{io, iter, mem};
 
 pub(super) use cstore_impl::provide;
@@ -109,9 +110,10 @@ pub(crate) struct CrateMetadata {
     dep_kind: Lock<CrateDepKind>,
     /// Filesystem location of this crate.
     source: Lrc<CrateSource>,
-    /// Whether or not this crate should be consider a private dependency
-    /// for purposes of the 'exported_private_dependencies' lint
-    private_dep: bool,
+    /// Whether or not this crate should be consider a private dependency.
+    /// Used by the 'exported_private_dependencies' lint, and for determining
+    /// whether to emit suggestions that reference this crate.
+    private_dep: AtomicBool,
     /// The hash for the host proc macro. Used to support `-Z dual-proc-macro`.
     host_hash: Option<Svh>,
 
@@ -692,12 +694,13 @@ impl MetadataBlob {
         writeln!(out, "=External Dependencies=")?;
 
         for (i, dep) in root.crate_deps.decode(self).enumerate() {
-            let CrateDep { name, extra_filename, hash, host_hash, kind } = dep;
+            let CrateDep { name, extra_filename, hash, host_hash, kind, is_private } = dep;
             let number = i + 1;
 
             writeln!(
                 out,
-                "{number} {name}{extra_filename} hash {hash} host_hash {host_hash:?} kind {kind:?}"
+                "{number} {name}{extra_filename} hash {hash} host_hash {host_hash:?} kind {kind:?} {privacy}",
+                privacy = if is_private { "private" } else { "public" }
             )?;
         }
         write!(out, "\n")?;
@@ -1627,7 +1630,7 @@ impl CrateMetadata {
             dependencies,
             dep_kind: Lock::new(dep_kind),
             source: Lrc::new(source),
-            private_dep,
+            private_dep: AtomicBool::new(private_dep),
             host_hash,
             extern_crate: Lock::new(None),
             hygiene_context: Default::default(),
@@ -1675,6 +1678,10 @@ impl CrateMetadata {
         self.dep_kind.with_lock(|dep_kind| *dep_kind = f(*dep_kind))
     }
 
+    pub(crate) fn update_and_private_dep(&self, private_dep: bool) {
+        self.private_dep.fetch_and(private_dep, Ordering::SeqCst);
+    }
+
     pub(crate) fn required_panic_strategy(&self) -> Option<PanicStrategy> {
         self.root.required_panic_strategy
     }

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

@@ -286,7 +286,13 @@ provide! { tcx, def_id, other, cdata,
     is_ctfe_mir_available => { cdata.is_ctfe_mir_available(def_id.index) }
 
     dylib_dependency_formats => { cdata.get_dylib_dependency_formats(tcx) }
-    is_private_dep => { cdata.private_dep }
+    is_private_dep => {
+        // Parallel compiler needs to synchronize type checking and linting (which use this flag)
+        // so that they happen strictly crate loading. Otherwise, the full list of available
+        // impls aren't loaded yet.
+        use std::sync::atomic::Ordering;
+        cdata.private_dep.load(Ordering::Acquire)
+    }
     is_panic_runtime => { cdata.root.panic_runtime }
     is_compiler_builtins => { cdata.root.compiler_builtins }
     has_global_allocator => { cdata.root.has_global_allocator }

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -1869,6 +1869,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
                     host_hash: self.tcx.crate_host_hash(cnum),
                     kind: self.tcx.dep_kind(cnum),
                     extra_filename: self.tcx.extra_filename(cnum).clone(),
+                    is_private: self.tcx.is_private_dep(cnum),
                 };
                 (cnum, dep)
             })

compiler/rustc_metadata/src/rmeta/mod.rs

@@ -302,6 +302,7 @@ pub(crate) struct CrateDep {
     pub host_hash: Option<Svh>,
     pub kind: CrateDepKind,
     pub extra_filename: String,
+    pub is_private: bool,
 }
 
 #[derive(MetadataEncodable, MetadataDecodable)]

compiler/rustc_middle/src/ty/util.rs

@@ -14,7 +14,7 @@ use rustc_data_structures::stable_hasher::{Hash64, HashStable, StableHasher};
 use rustc_errors::ErrorGuaranteed;
 use rustc_hir as hir;
 use rustc_hir::def::{CtorOf, DefKind, Res};
-use rustc_hir::def_id::{DefId, LocalDefId};
+use rustc_hir::def_id::{CrateNum, DefId, LocalDefId};
 use rustc_index::bit_set::GrowableBitSet;
 use rustc_index::{Idx, IndexVec};
 use rustc_macros::HashStable;
@@ -820,6 +820,26 @@ impl<'tcx> TyCtxt<'tcx> {
             _ => def_kind.article(),
         }
     }
+
+    /// Return `true` if the supplied `CrateNum` is "user-visible," meaning either a [public]
+    /// dependency, or a [direct] private dependency. This is used to decide whether the crate can
+    /// be shown in `impl` suggestions.
+    ///
+    /// [public]: TyCtxt::is_private_dep
+    /// [direct]: rustc_session::cstore::ExternCrate::is_direct
+    pub fn is_user_visible_dep(self, key: CrateNum) -> bool {
+        // | Private | Direct | Visible |                    |
+        // |---------|--------|---------|--------------------|
+        // | Yes     | Yes    | Yes     | !true || true   |
+        // | No      | Yes    | Yes     | !false || true  |
+        // | Yes     | No     | No      | !true || false  |
+        // | No      | No     | Yes     | !false || false |
+        !self.is_private_dep(key)
+            // If `extern_crate` is `None`, then the crate was injected (e.g., by the allocator).
+            // Treat that kind of crate as "indirect", since it's an implementation detail of
+            // the language.
+            || self.extern_crate(key.as_def_id()).map_or(false, |e| e.is_direct())
+    }
 }
 
 struct OpaqueTypeExpander<'tcx> {

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

@@ -1775,6 +1775,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                     || !trait_pred
                         .skip_binder()
                         .is_constness_satisfied_by(self.tcx.constness(def_id))
+                    || !self.tcx.is_user_visible_dep(def_id.krate)
                 {
                     return None;
                 }

library/std/Cargo.toml

@@ -1,3 +1,5 @@
+cargo-features = ["public-dependency"]
+
 [package]
 name = "std"
 version = "0.0.0"
@@ -10,12 +12,12 @@ edition = "2021"
 crate-type = ["dylib", "rlib"]
 
 [dependencies]
-alloc = { path = "../alloc" }
+alloc = { path = "../alloc", public = true }
 cfg-if = { version = "1.0", features = ['rustc-dep-of-std'] }
 panic_unwind = { path = "../panic_unwind", optional = true }
 panic_abort = { path = "../panic_abort" }
-core = { path = "../core" }
-libc = { version = "0.2.143", default-features = false, features = ['rustc-dep-of-std'] }
+core = { path = "../core", public = true }
+libc = { version = "0.2.143", default-features = false, features = ['rustc-dep-of-std'], public = true }
 compiler_builtins = { version = "0.1.91" }
 profiler_builtins = { path = "../profiler_builtins", optional = true }
 unwind = { path = "../unwind" }
@@ -25,7 +27,7 @@ std_detect = { path = "../stdarch/crates/std_detect", default-features = false,
 # Dependencies of the `backtrace` crate
 addr2line = { version = "0.19.0", optional = true, default-features = false }
 rustc-demangle = { version = "0.1.21", features = ['rustc-dep-of-std'] }
-miniz_oxide = { version = "0.6.0", optional = true, default-features = false }
+miniz_oxide = { version = "0.6.0", optional = true, default-features = false, public = false }
 [dependencies.object]
 version = "0.30.0"
 optional = true

library/std/tests/common/mod.rs

@@ -20,15 +20,15 @@ pub(crate) fn test_rng() -> rand_xorshift::XorShiftRng {
 }
 
 // Copied from std::sys_common::io
-pub struct TempDir(PathBuf);
+pub(crate) struct TempDir(PathBuf);
 
 impl TempDir {
-    pub fn join(&self, path: &str) -> PathBuf {
+    pub(crate) fn join(&self, path: &str) -> PathBuf {
         let TempDir(ref p) = *self;
         p.join(path)
     }
 
-    pub fn path(&self) -> &Path {
+    pub(crate) fn path(&self) -> &Path {
         let TempDir(ref p) = *self;
         p
     }
@@ -49,7 +49,7 @@ impl Drop for TempDir {
 }
 
 #[track_caller] // for `test_rng`
-pub fn tmpdir() -> TempDir {
+pub(crate) fn tmpdir() -> TempDir {
     let p = env::temp_dir();
     let mut r = test_rng();
     let ret = p.join(&format!("rust-{}", r.next_u32()));

src/bootstrap/metadata.rs

@@ -74,6 +74,9 @@ fn workspace_members(build: &Build) -> impl Iterator<Item = Package> {
     let collect_metadata = |manifest_path| {
         let mut cargo = Command::new(&build.initial_cargo);
         cargo
+            // Will read the libstd Cargo.toml
+            // which uses the unstable `public-dependency` feature.
+            .env("RUSTC_BOOTSTRAP", "1")
             .arg("metadata")
             .arg("--format-version")
             .arg("1")

src/tools/rust-installer/combine-installers.sh

@@ -11,5 +11,9 @@ abs_path() {
     (unset CDPATH && cd "$path" > /dev/null && pwd)
 }
 
+# Running cargo will read the libstd Cargo.toml
+# which uses the unstable `public-dependency` feature.
+export RUSTC_BOOTSTRAP=1
+
 src_dir="$(abs_path $(dirname "$0"))"
 $CARGO run --manifest-path="$src_dir/Cargo.toml" -- combine "$@"

src/tools/rust-installer/gen-installer.sh

@@ -11,5 +11,9 @@ abs_path() {
     (unset CDPATH && cd "$path" > /dev/null && pwd)
 }
 
+# Running cargo will read the libstd Cargo.toml
+# which uses the unstable `public-dependency` feature.
+export RUSTC_BOOTSTRAP=1
+
 src_dir="$(abs_path $(dirname "$0"))"
 $CARGO run --manifest-path="$src_dir/Cargo.toml" -- generate "$@"

src/tools/rust-installer/make-tarballs.sh

@@ -11,5 +11,9 @@ abs_path() {
     (unset CDPATH && cd "$path" > /dev/null && pwd)
 }
 
+# Running cargo will read the libstd Cargo.toml
+# which uses the unstable `public-dependency` feature.
+export RUSTC_BOOTSTRAP=1
+
 src_dir="$(abs_path $(dirname "$0"))"
 $CARGO run --manifest-path="$src_dir/Cargo.toml" -- tarball "$@"

src/tools/tidy/src/main.rs

@@ -16,6 +16,12 @@ use std::sync::atomic::{AtomicBool, Ordering};
 use std::thread::{self, scope, ScopedJoinHandle};
 
 fn main() {
+    // Running Cargo will read the libstd Cargo.toml
+    // which uses the unstable `public-dependency` feature.
+    //
+    // `setenv` might not be thread safe, so run it before using multiple threads.
+    env::set_var("RUSTC_BOOTSTRAP", "1");
+
     let root_path: PathBuf = env::args_os().nth(1).expect("need path to root of repo").into();
     let cargo: PathBuf = env::args_os().nth(2).expect("need path to cargo").into();
     let output_directory: PathBuf =

tests/ui/suggestions/issue-88696.rs

@@ -0,0 +1,14 @@
+// This test case should ensure that miniz_oxide isn't
+// suggested, since it's not a direct dependency.
+
+fn a() -> Result<u64, i32> {
+    Err(1)
+}
+
+fn b() -> Result<u32, i32> {
+    a().into() //~ERROR [E0277]
+}
+
+fn main() {
+    let _ = dbg!(b());
+}

tests/ui/suggestions/issue-88696.stderr

@@ -0,0 +1,11 @@
+error[E0277]: the trait bound `Result<u32, i32>: From<Result<u64, i32>>` is not satisfied
+  --> $DIR/issue-88696.rs:9:9
+   |
+LL |     a().into()
+   |         ^^^^ the trait `From<Result<u64, i32>>` is not implemented for `Result<u32, i32>`
+   |
+   = note: required for `Result<u64, i32>` to implement `Into<Result<u32, i32>>`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0277`.