Skip to content

Commit

Permalink
Auto merge of rust-lang#135278 - tgross35:ignore-std-dep-crates, r=<try>
Browse files Browse the repository at this point in the history
Exclude dependencies of `std` for diagnostics

Currently crates in the sysroot can show up in diagnostic suggestions, such as in rust-lang#135232. To prevent this, duplicate `all_traits` into `visible_traits` which only shows traits in non-private crates.

Setting `#![feature(rustc_private)]` overrides this and makes items in private crates visible as well, since `rustc_private` enables use of `std`'s private dependencies.

This may be reviewed per-commit.

Fixes: rust-lang#135232
  • Loading branch information
bors committed Jan 13, 2025
2 parents 047bc17 + c0eb4af commit c9c7ddb
Show file tree
Hide file tree
Showing 11 changed files with 182 additions and 30 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// all visible traits. If there's one clear winner, just suggest that.

let visible_traits: Vec<_> = tcx
.all_traits()
.visible_traits()
.filter(|trait_def_id| {
let viz = tcx.visibility(*trait_def_id);
let def_id = self.item_def_id();
Expand Down
49 changes: 29 additions & 20 deletions compiler/rustc_metadata/src/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,10 +390,12 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
None
}

// The `dependency` type is determined by the command line arguments(`--extern`) and
// `private_dep`. However, sometimes the directly dependent crate is not specified by
// `--extern`, in this case, `private-dep` is none during loading. This is equivalent to the
// scenario where the command parameter is set to `public-dependency`
/// The `dependency` type is determined by the command line arguments(`--extern`) and
/// `private_dep`.
///
/// Sometimes the directly dependent crate is not specified by `--extern`, in this case,
/// `private-dep` is none during loading. This is equivalent to the scenario where the
/// command parameter is set to `public-dependency`
fn is_private_dep(&self, name: &str, private_dep: Option<bool>) -> bool {
self.sess.opts.externs.get(name).map_or(private_dep.unwrap_or(false), |e| e.is_private_dep)
&& private_dep.unwrap_or(true)
Expand All @@ -402,7 +404,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
fn register_crate(
&mut self,
host_lib: Option<Library>,
root: Option<&CratePaths>,
dep_root: Option<&CratePaths>,
lib: Library,
dep_kind: CrateDepKind,
name: Symbol,
Expand Down Expand Up @@ -430,14 +432,14 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
// Maintain a reference to the top most crate.
// Stash paths for top-most crate locally if necessary.
let crate_paths;
let root = if let Some(root) = root {
root
let dep_root = if let Some(dep_root) = dep_root {
dep_root
} else {
crate_paths = CratePaths::new(crate_root.name(), source.clone());
&crate_paths
};

let cnum_map = self.resolve_crate_deps(root, &crate_root, &metadata, cnum, dep_kind)?;
let cnum_map = self.resolve_crate_deps(dep_root, &crate_root, &metadata, cnum, dep_kind)?;

let raw_proc_macros = if crate_root.is_proc_macro_crate() {
let temp_root;
Expand Down Expand Up @@ -559,15 +561,15 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
&'b mut self,
name: Symbol,
mut dep_kind: CrateDepKind,
dep: Option<(&'b CratePaths, &'b CrateDep)>,
dep_of: Option<(&'b CratePaths, &'b CrateDep)>,
) -> Result<CrateNum, CrateError> {
info!("resolving crate `{}`", name);
if !name.as_str().is_ascii() {
return Err(CrateError::NonAsciiName(name));
}
let (root, hash, host_hash, extra_filename, path_kind, private_dep) = match dep {
Some((root, dep)) => (
Some(root),
let (dep_root, hash, host_hash, extra_filename, path_kind, private_dep) = match dep_of {
Some((dep_root, dep)) => (
Some(dep_root),
Some(dep.hash),
dep.host_hash,
Some(&dep.extra_filename[..]),
Expand Down Expand Up @@ -599,7 +601,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
dep_kind = CrateDepKind::MacrosOnly;
match self.load_proc_macro(&mut locator, path_kind, host_hash)? {
Some(res) => res,
None => return Err(locator.into_error(root.cloned())),
None => return Err(locator.into_error(dep_root.cloned())),
}
}
}
Expand All @@ -623,7 +625,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
}
(LoadResult::Loaded(library), host_library) => {
info!("register newly loaded library for `{}`", name);
self.register_crate(host_library, root, library, dep_kind, name, private_dep)
self.register_crate(host_library, dep_root, library, dep_kind, name, private_dep)
}
_ => panic!(),
}
Expand Down Expand Up @@ -663,16 +665,20 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
}))
}

// Go through the crate metadata and load any crates that it references
/// Go through the crate metadata and load any crates that it references.
fn resolve_crate_deps(
&mut self,
root: &CratePaths,
dep_root: &CratePaths,
crate_root: &CrateRoot,
metadata: &MetadataBlob,
krate: CrateNum,
dep_kind: CrateDepKind,
) -> Result<CrateNumMap, CrateError> {
debug!("resolving deps of external crate");
debug!(
"resolving deps of external crate `{}` with dep root `{}`",
crate_root.name(),
dep_root.name
);
if crate_root.is_proc_macro_crate() {
return Ok(CrateNumMap::new());
}
Expand All @@ -685,14 +691,17 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
crate_num_map.push(krate);
for dep in deps {
info!(
"resolving dep crate {} hash: `{}` extra filename: `{}`",
dep.name, dep.hash, dep.extra_filename
"resolving dep `{}`->`{}` hash: `{}` extra filename: `{}`",
crate_root.name(),
dep.name,
dep.hash,
dep.extra_filename
);
let dep_kind = match dep_kind {
CrateDepKind::MacrosOnly => CrateDepKind::MacrosOnly,
_ => dep.kind,
};
let cnum = self.maybe_resolve_crate(dep.name, dep_kind, Some((root, &dep)))?;
let cnum = self.maybe_resolve_crate(dep.name, dep_kind, Some((dep_root, &dep)))?;
crate_num_map.push(cnum);
}

Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_metadata/src/locator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ pub(crate) struct CrateLocator<'a> {

#[derive(Clone)]
pub(crate) struct CratePaths {
name: Symbol,
pub(crate) name: Symbol,
source: CrateSource,
}

Expand Down Expand Up @@ -765,10 +765,10 @@ impl<'a> CrateLocator<'a> {
self.extract_lib(rlibs, rmetas, dylibs).map(|opt| opt.map(|(_, lib)| lib))
}

pub(crate) fn into_error(self, root: Option<CratePaths>) -> CrateError {
pub(crate) fn into_error(self, dep_root: Option<CratePaths>) -> CrateError {
CrateError::LocatorCombined(Box::new(CombinedLocatorError {
crate_name: self.crate_name,
root,
dep_root,
triple: self.tuple,
dll_prefix: self.target.dll_prefix.to_string(),
dll_suffix: self.target.dll_suffix.to_string(),
Expand Down Expand Up @@ -914,7 +914,7 @@ struct CrateRejections {
/// otherwise they are ignored.
pub(crate) struct CombinedLocatorError {
crate_name: Symbol,
root: Option<CratePaths>,
dep_root: Option<CratePaths>,
triple: TargetTuple,
dll_prefix: String,
dll_suffix: String,
Expand Down Expand Up @@ -987,7 +987,7 @@ impl CrateError {
}
CrateError::LocatorCombined(locator) => {
let crate_name = locator.crate_name;
let add_info = match &locator.root {
let add_info = match &locator.dep_root {
None => String::new(),
Some(r) => format!(" which `{}` depends on", r.name),
};
Expand All @@ -1012,7 +1012,7 @@ impl CrateError {
path.display()
));
}
if let Some(r) = locator.root {
if let Some(r) = locator.dep_root {
for path in r.source.paths() {
found_crates.push_str(&format!(
"\ncrate `{}`: {}",
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2128,6 +2128,8 @@ rustc_queries! {
eval_always
desc { "calculating the stability index for the local crate" }
}
/// All available crates in the graph, including those that should not be user-facing
/// (such as private crates).
query crates(_: ()) -> &'tcx [CrateNum] {
eval_always
desc { "fetching all foreign CrateNum instances" }
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2078,12 +2078,23 @@ impl<'tcx> TyCtxt<'tcx> {
self.limits(()).move_size_limit
}

/// All traits in the crate graph, including those not visible to the user.
pub fn all_traits(self) -> impl Iterator<Item = DefId> + 'tcx {
iter::once(LOCAL_CRATE)
.chain(self.crates(()).iter().copied())
.flat_map(move |cnum| self.traits(cnum).iter().copied())
}

/// All traits that are visible within the crate graph (i.e. excluding private dependencies).
pub fn visible_traits(self) -> impl Iterator<Item = DefId> + 'tcx {
let visible_crates =
self.crates(()).iter().copied().filter(move |cnum| self.is_user_visible_dep(*cnum));

iter::once(LOCAL_CRATE)
.chain(visible_crates)
.flat_map(move |cnum| self.traits(cnum).iter().copied())
}

#[inline]
pub fn local_visibility(self, def_id: LocalDefId) -> Visibility {
self.visibility(def_id).expect_local()
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,11 @@ impl<'tcx> TyCtxt<'tcx> {
/// [public]: TyCtxt::is_private_dep
/// [direct]: rustc_session::cstore::ExternCrate::is_direct
pub fn is_user_visible_dep(self, key: CrateNum) -> bool {
// `#![rustc_private]` overrides defaults to make private dependencies usable.
if self.features().enabled(sym::rustc_private) {
return true;
}

// | Private | Direct | Visible | |
// |---------|--------|---------|--------------------|
// | Yes | Yes | Yes | !true || true |
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_passes/src/diagnostic_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,14 @@ fn all_diagnostic_items(tcx: TyCtxt<'_>, (): ()) -> DiagnosticItems {
// Initialize the collector.
let mut items = DiagnosticItems::default();

// Collect diagnostic items in other crates.
for &cnum in tcx.crates(()).iter().chain(std::iter::once(&LOCAL_CRATE)) {
// Collect diagnostic items in visible crates.
for cnum in tcx
.crates(())
.iter()
.copied()
.filter(|cnum| tcx.is_user_visible_dep(*cnum))
.chain(std::iter::once(LOCAL_CRATE))
{
for (&name, &def_id) in &tcx.diagnostic_items(cnum).name_to_id {
collect_item(tcx, &mut items, name, def_id);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2189,7 +2189,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
let required_trait_path = self.tcx.def_path_str(trait_ref.def_id());
let traits_with_same_path: UnordSet<_> = self
.tcx
.all_traits()
.visible_traits()
.filter(|trait_def_id| *trait_def_id != trait_ref.def_id())
.map(|trait_def_id| (self.tcx.def_path_str(trait_def_id), trait_def_id))
.filter(|(p, _)| *p == required_trait_path)
Expand Down
39 changes: 39 additions & 0 deletions tests/ui/std/sysroot-private.default.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
error[E0405]: cannot find trait `Equivalent` in this scope
--> $DIR/sysroot-private.rs:25:18
|
LL | trait Trait2<K>: Equivalent<K> {}
| ^^^^^^^^^^ not found in this scope

error[E0412]: cannot find type `K` in this scope
--> $DIR/sysroot-private.rs:30:35
|
LL | fn trait_member<T>(val: &T, key: &K) -> bool {
| - ^
| |
| similarly named type parameter `T` defined here
|
help: a type parameter with a similar name exists
|
LL | fn trait_member<T>(val: &T, key: &T) -> bool {
| ~
help: you might be missing a type parameter
|
LL | fn trait_member<T, K>(val: &T, key: &K) -> bool {
| +++

error[E0220]: associated type `Buf` not found for `Trait`
--> $DIR/sysroot-private.rs:20:31
|
LL | type AssociatedTy = dyn Trait<Buf = i32, Bar = i32>;
| ^^^ help: `Trait` has the following associated type: `Bar`

error[E0425]: cannot find function `memchr2` in this scope
--> $DIR/sysroot-private.rs:38:5
|
LL | memchr2(b'a', b'b', buf)
| ^^^^^^^ not found in this scope

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0220, E0405, E0412, E0425.
For more information about an error, try `rustc --explain E0220`.
41 changes: 41 additions & 0 deletions tests/ui/std/sysroot-private.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
//! Test that private dependencies of `std` that live in the sysroot do not reach through to
//! diagnostics.
//!
//! This test would be more robust if we could patch the sysroot with an "evil" crate that
//! provided known types that we control; however, this would effectively require rebuilding
//! `std` (or patching crate metadata). So, this test relies on what is currently public API
//! of `std`'s dependencies, but may not be robust against dependency upgrades/changes.
//@ revisions: default rustc_private_enabled

// Enabling `rustc_private` should `std`'s dependencies accessible, so they should show up
// in diagnostics. NB: not all diagnostics are affected by this.
#![cfg_attr(rustc_private_enabled, feature(rustc_private))]
#![crate_type = "lib"]

trait Trait { type Bar; }

// Attempt to get a suggestion for `addr2line::LookupContinuation`, which has member `Buf`
// Note that the suggestion only happens when `rustc_private` is enabled.
type AssociatedTy = dyn Trait<Buf = i32, Bar = i32>;
//~^ ERROR associated type `Buf` not found
//[rustc_private_enabled]~| NOTE there is an associated type `Buf` in the trait `addr2line::lookup::LookupContinuation`

// Attempt to get a suggestion for `hashbrown::Equivalent`
trait Trait2<K>: Equivalent<K> {}
//~^ ERROR cannot find trait
//~| NOTE not found

// Attempt to get a suggestion for `hashbrown::Equivalent::equivalent`
fn trait_member<T>(val: &T, key: &K) -> bool {
//~^ ERROR cannot find type `K`
//~| NOTE similarly named
val.equivalent(key)
}

// Attempt to get a suggestion for `memchr::memchr2`
fn free_function(buf: &[u8]) -> Option<usize> {
memchr2(b'a', b'b', buf)
//~^ ERROR cannot find function
//~| NOTE not found
}
39 changes: 39 additions & 0 deletions tests/ui/std/sysroot-private.rustc_private_enabled.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
error[E0405]: cannot find trait `Equivalent` in this scope
--> $DIR/sysroot-private.rs:25:18
|
LL | trait Trait2<K>: Equivalent<K> {}
| ^^^^^^^^^^ not found in this scope

error[E0412]: cannot find type `K` in this scope
--> $DIR/sysroot-private.rs:30:35
|
LL | fn trait_member<T>(val: &T, key: &K) -> bool {
| - ^
| |
| similarly named type parameter `T` defined here
|
help: a type parameter with a similar name exists
|
LL | fn trait_member<T>(val: &T, key: &T) -> bool {
| ~
help: you might be missing a type parameter
|
LL | fn trait_member<T, K>(val: &T, key: &K) -> bool {
| +++

error[E0220]: associated type `Buf` not found for `Trait`
--> $DIR/sysroot-private.rs:20:31
|
LL | type AssociatedTy = dyn Trait<Buf = i32, Bar = i32>;
| ^^^ there is an associated type `Buf` in the trait `addr2line::lookup::LookupContinuation`

error[E0425]: cannot find function `memchr2` in this scope
--> $DIR/sysroot-private.rs:38:5
|
LL | memchr2(b'a', b'b', buf)
| ^^^^^^^ not found in this scope

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0220, E0405, E0412, E0425.
For more information about an error, try `rustc --explain E0220`.

0 comments on commit c9c7ddb

Please sign in to comment.