Skip to content

Deconstruct values in the HIR visitor #141931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 3, 2025
Merged

Deconstruct values in the HIR visitor #141931

merged 1 commit into from
Jun 3, 2025

Conversation

ArtemIsmagilov
Copy link
Contributor

@ArtemIsmagilov ArtemIsmagilov commented Jun 2, 2025 •
edited
Loading

I continue to add deconstruction for task #141849
The changes concern a more complex part of the task compiler/rustc_hir/src/intravisit.rs
r? @nnethercote

@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 Jun 2, 2025
@ArtemIsmagilov ArtemIsmagilov changed the title deconstruct2 compiler/rustc_hir/src/intravisit.rs deconstruct2 compiler/rustc_hir/src/intravisit.rs Jun 2, 2025
@nnethercote nnethercote changed the title deconstruct2 compiler/rustc_hir/src/intravisit.rs Deconstruct values in the THIR visitor Jun 3, 2025
nnethercote
nnethercote reviewed Jun 3, 2025
View reviewed changes
compiler/rustc_hir/src/intravisit.rs
@@ -1018,8 +1040,9 @@ pub fn walk_const_arg<'v, V: Visitor<'v>>(
match const_arg.try_as_ambig_ct() {
Some(ambig_ct) => visitor.visit_const_arg(ambig_ct),
None => {
try_visit!(visitor.visit_id(const_arg.hir_id));
visitor.visit_infer(const_arg.hir_id, const_arg.span(), InferKind::Const(const_arg))
let ConstArg { hir_id, kind: _ } = const_arg;
Copy link
Contributor

Choose a reason for hiding this comment

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

kind being skipped here might be a bug. No need to do anything about it in this PR, we can fix it elsewhere, but great that this PR has identified this!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

need reverse?

Copy link
Contributor

Choose a reason for hiding this comment

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

Leave it unchanged. If it needs fixing we can do in a follow-up.

nnethercote
nnethercote reviewed Jun 3, 2025
View reviewed changes
compiler/rustc_hir/src/intravisit.rs
@@ -1288,16 +1332,20 @@ pub fn walk_precise_capturing_arg<'v, V: Visitor<'v>>(
) -> V::Result {
match *arg {
PreciseCapturingArg::Lifetime(lt) => visitor.visit_lifetime(lt),
PreciseCapturingArg::Param(param) => visitor.visit_id(param.hir_id),
PreciseCapturingArg::Param(param) => {
let PreciseCapturingNonLifetimeArg { hir_id, ident: _, res: _ } = param;
Copy link
Contributor

Choose a reason for hiding this comment

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

Skipping the ident here is also suspicious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

need reverse?

Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor

Choose a reason for hiding this comment

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

I filed #141999 to fix this.

@nnethercote
Copy link
Contributor

Can you change the commit message to say "Deconstruct values in the THIR visitor"? Then this will be ready to merge. Thanks.

@ArtemIsmagilov
Copy link
Contributor Author

Thanks for the review! Tell me, taking into account your comments, should I still remove the destructor in match expressions?

@nnethercote
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 3, 2025

📌 Commit 624e1cd has been approved by nnethercote

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 Jun 3, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 3, 2025
Merged
bors added a commit that referenced this pull request Jun 3, 2025
@bors
Auto merge of #141961 - matthiaskrgr:rollup-r09j2sp, r=matthiaskrgr
a124fb3 
Rollup of 8 pull requests

Successful merges:

 - #141724 (fix(#141141): When expanding `PartialEq`, check equality of scalar types first.)
 - #141833 (`tests/ui`: A New Order [2/N])
 - #141861 (Switch `x86_64-msvc-{1,2}` back to Windows Server 2025 images)
 - #141914 (redesign stage 0 std follow-ups)
 - #141918 (Deconstruct values in the THIR visitor)
 - #141923 (Update books)
 - #141931 (Deconstruct values in the THIR visitor)
 - #141956 (Remove two trait methods from cg_ssa)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 757d0e7 into rust-lang:master Jun 3, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 3, 2025
rust-timer added a commit that referenced this pull request Jun 3, 2025
@rust-timer
Unrolled build for #141931
cf65316 
Rollup merge of #141931 - ArtemIsmagilov:issue-141849_2, r=nnethercote

Deconstruct values in the THIR visitor

I continue to add deconstruction for task #141849
The changes concern a more complex part of the task `compiler/rustc_hir/src/intravisit.rs`
r? `@nnethercote`
@nnethercote nnethercote changed the title Deconstruct values in the THIR visitor Deconstruct values in the HIR visitor Jun 3, 2025
@nnethercote
Copy link
Contributor

Whoops, the commit message should have said "HIR visitor" instead of "THIR visitor". Oh well, too late now :)

@ArtemIsmagilov ArtemIsmagilov deleted the issue-141849_2 branch June 4, 2025 05:45
@ArtemIsmagilov
Copy link
Contributor Author

Thanks again for your help with the first rust commit!

@nnethercote nnethercote mentioned this pull request Jun 4, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nnethercote nnethercote nnethercote left review comments

Assignees

@nnethercote nnethercote

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
Milestone
1.89.0
Development

Successfully merging this pull request may close these issues.
4 participants
@ArtemIsmagilov @nnethercote @bors @rustbot