From 879f8de4096b2db4769e64e4c1af5ffb10b53a22 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 2 May 2023 14:35:34 +0200 Subject: [PATCH 01/11] Correctly handle associated items of a trait inside a `#[doc(hidden)]` item --- src/librustdoc/formats/cache.rs | 8 ++++++- src/librustdoc/passes/strip_hidden.rs | 30 ++++++++++++++++++++++----- src/librustdoc/visit_ast.rs | 11 +++------- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 841abfab666bf..c0730e90740eb 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -195,7 +195,13 @@ impl Cache { impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { fn fold_item(&mut self, item: clean::Item) -> Option { if item.item_id.is_local() { - debug!("folding {} \"{:?}\", id {:?}", item.type_(), item.name, item.item_id); + let is_stripped = matches!(*item.kind, clean::ItemKind::StrippedItem(..)); + debug!( + "folding {} (stripped: {is_stripped:?}) \"{:?}\", id {:?}", + item.type_(), + item.name, + item.item_id + ); } // If this is a stripped module, diff --git a/src/librustdoc/passes/strip_hidden.rs b/src/librustdoc/passes/strip_hidden.rs index a688aa14863a9..972b0c5ec190e 100644 --- a/src/librustdoc/passes/strip_hidden.rs +++ b/src/librustdoc/passes/strip_hidden.rs @@ -1,5 +1,6 @@ //! Strip all doc(hidden) items from the output. +use rustc_hir::def_id::LocalDefId; use rustc_middle::ty::TyCtxt; use rustc_span::symbol::sym; use std::mem; @@ -29,6 +30,7 @@ pub(crate) fn strip_hidden(krate: clean::Crate, cx: &mut DocContext<'_>) -> clea update_retained: true, tcx: cx.tcx, is_in_hidden_item: false, + last_reexport: None, }; stripper.fold_crate(krate) }; @@ -49,13 +51,24 @@ struct Stripper<'a, 'tcx> { update_retained: bool, tcx: TyCtxt<'tcx>, is_in_hidden_item: bool, + last_reexport: Option, } impl<'a, 'tcx> Stripper<'a, 'tcx> { + fn set_last_reexport_then_fold_item(&mut self, i: Item) -> Item { + let prev_from_reexport = self.last_reexport; + if i.inline_stmt_id.is_some() { + self.last_reexport = i.item_id.as_def_id().and_then(|def_id| def_id.as_local()); + } + let ret = self.fold_item_recur(i); + self.last_reexport = prev_from_reexport; + ret + } + fn set_is_in_hidden_item_and_fold(&mut self, is_in_hidden_item: bool, i: Item) -> Item { let prev = self.is_in_hidden_item; self.is_in_hidden_item |= is_in_hidden_item; - let ret = self.fold_item_recur(i); + let ret = self.set_last_reexport_then_fold_item(i); self.is_in_hidden_item = prev; ret } @@ -64,7 +77,7 @@ impl<'a, 'tcx> Stripper<'a, 'tcx> { /// of `is_in_hidden_item` to `true` because the impl children inherit its visibility. fn recurse_in_impl_or_exported_macro(&mut self, i: Item) -> Item { let prev = mem::replace(&mut self.is_in_hidden_item, false); - let ret = self.fold_item_recur(i); + let ret = self.set_last_reexport_then_fold_item(i); self.is_in_hidden_item = prev; ret } @@ -86,13 +99,20 @@ impl<'a, 'tcx> DocFolder for Stripper<'a, 'tcx> { if !is_impl_or_exported_macro { is_hidden = self.is_in_hidden_item || has_doc_hidden; if !is_hidden && i.inline_stmt_id.is_none() { - // We don't need to check if it's coming from a reexport since the reexport itself was - // already checked. + // `i.inline_stmt_id` is `Some` if the item is directly reexported. If it is, we + // don't need to check it, because the reexport itself was already checked. + // + // If this item is the child of a reexported module, `self.last_reexport` will be + // `Some` even though `i.inline_stmt_id` is `None`. Hiddenness inheritance needs to + // account for the possibility that an item's true parent module is hidden, but it's + // inlined into a visible module true. This code shouldn't be reachable if the + // module's reexport is itself hidden, for the same reason it doesn't need to be + // checked if `i.inline_stmt_id` is Some: hidden reexports are never inlined. is_hidden = i .item_id .as_def_id() .and_then(|def_id| def_id.as_local()) - .map(|def_id| inherits_doc_hidden(self.tcx, def_id)) + .map(|def_id| inherits_doc_hidden(self.tcx, def_id, self.last_reexport)) .unwrap_or(false); } } diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index a6089680fae9d..a6c2404138052 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -500,19 +500,14 @@ impl<'a, 'tcx> Visitor<'tcx> for RustdocVisitor<'a, 'tcx> { fn visit_item(&mut self, i: &'tcx hir::Item<'tcx>) { self.visit_item_inner(i, None, None); - let new_value = if self.is_importable_from_parent { - matches!( + let new_value = self.is_importable_from_parent + && matches!( i.kind, hir::ItemKind::Mod(..) | hir::ItemKind::ForeignMod { .. } | hir::ItemKind::Impl(..) | hir::ItemKind::Trait(..) - ) - } else { - // Whatever the context, if it's an impl block, the items inside it can be used so they - // should be visible. - matches!(i.kind, hir::ItemKind::Impl(..)) - }; + ); let prev = mem::replace(&mut self.is_importable_from_parent, new_value); walk_item(self, i); self.is_importable_from_parent = prev; From fb160d5d3b30ad4a522149d309002fd76137b048 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 5 May 2023 15:32:56 +0200 Subject: [PATCH 02/11] Modules can be reexported and it should be handled by rustdoc --- src/librustdoc/clean/mod.rs | 23 ++++++++++- .../passes/check_doc_test_visibility.rs | 2 +- src/librustdoc/visit_ast.rs | 40 +++++++++++++++---- 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 8ccdb16b784ef..d21a8a5477f7a 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -119,7 +119,28 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext< }); let kind = ModuleItem(Module { items, span }); - Item::from_def_id_and_parts(doc.def_id.to_def_id(), Some(doc.name), kind, cx) + let def_id = doc.def_id.to_def_id(); + let target_attrs = inline::load_attrs(cx, def_id); + let attrs = if let Some(import_id) = doc.import_id { + let is_inline = inline::load_attrs(cx, import_id.to_def_id()) + .lists(sym::doc) + .get_word_attr(sym::inline) + .is_some(); + let mut attrs = get_all_import_attributes(cx, import_id, doc.def_id, is_inline); + add_without_unwanted_attributes(&mut attrs, target_attrs, is_inline, None); + attrs + } else { + // We only keep the item's attributes. + target_attrs.iter().map(|attr| (Cow::Borrowed(attr), None)).collect() + }; + + let cfg = attrs.cfg(cx.tcx, &cx.cache.hidden_cfg); + let attrs = Attributes::from_ast_iter(attrs.iter().map(|(attr, did)| (&**attr, *did)), false); + + let name = doc.renamed.or_else(|| Some(doc.name)); + let mut item = Item::from_def_id_and_attrs_and_parts(def_id, name, kind, Box::new(attrs), cfg); + item.inline_stmt_id = doc.import_id.map(|local| local.to_def_id()); + item } fn clean_generic_bound<'tcx>( diff --git a/src/librustdoc/passes/check_doc_test_visibility.rs b/src/librustdoc/passes/check_doc_test_visibility.rs index 6b13e6c9581ae..10295cbd189b1 100644 --- a/src/librustdoc/passes/check_doc_test_visibility.rs +++ b/src/librustdoc/passes/check_doc_test_visibility.rs @@ -95,7 +95,7 @@ pub(crate) fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) - } if cx.tcx.is_doc_hidden(def_id.to_def_id()) - || inherits_doc_hidden(cx.tcx, def_id) + || inherits_doc_hidden(cx.tcx, def_id, None) || cx.tcx.def_span(def_id.to_def_id()).in_derive_expansion() { return false; diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index a6c2404138052..f7c525042c2aa 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -27,6 +27,8 @@ pub(crate) struct Module<'hir> { pub(crate) where_inner: Span, pub(crate) mods: Vec>, pub(crate) def_id: LocalDefId, + pub(crate) renamed: Option, + pub(crate) import_id: Option, /// The key is the item `ItemId` and the value is: (item, renamed, import_id). /// We use `FxIndexMap` to keep the insert order. pub(crate) items: FxIndexMap< @@ -37,11 +39,19 @@ pub(crate) struct Module<'hir> { } impl Module<'_> { - pub(crate) fn new(name: Symbol, def_id: LocalDefId, where_inner: Span) -> Self { + pub(crate) fn new( + name: Symbol, + def_id: LocalDefId, + where_inner: Span, + renamed: Option, + import_id: Option, + ) -> Self { Module { name, def_id, where_inner, + renamed, + import_id, mods: Vec::new(), items: FxIndexMap::default(), foreigns: Vec::new(), @@ -60,9 +70,16 @@ fn def_id_to_path(tcx: TyCtxt<'_>, did: DefId) -> Vec { std::iter::once(crate_name).chain(relative).collect() } -pub(crate) fn inherits_doc_hidden(tcx: TyCtxt<'_>, mut def_id: LocalDefId) -> bool { +pub(crate) fn inherits_doc_hidden( + tcx: TyCtxt<'_>, + mut def_id: LocalDefId, + stop_at: Option, +) -> bool { let hir = tcx.hir(); while let Some(id) = tcx.opt_local_parent(def_id) { + if let Some(stop_at) = stop_at && id == stop_at { + return false; + } def_id = id; if tcx.is_doc_hidden(def_id.to_def_id()) { return true; @@ -100,6 +117,8 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { cx.tcx.crate_name(LOCAL_CRATE), CRATE_DEF_ID, cx.tcx.hir().root_module().spans.inner_span, + None, + None, ); RustdocVisitor { @@ -260,7 +279,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { let is_private = !self.cx.cache.effective_visibilities.is_directly_public(self.cx.tcx, ori_res_did); - let is_hidden = inherits_doc_hidden(self.cx.tcx, res_did); + let is_hidden = inherits_doc_hidden(self.cx.tcx, res_did, None); // Only inline if requested or if the item would otherwise be stripped. if (!please_inline && !is_private && !is_hidden) || is_no_inline { @@ -277,7 +296,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { .cache .effective_visibilities .is_directly_public(self.cx.tcx, item_def_id.to_def_id()) && - !inherits_doc_hidden(self.cx.tcx, item_def_id) + !inherits_doc_hidden(self.cx.tcx, item_def_id, None) { // The imported item is public and not `doc(hidden)` so no need to inline it. return false; @@ -426,7 +445,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { } } hir::ItemKind::Mod(ref m) => { - self.enter_mod(item.owner_id.def_id, m, name); + self.enter_mod(item.owner_id.def_id, m, name, renamed, import_id); } hir::ItemKind::Fn(..) | hir::ItemKind::ExternCrate(..) @@ -479,8 +498,15 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { /// This method will create a new module and push it onto the "modules stack" then call /// `visit_mod_contents`. Once done, it'll remove it from the "modules stack" and instead /// add into the list of modules of the current module. - fn enter_mod(&mut self, id: LocalDefId, m: &'tcx hir::Mod<'tcx>, name: Symbol) { - self.modules.push(Module::new(name, id, m.spans.inner_span)); + fn enter_mod( + &mut self, + id: LocalDefId, + m: &'tcx hir::Mod<'tcx>, + name: Symbol, + renamed: Option, + import_id: Option, + ) { + self.modules.push(Module::new(name, id, m.spans.inner_span, renamed, import_id)); self.visit_mod_contents(id, m); From cbc6daa5597916c12ee24a6870f7611adf989878 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 5 May 2023 15:59:41 +0200 Subject: [PATCH 03/11] Improve code to remove duplication --- src/librustdoc/clean/mod.rs | 52 +++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index d21a8a5477f7a..53b631b986d6f 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -119,14 +119,25 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext< }); let kind = ModuleItem(Module { items, span }); - let def_id = doc.def_id.to_def_id(); + generate_item_with_correct_attrs(cx, kind, doc.def_id, doc.name, doc.import_id, doc.renamed) +} + +fn generate_item_with_correct_attrs( + cx: &mut DocContext<'_>, + kind: ItemKind, + local_def_id: LocalDefId, + name: Symbol, + import_id: Option, + renamed: Option, +) -> Item { + let def_id = local_def_id.to_def_id(); let target_attrs = inline::load_attrs(cx, def_id); - let attrs = if let Some(import_id) = doc.import_id { + let attrs = if let Some(import_id) = import_id { let is_inline = inline::load_attrs(cx, import_id.to_def_id()) .lists(sym::doc) .get_word_attr(sym::inline) .is_some(); - let mut attrs = get_all_import_attributes(cx, import_id, doc.def_id, is_inline); + let mut attrs = get_all_import_attributes(cx, import_id, local_def_id, is_inline); add_without_unwanted_attributes(&mut attrs, target_attrs, is_inline, None); attrs } else { @@ -137,9 +148,9 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext< let cfg = attrs.cfg(cx.tcx, &cx.cache.hidden_cfg); let attrs = Attributes::from_ast_iter(attrs.iter().map(|(attr, did)| (&**attr, *did)), false); - let name = doc.renamed.or_else(|| Some(doc.name)); + let name = renamed.or(Some(name)); let mut item = Item::from_def_id_and_attrs_and_parts(def_id, name, kind, Box::new(attrs), cfg); - item.inline_stmt_id = doc.import_id.map(|local| local.to_def_id()); + item.inline_stmt_id = import_id.map(|local| local.to_def_id()); item } @@ -2309,29 +2320,14 @@ fn clean_maybe_renamed_item<'tcx>( _ => unreachable!("not yet converted"), }; - let target_attrs = inline::load_attrs(cx, def_id); - let attrs = if let Some(import_id) = import_id { - let is_inline = inline::load_attrs(cx, import_id.to_def_id()) - .lists(sym::doc) - .get_word_attr(sym::inline) - .is_some(); - let mut attrs = - get_all_import_attributes(cx, import_id, item.owner_id.def_id, is_inline); - add_without_unwanted_attributes(&mut attrs, target_attrs, is_inline, None); - attrs - } else { - // We only keep the item's attributes. - target_attrs.iter().map(|attr| (Cow::Borrowed(attr), None)).collect() - }; - - let cfg = attrs.cfg(cx.tcx, &cx.cache.hidden_cfg); - let attrs = - Attributes::from_ast_iter(attrs.iter().map(|(attr, did)| (&**attr, *did)), false); - - let mut item = - Item::from_def_id_and_attrs_and_parts(def_id, Some(name), kind, Box::new(attrs), cfg); - item.inline_stmt_id = import_id.map(|local| local.to_def_id()); - vec![item] + vec![generate_item_with_correct_attrs( + cx, + kind, + item.owner_id.def_id, + name, + import_id, + renamed, + )] }) } From 8de4308b43e9694384d1292f22aef384dc44a5df Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 2 May 2023 14:35:53 +0200 Subject: [PATCH 04/11] Add regression test for #111064 --- ...sue-111064-reexport-trait-from-hidden-2.rs | 31 +++++++++++++++++++ ...issue-111064-reexport-trait-from-hidden.rs | 21 +++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 tests/rustdoc/issue-111064-reexport-trait-from-hidden-2.rs create mode 100644 tests/rustdoc/issue-111064-reexport-trait-from-hidden.rs diff --git a/tests/rustdoc/issue-111064-reexport-trait-from-hidden-2.rs b/tests/rustdoc/issue-111064-reexport-trait-from-hidden-2.rs new file mode 100644 index 0000000000000..8e1029a1ca3df --- /dev/null +++ b/tests/rustdoc/issue-111064-reexport-trait-from-hidden-2.rs @@ -0,0 +1,31 @@ +#![feature(no_core)] +#![no_core] +#![crate_name = "foo"] + +// @!has 'foo/hidden/index.html' +// FIXME: add missing `@` for the two next tests once issue is fixed! +// To be done in . +// !has 'foo/hidden/inner/index.html' +// !has 'foo/hidden/inner/trait.Foo.html' +#[doc(hidden)] +pub mod hidden { + pub mod inner { + pub trait Foo { + /// Hello, world! + fn test(); + } + } +} + +// @has 'foo/visible/index.html' +// @has 'foo/visible/trait.Foo.html' +#[doc(inline)] +pub use hidden::inner as visible; + +// @has 'foo/struct.Bar.html' +// @count - '//*[@id="impl-Foo-for-Bar"]' 1 +pub struct Bar; + +impl visible::Foo for Bar { + fn test() {} +} diff --git a/tests/rustdoc/issue-111064-reexport-trait-from-hidden.rs b/tests/rustdoc/issue-111064-reexport-trait-from-hidden.rs new file mode 100644 index 0000000000000..a9ce4a34507e6 --- /dev/null +++ b/tests/rustdoc/issue-111064-reexport-trait-from-hidden.rs @@ -0,0 +1,21 @@ +// Regression test for . +// Methods from a re-exported trait inside a `#[doc(hidden)]` item should +// be visible. + +#![crate_name = "foo"] + +// @has 'foo/index.html' +// @has - '//*[@id="main-content"]//*[@class="item-name"]/a[@href="trait.Foo.html"]' 'Foo' + +// @has 'foo/trait.Foo.html' +// @has - '//*[@id="main-content"]//*[@class="code-header"]' 'fn test()' + +#[doc(hidden)] +mod hidden { + pub trait Foo { + /// Hello, world! + fn test(); + } +} + +pub use hidden::Foo; From 7f74ae57e8e24e87b7177af43db6ec0e8262cd5d Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 24 Apr 2023 00:47:01 +0000 Subject: [PATCH 05/11] Create a trait to abstract over the smir API --- compiler/rustc_smir/src/lib.rs | 2 + compiler/rustc_smir/src/rustc_internal/mod.rs | 45 +++++++--- compiler/rustc_smir/src/rustc_smir/mod.rs | 86 +++++++++---------- compiler/rustc_smir/src/stable_mir/mod.rs | 66 ++++++++++++-- tests/ui-fulldeps/stable-mir/crate-info.rs | 2 +- 5 files changed, 137 insertions(+), 64 deletions(-) diff --git a/compiler/rustc_smir/src/lib.rs b/compiler/rustc_smir/src/lib.rs index 54d474db038e9..b00f0a1c15312 100644 --- a/compiler/rustc_smir/src/lib.rs +++ b/compiler/rustc_smir/src/lib.rs @@ -11,6 +11,8 @@ test(attr(allow(unused_variables), deny(warnings))) )] #![cfg_attr(not(feature = "default"), feature(rustc_private))] +#![feature(local_key_cell_methods)] +#![feature(ptr_metadata)] pub mod rustc_internal; pub mod stable_mir; diff --git a/compiler/rustc_smir/src/rustc_internal/mod.rs b/compiler/rustc_smir/src/rustc_internal/mod.rs index 5998c8b650045..e643ad28c0326 100644 --- a/compiler/rustc_smir/src/rustc_internal/mod.rs +++ b/compiler/rustc_smir/src/rustc_internal/mod.rs @@ -3,30 +3,49 @@ //! For that, we define APIs that will temporarily be public to 3P that exposes rustc internal APIs //! until stable MIR is complete. -use std::sync::RwLock; - -use crate::stable_mir; +use crate::{ + rustc_smir::Tables, + stable_mir::{self, with}, +}; +use rustc_middle::ty::TyCtxt; pub use rustc_span::def_id::{CrateNum, DefId}; -static DEF_ID_MAP: RwLock> = RwLock::new(Vec::new()); +fn with_tables(mut f: impl FnMut(&mut Tables<'_>) -> R) -> R { + let mut ret = None; + with(|tables| tables.rustc_tables(&mut |t| ret = Some(f(t)))); + ret.unwrap() +} pub fn item_def_id(item: &stable_mir::CrateItem) -> DefId { - DEF_ID_MAP.read().unwrap()[item.0] + with_tables(|t| t.item_def_id(item)) } pub fn crate_item(did: DefId) -> stable_mir::CrateItem { - // FIXME: this becomes inefficient when we have too many ids - let mut map = DEF_ID_MAP.write().unwrap(); - for (i, &d) in map.iter().enumerate() { - if d == did { - return stable_mir::CrateItem(i); + with_tables(|t| t.crate_item(did)) +} + +impl<'tcx> Tables<'tcx> { + pub fn item_def_id(&self, item: &stable_mir::CrateItem) -> DefId { + self.def_ids[item.0] + } + + pub fn crate_item(&mut self, did: DefId) -> stable_mir::CrateItem { + // FIXME: this becomes inefficient when we have too many ids + for (i, &d) in self.def_ids.iter().enumerate() { + if d == did { + return stable_mir::CrateItem(i); + } } + let id = self.def_ids.len(); + self.def_ids.push(did); + stable_mir::CrateItem(id) } - let id = map.len(); - map.push(did); - stable_mir::CrateItem(id) } pub fn crate_num(item: &stable_mir::Crate) -> CrateNum { item.id.into() } + +pub fn run(tcx: TyCtxt<'_>, f: impl FnOnce()) { + crate::stable_mir::run(Tables { tcx, def_ids: vec![] }, f); +} diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index 241cd182059ba..81ecce415d553 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -7,55 +7,36 @@ //! //! For now, we are developing everything inside `rustc`, thus, we keep this module private. -use crate::{ - rustc_internal::{crate_item, item_def_id}, - stable_mir::{self}, -}; -use rustc_middle::ty::{tls::with, TyCtxt}; -use rustc_span::def_id::{CrateNum, LOCAL_CRATE}; +use crate::stable_mir::{self, Context}; +use rustc_middle::ty::TyCtxt; +use rustc_span::def_id::{CrateNum, DefId, LOCAL_CRATE}; use tracing::debug; -/// Get information about the local crate. -pub fn local_crate() -> stable_mir::Crate { - with(|tcx| smir_crate(tcx, LOCAL_CRATE)) -} +impl<'tcx> Context for Tables<'tcx> { + fn local_crate(&self) -> stable_mir::Crate { + smir_crate(self.tcx, LOCAL_CRATE) + } -/// Retrieve a list of all external crates. -pub fn external_crates() -> Vec { - with(|tcx| tcx.crates(()).iter().map(|crate_num| smir_crate(tcx, *crate_num)).collect()) -} + fn external_crates(&self) -> Vec { + self.tcx.crates(()).iter().map(|crate_num| smir_crate(self.tcx, *crate_num)).collect() + } -/// Find a crate with the given name. -pub fn find_crate(name: &str) -> Option { - with(|tcx| { - [LOCAL_CRATE].iter().chain(tcx.crates(()).iter()).find_map(|crate_num| { - let crate_name = tcx.crate_name(*crate_num).to_string(); - (name == crate_name).then(|| smir_crate(tcx, *crate_num)) + fn find_crate(&self, name: &str) -> Option { + [LOCAL_CRATE].iter().chain(self.tcx.crates(()).iter()).find_map(|crate_num| { + let crate_name = self.tcx.crate_name(*crate_num).to_string(); + (name == crate_name).then(|| smir_crate(self.tcx, *crate_num)) }) - }) -} - -/// Retrieve all items of the local crate that have a MIR associated with them. -pub fn all_local_items() -> stable_mir::CrateItems { - with(|tcx| tcx.mir_keys(()).iter().map(|item| crate_item(item.to_def_id())).collect()) -} - -pub fn entry_fn() -> Option { - with(|tcx| Some(crate_item(tcx.entry_fn(())?.0))) -} - -/// Build a stable mir crate from a given crate number. -fn smir_crate(tcx: TyCtxt<'_>, crate_num: CrateNum) -> stable_mir::Crate { - let crate_name = tcx.crate_name(crate_num).to_string(); - let is_local = crate_num == LOCAL_CRATE; - debug!(?crate_name, ?crate_num, "smir_crate"); - stable_mir::Crate { id: crate_num.into(), name: crate_name, is_local } -} + } -pub fn mir_body(item: &stable_mir::CrateItem) -> stable_mir::mir::Body { - with(|tcx| { - let def_id = item_def_id(item); - let mir = tcx.optimized_mir(def_id); + fn all_local_items(&mut self) -> stable_mir::CrateItems { + self.tcx.mir_keys(()).iter().map(|item| self.crate_item(item.to_def_id())).collect() + } + fn entry_fn(&mut self) -> Option { + Some(self.crate_item(self.tcx.entry_fn(())?.0)) + } + fn mir_body(&self, item: &stable_mir::CrateItem) -> stable_mir::mir::Body { + let def_id = self.item_def_id(item); + let mir = self.tcx.optimized_mir(def_id); stable_mir::mir::Body { blocks: mir .basic_blocks @@ -66,7 +47,24 @@ pub fn mir_body(item: &stable_mir::CrateItem) -> stable_mir::mir::Body { }) .collect(), } - }) + } + + fn rustc_tables(&mut self, f: &mut dyn FnMut(&mut Tables<'_>)) { + f(self) + } +} + +pub struct Tables<'tcx> { + pub tcx: TyCtxt<'tcx>, + pub def_ids: Vec, +} + +/// Build a stable mir crate from a given crate number. +fn smir_crate(tcx: TyCtxt<'_>, crate_num: CrateNum) -> stable_mir::Crate { + let crate_name = tcx.crate_name(crate_num).to_string(); + let is_local = crate_num == LOCAL_CRATE; + debug!(?crate_name, ?crate_num, "smir_crate"); + stable_mir::Crate { id: crate_num.into(), name: crate_name, is_local } } fn rustc_statement_to_statement( diff --git a/compiler/rustc_smir/src/stable_mir/mod.rs b/compiler/rustc_smir/src/stable_mir/mod.rs index 1d2efb5eab947..612777b9c7539 100644 --- a/compiler/rustc_smir/src/stable_mir/mod.rs +++ b/compiler/rustc_smir/src/stable_mir/mod.rs @@ -11,7 +11,14 @@ //! There shouldn't be any direct references to internal compiler constructs in this module. //! If you need an internal construct, consider using `rustc_internal` or `rustc_smir`. +use std::cell::Cell; + +use crate::rustc_smir::Tables; + +use self::ty::{Ty, TyKind}; + pub mod mir; +pub mod ty; /// Use String for now but we should replace it. pub type Symbol = String; @@ -41,7 +48,7 @@ pub struct CrateItem(pub(crate) DefId); impl CrateItem { pub fn body(&self) -> mir::Body { - crate::rustc_smir::mir_body(self) + with(|cx| cx.mir_body(self)) } } @@ -49,25 +56,72 @@ impl CrateItem { /// crate defines that. This is usually `main`, but could be /// `start` if the crate is a no-std crate. pub fn entry_fn() -> Option { - crate::rustc_smir::entry_fn() + with(|cx| cx.entry_fn()) } /// Access to the local crate. pub fn local_crate() -> Crate { - crate::rustc_smir::local_crate() + with(|cx| cx.local_crate()) } /// Try to find a crate with the given name. pub fn find_crate(name: &str) -> Option { - crate::rustc_smir::find_crate(name) + with(|cx| cx.find_crate(name)) } /// Try to find a crate with the given name. pub fn external_crates() -> Vec { - crate::rustc_smir::external_crates() + with(|cx| cx.external_crates()) } /// Retrieve all items in the local crate that have a MIR associated with them. pub fn all_local_items() -> CrateItems { - crate::rustc_smir::all_local_items() + with(|cx| cx.all_local_items()) +} + +pub trait Context { + fn entry_fn(&mut self) -> Option; + /// Retrieve all items of the local crate that have a MIR associated with them. + fn all_local_items(&mut self) -> CrateItems; + fn mir_body(&mut self, item: &CrateItem) -> mir::Body; + /// Get information about the local crate. + fn local_crate(&self) -> Crate; + /// Retrieve a list of all external crates. + fn external_crates(&self) -> Vec; + + /// Find a crate with the given name. + fn find_crate(&self, name: &str) -> Option; + + /// Obtain the representation of a type. + fn ty_kind(&mut self, ty: Ty) -> TyKind; + + /// HACK: Until we have fully stable consumers, we need an escape hatch + /// to get `DefId`s out of `CrateItem`s. + fn rustc_tables(&mut self, f: &mut dyn FnMut(&mut Tables<'_>)); +} + +thread_local! { + /// A thread local variable that stores a pointer to the tables mapping between TyCtxt + /// datastructures and stable MIR datastructures. + static TLV: Cell<*mut ()> = const { Cell::new(std::ptr::null_mut()) }; +} + +pub fn run(mut context: impl Context, f: impl FnOnce()) { + assert!(TLV.get().is_null()); + fn g<'a>(mut context: &mut (dyn Context + 'a), f: impl FnOnce()) { + TLV.set(&mut context as *mut &mut _ as _); + f(); + TLV.replace(std::ptr::null_mut()); + } + g(&mut context, f); +} + +/// Loads the current context and calls a function with it. +/// Do not nest these, as that will ICE. +pub(crate) fn with(f: impl FnOnce(&mut dyn Context) -> R) -> R { + let ptr = TLV.replace(std::ptr::null_mut()) as *mut &mut dyn Context; + assert!(!ptr.is_null()); + let ret = f(unsafe { *ptr }); + TLV.set(ptr as _); + ret } diff --git a/tests/ui-fulldeps/stable-mir/crate-info.rs b/tests/ui-fulldeps/stable-mir/crate-info.rs index 1454d6dde6c97..b0fcc36b0a805 100644 --- a/tests/ui-fulldeps/stable-mir/crate-info.rs +++ b/tests/ui-fulldeps/stable-mir/crate-info.rs @@ -123,7 +123,7 @@ impl Callbacks for SMirCalls { queries: &'tcx Queries<'tcx>, ) -> Compilation { queries.global_ctxt().unwrap().enter(|tcx| { - test_stable_mir(tcx); + rustc_smir::rustc_internal::run(tcx, || test_stable_mir(tcx)); }); // No need to keep going. Compilation::Stop From 5c6e2342f61ed9296fafb3781f0a89675a470eb9 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 24 Apr 2023 01:04:44 +0000 Subject: [PATCH 06/11] Encode types in SMIR --- compiler/rustc_smir/src/rustc_internal/mod.rs | 2 +- compiler/rustc_smir/src/rustc_smir/mod.rs | 57 ++++++++++++++++++- .../rustc_smir/src/stable_mir/mir/body.rs | 3 + compiler/rustc_smir/src/stable_mir/ty.rs | 15 +++++ tests/ui-fulldeps/stable-mir/crate-info.rs | 2 + 5 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 compiler/rustc_smir/src/stable_mir/ty.rs diff --git a/compiler/rustc_smir/src/rustc_internal/mod.rs b/compiler/rustc_smir/src/rustc_internal/mod.rs index e643ad28c0326..609a04d263c96 100644 --- a/compiler/rustc_smir/src/rustc_internal/mod.rs +++ b/compiler/rustc_smir/src/rustc_internal/mod.rs @@ -47,5 +47,5 @@ pub fn crate_num(item: &stable_mir::Crate) -> CrateNum { } pub fn run(tcx: TyCtxt<'_>, f: impl FnOnce()) { - crate::stable_mir::run(Tables { tcx, def_ids: vec![] }, f); + crate::stable_mir::run(Tables { tcx, def_ids: vec![], types: vec![] }, f); } diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index 81ecce415d553..6af43f5d3f358 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -7,8 +7,8 @@ //! //! For now, we are developing everything inside `rustc`, thus, we keep this module private. -use crate::stable_mir::{self, Context}; -use rustc_middle::ty::TyCtxt; +use crate::stable_mir::{self, ty::TyKind, Context}; +use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_span::def_id::{CrateNum, DefId, LOCAL_CRATE}; use tracing::debug; @@ -34,7 +34,7 @@ impl<'tcx> Context for Tables<'tcx> { fn entry_fn(&mut self) -> Option { Some(self.crate_item(self.tcx.entry_fn(())?.0)) } - fn mir_body(&self, item: &stable_mir::CrateItem) -> stable_mir::mir::Body { + fn mir_body(&mut self, item: &stable_mir::CrateItem) -> stable_mir::mir::Body { let def_id = self.item_def_id(item); let mir = self.tcx.optimized_mir(def_id); stable_mir::mir::Body { @@ -46,17 +46,68 @@ impl<'tcx> Context for Tables<'tcx> { statements: block.statements.iter().map(rustc_statement_to_statement).collect(), }) .collect(), + locals: mir.local_decls.iter().map(|decl| self.intern_ty(decl.ty)).collect(), } } fn rustc_tables(&mut self, f: &mut dyn FnMut(&mut Tables<'_>)) { f(self) } + + fn ty_kind(&mut self, ty: crate::stable_mir::ty::Ty) -> TyKind { + self.rustc_ty_to_ty(self.types[ty.0]) + } } pub struct Tables<'tcx> { pub tcx: TyCtxt<'tcx>, pub def_ids: Vec, + pub types: Vec>, +} + +impl<'tcx> Tables<'tcx> { + fn rustc_ty_to_ty(&mut self, ty: Ty<'tcx>) -> TyKind { + match ty.kind() { + ty::Bool => TyKind::Bool, + ty::Char => todo!(), + ty::Int(_) => todo!(), + ty::Uint(_) => todo!(), + ty::Float(_) => todo!(), + ty::Adt(_, _) => todo!(), + ty::Foreign(_) => todo!(), + ty::Str => todo!(), + ty::Array(_, _) => todo!(), + ty::Slice(_) => todo!(), + ty::RawPtr(_) => todo!(), + ty::Ref(_, _, _) => todo!(), + ty::FnDef(_, _) => todo!(), + ty::FnPtr(_) => todo!(), + ty::Placeholder(..) => todo!(), + ty::Dynamic(_, _, _) => todo!(), + ty::Closure(_, _) => todo!(), + ty::Generator(_, _, _) => todo!(), + ty::GeneratorWitness(_) => todo!(), + ty::GeneratorWitnessMIR(_, _) => todo!(), + ty::Never => todo!(), + ty::Tuple(fields) => { + TyKind::Tuple(fields.iter().map(|ty| self.intern_ty(ty)).collect()) + } + ty::Alias(_, _) => todo!(), + ty::Param(_) => todo!(), + ty::Bound(_, _) => todo!(), + ty::Infer(_) => todo!(), + ty::Error(_) => todo!(), + } + } + + fn intern_ty(&mut self, ty: Ty<'tcx>) -> stable_mir::ty::Ty { + if let Some(id) = self.types.iter().position(|&t| t == ty) { + return stable_mir::ty::Ty(id); + } + let id = self.types.len(); + self.types.push(ty); + stable_mir::ty::Ty(id) + } } /// Build a stable mir crate from a given crate number. diff --git a/compiler/rustc_smir/src/stable_mir/mir/body.rs b/compiler/rustc_smir/src/stable_mir/mir/body.rs index 4baf3f1f75eac..6328c35aa5982 100644 --- a/compiler/rustc_smir/src/stable_mir/mir/body.rs +++ b/compiler/rustc_smir/src/stable_mir/mir/body.rs @@ -1,6 +1,9 @@ +use crate::stable_mir::ty::Ty; + #[derive(Clone, Debug)] pub struct Body { pub blocks: Vec, + pub locals: Vec, } #[derive(Clone, Debug)] diff --git a/compiler/rustc_smir/src/stable_mir/ty.rs b/compiler/rustc_smir/src/stable_mir/ty.rs new file mode 100644 index 0000000000000..f27801b0f6cae --- /dev/null +++ b/compiler/rustc_smir/src/stable_mir/ty.rs @@ -0,0 +1,15 @@ +use super::with; + +#[derive(Copy, Clone, Debug)] +pub struct Ty(pub usize); + +impl Ty { + pub fn kind(&self) -> TyKind { + with(|context| context.ty_kind(*self)) + } +} + +pub enum TyKind { + Bool, + Tuple(Vec), +} diff --git a/tests/ui-fulldeps/stable-mir/crate-info.rs b/tests/ui-fulldeps/stable-mir/crate-info.rs index b0fcc36b0a805..a3db2e9ef24c4 100644 --- a/tests/ui-fulldeps/stable-mir/crate-info.rs +++ b/tests/ui-fulldeps/stable-mir/crate-info.rs @@ -40,6 +40,7 @@ fn test_stable_mir(tcx: TyCtxt<'_>) { let bar = get_item(tcx, &items, (DefKind::Fn, "bar")).unwrap(); let body = bar.body(); + assert_eq!(body.locals.len(), 2); assert_eq!(body.blocks.len(), 1); let block = &body.blocks[0]; assert_eq!(block.statements.len(), 1); @@ -54,6 +55,7 @@ fn test_stable_mir(tcx: TyCtxt<'_>) { let foo_bar = get_item(tcx, &items, (DefKind::Fn, "foo_bar")).unwrap(); let body = foo_bar.body(); + assert_eq!(body.locals.len(), 7); assert_eq!(body.blocks.len(), 4); let block = &body.blocks[0]; match &block.terminator { From b7f570fff3e582e2ffb8bce3ea698b6bc8f4ccef Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 9 May 2023 07:54:18 +0000 Subject: [PATCH 07/11] Keep encoding attributes for closures --- compiler/rustc_metadata/src/rmeta/encoder.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 82c66b9dfb9f6..29cf432b8f918 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -862,6 +862,11 @@ fn should_encode_attrs(def_kind: DefKind) -> bool { | DefKind::Macro(_) | DefKind::Field | DefKind::Impl { .. } => true, + // Tools may want to be able to detect their tool lints on + // closures from upstream crates, too. This is used by + // https://github.com/model-checking/kani and is not a performance + // or maintenance issue for us. + DefKind::Closure => true, DefKind::TyParam | DefKind::ConstParam | DefKind::Ctor(..) @@ -874,7 +879,6 @@ fn should_encode_attrs(def_kind: DefKind) -> bool { | DefKind::ImplTraitPlaceholder | DefKind::LifetimeParam | DefKind::GlobalAsm - | DefKind::Closure | DefKind::Generator => false, } } From 0dbaae4165b8ba41397a9b0f714acb2cd3610d65 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 22 Apr 2023 03:11:25 +0000 Subject: [PATCH 08/11] Make alias bounds sound in the new solver --- .../src/solve/assembly/mod.rs | 116 +++++++++++++++++- .../src/solve/project_goals.rs | 24 ++++ .../src/solve/trait_goals.rs | 24 ++++ .../traits/new-solver/alias-bound-unsound.rs | 27 ++++ .../new-solver/alias-bound-unsound.stderr | 24 ++++ .../traits/new-solver/nested-alias-bound.rs | 20 +++ 6 files changed, 233 insertions(+), 2 deletions(-) create mode 100644 tests/ui/traits/new-solver/alias-bound-unsound.rs create mode 100644 tests/ui/traits/new-solver/alias-bound-unsound.stderr create mode 100644 tests/ui/traits/new-solver/nested-alias-bound.rs diff --git a/compiler/rustc_trait_selection/src/solve/assembly/mod.rs b/compiler/rustc_trait_selection/src/solve/assembly/mod.rs index bacb0e32efc1a..0f42d5f12628f 100644 --- a/compiler/rustc_trait_selection/src/solve/assembly/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/assembly/mod.rs @@ -8,6 +8,7 @@ use rustc_data_structures::fx::FxIndexSet; use rustc_hir::def_id::DefId; use rustc_infer::traits::query::NoSolution; use rustc_infer::traits::util::elaborate; +use rustc_infer::traits::Reveal; use rustc_middle::traits::solve::{CanonicalResponse, Certainty, Goal, MaybeCause, QueryResult}; use rustc_middle::ty::fast_reject::TreatProjections; use rustc_middle::ty::TypeFoldable; @@ -87,7 +88,9 @@ pub(super) enum CandidateSource { } /// Methods used to assemble candidates for either trait or projection goals. -pub(super) trait GoalKind<'tcx>: TypeFoldable> + Copy + Eq { +pub(super) trait GoalKind<'tcx>: + TypeFoldable> + Copy + Eq + std::fmt::Display +{ fn self_ty(self) -> Ty<'tcx>; fn trait_ref(self, tcx: TyCtxt<'tcx>) -> ty::TraitRef<'tcx>; @@ -106,6 +109,16 @@ pub(super) trait GoalKind<'tcx>: TypeFoldable> + Copy + Eq { requirements: impl IntoIterator>>, ) -> QueryResult<'tcx>; + /// Consider a bound originating from the item bounds of an alias. For this we + /// require that the well-formed requirements of the self type of the goal + /// are "satisfied from the param-env". + /// See [`EvalCtxt::validate_alias_bound_self_from_param_env`]. + fn consider_alias_bound_candidate( + ecx: &mut EvalCtxt<'_, 'tcx>, + goal: Goal<'tcx, Self>, + assumption: ty::Predicate<'tcx>, + ) -> QueryResult<'tcx>; + // Consider a clause specifically for a `dyn Trait` self type. This requires // additionally checking all of the supertraits and object bounds to hold, // since they're not implied by the well-formedness of the object type. @@ -463,7 +476,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { for assumption in self.tcx().item_bounds(alias_ty.def_id).subst(self.tcx(), alias_ty.substs) { - match G::consider_implied_clause(self, goal, assumption, []) { + match G::consider_alias_bound_candidate(self, goal, assumption) { Ok(result) => { candidates.push(Candidate { source: CandidateSource::AliasBound, result }) } @@ -472,6 +485,105 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { } } + /// Check that we are allowed to use an alias bound originating from the self + /// type of this goal. This means something different depending on the self type's + /// alias kind. + /// + /// * Projection: Given a goal with a self type such as `::Assoc`, + /// we require that the bound `Ty: Trait` can be proven using either a nested alias + /// bound candidate, or a param-env candidate. + /// + /// * Opaque: The param-env must be in `Reveal::UserFacing` mode. Otherwise, + /// the goal should be proven by using the hidden type instead. + #[instrument(level = "debug", skip(self), ret)] + pub(super) fn validate_alias_bound_self_from_param_env>( + &mut self, + goal: Goal<'tcx, G>, + ) -> QueryResult<'tcx> { + match *goal.predicate.self_ty().kind() { + ty::Alias(ty::Projection, projection_ty) => { + let mut param_env_candidates = vec![]; + let self_trait_ref = projection_ty.trait_ref(self.tcx()); + + if self_trait_ref.self_ty().is_ty_var() { + return self + .evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS); + } + + let trait_goal: Goal<'_, ty::TraitPredicate<'tcx>> = goal.with( + self.tcx(), + ty::TraitPredicate { + trait_ref: self_trait_ref, + constness: ty::BoundConstness::NotConst, + polarity: ty::ImplPolarity::Positive, + }, + ); + + self.assemble_param_env_candidates(trait_goal, &mut param_env_candidates); + // FIXME: We probably need some sort of recursion depth check here. + // Can't come up with an example yet, though, and the worst case + // we can have is a compiler stack overflow... + self.assemble_alias_bound_candidates(trait_goal, &mut param_env_candidates); + + // FIXME: We must also consider alias-bound candidates for a peculiar + // class of built-in candidates that I'll call "defaulted" built-ins. + // + // For example, we always know that `T: Pointee` is implemented, but + // we do not always know what `::Metadata` actually is, + // similar to if we had a user-defined impl with a `default type ...`. + // For these traits, since we're not able to always normalize their + // associated types to a concrete type, we must consider their alias bounds + // instead, so we can prove bounds such as `::Metadata: Copy`. + self.assemble_alias_bound_candidates_for_builtin_impl_default_items( + trait_goal, + &mut param_env_candidates, + ); + + self.merge_candidates(param_env_candidates) + } + ty::Alias(ty::Opaque, _opaque_ty) => match goal.param_env.reveal() { + Reveal::UserFacing => { + self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes) + } + Reveal::All => return Err(NoSolution), + }, + _ => bug!("only expected to be called on alias tys"), + } + } + + /// Assemble a subset of builtin impl candidates for a class of candidates called + /// "defaulted" built-in traits. + /// + /// For example, we always know that `T: Pointee` is implemented, but we do not + /// always know what `::Metadata` actually is! See the comment in + /// [`EvalCtxt::validate_alias_bound_self_from_param_env`] for more detail. + #[instrument(level = "debug", skip_all)] + fn assemble_alias_bound_candidates_for_builtin_impl_default_items>( + &mut self, + goal: Goal<'tcx, G>, + candidates: &mut Vec>, + ) { + let lang_items = self.tcx().lang_items(); + let trait_def_id = goal.predicate.trait_def_id(self.tcx()); + + // You probably shouldn't add anything to this list unless you + // know what you're doing. + let result = if lang_items.pointee_trait() == Some(trait_def_id) { + G::consider_builtin_pointee_candidate(self, goal) + } else if lang_items.discriminant_kind_trait() == Some(trait_def_id) { + G::consider_builtin_discriminant_kind_candidate(self, goal) + } else { + Err(NoSolution) + }; + + match result { + Ok(result) => { + candidates.push(Candidate { source: CandidateSource::BuiltinImpl, result }) + } + Err(NoSolution) => (), + } + } + #[instrument(level = "debug", skip_all)] fn assemble_object_bound_candidates>( &mut self, diff --git a/compiler/rustc_trait_selection/src/solve/project_goals.rs b/compiler/rustc_trait_selection/src/solve/project_goals.rs index e5d51064c8d8f..85ac7f15a2fb6 100644 --- a/compiler/rustc_trait_selection/src/solve/project_goals.rs +++ b/compiler/rustc_trait_selection/src/solve/project_goals.rs @@ -83,6 +83,30 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> { } } + fn consider_alias_bound_candidate( + ecx: &mut EvalCtxt<'_, 'tcx>, + goal: Goal<'tcx, Self>, + assumption: ty::Predicate<'tcx>, + ) -> QueryResult<'tcx> { + if let Some(poly_projection_pred) = assumption.to_opt_poly_projection_pred() + && poly_projection_pred.projection_def_id() == goal.predicate.def_id() + { + ecx.probe(|ecx| { + let assumption_projection_pred = + ecx.instantiate_binder_with_infer(poly_projection_pred); + ecx.eq( + goal.param_env, + goal.predicate.projection_ty, + assumption_projection_pred.projection_ty, + )?; + ecx.eq(goal.param_env, goal.predicate.term, assumption_projection_pred.term)?; + ecx.validate_alias_bound_self_from_param_env(goal) + }) + } else { + Err(NoSolution) + } + } + fn consider_object_bound_candidate( ecx: &mut EvalCtxt<'_, 'tcx>, goal: Goal<'tcx, Self>, diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals.rs b/compiler/rustc_trait_selection/src/solve/trait_goals.rs index 04b38edc1265f..0f6fd4059b445 100644 --- a/compiler/rustc_trait_selection/src/solve/trait_goals.rs +++ b/compiler/rustc_trait_selection/src/solve/trait_goals.rs @@ -105,6 +105,30 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> { } } + fn consider_alias_bound_candidate( + ecx: &mut EvalCtxt<'_, 'tcx>, + goal: Goal<'tcx, Self>, + assumption: ty::Predicate<'tcx>, + ) -> QueryResult<'tcx> { + if let Some(poly_trait_pred) = assumption.to_opt_poly_trait_pred() + && poly_trait_pred.def_id() == goal.predicate.def_id() + { + // FIXME: Constness and polarity + ecx.probe(|ecx| { + let assumption_trait_pred = + ecx.instantiate_binder_with_infer(poly_trait_pred); + ecx.eq( + goal.param_env, + goal.predicate.trait_ref, + assumption_trait_pred.trait_ref, + )?; + ecx.validate_alias_bound_self_from_param_env(goal) + }) + } else { + Err(NoSolution) + } + } + fn consider_object_bound_candidate( ecx: &mut EvalCtxt<'_, 'tcx>, goal: Goal<'tcx, Self>, diff --git a/tests/ui/traits/new-solver/alias-bound-unsound.rs b/tests/ui/traits/new-solver/alias-bound-unsound.rs new file mode 100644 index 0000000000000..00294c708f1fa --- /dev/null +++ b/tests/ui/traits/new-solver/alias-bound-unsound.rs @@ -0,0 +1,27 @@ +// compile-flags: -Ztrait-solver=next + +// Makes sure that alias bounds are not unsound! + +#![feature(trivial_bounds)] + +trait Foo { + type Item: Copy + where + ::Item: Copy; + + fn copy_me(x: &Self::Item) -> Self::Item { + *x + } +} + +impl Foo for () { + type Item = String where String: Copy; +} + +fn main() { + let x = String::from("hello, world"); + drop(<() as Foo>::copy_me(&x)); + //~^ ERROR `<() as Foo>::Item: Copy` is not satisfied + //~| ERROR `<() as Foo>::Item` is not well-formed + println!("{x}"); +} diff --git a/tests/ui/traits/new-solver/alias-bound-unsound.stderr b/tests/ui/traits/new-solver/alias-bound-unsound.stderr new file mode 100644 index 0000000000000..9a43d2a6639ce --- /dev/null +++ b/tests/ui/traits/new-solver/alias-bound-unsound.stderr @@ -0,0 +1,24 @@ +error[E0277]: the trait bound `<() as Foo>::Item: Copy` is not satisfied + --> $DIR/alias-bound-unsound.rs:23:10 + | +LL | drop(<() as Foo>::copy_me(&x)); + | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `<() as Foo>::Item` + | +note: required by a bound in `Foo::Item` + --> $DIR/alias-bound-unsound.rs:10:30 + | +LL | type Item: Copy + | ---- required by a bound in this associated type +LL | where +LL | ::Item: Copy; + | ^^^^ required by this bound in `Foo::Item` + +error: the type `<() as Foo>::Item` is not well-formed + --> $DIR/alias-bound-unsound.rs:23:10 + | +LL | drop(<() as Foo>::copy_me(&x)); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/traits/new-solver/nested-alias-bound.rs b/tests/ui/traits/new-solver/nested-alias-bound.rs new file mode 100644 index 0000000000000..c365902dbe5e8 --- /dev/null +++ b/tests/ui/traits/new-solver/nested-alias-bound.rs @@ -0,0 +1,20 @@ +// compile-flags: -Ztrait-solver=next +// check-pass + +trait A { + type A: B; +} + +trait B { + type B: C; +} + +trait C {} + +fn needs_c() {} + +fn test() { + needs_c::<::B>(); +} + +fn main() {} From 3a863e534bcba819a4ff8d8362df442001a3dd8a Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 22 Apr 2023 04:51:35 +0000 Subject: [PATCH 09/11] Consolidate the 'match assumption' type methods in GoalKind --- .../src/solve/assembly/mod.rs | 44 +++++++++++- .../src/solve/project_goals.rs | 72 +------------------ .../src/solve/trait_goals.rs | 71 +----------------- 3 files changed, 47 insertions(+), 140 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/assembly/mod.rs b/compiler/rustc_trait_selection/src/solve/assembly/mod.rs index 0f42d5f12628f..25cc82f01d553 100644 --- a/compiler/rustc_trait_selection/src/solve/assembly/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/assembly/mod.rs @@ -99,6 +99,17 @@ pub(super) trait GoalKind<'tcx>: fn trait_def_id(self, tcx: TyCtxt<'tcx>) -> DefId; + // Try equating an assumption predicate against a goal's predicate. If it + // holds, then execute the `then` callback, which should do any additional + // work, then produce a response (typically by executing + // [`EvalCtxt::evaluate_added_goals_and_make_canonical_response`]). + fn probe_and_match_goal_against_assumption( + ecx: &mut EvalCtxt<'_, 'tcx>, + goal: Goal<'tcx, Self>, + assumption: ty::Predicate<'tcx>, + then: impl FnOnce(&mut EvalCtxt<'_, 'tcx>) -> QueryResult<'tcx>, + ) -> QueryResult<'tcx>; + // Consider a clause, which consists of a "assumption" and some "requirements", // to satisfy a goal. If the requirements hold, then attempt to satisfy our // goal by equating it with the assumption. @@ -107,7 +118,12 @@ pub(super) trait GoalKind<'tcx>: goal: Goal<'tcx, Self>, assumption: ty::Predicate<'tcx>, requirements: impl IntoIterator>>, - ) -> QueryResult<'tcx>; + ) -> QueryResult<'tcx> { + Self::probe_and_match_goal_against_assumption(ecx, goal, assumption, |ecx| { + ecx.add_goals(requirements); + ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes) + }) + } /// Consider a bound originating from the item bounds of an alias. For this we /// require that the well-formed requirements of the self type of the goal @@ -117,7 +133,11 @@ pub(super) trait GoalKind<'tcx>: ecx: &mut EvalCtxt<'_, 'tcx>, goal: Goal<'tcx, Self>, assumption: ty::Predicate<'tcx>, - ) -> QueryResult<'tcx>; + ) -> QueryResult<'tcx> { + Self::probe_and_match_goal_against_assumption(ecx, goal, assumption, |ecx| { + ecx.validate_alias_bound_self_from_param_env(goal) + }) + } // Consider a clause specifically for a `dyn Trait` self type. This requires // additionally checking all of the supertraits and object bounds to hold, @@ -126,7 +146,25 @@ pub(super) trait GoalKind<'tcx>: ecx: &mut EvalCtxt<'_, 'tcx>, goal: Goal<'tcx, Self>, assumption: ty::Predicate<'tcx>, - ) -> QueryResult<'tcx>; + ) -> QueryResult<'tcx> { + Self::probe_and_match_goal_against_assumption(ecx, goal, assumption, |ecx| { + let tcx = ecx.tcx(); + let ty::Dynamic(bounds, _, _) = *goal.predicate.self_ty().kind() else { + bug!("expected object type in `consider_object_bound_candidate`"); + }; + ecx.add_goals( + structural_traits::predicates_for_object_candidate( + &ecx, + goal.param_env, + goal.predicate.trait_ref(tcx), + bounds, + ) + .into_iter() + .map(|pred| goal.with(tcx, pred)), + ); + ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes) + }) + } fn consider_impl_candidate( ecx: &mut EvalCtxt<'_, 'tcx>, diff --git a/compiler/rustc_trait_selection/src/solve/project_goals.rs b/compiler/rustc_trait_selection/src/solve/project_goals.rs index 85ac7f15a2fb6..20ce2d9416e73 100644 --- a/compiler/rustc_trait_selection/src/solve/project_goals.rs +++ b/compiler/rustc_trait_selection/src/solve/project_goals.rs @@ -56,11 +56,11 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> { self.trait_def_id(tcx) } - fn consider_implied_clause( + fn probe_and_match_goal_against_assumption( ecx: &mut EvalCtxt<'_, 'tcx>, goal: Goal<'tcx, Self>, assumption: ty::Predicate<'tcx>, - requirements: impl IntoIterator>>, + then: impl FnOnce(&mut EvalCtxt<'_, 'tcx>) -> QueryResult<'tcx>, ) -> QueryResult<'tcx> { if let Some(poly_projection_pred) = assumption.to_opt_poly_projection_pred() && poly_projection_pred.projection_def_id() == goal.predicate.def_id() @@ -75,73 +75,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> { )?; ecx.eq(goal.param_env, goal.predicate.term, assumption_projection_pred.term) .expect("expected goal term to be fully unconstrained"); - ecx.add_goals(requirements); - ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes) - }) - } else { - Err(NoSolution) - } - } - - fn consider_alias_bound_candidate( - ecx: &mut EvalCtxt<'_, 'tcx>, - goal: Goal<'tcx, Self>, - assumption: ty::Predicate<'tcx>, - ) -> QueryResult<'tcx> { - if let Some(poly_projection_pred) = assumption.to_opt_poly_projection_pred() - && poly_projection_pred.projection_def_id() == goal.predicate.def_id() - { - ecx.probe(|ecx| { - let assumption_projection_pred = - ecx.instantiate_binder_with_infer(poly_projection_pred); - ecx.eq( - goal.param_env, - goal.predicate.projection_ty, - assumption_projection_pred.projection_ty, - )?; - ecx.eq(goal.param_env, goal.predicate.term, assumption_projection_pred.term)?; - ecx.validate_alias_bound_self_from_param_env(goal) - }) - } else { - Err(NoSolution) - } - } - - fn consider_object_bound_candidate( - ecx: &mut EvalCtxt<'_, 'tcx>, - goal: Goal<'tcx, Self>, - assumption: ty::Predicate<'tcx>, - ) -> QueryResult<'tcx> { - if let Some(poly_projection_pred) = assumption.to_opt_poly_projection_pred() - && poly_projection_pred.projection_def_id() == goal.predicate.def_id() - { - ecx.probe(|ecx| { - let tcx = ecx.tcx(); - - let assumption_projection_pred = - ecx.instantiate_binder_with_infer(poly_projection_pred); - ecx.eq( - goal.param_env, - goal.predicate.projection_ty, - assumption_projection_pred.projection_ty, - )?; - - let ty::Dynamic(bounds, _, _) = *goal.predicate.self_ty().kind() else { - bug!("expected object type in `consider_object_bound_candidate`"); - }; - ecx.add_goals( - structural_traits::predicates_for_object_candidate( - &ecx, - goal.param_env, - goal.predicate.projection_ty.trait_ref(tcx), - bounds, - ) - .into_iter() - .map(|pred| goal.with(tcx, pred)), - ); - ecx.eq(goal.param_env, goal.predicate.term, assumption_projection_pred.term) - .expect("expected goal term to be fully unconstrained"); - ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes) + then(ecx) }) } else { Err(NoSolution) diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals.rs b/compiler/rustc_trait_selection/src/solve/trait_goals.rs index 0f6fd4059b445..dcfa33ae842e9 100644 --- a/compiler/rustc_trait_selection/src/solve/trait_goals.rs +++ b/compiler/rustc_trait_selection/src/solve/trait_goals.rs @@ -78,11 +78,11 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> { }) } - fn consider_implied_clause( + fn probe_and_match_goal_against_assumption( ecx: &mut EvalCtxt<'_, 'tcx>, goal: Goal<'tcx, Self>, assumption: ty::Predicate<'tcx>, - requirements: impl IntoIterator>>, + then: impl FnOnce(&mut EvalCtxt<'_, 'tcx>) -> QueryResult<'tcx>, ) -> QueryResult<'tcx> { if let Some(poly_trait_pred) = assumption.to_opt_poly_trait_pred() && poly_trait_pred.def_id() == goal.predicate.def_id() @@ -97,72 +97,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> { goal.predicate.trait_ref, assumption_trait_pred.trait_ref, )?; - ecx.add_goals(requirements); - ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes) - }) - } else { - Err(NoSolution) - } - } - - fn consider_alias_bound_candidate( - ecx: &mut EvalCtxt<'_, 'tcx>, - goal: Goal<'tcx, Self>, - assumption: ty::Predicate<'tcx>, - ) -> QueryResult<'tcx> { - if let Some(poly_trait_pred) = assumption.to_opt_poly_trait_pred() - && poly_trait_pred.def_id() == goal.predicate.def_id() - { - // FIXME: Constness and polarity - ecx.probe(|ecx| { - let assumption_trait_pred = - ecx.instantiate_binder_with_infer(poly_trait_pred); - ecx.eq( - goal.param_env, - goal.predicate.trait_ref, - assumption_trait_pred.trait_ref, - )?; - ecx.validate_alias_bound_self_from_param_env(goal) - }) - } else { - Err(NoSolution) - } - } - - fn consider_object_bound_candidate( - ecx: &mut EvalCtxt<'_, 'tcx>, - goal: Goal<'tcx, Self>, - assumption: ty::Predicate<'tcx>, - ) -> QueryResult<'tcx> { - if let Some(poly_trait_pred) = assumption.to_opt_poly_trait_pred() - && poly_trait_pred.def_id() == goal.predicate.def_id() - && poly_trait_pred.polarity() == goal.predicate.polarity - { - // FIXME: Constness and polarity - ecx.probe(|ecx| { - let assumption_trait_pred = - ecx.instantiate_binder_with_infer(poly_trait_pred); - ecx.eq( - goal.param_env, - goal.predicate.trait_ref, - assumption_trait_pred.trait_ref, - )?; - - let tcx = ecx.tcx(); - let ty::Dynamic(bounds, _, _) = *goal.predicate.self_ty().kind() else { - bug!("expected object type in `consider_object_bound_candidate`"); - }; - ecx.add_goals( - structural_traits::predicates_for_object_candidate( - &ecx, - goal.param_env, - goal.predicate.trait_ref, - bounds, - ) - .into_iter() - .map(|pred| goal.with(tcx, pred)), - ); - ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes) + then(ecx) }) } else { Err(NoSolution) From 55d86b9da864ac4540b164f51b42fe05bbb4912c Mon Sep 17 00:00:00 2001 From: "Thomas M. DuBuisson" Date: Tue, 9 May 2023 14:08:13 -0700 Subject: [PATCH 10/11] Fix incorrect implication of transmuting slices transmute<&[u8]> would be useful and as a beginner it is confusing to see documents casually confuse the types of &[u8] and [u8; SZ] --- library/core/src/intrinsics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 3f2b1595d62b2..279a186689209 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -1187,7 +1187,7 @@ extern "rust-intrinsic" { /// Below are common applications of `transmute` which can be replaced with safer /// constructs. /// - /// Turning raw bytes (`&[u8]`) into `u32`, `f64`, etc.: + /// Turning raw bytes (`[u8; SZ]`) into `u32`, `f64`, etc.: /// /// ``` /// let raw_bytes = [0x78, 0x56, 0x34, 0x12]; From 26dc139b3702e7d3d2ea9dd0915da1d307f55011 Mon Sep 17 00:00:00 2001 From: Kyle Matsuda Date: Tue, 9 May 2023 16:02:52 -0600 Subject: [PATCH 11/11] add EarlyBinder to thir_abstract_const; remove tcx.bound_abstract_const --- compiler/rustc_infer/src/infer/mod.rs | 2 +- compiler/rustc_metadata/src/rmeta/mod.rs | 2 +- compiler/rustc_middle/src/query/erase.rs | 7 ++++--- compiler/rustc_middle/src/query/mod.rs | 2 +- compiler/rustc_middle/src/ty/abstract_const.rs | 9 +-------- compiler/rustc_ty_utils/src/consts.rs | 4 ++-- 6 files changed, 10 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index a89b993159902..9a95a9c8375e0 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -1530,7 +1530,7 @@ impl<'tcx> InferCtxt<'tcx> { // variables let tcx = self.tcx; if substs.has_non_region_infer() { - if let Some(ct) = tcx.bound_abstract_const(unevaluated.def)? { + if let Some(ct) = tcx.thir_abstract_const(unevaluated.def)? { let ct = tcx.expand_abstract_consts(ct.subst(tcx, substs)); if let Err(e) = ct.error_reported() { return Err(ErrorHandled::Reported(e)); diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index 84f6b7f934dc3..e2f6acb186b72 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -394,7 +394,7 @@ define_tables! { mir_for_ctfe: Table>>, mir_generator_witnesses: Table>>, promoted_mir: Table>>>, - thir_abstract_const: Table>>, + thir_abstract_const: Table>>>, impl_parent: Table, impl_polarity: Table, constness: Table, diff --git a/compiler/rustc_middle/src/query/erase.rs b/compiler/rustc_middle/src/query/erase.rs index 7d9aea022898d..28a9c1eef1a6d 100644 --- a/compiler/rustc_middle/src/query/erase.rs +++ b/compiler/rustc_middle/src/query/erase.rs @@ -82,9 +82,10 @@ impl EraseType for Result>, rustc_errors::ErrorGuarantee [u8; size_of::>, rustc_errors::ErrorGuaranteed>>()]; } -impl EraseType for Result>, rustc_errors::ErrorGuaranteed> { - type Result = - [u8; size_of::>, rustc_errors::ErrorGuaranteed>>()]; +impl EraseType for Result>>, rustc_errors::ErrorGuaranteed> { + type Result = [u8; size_of::< + Result>>, rustc_errors::ErrorGuaranteed>, + >()]; } impl EraseType for Result, traits::query::NoSolution> { diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index d5b185e45d6b4..dd89283d704e8 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -402,7 +402,7 @@ rustc_queries! { /// Try to build an abstract representation of the given constant. query thir_abstract_const( key: DefId - ) -> Result>, ErrorGuaranteed> { + ) -> Result>>, ErrorGuaranteed> { desc { |tcx| "building an abstract representation for `{}`", tcx.def_path_str(key), } diff --git a/compiler/rustc_middle/src/ty/abstract_const.rs b/compiler/rustc_middle/src/ty/abstract_const.rs index bfb740ab3560b..972c417cbbabd 100644 --- a/compiler/rustc_middle/src/ty/abstract_const.rs +++ b/compiler/rustc_middle/src/ty/abstract_const.rs @@ -4,7 +4,6 @@ use crate::ty::{ TypeVisitableExt, }; use rustc_errors::ErrorGuaranteed; -use rustc_hir::def_id::DefId; #[derive(Hash, Debug, Clone, Copy, Ord, PartialOrd, PartialEq, Eq)] #[derive(TyDecodable, TyEncodable, HashStable, TypeVisitable, TypeFoldable)] @@ -35,12 +34,6 @@ TrivialTypeTraversalAndLiftImpls! { pub type BoundAbstractConst<'tcx> = Result>>, ErrorGuaranteed>; impl<'tcx> TyCtxt<'tcx> { - /// Returns a const without substs applied - pub fn bound_abstract_const(self, uv: DefId) -> BoundAbstractConst<'tcx> { - let ac = self.thir_abstract_const(uv); - Ok(ac?.map(|ac| EarlyBinder(ac))) - } - pub fn expand_abstract_consts>>(self, ac: T) -> T { struct Expander<'tcx> { tcx: TyCtxt<'tcx>, @@ -59,7 +52,7 @@ impl<'tcx> TyCtxt<'tcx> { } fn fold_const(&mut self, c: Const<'tcx>) -> Const<'tcx> { let ct = match c.kind() { - ty::ConstKind::Unevaluated(uv) => match self.tcx.bound_abstract_const(uv.def) { + ty::ConstKind::Unevaluated(uv) => match self.tcx.thir_abstract_const(uv.def) { Err(e) => self.tcx.const_error_with_guaranteed(c.ty(), e), Ok(Some(bac)) => { let substs = self.tcx.erase_regions(uv.substs); diff --git a/compiler/rustc_ty_utils/src/consts.rs b/compiler/rustc_ty_utils/src/consts.rs index b08a92570ed71..3dd1d056be24f 100644 --- a/compiler/rustc_ty_utils/src/consts.rs +++ b/compiler/rustc_ty_utils/src/consts.rs @@ -394,7 +394,7 @@ impl<'a, 'tcx> visit::Visitor<'a, 'tcx> for IsThirPolymorphic<'a, 'tcx> { pub fn thir_abstract_const( tcx: TyCtxt<'_>, def: LocalDefId, -) -> Result>, ErrorGuaranteed> { +) -> Result>>, ErrorGuaranteed> { if !tcx.features().generic_const_exprs { return Ok(None); } @@ -420,7 +420,7 @@ pub fn thir_abstract_const( let root_span = body.exprs[body_id].span; - Some(recurse_build(tcx, body, body_id, root_span)).transpose() + Ok(Some(ty::EarlyBinder(recurse_build(tcx, body, body_id, root_span)?))) } pub fn provide(providers: &mut ty::query::Providers) {