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
Copy link
Collaborator

rustbot commented Feb 7, 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.

@JohnTitor
Copy link
Member Author

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

@rust-bors
Copy link
Contributor

rust-bors bot commented Feb 9, 2026

☔ The latest upstream changes (presumably #152373) made this pull request unmergeable. Please resolve the merge conflicts.

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(());
}

}
};

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
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