Auto merge of #149114 - BoxyUwU:mgca_adt_exprs, r=lcnr

MGCA: Support struct expressions without intermediary anon consts

r? oli-obk

tracking issue: #132980

Fixes #127972
Fixes #137888
Fixes #140275

due to delaying a bug instead of ICEing in HIR ty lowering.

### High level goal

Under `feature(min_generic_const_args)` this PR adds another kind of const argument. A struct/variant construction const arg kind. We represent the values of the fields as themselves being const arguments which allows for uses of generic parameters subject to the existing restrictions present in `min_generic_const_args`:
```rust
fn foo<const N: Option<u32>>() {}

trait Trait {
    #[type_const]
    const ASSOC: usize;
}

fn bar<T: Trait, const N: u32>() {
    // the initializer of `_0` is a `N` which is a legal const argument
    // so this is ok.
    foo::<{ Some::<u32> { 0: N } }>();

    // this is allowed as mgca supports uses of assoc consts in the
    // type system. ie `<T as Trait>::ASSOC` is a legal const argument
    foo::<{ Some::<u32> { 0: <T as Trait>::ASSOC } }>();

    // this on the other hand is not allowed as `N + 1` is not a legal
    // const argument
    foo::<{ Some::<u32> { 0: N + 1 } }>();
}
```

This PR does not support uses of const ctors, e.g. `None`. And also does not support tuple constructors, e.g. `Some(N)`. I believe that it would not be difficult to add support for such functionality after this PR lands so have left it out deliberately.

We currently require that all generic parameters on the type being constructed be explicitly specified. I haven't really looked into why that is but it doesn't seem desirable to me as it should be legal to write `Some { ... }` in a const argument inside of a body and have that desugar to `Some::<_> { ... }`. Regardless this can definitely be a follow-up PR and I assume this is some underlying consistency with the way that elided args are handled with type paths elsewhere.

This PRs implementation of supporting struct expressions is somewhat incomplete. We don't handle `Foo { ..expr }` at all and aren't handling privacy/stability. The printing of `ConstArgKind::Struct` HIR nodes doesn't really exist either :')

I've tried to keep the implementation here somewhat deliberately incomplete as I think a number of these issues are actually quite small and self contained after this PR lands and I'm hoping it could be a good set of issues to mentor newer contributors on 🤔 I just wanted the "bare minimum" required to actually demonstrate that the previous changes are "necessary".

### `ValTree` now recurse through `ty::Const`

In order to actually represent struct/variant construction in `ty::Const` without going through an anon const we would need to introduce some new `ConstKind` variant. Let's say some hypothetical `ConstKind::ADT(Ty<'tcx>, List<Const<'tcx>>)`.

This variant would represent things the same way that `ValTree` does with the first element representing the `VariantIdx` of the enum (if its an enum), and then followed by a list of field values in definition order.

This *could* work but there are a few reasons why it's suboptimal.

First it would mean we have a second kind of `Const` that can be normalized. Right now we only have `ConstKind::Unevaluated` which possibly needs normalization. Similarly with `TyKind` we *only* have `TyKind::Alias`. If we introduced `ConstKind::ADT` it would need to be normalized to a `ConstKind::Value` eventually. This feels to me like it has the potential to cause bugs in the long run where only `ConstKind::Unevaluated` is handled by some code paths.

Secondly it would make type equality/inference be kind of... weird... It's desirable for `Some { 0: ?x } eq Some { 0: 1_u32 }` to result in `?x=1_u32`.  I can't see a way for this to work with this `ConstKind::ADT` design under the current architecture for how we represent types/consts and generally do equality operations.

We would need to wholly special case these two variants in type equality and have a custom recursive walker separate from the existing architecture for doing type equality. It would also be somewhat unique in that it's a non-rigid `ty::Const` (it can be normalized more later on in type inference) while also having somewhat "structural" equality behaviour.

Lastly, it's worth noting that its not *actually* `ConstKind::ADT` that we want. It's desirable to extend this setup to also support tuples and arrays, or even references if we wind up supporting those in const generics. Therefore this isn't really `ConstKind::ADT` but a more general `ConstKind::ShallowValue` or something to that effect. It represents at least one "layer" of a types value :')

