always create DefIds when lowering anon-consts

Author: lcnr

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

@@ -224,16 +224,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                             // Wrap the expression in an AnonConst.
                             let parent_def_id = self.current_def_id_parent;
                             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

@@ -454,13 +454,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
             if legacy_args_idx.contains(&idx) {
                 let parent_def_id = self.current_def_id_parent;
                 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 {

compiler/rustc_ast_lowering/src/lib.rs

@@ -2159,7 +2159,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,20 +2172,6 @@ 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);

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);

compiler/rustc_resolve/src/def_collector.rs

@@ -12,37 +12,23 @@ use rustc_span::hygiene::LocalExpnId;
 use rustc_span::symbol::{Symbol, kw, sym};
 use tracing::debug;
 
-use crate::{ImplTraitContext, InvocationParent, PendingAnonConstInfo, Resolver};
+use crate::{ImplTraitContext, InvocationParent, Resolver};
 
 pub(crate) fn collect_definitions(
     resolver: &mut Resolver<'_, '_>,
     fragment: &AstFragment,
     expansion: LocalExpnId,
 ) {
-    let InvocationParent { parent_def, pending_anon_const_info, impl_trait_context, in_attr } =
+    let InvocationParent { parent_def, impl_trait_context, in_attr } =
         resolver.invocation_parents[&expansion];
-    let mut visitor = DefCollector {
-        resolver,
-        parent_def,
-        pending_anon_const_info,
-        expansion,
-        impl_trait_context,
-        in_attr,
-    };
+    let mut visitor = DefCollector { resolver, parent_def, expansion, impl_trait_context, in_attr };
     fragment.visit_with(&mut visitor);
 }
 
 /// Creates `DefId`s for nodes in the AST.
 struct DefCollector<'a, 'ra, 'tcx> {
     resolver: &'a mut Resolver<'ra, 'tcx>,
     parent_def: LocalDefId,
-    /// If we have an anon const that consists of a macro invocation, e.g. `Foo<{ m!() }>`,
-    /// we need to wait until we know what the macro expands to before we create the def for
-    /// the anon const. That's because we lower some anon consts into `hir::ConstArgKind::Path`,
-    /// which don't have defs.
-    ///
-    /// See `Self::visit_anon_const()`.
-    pending_anon_const_info: Option<PendingAnonConstInfo>,
     impl_trait_context: ImplTraitContext,
     in_attr: bool,
     expansion: LocalExpnId,
@@ -110,61 +96,13 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
 
     fn visit_macro_invoc(&mut self, id: NodeId) {
         let id = id.placeholder_to_expn_id();
-        let pending_anon_const_info = self.pending_anon_const_info.take();
         let old_parent = self.resolver.invocation_parents.insert(id, InvocationParent {
             parent_def: self.parent_def,
-            pending_anon_const_info,
             impl_trait_context: self.impl_trait_context,
             in_attr: self.in_attr,
         });
         assert!(old_parent.is_none(), "parent `LocalDefId` is reset for an invocation");
     }
