Skip to content

Conversation

@JohnTitor
Copy link
Member

@JohnTitor JohnTitor commented Feb 3, 2026

Fixes #151631, fixes #151477
r? @fmease

I'd recommend reviewing commit-by-commit, the diff is less-readable to address a cyclic issue.

@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 Feb 3, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 3, 2026

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

if !type_const_stack.insert(uv.def) {
let guar = tcx.dcx().span_err(
tcx.def_span(uv.def),
"cycle detected when evaluating type-level constant",
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if we should detect a cyclic error in this place though, maybe we can handle it better?

Copy link
Member

@fmease fmease Feb 10, 2026

Choose a reason for hiding this comment

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

I'm don't know how/where/when we invoke try_evaluate_const (during normalization of course, but …) but ideally (?) either the query system would detect cycles / overflows for us (but I guess no queries are involved here) or we have a simple depth param similar to what type normalization does instead of storing visited definitions. idk, imma wait for Boxy

@Human9000-bit
Copy link
Contributor

Does this code compiles under your impl?

@rustbot

This comment has been minimized.

@JohnTitor
Copy link
Member Author

@Human9000-bit Yeah, it should. Added a rev so that we can make sure it.

@rust-bors

This comment has been minimized.

Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

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

Some preliminary comments & suggestions. I'm now gonna pass the assignment to Boxy since I don't feel comfortable with this part of the code (yet).

View changes since this review


trait NewTrait {}

impl NewTrait for () {}
Copy link
Member

Choose a reason for hiding this comment

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

By adding this impl, this test doesn't reproduce the ICE on nightly/main.

Suggested change
impl NewTrait for () {}

#![feature(generic_const_items, min_generic_const_args)]
#![feature(adt_const_params)]
#![allow(incomplete_features)]
#![allow(dead_code)]
Copy link
Member

Choose a reason for hiding this comment

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

This is implied in UI tests.

Suggested change
#![allow(dead_code)]

Copy link
Member

Choose a reason for hiding this comment

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

This test isn't minimized and contains a lot of unrelated elements. Here's a smaller one:

// issue: <https://github.com/rust-lang/rust/issues/151631>
//@ compile-flags: -Znext-solver
#![feature(min_generic_const_args)]
#![expect(incomplete_features)]

trait SuperTrait {}
trait Trait: SuperTrait {
    type const K: u32;
}
impl Trait for () {
    type const K: u32 = const { 1 };
}

fn check(_: impl Trait<K = 0>) {}

fn main() {
    check(());
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I tweaked during developing then forgot to revert. Thank you! Fixed in 1641602.

}
};

if tcx.is_type_const(uv.def) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if this change makes sense, I'm not an expert in mGCA. Therefore I'll assign @BoxyUwU instead.

trait Owner {
#[type_const]
const C<const N: u32>: u32;
//~^ ERROR cycle detected when evaluating type-level constant
Copy link
Member

Choose a reason for hiding this comment

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

Ideally this error would point to the C in the trait impl.

if !type_const_stack.insert(uv.def) {
let guar = tcx.dcx().span_err(
tcx.def_span(uv.def),
"cycle detected when evaluating type-level constant",
Copy link
Member

@fmease fmease Feb 10, 2026

Choose a reason for hiding this comment

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

I'm don't know how/where/when we invoke try_evaluate_const (during normalization of course, but …) but ideally (?) either the query system would detect cycles / overflows for us (but I guess no queries are involved here) or we have a simple depth param similar to what type normalization does instead of storing visited definitions. idk, imma wait for Boxy

@fmease fmease assigned BoxyUwU and unassigned fmease Feb 10, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 11, 2026

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.

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

5 participants