-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Restrict sysroot crate imports to those defined in this repo. #143548
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -43,6 +43,7 @@ use std::fmt; | |||||||
pub use atomic_ref::AtomicRef; | ||||||||
pub use ena::{snapshot_vec, undo_log, unify}; | ||||||||
pub use rustc_index::static_assert_size; | ||||||||
pub use {either, indexmap, smallvec, thin_vec}; | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
or something like the comment on tracing? |
||||||||
|
||||||||
pub mod aligned; | ||||||||
pub mod base_n; | ||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -36,15 +36,15 @@ use std::str::{self, CharIndices}; | |||||||||||||
use std::sync::atomic::AtomicUsize; | ||||||||||||||
use std::sync::{Arc, Weak}; | ||||||||||||||
|
||||||||||||||
use pulldown_cmark::{ | ||||||||||||||
BrokenLink, CodeBlockKind, CowStr, Event, LinkType, Options, Parser, Tag, TagEnd, html, | ||||||||||||||
}; | ||||||||||||||
use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; | ||||||||||||||
use rustc_errors::{Diag, DiagMessage}; | ||||||||||||||
use rustc_hir::def_id::LocalDefId; | ||||||||||||||
use rustc_middle::ty::TyCtxt; | ||||||||||||||
pub(crate) use rustc_resolve::rustdoc::main_body_opts; | ||||||||||||||
use rustc_resolve::rustdoc::may_be_doc_link; | ||||||||||||||
use rustc_resolve::rustdoc::pulldown_cmark::{ | ||||||||||||||
self, BrokenLink, CodeBlockKind, CowStr, Event, LinkType, Options, Parser, Tag, TagEnd, html, | ||||||||||||||
}; | ||||||||||||||
use rustc_span::edition::Edition; | ||||||||||||||
use rustc_span::{Span, Symbol}; | ||||||||||||||
use tracing::{debug, trace}; | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are still tracing imports all over rustdoc... and indeed, tracing is oddly a cargo dependency here. That seems wrong? Tracing has a global singleton so rustc drivers should use the version from the sysroot, not their own, to ensure there's one shared logging system across the rustc core + driver. Probably best to do that in a separate PR, but maybe add a FIXME in the Cargo.toml to at least document this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rustdoc intentionally doesn't use rustc's copy of tracing to be able to enable trace logs for rustdoc without having to recompile rustc: Lines 165 to 170 in c720f49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, interesting. In Miri we use higher log levels to avoid that, since having two entirely separate loggers became quite annoying, but maybe that's less of a problem for rustdoc. |
||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this version compatibility still a concern?
I'm not sure what it's referring to, but I just want to make sure that removing this is not going to break something else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this comment in a previous PR as essentially a TODO for this PR. Now that crates are re-exported there's no need to pull them by name from the sysroot in order to ensure the versions match.