Skip to content

yeet alias new_from_def_id - #162760

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
khyperia:yeet-new_from_def_id
Sep 17, 2026
Merged

rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
khyperia:yeet-new_from_def_id

Conversation

@khyperia

@khyperia khyperia commented Sep 14, 2026

Copy link
Copy Markdown
Member

this PR fixes the // FIXME: remove in favor of explicit construction on alias_term_kind_from_def_id

vaguely related to rust-lang/project-const-generics#98 / #152245

fyi @lcnr who didn't like AliasConstInherentArgsKind #161929 (comment) (it gets yeeted in this PR)

r? @BoxyUwU

@rustbot rustbot added PG-exploit-mitigations Project group: Exploit mitigations 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 Sep 14, 2026
@rustbot

rustbot commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

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

@khyperia
khyperia marked this pull request as ready for review September 14, 2026 10:56
@rustbot

rustbot commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in compiler/rustc_sanitizers

cc @rcvalle

Some changes occurred in match checking

cc @Nadrieril

Some changes occurred in rustc_ty_utils::consts.rs

cc @BoxyUwU

HIR ty lowering was modified

cc @fmease

@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 Sep 14, 2026
ct: ty::AliasConst::new(
self.tcx,
ty::AliasConstKind::Anon { def_id: did },
self.tcx.mk_args(&[]),

@khyperia khyperia Sep 14, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

debating if I should pull this change out into another PR:

this changes the args from the ADT's args to []. This is because discriminant anon consts have no generics, but we were passing the ADT's generics to it here. This is wrong, and with this change, the debug assert in AliasConst::new for correct generic args triggers. Before, we were simply harmlessly ignoring the extra generic args.

View changes since the review

@bit-aloo bit-aloo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM 🚀 . Few questions though..

View changes since this review

Comment on lines 73 to 76
let user_ty = user_ty.as_ref().and_then(push_cuta);
// Under generic_const_args, `def_id` might be a regular const declared in a trait, but
// is `impl`d as a directly represented const. We do not know whether it is here, so we
// must use type system normalization for all consts under generic_const_args.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we reword this?

Comment thread compiler/rustc_mir_build/src/thir/cx/expr.rs
@rust-bors

This comment has been minimized.

@khyperia
khyperia force-pushed the yeet-new_from_def_id branch from 7fc2f33 to 0ace795 Compare September 16, 2026 04:43
@rustbot

rustbot commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

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.

@BoxyUwU BoxyUwU left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thx bit-aloo for reviews :3

View changes since this review

Comment on lines -531 to -532
def_id: DefId,
args: GenericArgsRef<'tcx>,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

unsure about this change, we want to maintain a fairly clear distinction between type system consts and non-type-system contains and this blurs that a bit 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

would quite like to not do this and then hand write the old logic into our handling of the THIR jank with a FIXME about how we don't want to be doing that 🤔

@khyperia
khyperia force-pushed the yeet-new_from_def_id branch from 0ace795 to caedbd8 Compare September 17, 2026 04:55
@khyperia

Copy link
Copy Markdown
Member Author

with the latest update, this bit in the PR description no longer applies, this PR is no longer perf-questionable:

this PR might be slightly perf-positive due to calling def_kind less. the change to THIR ExprKind is maybe very tiny bit different perf characteristics too but highly unlikely.

well, there's still one or tw.o less calls to def_kind, but less significant than it was, and the change to THIR ExprKind has been reverted

noting it here rather than just doing a strikethrough in the PR description since the PR description gets committed ✨

@BoxyUwU BoxyUwU left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

did you handle bit-aloo's review about rewording a comment already?

View changes since this review

@khyperia

Copy link
Copy Markdown
Member Author

did you handle bit-aloo's review about rewording a comment already?

the review is no longer applicable - the rewording was because I redid the def_id field to ct, so the comment should have said ct instead of def_id. but then I reverted the thing, so the comment should stay as-is.

@BoxyUwU

BoxyUwU commented Sep 17, 2026

Copy link
Copy Markdown
Member

@bors r+

@rust-bors

rust-bors Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

📌 Commit caedbd8 has been approved by BoxyUwU

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 Sep 17, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 17, 2026
…oxyUwU

yeet alias new_from_def_id

this PR fixes the `// FIXME: remove in favor of explicit construction` on `alias_term_kind_from_def_id`

vaguely related to rust-lang/project-const-generics#98 / rust-lang#152245

fyi @lcnr who didn't like `AliasConstInherentArgsKind` rust-lang#161929 (comment) (it gets yeeted in this PR)

r? @BoxyUwU
rust-bors Bot pushed a commit that referenced this pull request Sep 17, 2026
…uwer

Rollup of 24 pull requests

Successful merges:

 - #161596 (coretests: Add more pattern tests.)
 - #162177 (Properly implement the gpu-kernel ABI for amdgpu)
 - #162411 (Make Receiver `#[rustc_dyn_incompatible_trait]`)
 - #162760 (yeet alias new_from_def_id)
 - #162796 (libtest: do not early exit from test runners)
 - #162844 (Add loan reachability traces to polonius MIR dumps)
 - #162876 (Move operations out of `rustc_middle::query::job`)
 - #160108 (Stabilize `windows_process_extensions_main_thread_handle`)
 - #160212 (traits: Fix rigid alias liveness matching)
 - #160544 (Stabilize `feature(trim_prefix_suffix)` (`{str, [T], Path}::trim_prefix` and `{str, [T]}::trim_suffix`))
 - #161305 (Use the entire type of a dropped local to compute variance (edge direction) for Polonius alpha)
 - #161838 (tests: accept LLVM 24 optimization in this test)
 - #162312 (core: Rewrite docs for try_as_dyn)
 - #162785 (Avoid creating overlapping assignments in MatchBranchSimplification)
 - #162805 (Add `must_use` lint to `ExitCode`)
 - #162825 (core: Add examples for `debug_closure_helpers`)
 - #162841 (enable asm tests for xtensa targets)
 - #162842 (reintroduce check RibKind::ConstParamTy did in direct consts)
 - #162845 (mgca: fix issue with mismatched array valtree/valtree tys)
 - #162856 (Stabilize CommandExt::show_window)
 - #162865 (Complex conjugate, negation and default)
 - #162874 (Add support for `annotate_snippets::snippet::AnnotationKind::Visible`)
 - #162881 (Simplify the macro for forwarding Decoder methods )
 - #162888 (Fix a typo on the Armv7-R platform docs page)
rust-bors Bot pushed a commit that referenced this pull request Sep 17, 2026
…uwer

Rollup of 23 pull requests

Successful merges:

 - #161596 (coretests: Add more pattern tests.)
 - #162411 (Make Receiver `#[rustc_dyn_incompatible_trait]`)
 - #162760 (yeet alias new_from_def_id)
 - #162796 (libtest: do not early exit from test runners)
 - #162844 (Add loan reachability traces to polonius MIR dumps)
 - #162876 (Move operations out of `rustc_middle::query::job`)
 - #160108 (Stabilize `windows_process_extensions_main_thread_handle`)
 - #160212 (traits: Fix rigid alias liveness matching)
 - #160544 (Stabilize `feature(trim_prefix_suffix)` (`{str, [T], Path}::trim_prefix` and `{str, [T]}::trim_suffix`))
 - #161305 (Use the entire type of a dropped local to compute variance (edge direction) for Polonius alpha)
 - #161838 (tests: accept LLVM 24 optimization in this test)
 - #162312 (core: Rewrite docs for try_as_dyn)
 - #162785 (Avoid creating overlapping assignments in MatchBranchSimplification)
 - #162805 (Add `must_use` lint to `ExitCode`)
 - #162825 (core: Add examples for `debug_closure_helpers`)
 - #162841 (enable asm tests for xtensa targets)
 - #162842 (reintroduce check RibKind::ConstParamTy did in direct consts)
 - #162845 (mgca: fix issue with mismatched array valtree/valtree tys)
 - #162856 (Stabilize CommandExt::show_window)
 - #162865 (Complex conjugate, negation and default)
 - #162874 (Add support for `annotate_snippets::snippet::AnnotationKind::Visible`)
 - #162881 (Simplify the macro for forwarding Decoder methods )
 - #162888 (Fix a typo on the Armv7-R platform docs page)
@rust-bors
rust-bors Bot merged commit 7ddb377 into rust-lang:main Sep 17, 2026
13 checks passed
@rustbot rustbot added this to the 1.100.0 milestone Sep 17, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 17, 2026
Rollup merge of #162760 - khyperia:yeet-new_from_def_id, r=BoxyUwU

yeet alias new_from_def_id

this PR fixes the `// FIXME: remove in favor of explicit construction` on `alias_term_kind_from_def_id`

vaguely related to rust-lang/project-const-generics#98 / #152245

fyi @lcnr who didn't like `AliasConstInherentArgsKind` #161929 (comment) (it gets yeeted in this PR)

r? @BoxyUwU
}
}
DefKind::Const => ty::AliasConstKind::Free { def_id },
_ => unreachable!(),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

reachable via #162923

Its always better here to print the actual Defkind or whatever we are not expecting to be reachable as this makes categorizing the ICE a bit easier imho :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

aaaah sorry!! #162937

@khyperia
khyperia deleted the yeet-new_from_def_id branch September 18, 2026 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PG-exploit-mitigations Project group: Exploit mitigations 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