Proof of Concept: add #[defines] attribute and require it for all type-alias-impl-trait sites that register a hidden type

compiler/rustc_ast/src/attr/mod.rs

@@ -19,6 +19,7 @@ use crate::token::{self, CommentKind, Delimiter, Token};
 use crate::tokenstream::{DelimSpan, LazyAttrTokenStream, Spacing, TokenStream, TokenTree};
 use crate::util::comments;
 use crate::util::literal::escape_string_symbol;
+use crate::NodeId;
 
 pub struct MarkedAttrs(GrowableBitSet<AttrId>);
 
@@ -501,6 +502,16 @@ impl MetaItemKind {
             },
         }
     }
+
+    pub fn defines(&self) -> Vec<NodeId> {
+        match self {
+            MetaItemKind::Word => vec![],
+            MetaItemKind::List(things) => {
+                things.iter().filter_map(|i| Some(i.meta_item()?.path.segments[0].id)).collect()
+            }
+            MetaItemKind::NameValue(_) => vec![],
+        }
+    }
 }
 
 impl MetaItemInner {

compiler/rustc_ast_lowering/src/lib.rs

@@ -93,6 +93,8 @@ struct LoweringContext<'a, 'hir> {
 
     /// Bodies inside the owner being lowered.
     bodies: Vec<(hir::ItemLocalId, &'hir hir::Body<'hir>)>,
+    /// Bodies inside the owner being lowered.
+    defines: SortedMap<hir::ItemLocalId, &'hir [LocalDefId]>,
     /// Attributes inside the owner being lowered.
     attrs: SortedMap<hir::ItemLocalId, &'hir [hir::Attribute]>,
     /// Collect items that were created by lowering the current owner.
@@ -144,6 +146,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
 
             // HirId handling.
             bodies: Vec::new(),
+            defines: SortedMap::default(),
             attrs: SortedMap::default(),
             children: Vec::default(),
             current_hir_id_owner: hir::CRATE_OWNER_ID,
@@ -533,6 +536,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
 
         let current_attrs = std::mem::take(&mut self.attrs);
         let current_bodies = std::mem::take(&mut self.bodies);
+        let current_defines = std::mem::take(&mut self.defines);
         let current_ident_and_label_to_local_id =
             std::mem::take(&mut self.ident_and_label_to_local_id);
 
@@ -565,6 +569,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         let info = self.make_owner_info(item);
 
         self.attrs = current_attrs;
+        self.defines = current_defines;
         self.bodies = current_bodies;
         self.ident_and_label_to_local_id = current_ident_and_label_to_local_id;
 
@@ -583,6 +588,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     }
 
     fn make_owner_info(&mut self, node: hir::OwnerNode<'hir>) -> &'hir hir::OwnerInfo<'hir> {
+        let defines = std::mem::take(&mut self.defines);
         let attrs = std::mem::take(&mut self.attrs);
         let mut bodies = std::mem::take(&mut self.bodies);
         let trait_map = std::mem::take(&mut self.trait_map);
@@ -600,11 +606,11 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
 
         // Don't hash unless necessary, because it's expensive.
         let (opt_hash_including_bodies, attrs_hash) =
-            self.tcx.hash_owner_nodes(node, &bodies, &attrs);
+            self.tcx.hash_owner_nodes(node, &bodies, &attrs, &defines);
         let num_nodes = self.item_local_id_counter.as_usize();
         let (nodes, parenting) = index::index_hir(self.tcx, node, &bodies, num_nodes);
         let nodes = hir::OwnerNodes { opt_hash_including_bodies, nodes, bodies };
-        let attrs = hir::AttributeMap { map: attrs, opt_hash: attrs_hash };
+        let attrs = hir::AttributeMap { map: attrs, defines, opt_hash: attrs_hash };
 
         self.arena.alloc(hir::OwnerInfo { nodes, parenting, attrs, trait_map })
     }
@@ -852,6 +858,29 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             debug_assert_eq!(id.owner, self.current_hir_id_owner);
             let ret = self.arena.alloc_from_iter(attrs.iter().map(|a| self.lower_attr(a)));
             debug_assert!(!ret.is_empty());
+
+            let mut defines = None;
+            for attr in ret.iter() {
+                if !attr.has_name(sym::defines) {
+                    continue;
+                }
+                let defines = defines.get_or_insert_with(Vec::new);
+
+                // TODO: error reporting for non-local items being mentioned and tests that go through these code paths
+                defines.extend(
+                    self.resolver
+                        .defines
+                        .get(&attr.id)
+                        .into_iter()
+                        .flatten()
+                        .map(|did| did.expect_local()),
+                );
+            }
+            trace!(?id, ?ret, ?defines, "lower_attrs");
+
+            if let Some(defines) = defines {
+                self.defines.insert(id.local_id, &*self.arena.alloc_from_iter(defines));
+            }
             self.attrs.insert(id.local_id, ret);
             ret
         }

compiler/rustc_feature/src/builtin_attrs.rs

@@ -573,6 +573,13 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         EncodeCrossCrate::No, coroutines, experimental!(coroutines)
     ),
 
