-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
diagnostics: exclude indirect private deps from trait impl suggest #111076
Conversation
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
This comment has been minimized.
This comment has been minimized.
8453818
to
8bf782f
Compare
This comment has been minimized.
This comment has been minimized.
e96a4b5
to
60ed017
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
06f0be5
to
2bffa4c
Compare
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.
The approach seems better than the previous one, but I'm not sure I fully understand what you're doing here since I don't really know the code in rustc_metadata
.
@@ -111,7 +111,7 @@ pub(crate) struct CrateMetadata { | |||
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, | |||
private_dep: Lock<bool>, |
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.
AtomicBool
should suffice here.
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.
Could you update the comment?
r? rust-lang/compiler |
@@ -111,7 +111,7 @@ pub(crate) struct CrateMetadata { | |||
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, | |||
private_dep: Lock<bool>, |
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.
Could you update the comment?
compiler/rustc_middle/src/ty/util.rs
Outdated
// | Yes | No | No | !(true && !false) | | ||
// | No | No | Yes | !(false && !false) | | ||
!(self.is_private_dep(key) | ||
&& !self.extern_crate(key.as_def_id()).expect("crate must exist").is_direct()) |
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.
This is not what None
means. See the doc on extern_crate
field in rustc_metadata::rmeta::decoder::CrateMetadata
.
☔ The latest upstream changes (presumably #111346) made this pull request unmergeable. Please resolve the merge conflicts. |
8482f07
to
394d31c
Compare
This comment has been minimized.
This comment has been minimized.
97264e2
to
fcee118
Compare
⌛ Testing commit 5fb752b with merge ee838d0ea3588d5441eb64196c145322243b5d00... |
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
@bors r+ |
☀️ Test successful - checks-actions |
Finished benchmarking commit (ad8304a): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 643.343s -> 642.658s (-0.11%) |
Hey, it looks like this PR breaks building the toolchain with the Cargo binaries specified in https://github.com/rust-lang/rust/blob/master/src/stage0.json#L28-L83 The public-dependencies feature is a nightly feature, but the Cargo given there is beta.
How do we continue building the toolchain in this state? Chromium bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1450894 |
Maybe answering for myself but it looks like specifying |
I'm not sure |
Since rust-lang#111076 enables unstable cargo feature (`public-dependency`), we need to ensure that unstable features are enabled before reading libstd Cargo.toml. Signed-off-by: onur-ozkan <work@onurozkan.dev>
enable unstable feature on `x clean [PATH]` Since rust-lang#111076 enables unstable cargo feature (`public-dependency`), we need to ensure that unstable features are enabled before reading libstd Cargo.toml. Fixes rust-lang#117762 cc `@Nilstrieb`
Fixes #88696