bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #128440 - oli-obk:defines, r=lcnr

Browse source 
Add `#[define_opaques]` attribute and require it for all type-alias-impl-trait sites that register a hidden type

Instead of relying on the signature of items to decide whether they are constraining an opaque type, the opaque types that the item constrains must be explicitly listed.

A previous version of this PR used an actual attribute, but had to keep the resolved `DefId`s in a side table.

Now we just lower to fields in the AST that have no surface syntax, instead a builtin attribute macro fills in those fields where applicable.

Note that for convenience referencing opaque types in associated types from associated methods on the same impl will not require an attribute. If that causes problems `#[defines()]` can be used to overwrite the default of searching for opaques in the signature.

One wart of this design is that closures and static items do not have generics. So since I stored the opaques in the generics of functions, consts and methods, I would need to add a custom field to closures and statics to track this information. During a T-types discussion we decided to just not do this for now.

fixes #131298
This commit is contained in:
bors 2025-03-11 18:13:31 +00:00
commit 6650252439
666 changed files with 3025 additions and 2582 deletions
Download patch file Download diff file Expand all files Collapse all files

20
compiler/rustc_hir_analysis/src/check/check.rs
View file

@ -392,6 +392,12 @@ fn best_definition_site_of_opaque<'tcx>(
return ControlFlow::Continue(());
}
let opaque_types_defined_by = self.tcx.opaque_types_defined_by(item_def_id);
// Don't try to check items that cannot possibly constrain the type.
if !opaque_types_defined_by.contains(&self.opaque_def_id) {
return ControlFlow::Continue(());
}
if let Some(hidden_ty) =
self.tcx.mir_borrowck(item_def_id).concrete_opaque_types.get(&self.opaque_def_id)
{
@ -451,19 +457,7 @@ fn best_definition_site_of_opaque<'tcx>(
None
}
hir::OpaqueTyOrigin::TyAlias { in_assoc_ty: false, .. } => {
let scope = tcx.hir_get_defining_scope(tcx.local_def_id_to_hir_id(opaque_def_id));
let found = if scope == hir::CRATE_HIR_ID {
tcx.hir_walk_toplevel_module(&mut locator)
} else {
match tcx.hir_node(scope) {
Node::Item(it) => locator.visit_item(it),
Node::ImplItem(it) => locator.visit_impl_item(it),
Node::TraitItem(it) => locator.visit_trait_item(it),
Node::ForeignItem(it) => locator.visit_foreign_item(it),
other => bug!("{:?} is not a valid scope for an opaque type item", other),
}
};
found.break_value()
tcx.hir_walk_toplevel_module(&mut locator).break_value()
}
}
}

80
compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs
View file