+    // `#[defines]` attribute to be applied to functions can and must define hidden types for
+    // the opaque types specified in the attribute.
+    gated!(
+        defines, Normal, template!(List: r#"path::to::Alias, OtherAlias"#), ErrorFollowing,
+        EncodeCrossCrate::No, type_alias_impl_trait, experimental!(defines)
+    ),
+
     // RFC 3543
     // `#[patchable_function_entry(prefix_nops = m, entry_nops = n)]`
     gated!(

compiler/rustc_hir/src/hir.rs

@@ -1188,13 +1188,18 @@ impl Attribute {
 #[derive(Debug)]
 pub struct AttributeMap<'tcx> {
     pub map: SortedMap<ItemLocalId, &'tcx [Attribute]>,
+    /// Preprocessed `#[defines]` attribute.
+    pub defines: SortedMap<ItemLocalId, &'tcx [LocalDefId]>,
     // Only present when the crate hash is needed.
     pub opt_hash: Option<Fingerprint>,
 }
 
 impl<'tcx> AttributeMap<'tcx> {
-    pub const EMPTY: &'static AttributeMap<'static> =
-        &AttributeMap { map: SortedMap::new(), opt_hash: Some(Fingerprint::ZERO) };
+    pub const EMPTY: &'static AttributeMap<'static> = &AttributeMap {
+        map: SortedMap::new(),
+        defines: SortedMap::new(),
+        opt_hash: Some(Fingerprint::ZERO),
+    };
 
     #[inline]
     pub fn get(&self, id: ItemLocalId) -> &'tcx [Attribute] {

compiler/rustc_hir/src/stable_hash_impls.rs

@@ -102,9 +102,9 @@ impl<'tcx, HirCtx: crate::HashStableContext> HashStable<HirCtx> for OwnerNodes<'
 
 impl<'tcx, HirCtx: crate::HashStableContext> HashStable<HirCtx> for AttributeMap<'tcx> {
     fn hash_stable(&self, hcx: &mut HirCtx, hasher: &mut StableHasher) {
-        // We ignore the `map` since it refers to information included in `opt_hash` which is
+        // We ignore the `map` and `defines` since they refer to information included in `opt_hash` which is
         // hashed in the collector and used for the crate hash.
-        let AttributeMap { opt_hash, map: _ } = *self;
+        let AttributeMap { opt_hash, map: _, defines: _ } = *self;
         opt_hash.unwrap().hash_stable(hcx, hasher);
     }
 }

compiler/rustc_hir_analysis/src/check/check.rs

@@ -459,6 +459,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)
             {
@@ -518,19 +524,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()
         }
     }
 }

compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs

@@ -1,15 +1,14 @@
 use rustc_errors::StashKey;
 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.
@@ -83,38 +82,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
@@ -138,12 +108,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_name(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)
     }
@@ -177,6 +142,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
@@ -216,29 +188,13 @@ 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 {
                 continue;
             }
             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,
@@ -311,19 +267,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);

compiler/rustc_hir_analysis/src/errors.rs

@@ -422,16 +422,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]

compiler/rustc_middle/src/hir/map/mod.rs

@@ -656,17 +656,6 @@ impl<'hir> Map<'hir> {
         None
     }
 
-    /// Returns the defining scope for an opaque type definition.
-    pub fn get_defining_scope(self, id: HirId) -> HirId {
-        let mut scope = id;
-        loop {
-            scope = self.get_enclosing_scope(scope).unwrap_or(CRATE_HIR_ID);
-            if scope == CRATE_HIR_ID || !matches!(self.tcx.hir_node(scope), Node::Block(_)) {
-                return scope;
-            }
-        }
-    }
-
     pub fn get_foreign_abi(self, hir_id: HirId) -> ExternAbi {
         let parent = self.get_parent_item(hir_id);
         if let OwnerNode::Item(Item { kind: ItemKind::ForeignMod { abi, .. }, .. }) =

compiler/rustc_middle/src/hir/mod.rs

@@ -152,6 +152,7 @@ impl<'tcx> TyCtxt<'tcx> {
         node: OwnerNode<'_>,
         bodies: &SortedMap<ItemLocalId, &Body<'_>>,
         attrs: &SortedMap<ItemLocalId, &[Attribute]>,
+        defines: &SortedMap<ItemLocalId, &[LocalDefId]>,
     ) -> (Option<Fingerprint>, Option<Fingerprint>) {
         if self.needs_crate_hash() {
             self.with_stable_hashing_context(|mut hcx| {
@@ -163,6 +164,7 @@ impl<'tcx> TyCtxt<'tcx> {
 
                 let mut stable_hasher = StableHasher::new();
                 attrs.hash_stable(&mut hcx, &mut stable_hasher);
+                defines.hash_stable(&mut hcx, &mut stable_hasher);
                 let h2 = stable_hasher.finish();
                 (Some(h1), Some(h2))
             })

compiler/rustc_middle/src/ty/context.rs

@@ -1231,7 +1231,8 @@ impl<'tcx> TyCtxtFeed<'tcx, LocalDefId> {
         let bodies = Default::default();
         let attrs = hir::AttributeMap::EMPTY;
 
-        let (opt_hash_including_bodies, _) = self.tcx.hash_owner_nodes(node, &bodies, &attrs.map);
+        let (opt_hash_including_bodies, _) =
+            self.tcx.hash_owner_nodes(node, &bodies, &attrs.map, &attrs.defines);
         let node = node.into();
         self.opt_hir_owner_nodes(Some(self.tcx.arena.alloc(hir::OwnerNodes {
             opt_hash_including_bodies,