Skip to content

mGCA: error on region vars lowering #157188

Open
Human9000-bit wants to merge 1 commit into
rust-lang:mainfrom
Human9000-bit:mgca-157147
Open

mGCA: error on region vars lowering #157188
Human9000-bit wants to merge 1 commit into
rust-lang:mainfrom
Human9000-bit:mgca-157147

Conversation

@Human9000-bit

@Human9000-bit Human9000-bit commented May 31, 2026

Copy link
Copy Markdown
Member

This also needs to error on min_generic_const_args feature

cc @BoxyUwU (didn't r her as she is unavailable rn)

Fixes #157147

@rustbot

rustbot commented May 31, 2026

Copy link
Copy Markdown
Collaborator

HIR ty lowering was modified

cc @fmease

@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 May 31, 2026
@rustbot

rustbot commented May 31, 2026

Copy link
Copy Markdown
Collaborator

Failed to set assignee to her: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@Human9000-bit

Copy link
Copy Markdown
Member Author

@rustbot reroll

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@reddevilmidzy reddevilmidzy 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.

And if you wrap it in backticks, it probably won't be recognized as a command.😄

View changes since this review

Comment thread tests/ui/const-generics/mgca/region_vars_hashed.rs Outdated
@BoxyUwU

BoxyUwU commented Jun 1, 2026

Copy link
Copy Markdown
Member

r? BoxyUwU

@rustbot rustbot assigned BoxyUwU and unassigned chenyukang Jun 1, 2026
@BoxyUwU

BoxyUwU commented Jun 1, 2026

Copy link
Copy Markdown
Member

I expect that a similar case should be possible without min_generic_const_args, instead using generic_const_parameter_types e.g. something like Foo<'a, const N: &'a ()> and then Foo<'_, { &() }> . In general I don't see a problem extending this fix to happen even on stable as it should never be meaningfully reachable (though it would be good to have a test case where it actually matters)

@Human9000-bit Human9000-bit force-pushed the mgca-157147 branch 3 times, most recently from 60f92d8 to e52b714 Compare June 2, 2026 13:46
@Human9000-bit

Human9000-bit commented Jun 2, 2026

Copy link
Copy Markdown
Member Author

Is that what you meant?

I found a case with generic_const_parameter_types, but it seems that there is no obvious case to trigger the ICE without const generics features
(lowk used ai to find gcpt case)

@Human9000-bit

Human9000-bit commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

it seems that that clanker ai hasn't checked that it triggers ICE even on non-incremental. Is there a problem with that?

EDIT: ofc there is. Removed the gcpt case

@BoxyUwU

BoxyUwU commented Jun 7, 2026

Copy link
Copy Markdown
Member

I actually am somewhat confused here, I thought that the has_free_regions check would trigger if there are region variables in the expected type. For example the following does error:

#![feature(generic_const_parameter_types, adt_const_params, const_param_ty_unchecked)]

fn evil<T, const R: T>() {}

fn foo<const N: usize>() {
    evil::<&u32, { &1_u32 }>();
}

what's the difference in expected types of the { &1_u32 } anon const in this example, and the const { &0_u8 } anon const in the mgca example? I would expect both of these to ICE not just the mGCA one

@BoxyUwU

BoxyUwU commented Jun 7, 2026

Copy link
Copy Markdown
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 7, 2026
@Human9000-bit

Human9000-bit commented Jun 14, 2026

Copy link
Copy Markdown
Member Author

I would expect both of these to ICE not just the mGCA one

Your example is correct in terms of it containing region var, but it being caught by existing check:

if tcx.features().generic_const_parameter_types()
&& (ty.has_free_regions() || ty.has_erased_regions())

, because gcpt enabled and const having free regions. Does that make sense?
@rustbot ready

@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 Jun 14, 2026
@BoxyUwU

BoxyUwU commented Jun 15, 2026

Copy link
Copy Markdown
Member

hmm, what I meant is why does the ICEing example not get caught by that existing check. it too should have free regions in the type. i got confused before though and I realise now it's jsut because the check for this stuff only happens under gcpt but mgca also allows for gcpts implicitly.

I also realise now there's operator precedence jank at play in this diff 😅 :

if tcx.features().generic_const_parameter_types()
    && (ty.has_free_regions() || ty.has_erased_regions())
    || ty.has_infer_regions()
{

is actually

if (tcx.features().generic_const_parameter_types()
    && (ty.has_free_regions() || ty.has_erased_regions()))
    || ty.has_infer_regions()
{

I think your original diff was fine- making it be ``if (mgca || gcpt) && (has_free_regions || has_erased_regions)` seems correct enough to me 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ICE]: region variables should not be hashed

6 participants