Compiler error doesn't show full query chain when evaluating const cycle

[davidhewitt]

Consider the following code (both on stable 1.39.0 and nightly)

trait Const {
    const A: i32 = Self::B;
    const B: i32 = Self::A;
    const C: i32 = Self::A;
}

impl <T> Const for T {}

pub fn main() -> () {
    dbg!(i32::C);
}

There's a cycle evaluating the constants C -> A -> B -> A -> ...

The dbg!(i32::C) line is the offending code which causes this cycle to be evaluated.

However, the emitted error message doesn't actually reference the usage dbg!(i32::C) which caused the evaluation, as below:

   Compiling playground v0.0.1 (/playground)
error[E0391]: cycle detected when const-evaluating `Const::A`
 --> src/main.rs:4:24
  |
4 |         const A: i32 = Self::B;
  |                        ^^^^^^^
  |
note: ...which requires const-evaluating `Const::B`...
 --> src/main.rs:5:24
  |
5 |         const B: i32 = Self::A;
  |                        ^^^^^^^
  = note: ...which again requires const-evaluating `Const::A`, completing the cycle
note: cycle used when const-evaluating `Const::C`
 --> src/main.rs:6:24
  |
6 |         const C: i32 = Self::A;
  |                        ^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0391`.
error: could not compile `playground`.

To learn more, run the command again with --verbose.

This error could probably be improved by having an additional note: showing that the cycle was used when evaluating dbg!(i32::C).

cc @oli-obk

[oli-obk]

While it would be easy to dump the entire query stack instead of just the last part, the output can contain very surprising things that may confuse even more and its root will change arbitrarily between runs (especially with a parallel compiler). You could have the situation where rerunning the same program would tell you a different root cause with every rerun.

[davidhewitt]

As far as I can tell, if the CycleError has the usage field set, then that part of the query stack is included as a note:, which is probably good enough. See snippet below:

rust/src/librustc/ty/query/plumbing.rs

Lines 308 to 313 in a916ac2

	if let Some((span, query)) = usage {
	err.span_note(
	fix_span(span, &query),
	&format!("cycle used when {}", query.describe(self)),
	);
	}

I think it would be deisirable to note something that caused the cycle to be evaluated, even if that something isn't perfect.

I think this because const cycles can exist in valid rust code as long as that const cycle is never evaluated. So a user doing innocent enough refactoring might trigger evaluation of a const cycle and see this cycle error even if they never touched the part of the program containing the cycle.

See a playground example here: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=e02eff4470d70269dce44ccb2440a537

In that playground, the use of the function get_a() triggers evaluating the cycle, but this is not reported at all in the cycle error.

Of course, my opinion may be differ from others and please disregard mine if the majority disagrees!

[oli-obk]

I think it would be deisirable to note something that caused the cycle to be evaluated, even if that something isn't perfect.

Yea that's definitely a good idea.

As far as I can tell, if the CycleError has the usage field set, then that part of the query stack is included as a note:, which is probably good enough. See snippet below:

Oh neat.

So I'm guessing

rust/src/librustc/ty/query/job.rs

Lines 95 to 97 in a916ac2

	// Find out why the cycle itself was used
	let usage =
	job.parent.as_ref().map(|parent| (job.info.span, parent.info.query.clone()));

is already doing the right thing, because either there's a parent query or there's not (in which case we are already at the top of the call stack and won't get any new information).

In

rust/src/librustc/ty/query/job.rs

Line 395 in a916ac2

let usage = usage.as_ref().map(|(span, query)| (*span, query.info.query.clone()));

we get the info from

rust/src/librustc/ty/query/job.rs

Line 386 in a916ac2

let (_, entry_point, usage) = pick_query(tcx, &entry_points, |e| (e.0, e.1.clone()));

which reads to me like it's already doing what this issue wants it to be doing.

So to debug this issue, you can compile your snippet with a nightly rustc and pass -Ztreat-err-as-bug=1 and set RUST_BACKTRACE=1. Then you should be able to a) see the real query stack and b) see what is causing the cycle to be evaluated. Maybe we can figure out from that call stack what's going on.

If the backtrace is useless due to missing line information, you can build your own rustc and activate debug-assertions = true in your config.toml

[davidhewitt]

Awesome! I'll try to find some time to dig further into this over the holidays.