-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Skip redundant frames in const recursion errors #136649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
estebank
commented
Feb 6, 2025
``` error[E0080]: evaluation of constant value failed --> $DIR/infinite-recursion-const-fn.rs:4:5 | LL | b() | ^^^ reached the configured maximum number of stack frames | note: inside `a` --> $DIR/infinite-recursion-const-fn.rs:4:5 | LL | b() | ^^^ note: inside `b` --> $DIR/infinite-recursion-const-fn.rs:7:5 | LL | a() | ^^^ note: [... 125 additional calls ...] inside `b` --> $DIR/infinite-recursion-const-fn.rs:7:5 | LL | a() | ^^^ note: inside `ARR::{constant#0}` --> $DIR/infinite-recursion-const-fn.rs:9:18 | LL | const ARR: [i32; a()] = [5; 6]; | ^^^ ```
r? @Noratrieb rustbot has assigned @Noratrieb. Use |
Some changes occurred to the CTFE machinery cc @rust-lang/wg-const-eval |
&& last_frame.where_ == frame.where_ | ||
&& last_frame.instance == frame.instance => | ||
{ | ||
Some(last_frame) if !seen.insert(frame.clone()) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add comments explaining what is done and why.
I am not sure I agree with removing duplicates from a backtrace -- that seems like it could be quite confusing. I can't quite follow the logic for printing the "N frames skipped" here, is that always printed when seeing a duplicate?
This new logic abuses the times
field, where it no longer means "we saw this function N+1 times" -- I think that's too hacky and confusing.
// Don't print [... additional calls ...] if the number of lines is small | ||
if frame.times < 3 { | ||
let times = frame.times; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this logic still correct? It seems to be we could now have a situation where we see 2 frames that both have been seen already, so last_frame.times
is 2, but these were actually 2 different frames and then this here will print the wrong thing.
LL | hint_unreachable() | ||
| ^^^^^^^^^^^^^^^^^^ | ||
note: inside `hint_unreachable` | ||
note: [... 124 additional calls ...] inside `hint_unreachable` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would probably look better as 2 separate notes, now that it doesn't represent 124 calls of the same function any more.
☔ The latest upstream changes (presumably #137848) made this pull request unmergeable. Please resolve the merge conflicts. |