Auto merge of rust-lang#130337 - BoxyUwU:anon_const_macro_call, r=camelid

Fix anon const def-creation when macros are involved take 2

Fixes rust-lang#130321

There were two cases that rust-lang#129137 did not handle correctly:

- Given a const argument `Foo<{ bar!() }>` in which `bar!()` expands to `N`, we would visit the anon const and then visit the `{ bar() }` expression instead of visiting the macro call. This meant that we would build a def for the anon const as `{ bar!() }` is not a trivial const argument as `bar!()` is not a path.
- Given a const argument `Foo<{ bar!() }>` is which `bar!()` expands to `{ qux!() }` in which `qux!()` expands to `N`, it should not be considered a trivial const argument as `{{ N }}` has two pairs of braces.  If we only looked at `qux`'s expansion it would *look* like a trivial const argument even though it is not. We have to track whether we have "unwrapped" a brace already when recursing into the expansions of `bar`/`qux`/any macro

r? `@camelid`

Author: bors

compiler/rustc_ast/src/ast.rs

@@ -1187,8 +1187,8 @@ impl Expr {
     /// `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) -> bool {
-        let this = self.maybe_unwrap_block();
+    pub fn is_potential_trivial_const_arg(&self, strip_identity_block: bool) -> bool {
+        let this = if strip_identity_block { self.maybe_unwrap_block().1 } else { self };
 
         if let ExprKind::Path(None, path) = &this.kind
             && path.is_potential_trivial_const_arg()
@@ -1199,14 +1199,15 @@ impl Expr {
         }
     }
 
-    pub fn maybe_unwrap_block(&self) -> &Expr {
+    /// Returns an expression with (when possible) *one* outter brace removed
+    pub fn maybe_unwrap_block(&self) -> (bool, &Expr) {
         if let ExprKind::Block(block, None) = &self.kind
             && let [stmt] = block.stmts.as_slice()
             && let StmtKind::Expr(expr) = &stmt.kind
         {
-            expr
+            (true, expr)
         } else {
-            self
+            (false, self)
         }
     }
 

compiler/rustc_ast_lowering/src/asm.rs

@@ -221,7 +221,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                             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() {
+                            if !expr.is_potential_trivial_const_arg(true) {
                                 self.create_def(
                                     parent_def_id,
                                     node_id,

compiler/rustc_ast_lowering/src/expr.rs

@@ -389,7 +389,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 let node_id = self.next_node_id();
 
                 // HACK(min_generic_const_args): see lower_anon_const
-                if !arg.is_potential_trivial_const_arg() {
+                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);
                 }

compiler/rustc_ast_lowering/src/lib.rs

@@ -2466,7 +2466,7 @@ 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() {
+        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)

compiler/rustc_resolve/src/def_collector.rs

@@ -138,6 +138,61 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
         );
         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)> {
+        let (block_was_stripped, expr) = anon_const.value.maybe_unwrap_block();
+        match expr {
+            Expr { kind: ExprKind::MacCall(..), id, .. } => Some((
+                PendingAnonConstInfo {
+                    id: anon_const.id,
+                    span: anon_const.value.span,
+                    block_was_stripped,
+                },
+                *id,
+            )),
+            _ => None,
+        }
+    }
+
+    /// 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:?}`"),
+        );
+
+        let (block_was_stripped, expr) = if pending_anon.block_was_stripped {
+            (true, const_arg_sub_expr)
+        } else {
+            const_arg_sub_expr.maybe_unwrap_block()
+        };
+
+        match expr {
+            Expr { kind: ExprKind::MacCall(..), id, .. } => {
+                Some((PendingAnonConstInfo { block_was_stripped, ..pending_anon }, *id))
+            }
+            _ => None,
+        }
+    }
 }
 
 impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> {
@@ -354,12 +409,12 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> {
         // items will be messed up, but that's ok because there can't be any if we're just looking
         // for bare idents.
 
-        if matches!(constant.value.maybe_unwrap_block().kind, ExprKind::MacCall(..)) {
-            // See self.pending_anon_const_info for explanation
-            self.pending_anon_const_info =
-                Some(PendingAnonConstInfo { id: constant.id, span: constant.value.span });
-            return visit::walk_anon_const(self, constant);
-        } else if constant.value.is_potential_trivial_const_arg() {
+        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);
         }
 
@@ -368,23 +423,36 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> {
     }
 
     fn visit_expr(&mut self, expr: &'a Expr) {
-        if matches!(expr.kind, ExprKind::MacCall(..)) {
-            return self.visit_macro_invoc(expr.id);
+        // 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);
         }
 
-        let grandparent_def = if let Some(pending_anon) = self.pending_anon_const_info.take() {
-            // See self.pending_anon_const_info for explanation
-            if !expr.is_potential_trivial_const_arg() {
+        // 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)
-            } else {
-                self.parent_def
-            }
-        } else {
-            self.parent_def
-        };
+            })
+            .unwrap_or(self.parent_def);
 
-        self.with_parent(grandparent_def, |this| {
+        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)
                 }

compiler/rustc_resolve/src/late.rs

@@ -4546,7 +4546,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
         );
 
         self.resolve_anon_const_manual(
-            constant.value.is_potential_trivial_const_arg(),
+            constant.value.is_potential_trivial_const_arg(true),
             anon_const_kind,
             |this| this.resolve_expr(&constant.value, None),
         )
@@ -4710,7 +4710,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(),
+                            argument.is_potential_trivial_const_arg(true),
                             AnonConstKind::ConstArg(IsRepeatExpr::No),
                             |this| this.resolve_expr(argument, None),
                         );

compiler/rustc_resolve/src/lib.rs

@@ -190,6 +190,11 @@ impl InvocationParent {
 
 #[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,
 }

tests/ui/const-generics/early/trivial-const-arg-macro-braced-expansion.rs

@@ -0,0 +1,14 @@
+macro_rules! y {
+    () => {
+        N
+    };
+}
+
+struct A<const N: usize>;
+
+fn foo<const N: usize>() -> A<{ y!() }> {
+    A::<1>
+    //~^ ERROR: mismatched types
+}
+
+fn main() {}

tests/ui/const-generics/early/trivial-const-arg-macro-braced-expansion.stderr

@@ -0,0 +1,14 @@
+error[E0308]: mismatched types
+  --> $DIR/trivial-const-arg-macro-braced-expansion.rs:10:5
+   |
+LL | fn foo<const N: usize>() -> A<{ y!() }> {
+   |                             ----------- expected `A<N>` because of return type
+LL |     A::<1>
+   |     ^^^^^^ expected `N`, found `1`
+   |
+   = note: expected struct `A<N>`
+              found struct `A<1>`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0308`.

tests/ui/const-generics/early/trivial-const-arg-macro-nested-braces-2.rs

@@ -0,0 +1,15 @@
+macro_rules! y {
+    () => {
+        N
+        //~^ ERROR: generic parameters may not be used in const operations
+    };
+}
+
+struct A<const N: usize>;
+
+#[rustfmt::skip]
+fn foo<const N: usize>() -> A<{{ y!() }}> {
+    A::<1>
+}
+
+fn main() {}