clean up handling of paths in mgca by khyperia · Pull Request #160216 · rust-lang/rust

khyperia · 2026-07-30T13:33:51Z

related tracking issue: #132980

Handling of ambiguous type-or-constant argument paths was really wonky and distinct from the regular can_lower_expr_to_const_arg_direct lowering paths. Clean that up and unify it, as discussed here #158479 (comment)

Also, do a bunch of other little cleanups at the same time while I'm here:

add support for parens to the inside of direct_const_arg! and macroless_generic_const_args (idk why this wasn't a thing before)
add constant and lifetime ribs when resolving TyKind::DirectConstArg - note the TyKind, not ExprKind (~~I am unsure if/how this changes behavior~~ added a test, direct-const-arg-correct-rib.rs) - this was discussed here allow mGCA const arguments to fall back to anon consts #158617 (comment)
minor debug logging change
various minor code shuffles/cleanups
add test for direct_const_arg!(_) inferring to a type - this test should have been added in implement #![feature(macroless_generic_const_args)] #159058 which added this behavior (well, changed the behavior from an ICE to inferring to a type)
add a test asserting the current behavior of parens/braces/underscores on stable - one or two of these cases already have tests elsewhere, but it's nice to assert the whole set, and also assert the whole set in the same place so it's easily visible to be able to give me confidence the whole set actually is tested (both positives and negatives are important here)
add a test similar to the previous one, but with mgca enabled, asserting all combinations now compile successfully

r? @BoxyUwU

rustbot · 2026-07-30T13:33:58Z

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

bit-aloo

LGTM. Few questions and nits.

View changes since this review

bit-aloo · 2026-07-31T16:18:39Z

+    }
+
+    fn can_lower_path_to_const_arg_direct(
+        &mut self,


nit:

Suggested change

&mut self,

&self,

thanks! I also removed the mut from can_lower_expr_to_const_arg_direct :3

bit-aloo · 2026-07-31T16:19:45Z

+            TyKind::DirectConstArg(expr) => self.resolve_anon_const_manual(
+                true,
+                AnonConstKind::ConstArg(IsRepeatExpr::No),
+                |this| this.resolve_expr(&expr, None),


nit:

Suggested change

|this| this.resolve_expr(&expr, None),

|this| this.resolve_expr(expr, None),

out of curiosity, how did you spot this? just a good eye, or do you have some sort of setup where you can run clippy locally or something, and you checked out my code?

Nothing really. I was reading through the code, fiddling around, and somehow stumbled 😄.

bit-aloo · 2026-07-31T16:37:37Z

+                    {
+                        let ct =
+                            self.lower_const_path_to_const_arg(&None, path, res, ty.id, ty.span);
+                        let ct = self.arena.alloc(ct);


Curious why we do arena alloc here?

GenericArg::Const(&'hir ConstArg<..>) is arena'd, so it needs an arena alloc somewhere

lower_anon_const_to_const_arg returns ConstArg by value (this has been the case since before I started working on this code)

there's a lower_anon_const_to_const_arg_and_alloc wrapper, fwiw

which requires lower_expr_to_const_arg_direct to return it by value

which requires lower_path_to_const_arg_direct to do the same

and then I said shrug, make lower_const_path_to_const_arg do the same, since it's a parallel to lower_anon_const_to_const_arg

so we arena alloc in the caller of lower_const_path_to_const_arg, here

there's no particular reason why this whole set of functions (lower_anon_const_to_const_arg/lower_expr_to_const_arg_direct/lower_const_path_to_const_arg/lower_path_to_const_arg_direct) returns by value instead of by &'hir, could go the other way around, but they should all be consistently the same IMO.

I guess in other words:

the diff adds an arena alloc here, because I removed it from lower_const_path_to_const_arg, because I wanted lower_const_path_to_const_arg to be more parallel to lower_anon_const_to_const_arg: they are conceptually extremely similar functions (take an X, attempt to lower it to a direct const arg, and fall back to anon const if that fails), just X is a path for one, and an AST anon const in the other.

Makes sense. Thanks for detailed explanation. 😄

BoxyUwU · 2026-08-10T11:44:31Z

@bors r=BoxyUwU,bit-aloo

rust-bors · 2026-08-10T11:44:34Z

📌 Commit 2f54b38 has been approved by BoxyUwU,bit-aloo

It is now in the queue for this repository.

…uwer Rollup of 10 pull requests Successful merges: - #160841 (`rust-analyzer` subtree update) - #160253 (Add CI job for checking stdlib semver compatibility) - #160216 (clean up handling of paths in mgca) - #160832 (Fix funding link) - #160833 (Remove `analysis` arg from `ResultsVisitor` methods) - #160834 (Update Enzyme submodule) - #160836 (Fix swapped documentation lines on `io::Read`) - #160839 (Stop updating npm lockfile by Renovatebot) - #160840 (Rename `SmallCopyList` to `SmallCopySet`) - #160846 (use rustc_attr_ir imports more)

@BoxyUwU

Rollup merge of #160216 - khyperia:gca-path-cleanup, r=BoxyUwU,bit-aloo clean up handling of paths in mgca related tracking issue: #132980 Handling of ambiguous type-or-constant argument paths was really wonky and distinct from the regular `can_lower_expr_to_const_arg_direct` lowering paths. Clean that up and unify it, as discussed here #158479 (comment) Also, do a bunch of other little cleanups at the same time while I'm here: - add support for parens to the inside of direct_const_arg! and macroless_generic_const_args (idk why this wasn't a thing before) - add constant and lifetime ribs when resolving TyKind::DirectConstArg - note the TyKind, not ExprKind (~~I am unsure if/how this changes behavior~~ added a test, `direct-const-arg-correct-rib.rs`) - this was discussed here #158617 (comment) - minor debug logging change - various minor code shuffles/cleanups - add test for `direct_const_arg!(_)` inferring to a type - this test should have been added in #159058 which added this behavior (well, changed the behavior from an ICE to inferring to a type) - add a test asserting the current behavior of parens/braces/underscores on stable - one or two of these cases already have tests elsewhere, but it's nice to assert the whole set, and also assert the whole set in the same place so it's easily visible to be able to give me confidence the whole set actually is tested (both positives and negatives are important here) - add a test similar to the previous one, but with mgca enabled, asserting all combinations now compile successfully r? @BoxyUwU

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 Jul 30, 2026

rustbot assigned BoxyUwU Jul 30, 2026

This comment has been minimized.

Sign in to view

khyperia force-pushed the gca-path-cleanup branch 2 times, most recently from 8dda2c6 to 5b41ff7 Compare July 31, 2026 07:44

bit-aloo reviewed Jul 31, 2026

View reviewed changes

clean up handling of paths in mgca

2f54b38

khyperia force-pushed the gca-path-cleanup branch from 5b41ff7 to 2f54b38 Compare August 1, 2026 10:38

BoxyUwU mentioned this pull request Aug 6, 2026

Full Const Generics rust-lang/goals#100

Open

7 tasks

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 Aug 10, 2026

JonathanBrouwer mentioned this pull request Aug 10, 2026

Rollup of 10 pull requests #160849

Merged

rust-bors Bot merged commit 34df6a0 into rust-lang:main Aug 10, 2026
13 checks passed

rustbot added this to the 1.99.0 milestone Aug 10, 2026

	\|this\| this.resolve_expr(&expr, None),
	\|this\| this.resolve_expr(expr, None),

Uh oh!

Uh oh!

Conversation

khyperia commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 30, 2026

Uh oh!

This comment has been minimized.

bit-aloo left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bit-aloo Aug 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BoxyUwU commented Aug 10, 2026

Uh oh!

rust-bors Bot commented Aug 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

khyperia commented Jul 30, 2026 •

edited

Loading

bit-aloo left a comment •

edited by rustbot

Loading

bit-aloo Aug 2, 2026 •

edited

Loading