-
-    /// Determines whether the const argument `AnonConst` is a simple macro call, optionally
-    /// surrounded with braces.
-    ///
-    /// If this const argument *is* a trivial macro call then the id for the macro call is
-    /// returned along with the information required to build the anon const's def if
-    /// the macro call expands to a non-trivial expression.
-    fn is_const_arg_trivial_macro_expansion(
-        &self,
-        anon_const: &'a AnonConst,
-    ) -> Option<(PendingAnonConstInfo, NodeId)> {
-        anon_const.value.optionally_braced_mac_call(false).map(|(block_was_stripped, id)| {
-            (
-                PendingAnonConstInfo {
-                    id: anon_const.id,
-                    span: anon_const.value.span,
-                    block_was_stripped,
-                },
-                id,
-            )
-        })
-    }
-
-    /// Determines whether the expression `const_arg_sub_expr` is a simple macro call, sometimes
-    /// surrounded with braces if a set of braces has not already been entered. This is required
-    /// as `{ N }` is treated as equivalent to a bare parameter `N` whereas `{{ N }}` is treated as
-    /// a real block expression and is lowered to an anonymous constant which is not allowed to use
-    /// generic parameters.
-    ///
-    /// If this expression is a trivial macro call then the id for the macro call is
-    /// returned along with the information required to build the anon const's def if
-    /// the macro call expands to a non-trivial expression.
-    fn is_const_arg_sub_expr_trivial_macro_expansion(
-        &self,
-        const_arg_sub_expr: &'a Expr,
-    ) -> Option<(PendingAnonConstInfo, NodeId)> {
-        let pending_anon = self.pending_anon_const_info.unwrap_or_else(||
-            panic!("Checking expr is trivial macro call without having entered anon const: `{const_arg_sub_expr:?}`"),
-        );
-
-        const_arg_sub_expr.optionally_braced_mac_call(pending_anon.block_was_stripped).map(
-            |(block_was_stripped, id)| {
-                (PendingAnonConstInfo { block_was_stripped, ..pending_anon }, id)
-            },
-        )
-    }
 }
 
 impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> {
@@ -376,78 +314,34 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> {
     }
 
     fn visit_anon_const(&mut self, constant: &'a AnonConst) {
-        // HACK(min_generic_const_args): don't create defs for anon consts if we think they will
-        // later be turned into ConstArgKind::Path's. because this is before resolve is done, we
-        // may accidentally identify a construction of a unit struct as a param and not create a
-        // def. we'll then create a def later in ast lowering in this case. the parent of nested
-        // items will be messed up, but that's ok because there can't be any if we're just looking
-        // for bare idents.
-
-        if let Some((pending_anon, macro_invoc)) =
-            self.is_const_arg_trivial_macro_expansion(constant)
-        {
-            self.pending_anon_const_info = Some(pending_anon);
-            return self.visit_macro_invoc(macro_invoc);
-        } else if constant.value.is_potential_trivial_const_arg(true) {
-            return visit::walk_anon_const(self, constant);
-        }
-
-        let def = self.create_def(constant.id, kw::Empty, DefKind::AnonConst, constant.value.span);
-        self.with_parent(def, |this| visit::walk_anon_const(this, constant));
+        let parent =
+            self.create_def(constant.id, kw::Empty, DefKind::AnonConst, constant.value.span);
+        self.with_parent(parent, |this| visit::walk_anon_const(this, constant));
     }
 
     fn visit_expr(&mut self, expr: &'a Expr) {
-        // If we're visiting the expression of a const argument that was a macro call then
-        // check if it is *still* unknown whether it is a trivial const arg or not. If so
-        // recurse into the macro call and delay creating the anon const def until expansion.
-        if self.pending_anon_const_info.is_some()
-            && let Some((pending_anon, macro_invoc)) =
-                self.is_const_arg_sub_expr_trivial_macro_expansion(expr)
-        {
-            self.pending_anon_const_info = Some(pending_anon);
-            return self.visit_macro_invoc(macro_invoc);
-        }
-
-        // See self.pending_anon_const_info for explanation
-        let parent_def = self
-            .pending_anon_const_info
-            .take()
-            // If we already stripped away a set of braces then do not do it again when determining
-            // if the macro expanded to a trivial const arg. This arises in cases such as:
-            // `Foo<{ bar!() }>` where `bar!()` expands to `{ N }`. This should not be considered a
-            // trivial const argument even though `{ N }` by itself *is*.
-            .filter(|pending_anon| {
-                !expr.is_potential_trivial_const_arg(!pending_anon.block_was_stripped)
-            })
-            .map(|pending_anon| {
-                self.create_def(pending_anon.id, kw::Empty, DefKind::AnonConst, pending_anon.span)
-            })
-            .unwrap_or(self.parent_def);
-
-        self.with_parent(parent_def, |this| {
-            let parent_def = match expr.kind {
-                ExprKind::MacCall(..) => return this.visit_macro_invoc(expr.id),
-                ExprKind::Closure(..) | ExprKind::Gen(..) => {
-                    this.create_def(expr.id, kw::Empty, DefKind::Closure, expr.span)
-                }
-                ExprKind::ConstBlock(ref constant) => {
-                    for attr in &expr.attrs {
-                        visit::walk_attribute(this, attr);
-                    }
-                    let def = this.create_def(
-                        constant.id,
-                        kw::Empty,
-                        DefKind::InlineConst,
-                        constant.value.span,
-                    );
-                    this.with_parent(def, |this| visit::walk_anon_const(this, constant));
-                    return;
+        let parent_def = match expr.kind {
+            ExprKind::MacCall(..) => return self.visit_macro_invoc(expr.id),
+            ExprKind::Closure(..) | ExprKind::Gen(..) => {
+                self.create_def(expr.id, kw::Empty, DefKind::Closure, expr.span)
+            }
+            ExprKind::ConstBlock(ref constant) => {
+                for attr in &expr.attrs {
+                    visit::walk_attribute(self, attr);
                 }
-                _ => this.parent_def,
-            };
+                let def = self.create_def(
+                    constant.id,
+                    kw::Empty,
+                    DefKind::InlineConst,
+                    constant.value.span,
+                );
+                self.with_parent(def, |this| visit::walk_anon_const(this, constant));
+                return;
+            }
+            _ => self.parent_def,
+        };
 
-            this.with_parent(parent_def, |this| visit::walk_expr(this, expr))
-        })
+        self.with_parent(parent_def, |this| visit::walk_expr(this, expr))
     }
 
     fn visit_ty(&mut self, ty: &'a Ty) {

compiler/rustc_resolve/src/late.rs

@@ -4521,7 +4521,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
         );
 
         self.resolve_anon_const_manual(
-            constant.value.is_potential_trivial_const_arg(true),
+            constant.value.is_potential_trivial_const_arg(),
             anon_const_kind,
             |this| this.resolve_expr(&constant.value, None),
         )
@@ -4685,7 +4685,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
                     // that is how they will be later lowered to HIR.
                     if const_args.contains(&idx) {
                         self.resolve_anon_const_manual(
-                            argument.is_potential_trivial_const_arg(true),
+                            argument.is_potential_trivial_const_arg(),
                             AnonConstKind::ConstArg(IsRepeatExpr::No),
                             |this| this.resolve_expr(argument, None),
                         );

compiler/rustc_resolve/src/lib.rs

@@ -174,31 +174,18 @@ impl<'ra> ParentScope<'ra> {
 #[derive(Copy, Debug, Clone)]
 struct InvocationParent {
     parent_def: LocalDefId,
-    pending_anon_const_info: Option<PendingAnonConstInfo>,
     impl_trait_context: ImplTraitContext,
     in_attr: bool,
 }
 
 impl InvocationParent {
     const ROOT: Self = Self {
         parent_def: CRATE_DEF_ID,
-        pending_anon_const_info: None,
         impl_trait_context: ImplTraitContext::Existential,
         in_attr: false,
     };
 }
 
-#[derive(Copy, Debug, Clone)]
-struct PendingAnonConstInfo {
-    // A const arg is only a "trivial" const arg if it has at *most* one set of braces
-    // around the argument. We track whether we have stripped an outter brace so that
-    // if a macro expands to a braced expression *and* the macro was itself inside of
-    // some braces then we can consider it to be a non-trivial const argument.
-    block_was_stripped: bool,
-    id: NodeId,
-    span: Span,
-}
-
 #[derive(Copy, Debug, Clone)]
 enum ImplTraitContext {
     Existential,

tests/ui/consts/issue-36163.stderr

@@ -1,10 +1,10 @@
-error[E0391]: cycle detected when simplifying constant for the type system `Foo::{constant#0}`
+error[E0391]: cycle detected when simplifying constant for the type system `Foo::B::{constant#0}`
   --> $DIR/issue-36163.rs:4:9
    |
 LL |     B = A,
    |         ^
    |
-note: ...which requires const-evaluating + checking `Foo::{constant#0}`...
+note: ...which requires const-evaluating + checking `Foo::B::{constant#0}`...
   --> $DIR/issue-36163.rs:4:9
    |
 LL |     B = A,
@@ -19,7 +19,7 @@ note: ...which requires const-evaluating + checking `A`...
    |
 LL | const A: isize = Foo::B as isize;
    |                  ^^^^^^^^^^^^^^^
-   = note: ...which again requires simplifying constant for the type system `Foo::{constant#0}`, completing the cycle
+   = note: ...which again requires simplifying constant for the type system `Foo::B::{constant#0}`, completing the cycle
 note: cycle used when checking that `Foo` is well-formed
   --> $DIR/issue-36163.rs:3:1
    |