Instead of doing this implementation choice we instead change `ValTree::Branch`:
```rust
enum ValTree<'tcx> {
    Leaf(ScalarInt),
    // Before this PR:
    Branch(Box<[ValTree<'tcx>]>),
    // After this PR
    Branch(Box<[Const<'tcx>]>),
}
```

The representation for so called "shallow values" is now the same as the representation for the *entire* full value. The desired inference/type equality behaviour just falls right out of this. We also don't wind up with these shallow values actually being non-rigid. And `ValTree` *already* supports references/tuples/arrays so we can handle those just fine.

I think in the future it might be worth considering inlining `ValTree` into `ty::ConstKind`. E.g:
```rust
enum ConstKind {
    Scalar(Ty<'tcx>, ScalarInt),
    ShallowValue(Ty<'tcx>, List<Const<'tcx>>),
    Unevaluated(UnevaluatedConst<'tcx>),
    ...
}
```

This would imply that the usage of `ValTree`s in patterns would now be using `ty::Const` but they already kind of are anyway and I think that's probably okay in the long run. It also would mean that the set of things we *could* represent in const patterns is greater which may be desirable in the long run for supporting things such as const patterns of const generic parameters.

Regardless, this PR doesn't actually inline `ValTree` into `ty::ConstKind`, it only changes `Branch` to recurse through `Const`. This change could be split out of this PR if desired.

I'm not sure if there'll be a perf impact from this change. It's somewhat plausible as now all const pattern values that have nesting will be interning a lot more `Ty`s. We shall see :>

### Forbidding generic parameters under mgca

Under mgca we now allow all const arguments to resolve paths to generic parameters. We then *later* actually validate that the const arg should be allowed to access generic parameters if it did wind up resolving to any.

This winds up just being a lot simpler to implement than trying to make name resolution "keep track" of whether we're inside of a non-anon-const const arg and then encounter a `const { ... }` indicating we should now stop allowing resolving to generic parameters.

It's also somewhat in line with what we'll need for a `feature(generic_const_args)` where we'll want to decide whether an anon const should have any generic parameters based off syntactically whether any generic parameters were used. Though that design is entirely hypothetical at this point :)

### Followup Work

- Make HIR ty lowering check whether lowering generic parameters is supported and if not lower to an error type/const. Should make the code cleaner, fix some other bugs, and maybe(?) recover perf since we'll be accessing less queries which I think is part of the perf regression of this PR
- Make the ValTree setup less scuffed. We should find a new name for `ConstKind::Value` and the `Val` part of `ValTree` and `ty::Value` as they no longer correspond to a fully normalized structure. It may also be worth looking into inlining `ValTreeKind` into `ConstKind` or atleast into `ty::Value` or sth 🤔
- Support tuple constructors and const constructors not just struct expressions.
- Reduce code duplication between HIR ty lowering's handling of struct expressions, and HIR typeck's handling of struct expressions
- Try fix perf #149114 (comment). Maybe this will clear up once we clean up `ValTree` a bit and stop doing double interning and whatnot

Author: bors

compiler/rustc_ast_lowering/src/index.rs

@@ -281,6 +281,13 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
         });
     }
 
+    fn visit_const_arg_expr_field(&mut self, field: &'hir ConstArgExprField<'hir>) {
+        self.insert(field.span, field.hir_id, Node::ConstArgExprField(field));
+        self.with_parent(field.hir_id, |this| {
+            intravisit::walk_const_arg_expr_field(this, field);
+        })
+    }
+
     fn visit_stmt(&mut self, stmt: &'hir Stmt<'hir>) {
         self.insert(stmt.span, stmt.hir_id, Node::Stmt(stmt));
 

compiler/rustc_ast_lowering/src/lib.rs

@@ -2410,6 +2410,47 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
 
                 ConstArg { hir_id: self.next_id(), kind: hir::ConstArgKind::Path(qpath) }
             }
+            ExprKind::Struct(se) => {
+                let path = self.lower_qpath(
+                    expr.id,
+                    &se.qself,
+                    &se.path,
+                    // FIXME(mgca): we may want this to be `Optional` instead, but
+                    // we would also need to make sure that HIR ty lowering errors
+                    // when these paths wind up in signatures.
+                    ParamMode::Explicit,
+                    AllowReturnTypeNotation::No,
+                    ImplTraitContext::Disallowed(ImplTraitPosition::Path),
+                    None,
+                );
+
+                let fields = self.arena.alloc_from_iter(se.fields.iter().map(|f| {
+                    let hir_id = self.lower_node_id(f.id);
+                    // FIXME(mgca): This might result in lowering attributes that
+                    // then go unused as the `Target::ExprField` is not actually
+                    // corresponding to `Node::ExprField`.
+                    self.lower_attrs(hir_id, &f.attrs, f.span, Target::ExprField);
+
+                    let expr = if let ExprKind::ConstBlock(anon_const) = &f.expr.kind {
+                        let def_id = self.local_def_id(anon_const.id);
+                        let def_kind = self.tcx.def_kind(def_id);
+                        assert_eq!(DefKind::AnonConst, def_kind);
+
+                        self.lower_anon_const_to_const_arg_direct(anon_const)
+                    } else {
+                        self.lower_expr_to_const_arg_direct(&f.expr)
+                    };
+
+                    &*self.arena.alloc(hir::ConstArgExprField {
+                        hir_id,
+                        field: self.lower_ident(f.ident),
+                        expr: self.arena.alloc(expr),
+                        span: self.lower_span(f.span),
+                    })
+                }));
+
+                ConstArg { hir_id: self.next_id(), kind: hir::ConstArgKind::Struct(path, fields) }
+            }
             ExprKind::Underscore => ConstArg {
                 hir_id: self.lower_node_id(expr.id),
                 kind: hir::ConstArgKind::Infer(expr.span, ()),

compiler/rustc_codegen_cranelift/src/intrinsics/simd.rs

@@ -130,7 +130,7 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(
                 return;
             }
 
-            let idx = generic_args[2].expect_const().to_value().valtree.unwrap_branch();
+            let idx = generic_args[2].expect_const().to_branch();
 
             assert_eq!(x.layout(), y.layout());
             let layout = x.layout();
@@ -143,7 +143,7 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(
 
             let total_len = lane_count * 2;
 
-            let indexes = idx.iter().map(|idx| idx.unwrap_leaf().to_u32()).collect::<Vec<u32>>();
+            let indexes = idx.iter().map(|idx| idx.to_leaf().to_u32()).collect::<Vec<u32>>();
 
             for &idx in &indexes {
                 assert!(u64::from(idx) < total_len, "idx {} out of range 0..{}", idx, total_len);
@@ -961,9 +961,8 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(
             let lane_clif_ty = fx.clif_type(val_lane_ty).unwrap();
             let ptr_val = ptr.load_scalar(fx);
 
-            let alignment = generic_args[3].expect_const().to_value().valtree.unwrap_branch()[0]
-                .unwrap_leaf()
-                .to_simd_alignment();
+            let alignment =
+                generic_args[3].expect_const().to_branch()[0].to_leaf().to_simd_alignment();
 
             let memflags = match alignment {
                 SimdAlign::Unaligned => MemFlags::new().with_notrap(),
@@ -1006,9 +1005,8 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(
             let lane_clif_ty = fx.clif_type(val_lane_ty).unwrap();
             let ret_lane_layout = fx.layout_of(ret_lane_ty);
 
-            let alignment = generic_args[3].expect_const().to_value().valtree.unwrap_branch()[0]
-                .unwrap_leaf()
-                .to_simd_alignment();
+            let alignment =
+                generic_args[3].expect_const().to_branch()[0].to_leaf().to_simd_alignment();
 
             let memflags = match alignment {
                 SimdAlign::Unaligned => MemFlags::new().with_notrap(),
@@ -1059,9 +1057,8 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>(
             let ret_lane_layout = fx.layout_of(ret_lane_ty);
             let ptr_val = ptr.load_scalar(fx);
 
-            let alignment = generic_args[3].expect_const().to_value().valtree.unwrap_branch()[0]
-                .unwrap_leaf()
-                .to_simd_alignment();
+            let alignment =
+                generic_args[3].expect_const().to_branch()[0].to_leaf().to_simd_alignment();
 
             let memflags = match alignment {
                 SimdAlign::Unaligned => MemFlags::new().with_notrap(),

compiler/rustc_codegen_llvm/src/intrinsic.rs

@@ -345,7 +345,7 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
                     _ => bug!(),
                 };
                 let ptr = args[0].immediate();
-                let locality = fn_args.const_at(1).to_value().valtree.unwrap_leaf().to_i32();
+                let locality = fn_args.const_at(1).to_leaf().to_i32();
                 self.call_intrinsic(
                     "llvm.prefetch",
                     &[self.val_ty(ptr)],
@@ -1527,7 +1527,7 @@ fn generic_simd_intrinsic<'ll, 'tcx>(
     }
 
     if name == sym::simd_shuffle_const_generic {
-        let idx = fn_args[2].expect_const().to_value().valtree.unwrap_branch();
+        let idx = fn_args[2].expect_const().to_branch();
         let n = idx.len() as u64;
 
         let (out_len, out_ty) = require_simd!(ret_ty, SimdReturn);
@@ -1546,7 +1546,7 @@ fn generic_simd_intrinsic<'ll, 'tcx>(
             .iter()
             .enumerate()
             .map(|(arg_idx, val)| {
-                let idx = val.unwrap_leaf().to_i32();
+                let idx = val.to_leaf().to_i32();
                 if idx >= i32::try_from(total_len).unwrap() {
                     bx.sess().dcx().emit_err(InvalidMonomorphization::SimdIndexOutOfBounds {
                         span,
@@ -1958,9 +1958,7 @@ fn generic_simd_intrinsic<'ll, 'tcx>(
         // those lanes whose `mask` bit is enabled.
         // The memory addresses corresponding to the “off” lanes are not accessed.
 
-        let alignment = fn_args[3].expect_const().to_value().valtree.unwrap_branch()[0]
-            .unwrap_leaf()
-            .to_simd_alignment();
+        let alignment = fn_args[3].expect_const().to_branch()[0].to_leaf().to_simd_alignment();
 
         // The element type of the "mask" argument must be a signed integer type of any width
         let mask_ty = in_ty;
@@ -2053,9 +2051,7 @@ fn generic_simd_intrinsic<'ll, 'tcx>(
         // those lanes whose `mask` bit is enabled.
         // The memory addresses corresponding to the “off” lanes are not accessed.
 
-        let alignment = fn_args[3].expect_const().to_value().valtree.unwrap_branch()[0]
-            .unwrap_leaf()
-            .to_simd_alignment();
+        let alignment = fn_args[3].expect_const().to_branch()[0].to_leaf().to_simd_alignment();
 
         // The element type of the "mask" argument must be a signed integer type of any width
         let mask_ty = in_ty;

compiler/rustc_codegen_ssa/src/mir/constant.rs

@@ -77,22 +77,21 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             .flatten()
             .map(|val| {
                 // A SIMD type has a single field, which is an array.
-                let fields = val.unwrap_branch();
+                let fields = val.to_branch();
                 assert_eq!(fields.len(), 1);
-                let array = fields[0].unwrap_branch();
+                let array = fields[0].to_branch();
                 // Iterate over the array elements to obtain the values in the vector.
                 let values: Vec<_> = array
                     .iter()
                     .map(|field| {
-                        if let Some(prim) = field.try_to_scalar() {
-                            let layout = bx.layout_of(field_ty);
-                            let BackendRepr::Scalar(scalar) = layout.backend_repr else {
-                                bug!("from_const: invalid ByVal layout: {:#?}", layout);
-                            };
-                            bx.scalar_to_backend(prim, scalar, bx.immediate_backend_type(layout))
-                        } else {
+                        let Some(prim) = field.try_to_scalar() else {
                             bug!("field is not a scalar {:?}", field)
-                        }
+                        };
+                        let layout = bx.layout_of(field_ty);
+                        let BackendRepr::Scalar(scalar) = layout.backend_repr else {
+                            bug!("from_const: invalid ByVal layout: {:#?}", layout);
+                        };
+                        bx.scalar_to_backend(prim, scalar, bx.immediate_backend_type(layout))
                     })
                     .collect();
                 bx.const_vector(&values)

compiler/rustc_codegen_ssa/src/mir/intrinsic.rs

@@ -102,7 +102,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         };
 
         let parse_atomic_ordering = |ord: ty::Value<'tcx>| {
-            let discr = ord.valtree.unwrap_branch()[0].unwrap_leaf();
+            let discr = ord.to_branch()[0].to_leaf();
             discr.to_atomic_ordering()
         };
 

compiler/rustc_const_eval/src/const_eval/valtrees.rs

@@ -36,13 +36,17 @@ fn branches<'tcx>(
     // For enums, we prepend their variant index before the variant's fields so we can figure out
     // the variant again when just seeing a valtree.
     if let Some(variant) = variant {
-        branches.push(ty::ValTree::from_scalar_int(*ecx.tcx, variant.as_u32().into()));
+        branches.push(ty::Const::new_value(
+            *ecx.tcx,
+            ty::ValTree::from_scalar_int(*ecx.tcx, variant.as_u32().into()),
+            ecx.tcx.types.u32,
+        ));
     }
 
     for i in 0..field_count {
         let field = ecx.project_field(&place, FieldIdx::from_usize(i)).unwrap();
         let valtree = const_to_valtree_inner(ecx, &field, num_nodes)?;
-        branches.push(valtree);
+        branches.push(ty::Const::new_value(*ecx.tcx, valtree, field.layout.ty));
     }
 
     // Have to account for ZSTs here
@@ -65,7 +69,7 @@ fn slice_branches<'tcx>(
     for i in 0..n {
         let place_elem = ecx.project_index(place, i).unwrap();
         let valtree = const_to_valtree_inner(ecx, &place_elem, num_nodes)?;
-        elems.push(valtree);
+        elems.push(ty::Const::new_value(*ecx.tcx, valtree, place_elem.layout.ty));
     }
 
     Ok(ty::ValTree::from_branches(*ecx.tcx, elems))
@@ -200,8 +204,8 @@ fn reconstruct_place_meta<'tcx>(
         &ObligationCause::dummy(),
         |ty| ty,
         || {
-            let branches = last_valtree.unwrap_branch();
-            last_valtree = *branches.last().unwrap();
+            let branches = last_valtree.to_branch();
+            last_valtree = branches.last().unwrap().to_value().valtree;
             debug!(?branches, ?last_valtree);
         },
     );
@@ -212,7 +216,7 @@ fn reconstruct_place_meta<'tcx>(
     };
 
     // Get the number of elements in the unsized field.
-    let num_elems = last_valtree.unwrap_branch().len();
+    let num_elems = last_valtree.to_branch().len();
     MemPlaceMeta::Meta(Scalar::from_target_usize(num_elems as u64, &tcx))
 }
 
@@ -274,7 +278,7 @@ pub fn valtree_to_const_value<'tcx>(
             mir::ConstValue::ZeroSized
         }
         ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char | ty::RawPtr(_, _) => {
-            mir::ConstValue::Scalar(Scalar::Int(cv.valtree.unwrap_leaf()))
+            mir::ConstValue::Scalar(Scalar::Int(cv.to_leaf()))
         }
         ty::Pat(ty, _) => {
             let cv = ty::Value { valtree: cv.valtree, ty };
@@ -301,12 +305,13 @@ pub fn valtree_to_const_value<'tcx>(
                     || matches!(cv.ty.kind(), ty::Adt(def, _) if def.is_struct()))
             {
                 // A Scalar tuple/struct; we can avoid creating an allocation.
-                let branches = cv.valtree.unwrap_branch();
+                let branches = cv.to_branch();
                 // Find the non-ZST field. (There can be aligned ZST!)
                 for (i, &inner_valtree) in branches.iter().enumerate() {
                     let field = layout.field(&LayoutCx::new(tcx, typing_env), i);
                     if !field.is_zst() {
-                        let cv = ty::Value { valtree: inner_valtree, ty: field.ty };
+                        let cv =
+                            ty::Value { valtree: inner_valtree.to_value().valtree, ty: field.ty };
                         return valtree_to_const_value(tcx, typing_env, cv);
                     }
                 }
@@ -381,7 +386,7 @@ fn valtree_into_mplace<'tcx>(
             // Zero-sized type, nothing to do.
         }
         ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char | ty::RawPtr(..) => {
-            let scalar_int = valtree.unwrap_leaf();
+            let scalar_int = valtree.to_leaf();
             debug!("writing trivial valtree {:?} to place {:?}", scalar_int, place);
             ecx.write_immediate(Immediate::Scalar(scalar_int.into()), place).unwrap();
         }
@@ -391,13 +396,13 @@ fn valtree_into_mplace<'tcx>(
             ecx.write_immediate(imm, place).unwrap();
         }
         ty::Adt(_, _) | ty::Tuple(_) | ty::Array(_, _) | ty::Str | ty::Slice(_) => {
-            let branches = valtree.unwrap_branch();
+            let branches = valtree.to_branch();
 
             // Need to downcast place for enums
             let (place_adjusted, branches, variant_idx) = match ty.kind() {
                 ty::Adt(def, _) if def.is_enum() => {
                     // First element of valtree corresponds to variant
-                    let scalar_int = branches[0].unwrap_leaf();
+                    let scalar_int = branches[0].to_leaf();
                     let variant_idx = VariantIdx::from_u32(scalar_int.to_u32());
                     let variant = def.variant(variant_idx);
                     debug!(?variant);
@@ -425,7 +430,7 @@ fn valtree_into_mplace<'tcx>(
                 };
 
                 debug!(?place_inner);
-                valtree_into_mplace(ecx, &place_inner, *inner_valtree);
+                valtree_into_mplace(ecx, &place_inner, inner_valtree.to_value().valtree);
                 dump_place(ecx, &place_inner);
             }
 

compiler/rustc_const_eval/src/interpret/intrinsics/simd.rs

@@ -545,15 +545,15 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 let (right, right_len) = self.project_to_simd(&args[1])?;
                 let (dest, dest_len) = self.project_to_simd(&dest)?;
 
-                let index = generic_args[2].expect_const().to_value().valtree.unwrap_branch();
+                let index = generic_args[2].expect_const().to_branch();
                 let index_len = index.len();
 
                 assert_eq!(left_len, right_len);
                 assert_eq!(u64::try_from(index_len).unwrap(), dest_len);
 
                 for i in 0..dest_len {
                     let src_index: u64 =
-                        index[usize::try_from(i).unwrap()].unwrap_leaf().to_u32().into();
+                        index[usize::try_from(i).unwrap()].to_leaf().to_u32().into();
                     let dest = self.project_index(&dest, i)?;
 
                     let val = if src_index < left_len {
@@ -657,9 +657,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 self.check_simd_ptr_alignment(
                     ptr,
                     dest_layout,
-                    generic_args[3].expect_const().to_value().valtree.unwrap_branch()[0]
-                        .unwrap_leaf()
-                        .to_simd_alignment(),
+                    generic_args[3].expect_const().to_branch()[0].to_leaf().to_simd_alignment(),
                 )?;
 
                 for i in 0..dest_len {
@@ -689,9 +687,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 self.check_simd_ptr_alignment(
                     ptr,
                     args[2].layout,
-                    generic_args[3].expect_const().to_value().valtree.unwrap_branch()[0]
-                        .unwrap_leaf()
-                        .to_simd_alignment(),
+                    generic_args[3].expect_const().to_branch()[0].to_leaf().to_simd_alignment(),
                 )?;
 
                 for i in 0..vals_len {