Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

diagnostics: exclude indirect private deps from trait impl suggest #111076

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
64025bb
bootstrap: enable Cargo `public-dependency` feature for `libstd`
notriddle May 1, 2023
e36020c
rustc_metadata: inherit dependency privacy flag
notriddle May 1, 2023
674a3d5
diagnostics: exclude indirect private deps from trait impl suggest
notriddle May 1, 2023
ffef807
Update compiler/rustc_metadata/src/creader.rs
notriddle May 1, 2023
8488e8a
rust-installer: include `RUSTC_BOOTSTRAP` when generating installer
notriddle May 1, 2023
3740243
Improve comments
notriddle May 8, 2023
0ca70be
rustc_metadata: fix private_dep logic in `register_crate`
notriddle May 8, 2023
d47dc32
diagnostics: don't crash if an injected crate shows up in suggestions
notriddle May 8, 2023
5a7fffe
rustc_metadata: use AtomicBool for privateness instead of Lock
notriddle May 8, 2023
1c14b0a
remove outdated comment from `is_user_visible_dep` docs
notriddle May 8, 2023
b537c1f
std: mark common functions in test crate `pub(crate)`
notriddle May 8, 2023
a12f50d
rustc_metadata: use configurable AtomicBool for privateness flag
notriddle May 8, 2023
bd90868
Use De Morgan's law to simplify logic
notriddle May 9, 2023
6a35896
rustc_metadata: specialize private_dep flag with `fetch_and`
notriddle May 9, 2023
2e52f4d
bootstrap: use RUSTC_BOOTSTRAP to vendor sources
notriddle May 16, 2023
8c21920
std: make internal-only items `pub(crate)`
notriddle May 18, 2023
5fb752b
std: make `fortanix-sgx-abi` a public depedenceny
notriddle May 21, 2023
64cfc21
bootstrap: use RUSTC_BOOTSTRAP in distcheck
notriddle May 25, 2023
52bd82f
rustc_data_structures: sync and atomic consistency
notriddle May 25, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions compiler/rustc_data_structures/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,14 @@ cfg_if! {

impl Atomic<bool> {
pub fn fetch_or(&self, val: bool, _: Ordering) -> bool {
let result = self.0.get() | val;
self.0.set(val);
result
let old = self.0.get();
self.0.set(val | old);
old
}
pub fn fetch_and(&self, val: bool, _: Ordering) -> bool {
let old = self.0.get();
self.0.set(val & old);
old
}
}

Expand Down
20 changes: 15 additions & 5 deletions compiler/rustc_metadata/src/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,15 +365,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()).is_some_and(|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)?;
Expand Down Expand Up @@ -518,15 +524,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)
Expand Down Expand Up @@ -562,10 +569,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!(),
}
Expand Down
21 changes: 14 additions & 7 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::owned_slice::OwnedSlice;
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};
Expand Down Expand Up @@ -40,6 +40,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;
Expand Down Expand Up @@ -111,9 +112,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,
notriddle marked this conversation as resolved.
Show resolved Hide resolved
/// The hash for the host proc macro. Used to support `-Z dual-proc-macro`.
host_hash: Option<Svh>,

Expand Down Expand Up @@ -690,12 +692,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")?;
Expand Down Expand Up @@ -1617,7 +1620,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(),
Expand Down Expand Up @@ -1665,6 +1668,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
}
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,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 }
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1880,6 +1880,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)
})
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
22 changes: 21 additions & 1 deletion compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,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;
Expand Down Expand Up @@ -857,6 +857,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> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
12 changes: 7 additions & 5 deletions library/std/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
cargo-features = ["public-dependency"]

[package]
name = "std"
version = "0.0.0"
Expand All @@ -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.92" }
profiler_builtins = { path = "../profiler_builtins", optional = true }
unwind = { path = "../unwind" }
Expand All @@ -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
Expand All @@ -40,7 +42,7 @@ rand_xorshift = "0.3.0"
dlmalloc = { version = "0.2.3", features = ['rustc-dep-of-std'] }

[target.x86_64-fortanix-unknown-sgx.dependencies]
fortanix-sgx-abi = { version = "0.5.0", features = ['rustc-dep-of-std'] }
fortanix-sgx-abi = { version = "0.5.0", features = ['rustc-dep-of-std'], public = true }

