Auto merge of rust-lang#133468 - lcnr:uwu4, r=BoxyUwU

always create `DefId`s for anon consts

but don't use them anywhere, we intentionally don't encode them in the crate metadata.

best reviewed by disabling whitespace.

This pretty much reimplements rust-lang#133285 while adding the tests of rust-lang#133455. Fixes rust-lang#133064

r? `@BoxyUwU` `@compiler-errors`

Author: bors

compiler/rustc_ast/src/ast.rs

@@ -1184,14 +1184,15 @@ pub struct Expr {
 }
 
 impl Expr {
-    /// Is this expr either `N`, or `{ N }`.
+    /// Could this expr be either `N`, or `{ N }`, where `N` is a const parameter.
     ///
     /// If this is not the case, name resolution does not resolve `N` when using
     /// `min_const_generics` as more complex expressions are not supported.
     ///
     /// Does not ensure that the path resolves to a const param, the caller should check this.
-    pub fn is_potential_trivial_const_arg(&self, strip_identity_block: bool) -> bool {
-        let this = if strip_identity_block { self.maybe_unwrap_block() } else { self };
+    /// This also does not consider macros, so it's only correct after macro-expansion.
+    pub fn is_potential_trivial_const_arg(&self) -> bool {
+        let this = self.maybe_unwrap_block();
 
         if let ExprKind::Path(None, path) = &this.kind
             && path.is_potential_trivial_const_arg()

compiler/rustc_ast_lowering/src/asm.rs

@@ -227,18 +227,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                             };
 
                             // Wrap the expression in an AnonConst.
-                            let parent_def_id = self.current_def_id_parent;
+                            let parent_def_id = self.current_hir_id_owner.def_id;
                             let node_id = self.next_node_id();
-                            // HACK(min_generic_const_args): see lower_anon_const
-                            if !expr.is_potential_trivial_const_arg(true) {
-                                self.create_def(
-                                    parent_def_id,
-                                    node_id,
-                                    kw::Empty,
-                                    DefKind::AnonConst,
-                                    *op_sp,
-                                );
-                            }
+                            self.create_def(
+                                parent_def_id,
+                                node_id,
+                                kw::Empty,
+                                DefKind::AnonConst,
+                                *op_sp,
+                            );
                             let anon_const = AnonConst { id: node_id, value: P(expr) };
                             hir::InlineAsmOperand::SymFn {
                                 anon_const: self.lower_anon_const_to_anon_const(&anon_const),

compiler/rustc_ast_lowering/src/expr.rs

@@ -109,9 +109,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                         hir::ConstBlock {
                             def_id,
                             hir_id: this.lower_node_id(c.id),
-                            body: this.with_def_id_parent(def_id, |this| {
-                                this.lower_const_body(c.value.span, Some(&c.value))
-                            }),
+                            body: this.lower_const_body(c.value.span, Some(&c.value)),
                         }
                     });
                     hir::ExprKind::ConstBlock(c)
@@ -452,15 +450,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let mut generic_args = ThinVec::new();
         for (idx, arg) in args.iter().cloned().enumerate() {
             if legacy_args_idx.contains(&idx) {
-                let parent_def_id = self.current_def_id_parent;
+                let parent_def_id = self.current_hir_id_owner.def_id;
                 let node_id = self.next_node_id();
-
-                // HACK(min_generic_const_args): see lower_anon_const
-                if !arg.is_potential_trivial_const_arg(true) {
-                    // Add a definition for the in-band const def.
-                    self.create_def(parent_def_id, node_id, kw::Empty, DefKind::AnonConst, f.span);
-                }
-
+                self.create_def(parent_def_id, node_id, kw::Empty, DefKind::AnonConst, f.span);
                 let mut visitor = WillCreateDefIdsVisitor {};
                 let const_value = if let ControlFlow::Break(span) = visitor.visit_expr(&arg) {
                     AstP(Expr {
@@ -759,19 +751,17 @@ impl<'hir> LoweringContext<'_, 'hir> {
             lifetime_elision_allowed: false,
         });
 
-        let body = self.with_def_id_parent(closure_def_id, move |this| {
-            this.lower_body(move |this| {
-                this.coroutine_kind = Some(coroutine_kind);
+        let body = self.lower_body(move |this| {
+            this.coroutine_kind = Some(coroutine_kind);
 
-                let old_ctx = this.task_context;
-                if task_context.is_some() {
-                    this.task_context = task_context;
-                }
-                let res = body(this);
-                this.task_context = old_ctx;
+            let old_ctx = this.task_context;
+            if task_context.is_some() {
+                this.task_context = task_context;
+            }
+            let res = body(this);
+            this.task_context = old_ctx;
 
-                (params, res)
-            })
+            (params, res)
         });
 
         // `static |<_task_context?>| -> <return_ty> { <body> }`:
@@ -1056,26 +1046,24 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let (binder_clause, generic_params) = self.lower_closure_binder(binder);
 
         let (body_id, closure_kind) = self.with_new_scopes(fn_decl_span, move |this| {
-            this.with_def_id_parent(closure_def_id, move |this| {
-                let mut coroutine_kind = if this
-                    .attrs
-                    .get(&closure_hir_id.local_id)
-                    .is_some_and(|attrs| attrs.iter().any(|attr| attr.has_name(sym::coroutine)))
-                {
-                    Some(hir::CoroutineKind::Coroutine(Movability::Movable))
-                } else {
-                    None
-                };
-                let body_id = this.lower_fn_body(decl, |this| {
-                    this.coroutine_kind = coroutine_kind;
-                    let e = this.lower_expr_mut(body);
-                    coroutine_kind = this.coroutine_kind;
-                    e
-                });
-                let coroutine_option =
-                    this.closure_movability_for_fn(decl, fn_decl_span, coroutine_kind, movability);
-                (body_id, coroutine_option)
-            })
+            let mut coroutine_kind = if this
+                .attrs
+                .get(&closure_hir_id.local_id)
+                .is_some_and(|attrs| attrs.iter().any(|attr| attr.has_name(sym::coroutine)))
+            {
+                Some(hir::CoroutineKind::Coroutine(Movability::Movable))
+            } else {
+                None
+            };
+            let body_id = this.lower_fn_body(decl, |this| {
+                this.coroutine_kind = coroutine_kind;
+                let e = this.lower_expr_mut(body);
+                coroutine_kind = this.coroutine_kind;
+                e
+            });
+            let coroutine_option =
+                this.closure_movability_for_fn(decl, fn_decl_span, coroutine_kind, movability);
+            (body_id, coroutine_option)
         });
 
         let bound_generic_params = self.lower_lifetime_binder(closure_id, generic_params);
@@ -1165,28 +1153,26 @@ impl<'hir> LoweringContext<'_, 'hir> {
         );
 
         let body = self.with_new_scopes(fn_decl_span, |this| {
-            this.with_def_id_parent(closure_def_id, |this| {
-                let inner_decl =
-                    FnDecl { inputs: decl.inputs.clone(), output: FnRetTy::Default(fn_decl_span) };
-
-                // Transform `async |x: u8| -> X { ... }` into
-                // `|x: u8| || -> X { ... }`.
-                let body_id = this.lower_body(|this| {
-                    let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments(
-                        &inner_decl,
-                        |this| this.with_new_scopes(fn_decl_span, |this| this.lower_expr_mut(body)),
-                        fn_decl_span,
-                        body.span,
-                        coroutine_kind,
-                        hir::CoroutineSource::Closure,
-                    );
+            let inner_decl =
+                FnDecl { inputs: decl.inputs.clone(), output: FnRetTy::Default(fn_decl_span) };
+
+            // Transform `async |x: u8| -> X { ... }` into
+            // `|x: u8| || -> X { ... }`.
+            let body_id = this.lower_body(|this| {
+                let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments(
+                    &inner_decl,
+                    |this| this.with_new_scopes(fn_decl_span, |this| this.lower_expr_mut(body)),
+                    fn_decl_span,
+                    body.span,
+                    coroutine_kind,
+                    hir::CoroutineSource::Closure,
+                );
 
-                    this.maybe_forward_track_caller(body.span, closure_hir_id, expr.hir_id);
+                this.maybe_forward_track_caller(body.span, closure_hir_id, expr.hir_id);
 
-                    (parameters, expr)
-                });
-                body_id
-            })
+                (parameters, expr)
+            });
+            body_id
         });
 
         let bound_generic_params = self.lower_lifetime_binder(closure_id, generic_params);

compiler/rustc_ast_lowering/src/lib.rs

@@ -117,18 +117,6 @@ struct LoweringContext<'a, 'hir> {
     is_in_dyn_type: bool,
 
     current_hir_id_owner: hir::OwnerId,
-    /// Why do we need this in addition to [`Self::current_hir_id_owner`]?
-    ///
-    /// Currently (as of June 2024), anonymous constants are not HIR owners; however,
-    /// they do get their own DefIds. Some of these DefIds have to be created during
-    /// AST lowering, rather than def collection, because we can't tell until after
-    /// name resolution whether an anonymous constant will end up instead being a
-    /// [`hir::ConstArgKind::Path`]. However, to compute which generics are
-    /// available to an anonymous constant nested inside another, we need to make
-    /// sure that the parent is recorded as the parent anon const, not the enclosing
-    /// item. So we need to track parent defs differently from HIR owners, since they
-    /// will be finer-grained in the case of anon consts.
-    current_def_id_parent: LocalDefId,
     item_local_id_counter: hir::ItemLocalId,
     trait_map: ItemLocalMap<Box<[TraitCandidate]>>,
 
@@ -161,7 +149,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             attrs: SortedMap::default(),
             children: Vec::default(),
             current_hir_id_owner: hir::CRATE_OWNER_ID,
-            current_def_id_parent: CRATE_DEF_ID,
             item_local_id_counter: hir::ItemLocalId::ZERO,
             ident_and_label_to_local_id: Default::default(),
             #[cfg(debug_assertions)]
@@ -565,7 +552,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             debug_assert_eq!(_old, None);
         }
 
-        let item = self.with_def_id_parent(def_id, f);
+        let item = f(self);
         debug_assert_eq!(def_id, item.def_id().def_id);
         // `f` should have consumed all the elements in these vectors when constructing `item`.
         debug_assert!(self.impl_trait_defs.is_empty());
@@ -590,13 +577,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         self.children.push((def_id, hir::MaybeOwner::Owner(info)));
     }
 
-    fn with_def_id_parent<T>(&mut self, parent: LocalDefId, f: impl FnOnce(&mut Self) -> T) -> T {
-        let current_def_id_parent = std::mem::replace(&mut self.current_def_id_parent, parent);
-        let result = f(self);
-        self.current_def_id_parent = current_def_id_parent;
-        result
-    }
-
     fn make_owner_info(&mut self, node: hir::OwnerNode<'hir>) -> &'hir hir::OwnerInfo<'hir> {
         let attrs = std::mem::take(&mut self.attrs);
         let mut bodies = std::mem::take(&mut self.bodies);
@@ -773,7 +753,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             LifetimeRes::Fresh { param, kind, .. } => {
                 // Late resolution delegates to us the creation of the `LocalDefId`.
                 let _def_id = self.create_def(
-                    self.current_hir_id_owner.def_id, // FIXME: should this use self.current_def_id_parent?
+                    self.current_hir_id_owner.def_id,
                     param,
                     kw::UnderscoreLifetime,
                     DefKind::LifetimeParam,
@@ -1466,17 +1446,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         let opaque_ty_hir_id = self.lower_node_id(opaque_ty_node_id);
         debug!(?opaque_ty_def_id, ?opaque_ty_hir_id);
 
-        let opaque_ty_def = self.with_def_id_parent(opaque_ty_def_id, |this| {
-            let bounds = lower_item_bounds(this);
-            let opaque_ty_def = hir::OpaqueTy {
-                hir_id: opaque_ty_hir_id,
-                def_id: opaque_ty_def_id,
-                bounds,
-                origin,
-                span: this.lower_span(opaque_ty_span),
-            };
-            this.arena.alloc(opaque_ty_def)
-        });
+        let bounds = lower_item_bounds(self);
+        let opaque_ty_def = hir::OpaqueTy {
+            hir_id: opaque_ty_hir_id,
+            def_id: opaque_ty_def_id,
+            bounds,
+            origin,
+            span: self.lower_span(opaque_ty_span),
+        };
+        let opaque_ty_def = self.arena.alloc(opaque_ty_def);
 
         hir::TyKind::OpaqueDef(opaque_ty_def)
     }
@@ -2084,7 +2062,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         } else {
             // Construct an AnonConst where the expr is the "ty"'s path.
 
-            let parent_def_id = self.current_def_id_parent;
+            let parent_def_id = self.current_hir_id_owner.def_id;
             let node_id = self.next_node_id();
             let span = self.lower_span(span);
 
@@ -2108,9 +2086,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 self.arena.alloc(hir::AnonConst {
                     def_id,
                     hir_id,
-                    body: this.with_def_id_parent(def_id, |this| {
-                        this.lower_const_body(path_expr.span, Some(&path_expr))
-                    }),
+                    body: this.lower_const_body(path_expr.span, Some(&path_expr)),
                     span,
                 })
             });
@@ -2159,7 +2135,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 None,
             );
 
-            return ConstArg { hir_id: self.next_id(), kind: hir::ConstArgKind::Path(qpath) };
+            return ConstArg {
+                hir_id: self.lower_node_id(anon.id),
+                kind: hir::ConstArgKind::Path(qpath),
+            };
         }
 
         let lowered_anon = self.lower_anon_const_to_anon_const(anon);
@@ -2169,29 +2148,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
     /// See [`hir::ConstArg`] for when to use this function vs
     /// [`Self::lower_anon_const_to_const_arg`].
     fn lower_anon_const_to_anon_const(&mut self, c: &AnonConst) -> &'hir hir::AnonConst {
-        if c.value.is_potential_trivial_const_arg(true) {
-            // HACK(min_generic_const_args): see DefCollector::visit_anon_const
-            // Over there, we guess if this is a bare param and only create a def if
-            // we think it's not. However we may can guess wrong (see there for example)
-            // in which case we have to create the def here.
-            self.create_def(
-                self.current_def_id_parent,
-                c.id,
-                kw::Empty,
-                DefKind::AnonConst,
-                c.value.span,
-            );
-        }
-
         self.arena.alloc(self.with_new_scopes(c.value.span, |this| {
             let def_id = this.local_def_id(c.id);
             let hir_id = this.lower_node_id(c.id);
             hir::AnonConst {
                 def_id,
                 hir_id,
-                body: this.with_def_id_parent(def_id, |this| {
-                    this.lower_const_body(c.value.span, Some(&c.value))
-                }),
+                body: this.lower_const_body(c.value.span, Some(&c.value)),
                 span: this.lower_span(c.value.span),
             }
         }))

compiler/rustc_hir/src/def.rs

@@ -109,7 +109,16 @@ pub enum DefKind {
     Use,
     /// An `extern` block.
     ForeignMod,
-    /// Anonymous constant, e.g. the `1 + 2` in `[u8; 1 + 2]`
+    /// Anonymous constant, e.g. the `1 + 2` in `[u8; 1 + 2]`.
+    ///
+    /// Not all anon-consts are actually still relevant in the HIR. We lower
+    /// trivial const-arguments directly to `hir::ConstArgKind::Path`, at which
+    /// point the definition for the anon-const ends up unused and incomplete.
+    ///
+    /// We do not provide any a `Span` for the definition and pretty much all other
+    /// queries also ICE when using this `DefId`. Given that the `DefId` of such
+    /// constants should only be reachable by iterating all definitions of a
+    /// given crate, you should not have to worry about this.
     AnonConst,
     /// An inline constant, e.g. `const { 1 + 2 }`
     InlineConst,

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -1383,6 +1383,20 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             let def_id = local_id.to_def_id();
             let def_kind = tcx.def_kind(local_id);
             self.tables.def_kind.set_some(def_id.index, def_kind);
+
+            // The `DefCollector` will sometimes create unnecessary `DefId`s
+            // for trivial const arguments which are directly lowered to
+            // `ConstArgKind::Path`. We never actually access this `DefId`
+            // anywhere so we don't need to encode it for other crates.
+            if def_kind == DefKind::AnonConst
+                && matches!(
+                    tcx.hir_node_by_def_id(local_id),
+                    hir::Node::ConstArg(hir::ConstArg { kind: hir::ConstArgKind::Path(..), .. })
+                )
+            {
+                continue;
+            }
+
             if should_encode_span(def_kind) {
                 let def_span = tcx.def_span(local_id);
                 record!(self.tables.def_span[def_id] <- def_span);