Skip to content

Conversation

@Kivooeo
Copy link
Member

@Kivooeo Kivooeo commented Dec 26, 2025

r? BoxyUwU

as you suggested yesterday (will add more context like links when it stops being a draft)

tried to split this into some meaningful commits hope it help ^^

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 26, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 26, 2025

BoxyUwU is currently at their maximum review capacity.
They may take a while to respond.

@rust-log-analyzer

This comment has been minimized.

@BoxyUwU
Copy link
Member

BoxyUwU commented Dec 27, 2025

btw dont worry about the commit history- I expect this PR is simple enough that just one is fine. thanks for splitting it up nicely though :-)

@rust-log-analyzer

This comment has been minimized.

@Kivooeo
Copy link
Member Author

Kivooeo commented Dec 27, 2025

oh no i forget about semicolon

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Kivooeo
Copy link
Member Author

Kivooeo commented Dec 27, 2025

seems like it also fixes #131052?

Copy link
Member

@BoxyUwU BoxyUwU left a comment

Choose a reason for hiding this comment

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

Can you squash all these commits then undraft the PR? I think its basically done after these reviews. I just want to spend a bit of time thinking over why this fixed that ICE but I don't expect that to take me long

View changes since this review

@Kivooeo
Copy link
Member Author

Kivooeo commented Dec 27, 2025

Can you squash all these commits then undraft the PR?

yeah, this commits are only for while it's draft

@Kivooeo
Copy link
Member Author

Kivooeo commented Dec 27, 2025

hopefully i didnt mess with commits

@rust-log-analyzer

This comment has been minimized.

}
(ty::ValTreeKind::Branch(fields), ty::Adt(def, args)) => {
let contents = cv.destructure_adt_const();
let fields = fields.iter().copied();
Copy link
Member

Choose a reason for hiding this comment

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

using fields here is wrong as it has more stuff in it than contents' fields

@Kivooeo Kivooeo marked this pull request as ready for review December 28, 2025 12:57
@rustbot
Copy link
Collaborator

rustbot commented Dec 28, 2025

This PR changes a file inside tests/crashes. If a crash was fixed, please move into the corresponding ui subdir and add 'Fixes #' to the PR description to autoclose the issue upon merge.

Some changes occurred in rustc_ty_utils::consts.rs

cc @BoxyUwU

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 28, 2025
@BoxyUwU
Copy link
Member

BoxyUwU commented Dec 28, 2025

pls squash commits and look at the two leftover reviews, thx

@rustbot

This comment has been minimized.

@rustbot

This comment was marked as resolved.

@rustbot
Copy link
Collaborator

rustbot commented Dec 28, 2025

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rustbot

This comment has been minimized.

@@ -0,0 +1,13 @@
//! Regression test for <https://github.com/rust-lang/rust/issues/131052>
Copy link
Member

Choose a reason for hiding this comment

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

I think what's happened here is that we're now masking an ICE that was originally caused by lit_to_const returning ValTree's with incorrect types 🤔 I can't think of a way to write a test that exposes that underlying brokenness so I think this is fine.

It would be nice to fix the underlying brokenness regardless. Would you be interested in doing that? The short explanation is that we lit_to_const for byte strings doesn't check that the type of the parameter the byte string is an argument to, is &[u8] is only checks if its a reference to a slice.

@BoxyUwU
Copy link
Member

BoxyUwU commented Dec 28, 2025

@bors r+ rollup

thanks for the PR ✨

@bors
Copy link
Collaborator

bors commented Dec 28, 2025

📌 Commit b29cc98 has been approved by BoxyUwU

It is now in the queue for this repository.

@bors bors 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 Dec 28, 2025
@Kivooeo
Copy link
Member Author

Kivooeo commented Dec 28, 2025

i also was wondering, during this i've seen a lot of FIXME(mgca) and some of them seemed pretty solvable, at least at a first glance, would this be reasonable to try to solve them, or it'd be better to ask you first?

@Kivooeo
Copy link
Member Author

Kivooeo commented Dec 28, 2025

Would you be interested in doing that?

yeah i guess i could take a look potentially, around next few days maybe

@BoxyUwU
Copy link
Member

BoxyUwU commented Dec 28, 2025

i also was wondering, during this i've seen a lot of FIXME(mgca) and some of them seemed pretty solvable, at least at a first glance, would this be reasonable to try to solve them, or it'd be better to ask you first?

some of them are probably a lot harder than others. if you ask me first I can tell you whether it's likely to be a huge pain or relatively simple :) in general though help with mgca is much appreciated

JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Dec 28, 2025
refactor `destructure_const`

r? BoxyUwU

as you suggested yesterday (will add more context like links when it stops being a draft)

tried to split this into some meaningful commits hope it help ^^
bors added a commit that referenced this pull request Dec 28, 2025
…uwer

Rollup of 8 pull requests

Successful merges:

 - #148321 (parser/lexer: bump to Unicode 17, use faster unicode-ident)
 - #149540 (std: sys: fs: uefi: Implement readdir)
 - #149582 (Implement `Duration::div_duration_{floor,ceil}`)
 - #149663 (Optimized implementation for uN::{gather,scatter}_bits)
 - #149667 (Fix ICE by rejecting const blocks in patterns during AST lowering (closes #148138))
 - #149947 (add several older crashtests)
 - #150011 (Add more `unbounded_sh[lr]` examples)
 - #150411 (refactor `destructure_const`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1117bd9 into rust-lang:main Dec 29, 2025
11 checks passed
rust-timer added a commit that referenced this pull request Dec 29, 2025
Rollup merge of #150411 - Kivooeo:somefunnystuff, r=BoxyUwU

refactor `destructure_const`

r? BoxyUwU

as you suggested yesterday (will add more context like links when it stops being a draft)

tried to split this into some meaningful commits hope it help ^^
@rustbot rustbot added this to the 1.94.0 milestone Dec 29, 2025
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.

5 participants