Skip to content

Box in ValTreeKind::Branch(Box<[I::Const]>) changed to List#152593

Merged
rust-bors[bot] merged 4 commits intorust-lang:mainfrom
spirali:valtreekind-list
Mar 6, 2026
Merged

Box in ValTreeKind::Branch(Box<[I::Const]>) changed to List#152593
rust-bors[bot] merged 4 commits intorust-lang:mainfrom
spirali:valtreekind-list

Conversation

@spirali
Copy link
Contributor

@spirali spirali commented Feb 13, 2026

This is related to trait system refactoring. It fixes the FIXME in ValTreeKind

   // FIXME(mgca): Use a `List` here instead of a boxed slice
    Branch(Box<[I::Const]>),

It introduces Interner::Consts, changes Branch(Box<[I::Const]>) to Branch(I::Consts), and updates all relevant places.

r? lcnr

@rustbot
Copy link
Collaborator

rustbot commented Feb 13, 2026

Some changes occurred in match lowering

cc @Nadrieril

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 13, 2026
@rust-log-analyzer

This comment has been minimized.

let ValTreeKind::Branch(branch) = *valtree else {
bug!("malformed valtree for an enum")
};
let Some(actual_variant_idx) = branch.get(0) else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we stop asserting that the len is 1 here

@@ -179,7 +179,7 @@ impl<I: Interner> ValTreeKind<I> {
#[inline]
pub fn to_branch(&self) -> &[I::Const] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub fn to_branch(&self) -> &[I::Const] {
pub fn to_branch(&self) -> I::Consts {

@@ -195,7 +195,7 @@ impl<I: Interner> ValTreeKind<I> {
/// Attempts to convert to a `ValTreeKind::Branch` value.
pub fn try_to_branch(&self) -> Option<&[I::Const]> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove the fn kind method from trait ValTree and instead implement IntoKind for it now.

VaTreeKind can now be Copy

View changes since this review

}
// Otherwise, print the array separated by commas (or if it's a tuple)
(ty::ValTreeKind::Branch(fields), ty::Array(..) | ty::Tuple(..)) => {
let fields_iter = fields.iter().copied();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consts should implement SliceLike so we can just do .iter() here now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consts implements SliceLike, but .iter() on fields returns &Const and the following code works only for iterator over Const:

error[E0277]: the trait bound `&ty::consts::Const<'_>: print::Print<'tcx, Self>` is not satisfied
    --> compiler/rustc_middle/src/ty/print/pretty.rs:1964:34
     |
1964 | ...                   self.comma_sep(fields)?;
     |                            ^^^^^^^^^ the nightly-only, unstable trait `print::Print<'tcx, Self>` is not implemented for `&ty::consts::Const<'_>`
     |
help: the trait `Print<'tcx, _>` is not implemented for `&ty::consts::Const<'_>`
      but trait `Print<'_, _>` is implemented for `ty::consts::Const<'_>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think the as_slice should not be needed 🤔

Though given that we're in rustc_middle i guess this is the inherent impl for List or via deref?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still relevant, can you try removing the as_slice?

Copy link
Contributor

@lcnr lcnr Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ (get rid of as_slice)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry, I have overlooked your original reply. It is fixed now.

Comment on lines 580 to 583
.as_slice()
.iter()
.zip(branches_b.as_slice().iter())
.all(|(a, b)| relation.relate(*a, *b).is_ok())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.as_slice()
.iter()
.zip(branches_b.as_slice().iter())
.all(|(a, b)| relation.relate(*a, *b).is_ok())
.iter()
.zip(branches_b.iter())
.all(|(a, b)| relation.relate(a, b).is_ok())

@spirali
Copy link
Contributor Author

spirali commented Feb 15, 2026

please remove the fn kind method from trait ValTree and instead implement IntoKind for it now.

VaTreeKind can now be Copy

View changes since this review

When ValTreeKind is Copy, I get the following error:

error[E0283]: type annotations needed
   --> compiler/rustc_middle/src/ty/context.rs:564:79
    |
564 |                 interners.valtree.intern(v, |v| InternedInSet(interners.arena.alloc(v))).0,
    |                                                                               ^^^^^ cannot infer type of the type parameter `C` declared on the method `alloc`
    |
note: multiple `impl`s satisfying `rustc_type_ir::ValTreeKind<context::TyCtxt<'tcx>>: arena::ArenaAllocatable<'_, _>` found
   --> compiler/rustc_arena/src/lib.rs:646:5
    |
628 |    pub macro declare_arena([$($a:tt $name:ident: $ty:ty,)*]) {
    |    ----------------------- in this expansion of `rustc_arena::declare_arena!` (#2)
...
646 |        impl<'tcx, T: Copy> ArenaAllocatable<'tcx, rustc_arena::IsCopy> for T {
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
662 |            impl<'tcx> ArenaAllocatable<'tcx, rustc_arena::IsNotCopy> for $ty {
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

If I try to fix it through adding explicit type:

InternedInSet(self.interners.arena.alloc::<_, rustc_arena::IsCopy>(v))

I get:

error[E0277]: the trait bound `Allocation: arena::ArenaAllocatable<'_, IsCopy>` is not satisfied

I do not understand what is the problem here.

@lcnr
Copy link
Contributor

lcnr commented Feb 15, 2026

Can you try removing the ValTreeKind from

[] valtree: rustc_middle::ty::ValTreeKind<rustc_middle::ty::TyCtxt<'tcx>>,
valtrees can now be stored in the dropless arena.

I don't 100% know how to do here and you could look at how e.g. Ty<'tcx> arena allocation is handled

@rustbot
Copy link
Collaborator

rustbot commented Feb 17, 2026

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

@spirali
Copy link
Contributor Author

spirali commented Feb 17, 2026

Can you try removing the ValTreeKind from

[] valtree: rustc_middle::ty::ValTreeKind<rustc_middle::ty::TyCtxt<'tcx>>,

valtrees can now be stored in the dropless arena.

I don't 100% know how to do here and you could look at how e.g. Ty<'tcx> arena allocation is handled

Thank you, it is fixed now.

ValTreeKind is now copy, but in the following test returns a different hash:

static fn function_names::const_generic_fn_non_int<{CONST#ffa3db4ca1d52dce}>();

With this change, it is safe to update the test to a new hash value or I would just cover a deeper problem?

@rust-log-analyzer

This comment has been minimized.

@lcnr
Copy link
Contributor

lcnr commented Feb 22, 2026

With this change, it is safe to update the test to a new hash value or I would just cover a deeper problem?

I think this is fine, but not 100% sure 🤔 feel free to bless it and I then look at the change before merge

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors bot pushed a commit that referenced this pull request Feb 22, 2026
Box in `ValTreeKind::Branch(Box<[I::Const]>)` changed to `List`
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 22, 2026
@rust-bors
Copy link
Contributor

rust-bors bot commented Feb 22, 2026

☀️ Try build successful (CI)
Build commit: 7e95d4f (7e95d4f9eee300952b21051884d058f1f5bddea3, parent: 1500f0f47f5fe8ddcd6528f6c6c031b210b4eac5)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (7e95d4f): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -0.4%, secondary 0.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
1.9% [1.5%, 2.2%] 2
Regressions ❌
(secondary)
3.0% [3.0%, 3.0%] 1
Improvements ✅
(primary)
-1.9% [-3.0%, -0.9%] 3
Improvements ✅
(secondary)
-1.1% [-1.1%, -1.1%] 2
All ❌✅ (primary) -0.4% [-3.0%, 2.2%] 5

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 483.037s -> 480.952s (-0.43%)
Artifact size: 397.95 MiB -> 398.00 MiB (0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 22, 2026
//@ gdb-command:info functions -q function_names::const_generic_fn.*
//@ gdb-check:[...]static fn function_names::const_generic_fn_bool<false>();
//@ gdb-check:[...]static fn function_names::const_generic_fn_non_int<{CONST#ffa3db4ca1d52dce}>();
//@ gdb-check:[...]static fn function_names::const_generic_fn_non_int<{CONST#6dd80cc0c950c171}>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's fine, different HashStable impl between List and boxed slices 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Does it need some additional work from my side? All comments above should be resolved

Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me after that nit

View changes since this review

}
// Otherwise, print the array separated by commas (or if it's a tuple)
(ty::ValTreeKind::Branch(fields), ty::Array(..) | ty::Tuple(..)) => {
let fields_iter = fields.iter().copied();
Copy link
Contributor

@lcnr lcnr Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ (get rid of as_slice)

@Kobzol
Copy link
Member

Kobzol commented Mar 5, 2026

@bors delegate

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 5, 2026

✌️ @spirali, you can now approve this pull request!

If @lcnr told you to "r=me" after making some further change, then please make that change and post @bors r=lcnr.

@spirali
Copy link
Contributor Author

spirali commented Mar 5, 2026

@bors r=lcnr

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 5, 2026

📌 Commit f8998e9 has been approved by lcnr

It is now in the queue for this repository.

@rust-bors rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 5, 2026
@Kobzol
Copy link
Member

Kobzol commented Mar 5, 2026

The perf. result was neutral on the last perf. run, so:

@bors rollup=maybe

jhpratt added a commit to jhpratt/rust that referenced this pull request Mar 6, 2026
Box in `ValTreeKind::Branch(Box<[I::Const]>)` changed to `List`

This is related to trait system refactoring. It fixes the FIXME in `ValTreeKind`

```
   // FIXME(mgca): Use a `List` here instead of a boxed slice
    Branch(Box<[I::Const]>),
```

It introduces `Internator::Consts`, changes `Branch(Box<[I::Const]>)` to `Branch(I::Consts)`, and updates all relevant places.

r? lcnr
rust-bors bot pushed a commit that referenced this pull request Mar 6, 2026
Rollup of 7 pull requests

Successful merges:

 - #153466 (`rust-analyzer` subtree update)
 - #151280 (Fix incorrect trailing comma suggested in no_accessible_fields)
 - #152593 (Box in `ValTreeKind::Branch(Box<[I::Const]>)` changed to `List`)
 - #153189 (refactor: move `check_align` to `parse_alignment`)
 - #153230 (Roll rustfmt reviewers for in-tree rustfmt)
 - #153321 (Add high-priority ICEs to tests/crashes)
 - #153445 (Consider try blocks as block-like for overflowed expr)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Mar 6, 2026
Box in `ValTreeKind::Branch(Box<[I::Const]>)` changed to `List`

This is related to trait system refactoring. It fixes the FIXME in `ValTreeKind`

```
   // FIXME(mgca): Use a `List` here instead of a boxed slice
    Branch(Box<[I::Const]>),
```

It introduces `Interner::Consts`, changes `Branch(Box<[I::Const]>)` to `Branch(I::Consts)`, and updates all relevant places.

r? lcnr
rust-bors bot pushed a commit that referenced this pull request Mar 6, 2026
…uwer

Rollup of 9 pull requests

Successful merges:

 - #153466 (`rust-analyzer` subtree update)
 - #151280 (Fix incorrect trailing comma suggested in no_accessible_fields)
 - #152593 (Box in `ValTreeKind::Branch(Box<[I::Const]>)` changed to `List`)
 - #153174 (std: add wasm64 to sync::Once and thread_parking atomics cfg guards)
 - #153189 (refactor: move `check_align` to `parse_alignment`)
 - #153230 (Roll rustfmt reviewers for in-tree rustfmt)
 - #153445 (Consider try blocks as block-like for overflowed expr)
 - #153476 (bootstrap.py: fix typo "parallle")
 - #153483 (Preserve parentheses around `Fn` trait bounds in pretty printer)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Mar 6, 2026
Box in `ValTreeKind::Branch(Box<[I::Const]>)` changed to `List`

This is related to trait system refactoring. It fixes the FIXME in `ValTreeKind`

```
   // FIXME(mgca): Use a `List` here instead of a boxed slice
    Branch(Box<[I::Const]>),
```

It introduces `Interner::Consts`, changes `Branch(Box<[I::Const]>)` to `Branch(I::Consts)`, and updates all relevant places.

r? lcnr
rust-bors bot pushed a commit that referenced this pull request Mar 6, 2026
…uwer

Rollup of 14 pull requests

Successful merges:

 - #153466 (`rust-analyzer` subtree update)
 - #151280 (Fix incorrect trailing comma suggested in no_accessible_fields)
 - #152593 (Box in `ValTreeKind::Branch(Box<[I::Const]>)` changed to `List`)
 - #153174 (std: add wasm64 to sync::Once and thread_parking atomics cfg guards)
 - #153485 (libcore float tests: replace macro shadowing by const-compatible macro)
 - #153495 (Fix ICE in `offset_of!` error recovery)
 - #152040 (Do not emit ConstEvaluatable goals if type-const)
 - #152741 (Suppress invalid suggestions in destructuring assignment)
 - #153189 (refactor: move `check_align` to `parse_alignment`)
 - #153230 (Roll rustfmt reviewers for in-tree rustfmt)
 - #153445 (Consider try blocks as block-like for overflowed expr)
 - #153452 (Cleanup unused diagnostic emission methods)
 - #153476 (bootstrap.py: fix typo "parallle")
 - #153483 (Preserve parentheses around `Fn` trait bounds in pretty printer)
@rust-bors rust-bors bot merged commit 77658bc into rust-lang:main Mar 6, 2026
11 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Mar 6, 2026
rust-timer added a commit that referenced this pull request Mar 6, 2026
Rollup merge of #152593 - spirali:valtreekind-list, r=lcnr

Box in `ValTreeKind::Branch(Box<[I::Const]>)` changed to `List`

This is related to trait system refactoring. It fixes the FIXME in `ValTreeKind`

```
   // FIXME(mgca): Use a `List` here instead of a boxed slice
    Branch(Box<[I::Const]>),
```

It introduces `Interner::Consts`, changes `Branch(Box<[I::Const]>)` to `Branch(I::Consts)`, and updates all relevant places.

r? lcnr
github-actions bot pushed a commit to rust-lang/rust-analyzer that referenced this pull request Mar 9, 2026
…uwer

Rollup of 14 pull requests

Successful merges:

 - rust-lang/rust#153466 (`rust-analyzer` subtree update)
 - rust-lang/rust#151280 (Fix incorrect trailing comma suggested in no_accessible_fields)
 - rust-lang/rust#152593 (Box in `ValTreeKind::Branch(Box<[I::Const]>)` changed to `List`)
 - rust-lang/rust#153174 (std: add wasm64 to sync::Once and thread_parking atomics cfg guards)
 - rust-lang/rust#153485 (libcore float tests: replace macro shadowing by const-compatible macro)
 - rust-lang/rust#153495 (Fix ICE in `offset_of!` error recovery)
 - rust-lang/rust#152040 (Do not emit ConstEvaluatable goals if type-const)
 - rust-lang/rust#152741 (Suppress invalid suggestions in destructuring assignment)
 - rust-lang/rust#153189 (refactor: move `check_align` to `parse_alignment`)
 - rust-lang/rust#153230 (Roll rustfmt reviewers for in-tree rustfmt)
 - rust-lang/rust#153445 (Consider try blocks as block-like for overflowed expr)
 - rust-lang/rust#153452 (Cleanup unused diagnostic emission methods)
 - rust-lang/rust#153476 (bootstrap.py: fix typo "parallle")
 - rust-lang/rust#153483 (Preserve parentheses around `Fn` trait bounds in pretty printer)
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Mar 21, 2026
…uwer

Rollup of 14 pull requests

Successful merges:

 - rust-lang/rust#153466 (`rust-analyzer` subtree update)
 - rust-lang/rust#151280 (Fix incorrect trailing comma suggested in no_accessible_fields)
 - rust-lang/rust#152593 (Box in `ValTreeKind::Branch(Box<[I::Const]>)` changed to `List`)
 - rust-lang/rust#153174 (std: add wasm64 to sync::Once and thread_parking atomics cfg guards)
 - rust-lang/rust#153485 (libcore float tests: replace macro shadowing by const-compatible macro)
 - rust-lang/rust#153495 (Fix ICE in `offset_of!` error recovery)
 - rust-lang/rust#152040 (Do not emit ConstEvaluatable goals if type-const)
 - rust-lang/rust#152741 (Suppress invalid suggestions in destructuring assignment)
 - rust-lang/rust#153189 (refactor: move `check_align` to `parse_alignment`)
 - rust-lang/rust#153230 (Roll rustfmt reviewers for in-tree rustfmt)
 - rust-lang/rust#153445 (Consider try blocks as block-like for overflowed expr)
 - rust-lang/rust#153452 (Cleanup unused diagnostic emission methods)
 - rust-lang/rust#153476 (bootstrap.py: fix typo "parallle")
 - rust-lang/rust#153483 (Preserve parentheses around `Fn` trait bounds in pretty printer)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants