-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
diagnostics: do not suggest masked crate impls #109688
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
Changes from 2 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 |
|---|---|---|
|
|
@@ -17,10 +17,10 @@ use rustc_middle::query::LocalCrate; | |
| use rustc_middle::ty::fast_reject::SimplifiedType; | ||
| use rustc_middle::ty::query::{ExternProviders, Providers}; | ||
| use rustc_middle::ty::{self, TyCtxt}; | ||
| use rustc_session::cstore::CrateStore; | ||
| use rustc_session::cstore::{CrateStore, ExternCrateSource}; | ||
| use rustc_session::{Session, StableCrateId}; | ||
| use rustc_span::hygiene::{ExpnHash, ExpnId}; | ||
| use rustc_span::symbol::{kw, Symbol}; | ||
| use rustc_span::symbol::{kw, sym, Symbol}; | ||
| use rustc_span::Span; | ||
|
|
||
| use rustc_data_structures::sync::Lrc; | ||
|
|
@@ -355,6 +355,7 @@ provide! { tcx, def_id, other, cdata, | |
| expn_that_defined => { cdata.get_expn_that_defined(def_id.index, tcx.sess) } | ||
| generator_diagnostic_data => { cdata.get_generator_diagnostic_data(tcx, def_id.index) } | ||
| is_doc_hidden => { cdata.get_attr_flags(def_id.index).contains(AttrFlags::IS_DOC_HIDDEN) } | ||
| doc_masked_crates => { tcx.arena.alloc_from_iter(cdata.get_doc_masked_crates()) } | ||
| doc_link_resolutions => { tcx.arena.alloc(cdata.get_doc_link_resolutions(def_id.index)) } | ||
| doc_link_traits_in_scope => { | ||
| tcx.arena.alloc_from_iter(cdata.get_doc_link_traits_in_scope(def_id.index)) | ||
|
|
@@ -495,6 +496,25 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) { | |
| tcx.untracked().cstore.leak(); | ||
| tcx.arena.alloc_from_iter(CStore::from_tcx(tcx).iter_crate_data().map(|(cnum, _)| cnum)) | ||
| }, | ||
| doc_masked_crates: |tcx, LocalCrate| { | ||
|
Contributor
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. Can you put this functionality in a function and only call that function in the closure here, please? I think keeping the
Contributor
Author
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. Okay. It's done. |
||
| let mut v: Vec<CrateNum> = CStore::from_tcx(tcx) | ||
| .iter_crate_data() | ||
| .filter_map(|(cnum, cdata)| { | ||
| if let Some(extern_crate) = *cdata.extern_crate.lock() && | ||
| let ExternCrateSource::Extern(did) = extern_crate.src && | ||
| tcx | ||
| .get_attrs(did, sym::doc) | ||
| .filter_map(|attr| attr.meta_item_list()) | ||
| .any(|items| items.iter().any(|item| item.has_name(sym::masked))) { | ||
| Some(cnum) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| .collect(); | ||
| v.sort_unstable(); | ||
| tcx.arena.alloc_from_iter(v) | ||
| }, | ||
| ..*providers | ||
| }; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2006,6 +2006,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { | |
| || !trait_pred | ||
| .skip_binder() | ||
| .is_constness_satisfied_by(self.tcx.constness(def_id)) | ||
| || self.tcx.is_doc_masked(def_id.krate) | ||
|
Contributor
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. hm I'm not sure whether relying on an attribute that seems to solely be used in rustdoc inside the trait system is such a good idea. 🤔 But I can't think of a better way to filter for crates we want to ignore here either, so I guess this is fine. Still can you put a
Contributor
Author
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. Okay, FIXME added. |
||
| { | ||
| return None; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| // derived from miniz_oxide | ||
|
|
||
| pub enum MZStatus {} | ||
| pub enum MZError {} | ||
|
|
||
| pub struct StreamResult; | ||
|
|
||
| pub type MZResult = Result<MZStatus, MZError>; | ||
|
|
||
| impl core::convert::From<StreamResult> for MZResult { | ||
| fn from(res: StreamResult) -> Self { | ||
| loop {} | ||
| } | ||
| } | ||
|
|
||
| impl core::convert::From<&StreamResult> for MZResult { | ||
| fn from(res: &StreamResult) -> Self { | ||
| loop {} | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| // aux-build:masked-crate.rs | ||
| // | ||
| // This test case should ensure that miniz_oxide isn't | ||
| // suggested, since it's not a direct dependency. | ||
|
|
||
| #![feature(doc_masked)] | ||
|
|
||
| #[doc(masked)] | ||
| extern crate masked_crate; | ||
|
Contributor
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. I don't understand why you include this
Contributor
Author
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. I wanted to make sure that this test case didn't rely on the specifics of how libstd is implemented right now, so I copied the relevant parts of miniz_oxide into the test case. When run without the fix, the error message lists both miniz_oxide and masked_crate. |
||
|
|
||
| fn a() -> Result<u64, i32> { | ||
| Err(1) | ||
| } | ||
|
|
||
| fn b() -> Result<u32, i32> { | ||
| a().into() //~ERROR [E0277] | ||
| } | ||
|
|
||
| fn main() { | ||
| let _ = dbg!(b()); | ||
| // make sure crate actually gets loaded | ||
| let _ = masked_crate::StreamResult; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| error[E0277]: the trait bound `Result<u32, i32>: From<Result<u64, i32>>` is not satisfied | ||
| --> $DIR/issue-88696.rs:16:9 | ||
| | | ||
| LL | a().into() | ||
| | ^^^^ the trait `From<Result<u64, i32>>` is not implemented for `Result<u32, i32>` | ||
| | | ||
| = note: required for `Result<u64, i32>` to implement `Into<Result<u32, i32>>` | ||
|
|
||
| error: aborting due to previous error | ||
|
|
||
| For more information about this error, try `rustc --explain E0277`. |
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.
Why do we need to sort 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.
I added a comment describing why it needs sorted. It's so that binary search can be used later.