Depth limit const eval query #135167
Depth limit const eval query #135167
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @oli-obk (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
A-query-system
| | ^^^^^^^^^^^ | ||
| | | ||
| = help: consider increasing the recursion limit by adding a `#![recursion_limit = "30"]` attribute to your crate (`recursive`) | ||
| = note: query depth increased by 17 when simplifying constant for the type system `RECUR` |
There was a problem hiding this comment.
Would be neat if we could report the query backtrace before that recursion
There was a problem hiding this comment.
I'm not exactly sure how this should work since the job.id never repeats in this case. I guess we could determine when the cycle is complete by finding the first parent query with job.dep_kind == eval_to_const_value_raw and a span that matches the one of the current query. What do you think about this?
Seems like a bit of a hack but I can't really think of a better solution.
|
@bors try |
|
Gonna run crater to see if the default limit is fine |
… r=<try> Depth limit const eval query Currently the const-eval query doesn't have a recursion limit or timeout, causing the complier to freeze in an infinite loop, see rust-lang#125718. This PR depth limits the `eval_to_const_value_raw` query (with the [`recursion_limit`](https://doc.rust-lang.org/reference/attributes/limits.html) attribute) and improves the diagnostics for query overflow errors, so spans are reported for other dep kinds than `layout_of` (e.g. `eval_to_const_value_raw`). fixes rust-lang#125718 fixes rust-lang#114192
|
☀️ Try build successful - checks-actions |
|
Should the code in #114192 (comment) also be included as a test? |
|
Yes, that would great imo! |
mzacho
force-pushed
the
depth-limit-const-eval-query
branch
from
bbb8201 to
4b81c5b
Compare
|
@craterbot check |
|
Don't think the job got enqueued? @oli-obk |
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
added
S-waiting-on-crater
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
|
craterbot
added
S-waiting-on-review
|
No regressions, just some disk failures @bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (3ff1b64): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary -3.9%, secondary -4.8%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (secondary 0.9%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 765.149s -> 764.23s (-0.12%) |
Currently the const-eval query doesn't have a recursion limit or timeout, causing the complier to freeze in an infinite loop, see #125718. This PR depth limits the
eval_to_const_value_rawquery (with therecursion_limitattribute) and improves the diagnostics for query overflow errors, so spans are reported for other dep kinds thanlayout_of(e.g.eval_to_const_value_raw).fixes #125718
fixes #114192