@ -1,14 +1,13 @@
use rustc_hir::def::DefKind;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{self as hir, Expr, ImplItem, Item, Node, TraitItem, def};
use rustc_hir::{self as hir, Expr, ImplItem, Item, Node, TraitItem, def, intravisit};
use rustc_middle::bug;
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};
use rustc_span::DUMMY_SP;
use tracing::{debug, instrument, trace};
use crate::errors::{TaitForwardCompat, TaitForwardCompat2, UnconstrainedOpaqueType};
use crate::errors::{TaitForwardCompat2, UnconstrainedOpaqueType};
/// Checks "defining uses" of opaque `impl Trait` in associated types.
/// These can only be defined by associated items of the same trait.
@ -82,38 +81,9 @@ pub(super) fn find_opaque_ty_constraints_for_impl_trait_in_assoc_type(
/// ```
#[instrument(skip(tcx), level = "debug")]
pub(super) fn find_opaque_ty_constraints_for_tait(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Ty<'_> {
let hir_id = tcx.local_def_id_to_hir_id(def_id);
let scope = tcx.hir_get_defining_scope(hir_id);
let mut locator = TaitConstraintLocator { def_id, tcx, found: None, typeck_types: vec![] };
debug!(?scope);
if scope == hir::CRATE_HIR_ID {
tcx.hir_walk_toplevel_module(&mut locator);
} else {
trace!("scope={:#?}", tcx.hir_node(scope));
match tcx.hir_node(scope) {
// We explicitly call `visit_*` methods, instead of using `intravisit::walk_*` methods
// This allows our visitor to process the defining item itself, causing
// it to pick up any 'sibling' defining uses.
//
// For example, this code:
// ```
// fn foo() {
// type Blah = impl Debug;
// let my_closure = || -> Blah { true };
// }
// ```
//
// requires us to explicitly process `foo()` in order
// to notice the defining usage of `Blah`.
Node::Item(it) => locator.visit_item(it),
Node::ImplItem(it) => locator.visit_impl_item(it),
Node::TraitItem(it) => locator.visit_trait_item(it),
Node::ForeignItem(it) => locator.visit_foreign_item(it),
other => bug!("{:?} is not a valid scope for an opaque type item", other),
}
}
tcx.hir_walk_toplevel_module(&mut locator);
if let Some(hidden) = locator.found {
// Only check against typeck if we didn't already error
@ -137,12 +107,7 @@ pub(super) fn find_opaque_ty_constraints_for_tait(tcx: TyCtxt<'_>, def_id: Local
let reported = tcx.dcx().emit_err(UnconstrainedOpaqueType {
span: tcx.def_span(def_id),
name: tcx.item_ident(parent_def_id.to_def_id()),
what: match tcx.hir_node(scope) {
_ if scope == hir::CRATE_HIR_ID => "module",
Node::Item(hir::Item { kind: hir::ItemKind::Mod(_), .. }) => "module",
Node::Item(hir::Item { kind: hir::ItemKind::Impl(_), .. }) => "impl",
_ => "item",
},
what: "crate",
});
Ty::new_error(tcx, reported)
}
@ -176,6 +141,13 @@ impl TaitConstraintLocator<'_> {
return;
}
let opaque_types_defined_by = self.tcx.opaque_types_defined_by(item_def_id);
// Don't try to check items that cannot possibly constrain the type.
if !opaque_types_defined_by.contains(&self.def_id) {
debug!("no constraint: no opaque types defined");
return;
}
// Function items with `_` in their return type already emit an error, skip any
// "non-defining use" errors for them.
// Note that we use `Node::fn_sig` instead of `Node::fn_decl` here, because the former
@ -215,8 +187,6 @@ impl TaitConstraintLocator<'_> {
return;
}
let opaque_types_defined_by = self.tcx.opaque_types_defined_by(item_def_id);
let mut constrained = false;
for (&opaque_type_key, &hidden_type) in &tables.concrete_opaque_types {
if opaque_type_key.def_id != self.def_id {
@ -224,20 +194,6 @@ impl TaitConstraintLocator<'_> {
}
constrained = true;
if !opaque_types_defined_by.contains(&self.def_id) {
let guar = self.tcx.dcx().emit_err(TaitForwardCompat {
span: hidden_type.span,
item_span: self
.tcx
.def_ident_span(item_def_id)
.unwrap_or_else(|| self.tcx.def_span(item_def_id)),
});
// Avoid "opaque type not constrained" errors on the opaque itself.
self.found = Some(ty::OpaqueHiddenType {
span: DUMMY_SP,
ty: Ty::new_error(self.tcx, guar),
});
}
let concrete_type =
self.tcx.erase_regions(hidden_type.remap_generic_params_to_declaration_params(
opaque_type_key,
@ -309,19 +265,13 @@ impl<'tcx> intravisit::Visitor<'tcx> for TaitConstraintLocator<'tcx> {
}
fn visit_item(&mut self, it: &'tcx Item<'tcx>) {
trace!(?it.owner_id);
// The opaque type itself or its children are not within its reveal scope.
if it.owner_id.def_id != self.def_id {
self.check(it.owner_id.def_id);
intravisit::walk_item(self, it);
}
self.check(it.owner_id.def_id);
intravisit::walk_item(self, it);
}
fn visit_impl_item(&mut self, it: &'tcx ImplItem<'tcx>) {
trace!(?it.owner_id);
// The opaque type itself or its children are not within its reveal scope.
if it.owner_id.def_id != self.def_id {
self.check(it.owner_id.def_id);
intravisit::walk_impl_item(self, it);
}
self.check(it.owner_id.def_id);
intravisit::walk_impl_item(self, it);
}
fn visit_trait_item(&mut self, it: &'tcx TraitItem<'tcx>) {
trace!(?it.owner_id);

10
compiler/rustc_hir_analysis/src/errors.rs
View file

@ -424,16 +424,6 @@ pub(crate) struct UnconstrainedOpaqueType {
pub what: &'static str,
}
#[derive(Diagnostic)]
#[diag(hir_analysis_tait_forward_compat)]
#[note]
pub(crate) struct TaitForwardCompat {
#[primary_span]
pub span: Span,
#[note]
pub item_span: Span,
}
#[derive(Diagnostic)]
#[diag(hir_analysis_tait_forward_compat2)]
#[note]