From 042fba16a44db9583b8392a1ebb75047ea8a733d Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Wed, 29 Jan 2025 06:31:50 +0000 Subject: [PATCH 1/8] Track where crate dependencies come from in `creader` Introduce an enum that represents the different possible sources for dependencies, and use them where possible. This will enable more fine grained control and provides better context than passing the `dep_root` tuple. Use this to ensure that injected crates always show up as private by default. --- compiler/rustc_metadata/src/creader.rs | 101 ++++++++++++++++++++----- 1 file changed, 81 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index a6cd0ecafd0de..3a021087c2e20 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -157,6 +157,43 @@ impl<'a> std::fmt::Debug for CrateDump<'a> { } } +/// Reason that a crate is being sourced as a dependency. +#[derive(Clone, Copy)] +enum CrateOrigin<'a> { + /// This crate was a dependency of another crate. + Dependency { + dep_root: &'a CratePaths, + /// Dependency info about this crate. + dep: &'a CrateDep, + }, + /// Injected by `rustc`. + Injected, + /// An extern that has been provided with the `force` option. + ForcedExtern, + /// Part of the extern prelude. + ExternPrelude, + /// Provided by `extern crate foo`. + AstExtern, +} + +impl<'a> CrateOrigin<'a> { + /// Return the dependency root, if any. + fn dep_root(&self) -> Option<&'a CratePaths> { + match self { + CrateOrigin::Dependency { dep_root, .. } => Some(dep_root), + _ => None, + } + } + + /// Return dependency information, if any. + fn dep(&self) -> Option<&'a CrateDep> { + match self { + CrateOrigin::Dependency { dep, .. } => Some(dep), + _ => None, + } + } +} + impl CStore { pub fn from_tcx(tcx: TyCtxt<'_>) -> FreezeReadGuard<'_, CStore> { FreezeReadGuard::map(tcx.untracked().cstore.read(), |cstore| { @@ -404,7 +441,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { &self, name: Symbol, private_dep: Option, - dep_root: Option<&CratePaths>, + origin: CrateOrigin<'_>, ) -> bool { // Standard library crates are never private. if STDLIB_STABLE_CRATES.contains(&name) { @@ -414,10 +451,14 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { let extern_private = self.sess.opts.externs.get(name.as_str()).map(|e| e.is_private_dep); + if matches!(origin, CrateOrigin::Injected) { + return true; + } + // Any descendants of `std` should be private. These crates are usually not marked // private in metadata, so we ignore that field. if extern_private.is_none() - && let Some(dep) = dep_root + && let Some(dep) = origin.dep_root() && STDLIB_STABLE_CRATES.contains(&dep.name) { return true; @@ -435,7 +476,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { fn register_crate( &mut self, host_lib: Option, - dep_root: Option<&CratePaths>, + origin: CrateOrigin<'_>, lib: Library, dep_kind: CrateDepKind, name: Symbol, @@ -447,7 +488,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { 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.is_private_dep(name, private_dep, dep_root); + let private_dep = self.is_private_dep(name, private_dep, origin); // Claim this crate number and cache it let feed = self.cstore.intern_stable_crate_id(&crate_root, self.tcx)?; @@ -463,7 +504,7 @@ 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 dep_root = if let Some(dep_root) = dep_root { + let dep_root = if let Some(dep_root) = origin.dep_root() { dep_root } else { crate_paths = CratePaths::new(crate_root.name(), source.clone()); @@ -571,17 +612,23 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { name: Symbol, span: Span, dep_kind: CrateDepKind, + origin: CrateOrigin<'_>, ) -> Option { self.used_extern_options.insert(name); - match self.maybe_resolve_crate(name, dep_kind, None) { + match self.maybe_resolve_crate(name, dep_kind, origin) { Ok(cnum) => { self.cstore.set_used_recursively(cnum); Some(cnum) } Err(err) => { debug!("failed to resolve crate {} {:?}", name, dep_kind); - let missing_core = - self.maybe_resolve_crate(sym::core, CrateDepKind::Explicit, None).is_err(); + let missing_core = self + .maybe_resolve_crate( + sym::core, + CrateDepKind::Explicit, + CrateOrigin::ExternPrelude, + ) + .is_err(); err.report(self.sess, span, missing_core); None } @@ -592,15 +639,15 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { &'b mut self, name: Symbol, mut dep_kind: CrateDepKind, - dep_of: Option<(&'b CratePaths, &'b CrateDep)>, + origin: CrateOrigin<'b>, ) -> Result { info!("resolving crate `{}`", name); if !name.as_str().is_ascii() { return Err(CrateError::NonAsciiName(name)); } - let dep_root = dep_of.map(|d| d.0); - let dep = dep_of.map(|d| d.1); + let dep_root = origin.dep_root(); + let dep = origin.dep(); let hash = dep.map(|d| d.hash); let host_hash = dep.map(|d| d.host_hash).flatten(); let extra_filename = dep.map(|d| &d.extra_filename[..]); @@ -643,7 +690,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { // not specified by `--extern` on command line parameters, it may be // `private-dependency` when `register_crate` is called for the first time. Then it must be updated to // `public-dependency` here. - let private_dep = self.is_private_dep(name, private_dep, dep_root); + let private_dep = self.is_private_dep(name, private_dep, origin); let data = self.cstore.get_crate_data_mut(cnum); if data.is_proc_macro_crate() { dep_kind = CrateDepKind::MacrosOnly; @@ -654,7 +701,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { } (LoadResult::Loaded(library), host_library) => { info!("register newly loaded library for `{}`", name); - self.register_crate(host_library, dep_root, library, dep_kind, name, private_dep) + self.register_crate(host_library, origin, library, dep_kind, name, private_dep) } _ => panic!(), } @@ -730,7 +777,10 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { CrateDepKind::MacrosOnly => CrateDepKind::MacrosOnly, _ => dep.kind, }; - let cnum = self.maybe_resolve_crate(dep.name, dep_kind, Some((dep_root, &dep)))?; + let cnum = self.maybe_resolve_crate(dep.name, dep_kind, CrateOrigin::Dependency { + dep_root, + dep: &dep, + })?; crate_num_map.push(cnum); } @@ -824,7 +874,9 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { }; info!("panic runtime not found -- loading {}", name); - let Some(cnum) = self.resolve_crate(name, DUMMY_SP, CrateDepKind::Implicit) else { + let Some(cnum) = + self.resolve_crate(name, DUMMY_SP, CrateDepKind::Implicit, CrateOrigin::Injected) + else { return; }; let data = self.cstore.get_crate_data(cnum); @@ -853,7 +905,9 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { info!("loading profiler"); let name = Symbol::intern(&self.sess.opts.unstable_opts.profiler_runtime); - let Some(cnum) = self.resolve_crate(name, DUMMY_SP, CrateDepKind::Implicit) else { + let Some(cnum) = + self.resolve_crate(name, DUMMY_SP, CrateDepKind::Implicit, CrateOrigin::Injected) + else { return; }; let data = self.cstore.get_crate_data(cnum); @@ -966,7 +1020,12 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { if entry.force { let name_interned = Symbol::intern(name); if !self.used_extern_options.contains(&name_interned) { - self.resolve_crate(name_interned, DUMMY_SP, CrateDepKind::Explicit); + self.resolve_crate( + name_interned, + DUMMY_SP, + CrateDepKind::Explicit, + CrateOrigin::ForcedExtern, + ); } } } @@ -1092,6 +1151,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { info!("{:?}", CrateDump(self.cstore)); } + /// Process an `extern crate foo` AST node. pub fn process_extern_crate( &mut self, item: &ast::Item, @@ -1117,7 +1177,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { CrateDepKind::Explicit }; - let cnum = self.resolve_crate(name, item.span, dep_kind)?; + let cnum = self.resolve_crate(name, item.span, dep_kind, CrateOrigin::AstExtern)?; let path_len = definitions.def_path(def_id).data.len(); self.cstore.update_extern_crate(cnum, ExternCrate { @@ -1133,7 +1193,8 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { } pub fn process_path_extern(&mut self, name: Symbol, span: Span) -> Option { - let cnum = self.resolve_crate(name, span, CrateDepKind::Explicit)?; + let cnum = + self.resolve_crate(name, span, CrateDepKind::Explicit, CrateOrigin::ExternPrelude)?; self.cstore.update_extern_crate(cnum, ExternCrate { src: ExternCrateSource::Path, @@ -1147,7 +1208,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { } pub fn maybe_process_path_extern(&mut self, name: Symbol) -> Option { - self.maybe_resolve_crate(name, CrateDepKind::Explicit, None).ok() + self.maybe_resolve_crate(name, CrateDepKind::Explicit, CrateOrigin::ExternPrelude).ok() } } From 89dd4ecfb5e5d4ab612c8f2501f263350cda5df8 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Wed, 29 Jan 2025 23:45:36 +0000 Subject: [PATCH 2/8] Improve debugging for metadata structures I had to do a lot of debug by printing; having these `Debug` traits in place made it easier. Additionally, add some more information to existing `info!` statements. --- compiler/rustc_metadata/src/creader.rs | 3 ++- compiler/rustc_metadata/src/locator.rs | 10 ++++++---- compiler/rustc_metadata/src/rmeta/decoder.rs | 4 ++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 3a021087c2e20..32caf12f693b8 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -142,6 +142,7 @@ impl<'a> std::fmt::Debug for CrateDump<'a> { writeln!(fmt, " cnum: {cnum}")?; writeln!(fmt, " hash: {}", data.hash())?; writeln!(fmt, " reqd: {:?}", data.dep_kind())?; + writeln!(fmt, " priv: {:?}", data.is_private_dep())?; let CrateSource { dylib, rlib, rmeta } = data.source(); if let Some(dylib) = dylib { writeln!(fmt, " dylib: {}", dylib.0.display())?; @@ -685,7 +686,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { match result { (LoadResult::Previous(cnum), None) => { - info!("library for `{}` was loaded previously", name); + info!("library for `{}` was loaded previously, cnum {cnum}", name); // When `private_dep` is none, it indicates the directly dependent crate. If it is // not specified by `--extern` on command line parameters, it may be // `private-dependency` when `register_crate` is called for the first time. Then it must be updated to diff --git a/compiler/rustc_metadata/src/locator.rs b/compiler/rustc_metadata/src/locator.rs index 2ddabeb49f70d..3f3e58384cbf4 100644 --- a/compiler/rustc_metadata/src/locator.rs +++ b/compiler/rustc_metadata/src/locator.rs @@ -260,7 +260,7 @@ pub(crate) struct CrateLocator<'a> { crate_rejections: CrateRejections, } -#[derive(Clone)] +#[derive(Clone, Debug)] pub(crate) struct CratePaths { pub(crate) name: Symbol, source: CrateSource, @@ -272,7 +272,7 @@ impl CratePaths { } } -#[derive(Copy, Clone, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq)] pub(crate) enum CrateFlavor { Rlib, Rmeta, @@ -893,13 +893,13 @@ fn get_flavor_from_path(path: &Path) -> CrateFlavor { // ------------------------------------------ Error reporting ------------------------------------- -#[derive(Clone)] +#[derive(Clone, Debug)] struct CrateMismatch { path: PathBuf, got: String, } -#[derive(Clone, Default)] +#[derive(Clone, Debug, Default)] struct CrateRejections { via_hash: Vec, via_triple: Vec, @@ -912,6 +912,7 @@ struct CrateRejections { /// Candidate rejection reasons collected during crate search. /// If no candidate is accepted, then these reasons are presented to the user, /// otherwise they are ignored. +#[derive(Debug)] pub(crate) struct CombinedLocatorError { crate_name: Symbol, dep_root: Option, @@ -921,6 +922,7 @@ pub(crate) struct CombinedLocatorError { crate_rejections: CrateRejections, } +#[derive(Debug)] pub(crate) enum CrateError { NonAsciiName(Symbol), ExternLocationNotExist(Symbol, PathBuf), diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index e02c4871f35bd..8694fb23d1844 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1920,6 +1920,10 @@ impl CrateMetadata { self.root.needs_panic_runtime } + pub(crate) fn is_private_dep(&self) -> bool { + self.private_dep + } + pub(crate) fn is_panic_runtime(&self) -> bool { self.root.panic_runtime } From 01124991d6002f899f2988c8e2bb550f9e7f6819 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Wed, 29 Jan 2025 23:54:15 +0000 Subject: [PATCH 3/8] Inject `compiler_builtins` during postprocessing rather than via AST `compiler_builtins` is currently injected as `extern crate compiler_builtins as _`. This has made gating via diagnostics difficult because it appears in the crate graph as a non-private dependency, and there isn't an easy way to differentiate between the injected AST and user-specified `extern crate compiler_builtins`. Resolve this by injecting `compiler_builtins` during postprocessing rather than early in the AST. Most of the time this isn't even needed because it shows up in `std` or `core`'s crate graph, but injection is still needed to ensure `#![no_core]` works correctly. A similar change was attempted at [1] but this encountered errors building `proc_macro` and `rustc-std-workspace-std`. Similar failures showed up while working on this patch, which were traced back to `compiler_builtins` showing up in the graph twice (once via dependency and once via injection). This is resolved by not injecting if a `#![compiler_builtins]` crate already exists. [1]: https://github.com/rust-lang/rust/pull/113634 --- .../src/standard_library_imports.rs | 49 +++++-------------- compiler/rustc_metadata/messages.ftl | 3 ++ compiler/rustc_metadata/src/creader.rs | 38 ++++++++++++++ compiler/rustc_metadata/src/errors.rs | 6 +++ compiler/rustc_metadata/src/rmeta/decoder.rs | 4 ++ .../issue-59191-replace-root-with-fn.rs | 1 + .../issue-59191-replace-root-with-fn.stderr | 9 +++- tests/ui/proc-macro/meta-macro-hygiene.stdout | 1 - .../nonterminal-token-hygiene.stdout | 1 - 9 files changed, 73 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/standard_library_imports.rs b/compiler/rustc_builtin_macros/src/standard_library_imports.rs index 1a3f4d2d44908..6933ca09349a1 100644 --- a/compiler/rustc_builtin_macros/src/standard_library_imports.rs +++ b/compiler/rustc_builtin_macros/src/standard_library_imports.rs @@ -19,16 +19,12 @@ pub fn inject( let edition = sess.psess.edition; // the first name in this list is the crate name of the crate with the prelude - let names: &[Symbol] = if attr::contains_name(pre_configured_attrs, sym::no_core) { + let name: Symbol = if attr::contains_name(pre_configured_attrs, sym::no_core) { return 0; } else if attr::contains_name(pre_configured_attrs, sym::no_std) { - if attr::contains_name(pre_configured_attrs, sym::compiler_builtins) { - &[sym::core] - } else { - &[sym::core, sym::compiler_builtins] - } + sym::core } else { - &[sym::std] + sym::std }; let expn_id = resolver.expansion_for_ast_pass( @@ -43,36 +39,16 @@ pub fn inject( let ecfg = ExpansionConfig::default("std_lib_injection".to_string(), features); let cx = ExtCtxt::new(sess, ecfg, resolver, None); - // .rev() to preserve ordering above in combination with insert(0, ...) - for &name in names.iter().rev() { - let ident_span = if edition >= Edition2018 { span } else { call_site }; - let item = if name == sym::compiler_builtins { - // compiler_builtins is a private implementation detail. We only - // need to insert it into the crate graph for linking and should not - // expose any of its public API. - // - // FIXME(#113634) We should inject this during post-processing like - // we do for the panic runtime, profiler runtime, etc. - cx.item( - span, - Ident::new(kw::Underscore, ident_span), - thin_vec![], - ast::ItemKind::ExternCrate(Some(name)), - ) - } else { - cx.item( - span, - Ident::new(name, ident_span), - thin_vec![cx.attr_word(sym::macro_use, span)], - ast::ItemKind::ExternCrate(None), - ) - }; - krate.items.insert(0, item); - } + let ident_span = if edition >= Edition2018 { span } else { call_site }; - // The crates have been injected, the assumption is that the first one is - // the one with the prelude. - let name = names[0]; + let item = cx.item( + span, + Ident::new(name, ident_span), + thin_vec![cx.attr_word(sym::macro_use, span)], + ast::ItemKind::ExternCrate(None), + ); + + krate.items.insert(0, item); let root = (edition == Edition2015).then_some(kw::PathRoot); @@ -88,6 +64,7 @@ pub fn inject( .map(|&symbol| Ident::new(symbol, span)) .collect(); + // Inject the relevant crate's prelude. let use_item = cx.item( span, Ident::empty(), diff --git a/compiler/rustc_metadata/messages.ftl b/compiler/rustc_metadata/messages.ftl index d2b5ae5318592..948503bb8bf73 100644 --- a/compiler/rustc_metadata/messages.ftl +++ b/compiler/rustc_metadata/messages.ftl @@ -47,6 +47,9 @@ metadata_crate_dep_rustc_driver = metadata_crate_location_unknown_type = extern location for {$crate_name} is of an unknown type: {$path} +metadata_crate_not_compiler_builtins = + the crate `{$crate_name}` resolved as `compiler_builtins` but is not `#![compiler_builtins]` + metadata_crate_not_panic_runtime = the crate `{$crate_name}` is not a panic runtime diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 32caf12f693b8..e9643a287b0af 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -1032,6 +1032,43 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { } } + /// Inject the `compiler_builtins` crate if it is not already in the graph. + fn inject_compiler_builtins(&mut self, krate: &ast::Crate) { + // `compiler_builtins` does not get extern builtins, nor do `#![no_core]` crates + if attr::contains_name(&krate.attrs, sym::compiler_builtins) + || attr::contains_name(&krate.attrs, sym::no_core) + { + info!("`compiler_builtins` unneeded"); + return; + } + + // If a `#![compiler_builtins]` crate already exists, avoid injecting it twice. This is + // the common case since usually it appears as a dependency of `std` or `core`. + for (cnum, cmeta) in self.cstore.iter_crate_data() { + if cmeta.is_compiler_builtins() { + info!("`compiler_builtins` already exists (cnum = {cnum}); skipping injection"); + return; + } + } + + // Allow builtins to remain unresolved, which will just mean linker errors if the + // relevant symbols aren't otherwise provided. + let Ok(cnum) = self.maybe_resolve_crate( + sym::compiler_builtins, + CrateDepKind::Explicit, + CrateOrigin::Injected, + ) else { + info!("`compiler_builtins` not resolved"); + return; + }; + + // Sanity check that the loaded crate is `#![compiler_builtins]` + let cmeta = self.cstore.get_crate_data(cnum); + if !cmeta.is_compiler_builtins() { + self.dcx().emit_err(errors::CrateNotCompilerBuiltins { crate_name: cmeta.name() }); + } + } + fn inject_dependency_if( &mut self, krate: CrateNum, @@ -1141,6 +1178,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { } pub fn postprocess(&mut self, krate: &ast::Crate) { + self.inject_compiler_builtins(krate); self.inject_forced_externs(); self.inject_profiler_runtime(); self.inject_allocator_crate(krate); diff --git a/compiler/rustc_metadata/src/errors.rs b/compiler/rustc_metadata/src/errors.rs index cefc6498f68fe..6e97c111a55eb 100644 --- a/compiler/rustc_metadata/src/errors.rs +++ b/compiler/rustc_metadata/src/errors.rs @@ -332,6 +332,12 @@ pub struct CrateNotPanicRuntime { pub crate_name: Symbol, } +#[derive(Diagnostic)] +#[diag(metadata_crate_not_compiler_builtins)] +pub struct CrateNotCompilerBuiltins { + pub crate_name: Symbol, +} + #[derive(Diagnostic)] #[diag(metadata_no_panic_strategy)] pub struct NoPanicStrategy { diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 8694fb23d1844..9b06df6bf4e32 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1932,6 +1932,10 @@ impl CrateMetadata { self.root.profiler_runtime } + pub(crate) fn is_compiler_builtins(&self) -> bool { + self.root.compiler_builtins + } + pub(crate) fn needs_allocator(&self) -> bool { self.root.needs_allocator } diff --git a/tests/ui/proc-macro/issue-59191-replace-root-with-fn.rs b/tests/ui/proc-macro/issue-59191-replace-root-with-fn.rs index c1b55fd99dfad..07d4b71e8182f 100644 --- a/tests/ui/proc-macro/issue-59191-replace-root-with-fn.rs +++ b/tests/ui/proc-macro/issue-59191-replace-root-with-fn.rs @@ -3,6 +3,7 @@ //@ edition:2018 //@ proc-macro: issue-59191.rs +//@ needs-unwind (affects error output) //@ error-pattern: requires `sized` lang_item #![feature(custom_inner_attributes)] diff --git a/tests/ui/proc-macro/issue-59191-replace-root-with-fn.stderr b/tests/ui/proc-macro/issue-59191-replace-root-with-fn.stderr index 08e679a7c559a..2cb1c9310b94f 100644 --- a/tests/ui/proc-macro/issue-59191-replace-root-with-fn.stderr +++ b/tests/ui/proc-macro/issue-59191-replace-root-with-fn.stderr @@ -1,4 +1,11 @@ +error: `#[panic_handler]` function required, but not found + +error: unwinding panics are not supported without std + | + = help: using nightly cargo, use -Zbuild-std with panic="abort" to avoid unwinding + = note: since the core library is usually precompiled with panic="unwind", rebuilding your crate with panic="abort" may not be enough to fix the problem + error: requires `sized` lang_item -error: aborting due to 1 previous error +error: aborting due to 3 previous errors diff --git a/tests/ui/proc-macro/meta-macro-hygiene.stdout b/tests/ui/proc-macro/meta-macro-hygiene.stdout index fae8446515af0..1ee7179e84c04 100644 --- a/tests/ui/proc-macro/meta-macro-hygiene.stdout +++ b/tests/ui/proc-macro/meta-macro-hygiene.stdout @@ -20,7 +20,6 @@ Respanned: TokenStream [Ident { ident: "$crate", span: $DIR/auxiliary/make-macro use core /* 0#1 */::prelude /* 0#1 */::rust_2018 /* 0#1 */::*; #[macro_use /* 0#1 */] extern crate core /* 0#1 */; -extern crate compiler_builtins /* NNN */ as _ /* 0#1 */; // Don't load unnecessary hygiene information from std extern crate std /* 0#0 */; diff --git a/tests/ui/proc-macro/nonterminal-token-hygiene.stdout b/tests/ui/proc-macro/nonterminal-token-hygiene.stdout index e7dda7d3c1606..c80a33206fb4f 100644 --- a/tests/ui/proc-macro/nonterminal-token-hygiene.stdout +++ b/tests/ui/proc-macro/nonterminal-token-hygiene.stdout @@ -39,7 +39,6 @@ PRINT-BANG INPUT (DEBUG): TokenStream [ use ::core /* 0#1 */::prelude /* 0#1 */::rust_2015 /* 0#1 */::*; #[macro_use /* 0#1 */] extern crate core /* 0#2 */; -extern crate compiler_builtins /* NNN */ as _ /* 0#2 */; // Don't load unnecessary hygiene information from std extern crate std /* 0#0 */; From b6febfc01336719ab8b19cd97040f9866f947797 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Tue, 14 Jan 2025 20:04:59 +0000 Subject: [PATCH 4/8] Partially revert ed63539282 ("Mark dependencies ... private by default") Remove the portion of ed63539282 that automatically sets crates private based on whether they are dependencies of `std`. Instead, this is controlled by dependency configuration in `Cargo.toml`. --- compiler/rustc_metadata/src/creader.rs | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index e9643a287b0af..00eacc0df9d74 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -29,7 +29,7 @@ use rustc_session::lint::{self, BuiltinLintDiag}; use rustc_session::output::validate_crate_name; use rustc_session::search_paths::PathKind; use rustc_span::edition::Edition; -use rustc_span::{DUMMY_SP, Ident, STDLIB_STABLE_CRATES, Span, Symbol, sym}; +use rustc_span::{DUMMY_SP, Ident, Span, Symbol, sym}; use rustc_target::spec::{PanicStrategy, Target, TargetTuple}; use tracing::{debug, info, trace}; @@ -444,27 +444,12 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { private_dep: Option, origin: CrateOrigin<'_>, ) -> bool { - // Standard library crates are never private. - if STDLIB_STABLE_CRATES.contains(&name) { - tracing::info!("returning false for {name} is private"); - return false; - } - let extern_private = self.sess.opts.externs.get(name.as_str()).map(|e| e.is_private_dep); if matches!(origin, CrateOrigin::Injected) { return true; } - // Any descendants of `std` should be private. These crates are usually not marked - // private in metadata, so we ignore that field. - if extern_private.is_none() - && let Some(dep) = origin.dep_root() - && STDLIB_STABLE_CRATES.contains(&dep.name) - { - return true; - } - match (extern_private, private_dep) { // Explicit non-private via `--extern`, explicit non-private from metadata, or // unspecified with default to public. From 5a49b8d901192c22ce9cf3a5fbe661f73db2ba3e Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Tue, 28 Jan 2025 22:25:23 +0000 Subject: [PATCH 5/8] Use `public-dependencies` in all sysroot crates In [1], most dependencies of `std` and other sysroot crates were marked private, but this did not happen for `alloc` and `test`. Update these here, marking public standard library crates as the only non-private dependencies. [1]: https://github.com/rust-lang/rust/pull/111076 --- ...stdlib-Disable-f16-and-f128-in-compiler-builtins.patch | 2 +- library/alloc/Cargo.toml | 4 +++- library/sysroot/Cargo.toml | 8 +++++--- library/test/Cargo.toml | 6 ++++-- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/patches/0029-stdlib-Disable-f16-and-f128-in-compiler-builtins.patch b/compiler/rustc_codegen_cranelift/patches/0029-stdlib-Disable-f16-and-f128-in-compiler-builtins.patch index bf58e48515855..1f5da027a7e77 100644 --- a/compiler/rustc_codegen_cranelift/patches/0029-stdlib-Disable-f16-and-f128-in-compiler-builtins.patch +++ b/compiler/rustc_codegen_cranelift/patches/0029-stdlib-Disable-f16-and-f128-in-compiler-builtins.patch @@ -15,7 +15,7 @@ index 7165c3e48af..968552ad435 100644 edition = "2021" [dependencies] - core = { path = "../core" } + core = { path = "../core", public = true } -compiler_builtins = { version = "=0.1.143", features = ['rustc-dep-of-std'] } +compiler_builtins = { version = "=0.1.143", features = ['rustc-dep-of-std', 'no-f16-f128'] } diff --git a/library/alloc/Cargo.toml b/library/alloc/Cargo.toml index 96caac890a35c..182ec0a9b1cec 100644 --- a/library/alloc/Cargo.toml +++ b/library/alloc/Cargo.toml @@ -1,3 +1,5 @@ +cargo-features = ["public-dependency"] + [package] name = "alloc" version = "0.0.0" @@ -9,7 +11,7 @@ autobenches = false edition = "2021" [dependencies] -core = { path = "../core" } +core = { path = "../core", public = true } compiler_builtins = { version = "=0.1.143", features = ['rustc-dep-of-std'] } [dev-dependencies] diff --git a/library/sysroot/Cargo.toml b/library/sysroot/Cargo.toml index aa6c3dc32e2ba..45a9eda445dfb 100644 --- a/library/sysroot/Cargo.toml +++ b/library/sysroot/Cargo.toml @@ -1,3 +1,5 @@ +cargo-features = ["public-dependency"] + [package] name = "sysroot" version = "0.0.0" @@ -5,10 +7,10 @@ edition = "2021" # this is a dummy crate to ensure that all required crates appear in the sysroot [dependencies] -proc_macro = { path = "../proc_macro" } +proc_macro = { path = "../proc_macro", public = true } profiler_builtins = { path = "../profiler_builtins", optional = true } -std = { path = "../std" } -test = { path = "../test" } +std = { path = "../std", public = true } +test = { path = "../test", public = true } # Forward features to the `std` crate as necessary [features] diff --git a/library/test/Cargo.toml b/library/test/Cargo.toml index 75cc7c00e389c..241ef324b0088 100644 --- a/library/test/Cargo.toml +++ b/library/test/Cargo.toml @@ -1,3 +1,5 @@ +cargo-features = ["public-dependency"] + [package] name = "test" version = "0.0.0" @@ -5,8 +7,8 @@ edition = "2021" [dependencies] getopts = { version = "0.2.21", features = ['rustc-dep-of-std'] } -std = { path = "../std" } -core = { path = "../core" } +std = { path = "../std", public = true } +core = { path = "../core", public = true } [target.'cfg(not(all(windows, target_env = "msvc")))'.dependencies] libc = { version = "0.2.150", default-features = false } From 5d3f112495259ab7a02e6a242011603383f73ede Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Tue, 28 Jan 2025 23:20:36 +0000 Subject: [PATCH 6/8] If the parent dependency is private, treat dependents as private Currently, marking a dependency private does not automatically make all its child dependencies private. Resolve this by making its children private by default as well. This also resolves some FIXMEs for tests that are intended to fail but previously passed. [1]: https://github.com/rust-lang/rust/pull/135501#issuecomment-2620242419 --- compiler/rustc_metadata/src/creader.rs | 26 ++++++++++++++++--- .../pub-priv-dep/reexport_from_priv.rs | 3 +-- .../pub-priv-dep/reexport_from_priv.stderr | 14 ++++++++++ .../privacy/pub-priv-dep/shared_indirect.rs | 3 +-- .../pub-priv-dep/shared_indirect.stderr | 14 ++++++++++ 5 files changed, 52 insertions(+), 8 deletions(-) create mode 100644 tests/ui/privacy/pub-priv-dep/reexport_from_priv.stderr create mode 100644 tests/ui/privacy/pub-priv-dep/shared_indirect.stderr diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 00eacc0df9d74..e40179e15951a 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -163,7 +163,10 @@ impl<'a> std::fmt::Debug for CrateDump<'a> { enum CrateOrigin<'a> { /// This crate was a dependency of another crate. Dependency { + /// Where this dependency was included from. dep_root: &'a CratePaths, + /// True if the parent is private, meaning the dependent should also be private. + parent_private: bool, /// Dependency info about this crate. dep: &'a CrateDep, }, @@ -193,6 +196,17 @@ impl<'a> CrateOrigin<'a> { _ => None, } } + + /// `Some(true)` if the dependency is private or its parent is private, `Some(false)` if the + /// dependency is not private, `None` if it could not be determined. + fn private_dep(&self) -> Option { + match self { + CrateOrigin::Dependency { parent_private, dep, .. } => { + Some(dep.is_private || *parent_private) + } + _ => None, + } + } } impl CStore { @@ -497,7 +511,8 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { &crate_paths }; - let cnum_map = self.resolve_crate_deps(dep_root, &crate_root, &metadata, cnum, dep_kind)?; + let cnum_map = + self.resolve_crate_deps(dep_root, &crate_root, &metadata, cnum, dep_kind, private_dep)?; let raw_proc_macros = if crate_root.is_proc_macro_crate() { let temp_root; @@ -638,7 +653,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { let host_hash = dep.map(|d| d.host_hash).flatten(); let extra_filename = dep.map(|d| &d.extra_filename[..]); let path_kind = if dep.is_some() { PathKind::Dependency } else { PathKind::Crate }; - let private_dep = dep.map(|d| d.is_private); + let private_dep = origin.private_dep(); let result = if let Some(cnum) = self.existing_match(name, hash, path_kind) { (LoadResult::Previous(cnum), None) @@ -735,6 +750,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { metadata: &MetadataBlob, krate: CrateNum, dep_kind: CrateDepKind, + parent_is_private: bool, ) -> Result { debug!( "resolving deps of external crate `{}` with dep root `{}`", @@ -753,11 +769,12 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { crate_num_map.push(krate); for dep in deps { info!( - "resolving dep `{}`->`{}` hash: `{}` extra filename: `{}`", + "resolving dep `{}`->`{}` hash: `{}` extra filename: `{}` private {}", crate_root.name(), dep.name, dep.hash, - dep.extra_filename + dep.extra_filename, + dep.is_private, ); let dep_kind = match dep_kind { CrateDepKind::MacrosOnly => CrateDepKind::MacrosOnly, @@ -765,6 +782,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { }; let cnum = self.maybe_resolve_crate(dep.name, dep_kind, CrateOrigin::Dependency { dep_root, + parent_private: parent_is_private, dep: &dep, })?; crate_num_map.push(cnum); diff --git a/tests/ui/privacy/pub-priv-dep/reexport_from_priv.rs b/tests/ui/privacy/pub-priv-dep/reexport_from_priv.rs index 3c6e9825e7288..136e0ceeb5bec 100644 --- a/tests/ui/privacy/pub-priv-dep/reexport_from_priv.rs +++ b/tests/ui/privacy/pub-priv-dep/reexport_from_priv.rs @@ -1,6 +1,5 @@ //@ aux-crate:priv:reexport=reexport.rs //@ compile-flags: -Zunstable-options -//@ check-pass // Checks the behavior of a reexported item from a private dependency. @@ -9,7 +8,7 @@ extern crate reexport; -// FIXME: This should trigger. pub fn leaks_priv() -> reexport::Shared { + //~^ ERROR type `Shared` from private dependency 'shared' in public interface reexport::Shared } diff --git a/tests/ui/privacy/pub-priv-dep/reexport_from_priv.stderr b/tests/ui/privacy/pub-priv-dep/reexport_from_priv.stderr new file mode 100644 index 0000000000000..f1573283ff2e3 --- /dev/null +++ b/tests/ui/privacy/pub-priv-dep/reexport_from_priv.stderr @@ -0,0 +1,14 @@ +error: type `Shared` from private dependency 'shared' in public interface + --> $DIR/reexport_from_priv.rs:11:1 + | +LL | pub fn leaks_priv() -> reexport::Shared { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/reexport_from_priv.rs:7:9 + | +LL | #![deny(exported_private_dependencies)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/privacy/pub-priv-dep/shared_indirect.rs b/tests/ui/privacy/pub-priv-dep/shared_indirect.rs index 34b624b4a1a40..2fe66ae80cf48 100644 --- a/tests/ui/privacy/pub-priv-dep/shared_indirect.rs +++ b/tests/ui/privacy/pub-priv-dep/shared_indirect.rs @@ -1,7 +1,6 @@ //@ aux-crate:priv:shared=shared.rs //@ aux-crate:priv:indirect1=indirect1.rs //@ compile-flags: -Zunstable-options -//@ check-pass // A shared dependency, where it is only indirectly public. // @@ -23,7 +22,7 @@ extern crate shared; extern crate indirect1; -// FIXME: This should trigger. pub fn leaks_priv() -> shared::Shared { + //~^ ERROR type `Shared` from private dependency 'shared' in public interface shared::Shared } diff --git a/tests/ui/privacy/pub-priv-dep/shared_indirect.stderr b/tests/ui/privacy/pub-priv-dep/shared_indirect.stderr new file mode 100644 index 0000000000000..dbc534713d16e --- /dev/null +++ b/tests/ui/privacy/pub-priv-dep/shared_indirect.stderr @@ -0,0 +1,14 @@ +error: type `Shared` from private dependency 'shared' in public interface + --> $DIR/shared_indirect.rs:25:1 + | +LL | pub fn leaks_priv() -> shared::Shared { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/shared_indirect.rs:20:9 + | +LL | #![deny(exported_private_dependencies)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + From 35b5ed10fd0c87b57d4013b6587def9bcb8cede8 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Thu, 16 Jan 2025 01:47:15 +0000 Subject: [PATCH 7/8] Update `compiler-builtins` to 0.1.144 The change between 0.1.143 and 0.1.144 includes refactoring that was in compiler-builtins before, but had to be reverted before landing in rust-lang/rust because the traits were leaking into diagnostics [1]. Recently a fix for this issue was merged [2] so the cleanup is reapplied here. This also acts as a regression test for [2]. [1]: https://github.com/rust-lang/rust/pull/128691#issuecomment-2272297610 [2]: https://github.com/rust-lang/rust/pull/135278 --- ...029-stdlib-Disable-f16-and-f128-in-compiler-builtins.patch | 4 ++-- library/Cargo.lock | 4 ++-- library/alloc/Cargo.toml | 2 +- library/std/Cargo.toml | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/patches/0029-stdlib-Disable-f16-and-f128-in-compiler-builtins.patch b/compiler/rustc_codegen_cranelift/patches/0029-stdlib-Disable-f16-and-f128-in-compiler-builtins.patch index 1f5da027a7e77..0831bd34f16ae 100644 --- a/compiler/rustc_codegen_cranelift/patches/0029-stdlib-Disable-f16-and-f128-in-compiler-builtins.patch +++ b/compiler/rustc_codegen_cranelift/patches/0029-stdlib-Disable-f16-and-f128-in-compiler-builtins.patch @@ -16,8 +16,8 @@ index 7165c3e48af..968552ad435 100644 [dependencies] core = { path = "../core", public = true } --compiler_builtins = { version = "=0.1.143", features = ['rustc-dep-of-std'] } -+compiler_builtins = { version = "=0.1.143", features = ['rustc-dep-of-std', 'no-f16-f128'] } +-compiler_builtins = { version = "=0.1.144", features = ['rustc-dep-of-std'] } ++compiler_builtins = { version = "=0.1.144", features = ['rustc-dep-of-std', 'no-f16-f128'] } [dev-dependencies] rand = { version = "0.8.5", default-features = false, features = ["alloc"] } diff --git a/library/Cargo.lock b/library/Cargo.lock index 30875482dc06e..70cffa61b7076 100644 --- a/library/Cargo.lock +++ b/library/Cargo.lock @@ -61,9 +61,9 @@ dependencies = [ [[package]] name = "compiler_builtins" -version = "0.1.143" +version = "0.1.144" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c85ba2077e3eab3dd81be4ece6b7fb2ad0887c1fb813e9a45400baf75c6c7c29" +checksum = "d18a7b7b5a56aa131e62314b4d862c9f6aa2860f615f3770094ec9064d7ec572" dependencies = [ "cc", "rustc-std-workspace-core", diff --git a/library/alloc/Cargo.toml b/library/alloc/Cargo.toml index 182ec0a9b1cec..f64ebee7cb45d 100644 --- a/library/alloc/Cargo.toml +++ b/library/alloc/Cargo.toml @@ -12,7 +12,7 @@ edition = "2021" [dependencies] core = { path = "../core", public = true } -compiler_builtins = { version = "=0.1.143", features = ['rustc-dep-of-std'] } +compiler_builtins = { version = "=0.1.144", features = ['rustc-dep-of-std'] } [dev-dependencies] rand = { version = "0.8.5", default-features = false, features = ["alloc"] } diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index 9eab75b06961f..ce98e90222faa 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -17,7 +17,7 @@ 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", public = true } -compiler_builtins = { version = "=0.1.143" } +compiler_builtins = { version = "=0.1.144" } unwind = { path = "../unwind" } hashbrown = { version = "0.15", default-features = false, features = [ 'rustc-dep-of-std', From 530eaf1a20cc3fdde24115e506dd114a13458657 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Thu, 30 Jan 2025 02:52:39 +0000 Subject: [PATCH 8/8] Replace some instances of `pub` with `pub(crate)` The recent fixes to private dependencies exposed some cases in the UEFI module where private dependencies are exposed in a public interface. These do not need to be crate-public, so change them to `pub(crate)`. --- library/std/src/sys/pal/uefi/process.rs | 2 +- library/std/src/sys/pal/uefi/time.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/pal/uefi/process.rs b/library/std/src/sys/pal/uefi/process.rs index 3077a72eac661..84685915fe878 100644 --- a/library/std/src/sys/pal/uefi/process.rs +++ b/library/std/src/sys/pal/uefi/process.rs @@ -358,7 +358,7 @@ mod uefi_command_internal { } } - pub fn start_image(&mut self) -> io::Result { + pub(crate) fn start_image(&mut self) -> io::Result { self.update_st_crc32()?; // Use our system table instead of the default one diff --git a/library/std/src/sys/pal/uefi/time.rs b/library/std/src/sys/pal/uefi/time.rs index 495ff2dc930ed..c4ff3015ac60d 100644 --- a/library/std/src/sys/pal/uefi/time.rs +++ b/library/std/src/sys/pal/uefi/time.rs @@ -84,7 +84,7 @@ pub(crate) mod system_time_internal { // This algorithm is based on the one described in the post // https://blog.reverberate.org/2020/05/12/optimizing-date-algorithms.html - pub const fn uefi_time_to_duration(t: r_efi::system::Time) -> Duration { + pub(crate) const fn uefi_time_to_duration(t: r_efi::system::Time) -> Duration { assert!(t.month <= 12); assert!(t.month != 0);