[target.'cfg(target_os = "hermit")'.dependencies]
hermit-abi = { version = "0.3.0", features = ['rustc-dep-of-std'] }
Expand Down
6 changes: 3 additions & 3 deletions library/std/src/sys/wasi/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl WasiFd {
unsafe { wasi::fd_sync(self.as_raw_fd() as wasi::Fd).map_err(err2io) }
}

pub fn advise(&self, offset: u64, len: u64, advice: wasi::Advice) -> io::Result<()> {
pub(crate) fn advise(&self, offset: u64, len: u64, advice: wasi::Advice) -> io::Result<()> {
unsafe {
wasi::fd_advise(self.as_raw_fd() as wasi::Fd, offset, len, advice).map_err(err2io)
}
Expand Down Expand Up @@ -179,7 +179,7 @@ impl WasiFd {
}
}

pub fn filestat_get(&self) -> io::Result<wasi::Filestat> {
pub(crate) fn filestat_get(&self) -> io::Result<wasi::Filestat> {
unsafe { wasi::fd_filestat_get(self.as_raw_fd() as wasi::Fd).map_err(err2io) }
}

Expand All @@ -199,7 +199,7 @@ impl WasiFd {
unsafe { wasi::fd_filestat_set_size(self.as_raw_fd() as wasi::Fd, size).map_err(err2io) }
}

pub fn path_filestat_get(
pub(crate) fn path_filestat_get(
&self,
flags: wasi::Lookupflags,
path: &str,
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/wasi/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl FileAttr {
Ok(SystemTime::from_wasi_timestamp(self.meta.ctim))
}

pub fn as_wasi(&self) -> &wasi::Filestat {
pub(crate) fn as_wasi(&self) -> &wasi::Filestat {
&self.meta
}
}
Expand Down Expand Up @@ -142,7 +142,7 @@ impl FileType {
self.bits == wasi::FILETYPE_SYMBOLIC_LINK
}

pub fn bits(&self) -> wasi::Filetype {
pub(crate) fn bits(&self) -> wasi::Filetype {
self.bits
}
}
Expand Down
8 changes: 4 additions & 4 deletions library/std/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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()));
Expand Down
3 changes: 3 additions & 0 deletions src/bootstrap/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,9 @@ impl Step for PlainSourceTarball {
.arg(builder.src.join("./compiler/rustc_codegen_cranelift/Cargo.toml"))
.arg("--sync")
.arg(builder.src.join("./src/bootstrap/Cargo.toml"))
// Will read the libstd Cargo.toml
// which uses the unstable `public-dependency` feature.
.env("RUSTC_BOOTSTRAP", "1")
notriddle marked this conversation as resolved.
Show resolved Hide resolved
.current_dir(&plain_dst_src);

let config = if !builder.config.dry_run() {
Expand Down
3 changes: 3 additions & 0 deletions src/bootstrap/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
cjgillot marked this conversation as resolved.
Show resolved Hide resolved
.arg("metadata")
.arg("--format-version")
.arg("1")
Expand Down
3 changes: 3 additions & 0 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2514,6 +2514,9 @@ impl Step for Distcheck {
let toml = dir.join("rust-src/lib/rustlib/src/rust/library/std/Cargo.toml");
builder.run(
Command::new(&builder.initial_cargo)
// Will read the libstd Cargo.toml
// which uses the unstable `public-dependency` feature.
.env("RUSTC_BOOTSTRAP", "1")
.arg("generate-lockfile")
.arg("--manifest-path")
.arg(&toml)
Expand Down
4 changes: 4 additions & 0 deletions src/tools/rust-installer/combine-installers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
cjgillot marked this conversation as resolved.
Show resolved Hide resolved

src_dir="$(abs_path $(dirname "$0"))"
$CARGO run --manifest-path="$src_dir/Cargo.toml" -- combine "$@"
4 changes: 4 additions & 0 deletions src/tools/rust-installer/gen-installer.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
cjgillot marked this conversation as resolved.
Show resolved Hide resolved

src_dir="$(abs_path $(dirname "$0"))"
$CARGO run --manifest-path="$src_dir/Cargo.toml" -- generate "$@"
4 changes: 4 additions & 0 deletions src/tools/rust-installer/make-tarballs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
cjgillot marked this conversation as resolved.
Show resolved Hide resolved

src_dir="$(abs_path $(dirname "$0"))"
$CARGO run --manifest-path="$src_dir/Cargo.toml" -- tarball "$@"
Loading