-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
add note for layout_of
when query depth overflows
#101801
Conversation
r? @cjgillot (rust-highfive has picked a reviewer for you, use r? to override) |
cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki |
This comment has been minimized.
This comment has been minimized.
0ccdbf0
to
5b890f4
Compare
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.
Can we add a test somewhere in src/test/ui
for this case?
Also, it'd be amazing if we could get a better span, but this is already an improvement.
@@ -23,3 +23,5 @@ query_system_cycle_recursive_trait_alias = trait aliases cannot be recursive | |||
query_system_cycle_which_requires = ...which requires {$desc}... | |||
|
|||
query_system_query_overflow = queries overflow the depth limit! | |||
|
|||
query_system_layout_of_depth = Query depth increased by {$depth} when {$desc}! |
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.
query_system_layout_of_depth = Query depth increased by {$depth} when {$desc}! | |
query_system_layout_of_depth = query depth increased by {$depth} when {$desc} |
It is somewhat problematic for the future to include $desc
in a message, because the output might be set up for another language, which will result in embedded english text.
#[note(query_system::layout_of_depth)] | ||
pub struct LayoutOfDepth { | ||
#[primary_span] | ||
pub span: Span, |
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.
It feels like ideally this span would be part of the main message, not of the note. I'm not sure if derive(SessionDiagnostic)
knows how to deal with Option<Span>
, but if it doesn't, you can get away with using DUMMY_SP
for the "default"/"empty" case.
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.
We support Option<Span>
:)
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.
That makes sense!
Do we want the diagnostic to mention |
Corrected the order of span. We can't get a better span now, I think this needs to optimize the span location acquisition of query calls. Currently only the |
0293cb7
to
89fd6ae
Compare
@bors r+ |
…ebank add note for `layout_of` when query depth overflows Fixes rust-lang#101747 Added `try_find_layout_root` function to add a note for `layout_of` when query depth overflows. This would make the error in rust-lang#101747 look like this: ``` error: queries overflow the depth limit! | note: Query depth increased by 66 when computing layout of `core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<core::option::Option<alloc::boxed::Box<alloc::string::String>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>`! --> D:\rust-backup\parallel_rust\query_depth.rs:40:1 | 40 | fn main() { | ^^^^^^^^^ error: aborting due to previous error ``` cc `@semicoleon`
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#101672 (array docs - advertise how to get array from slice) - rust-lang#101781 (Extend list of targets that support dyanmic linking for llvm tools) - rust-lang#101783 (Improve handing of env vars during bootstrap process) - rust-lang#101801 (add note for `layout_of` when query depth overflows) - rust-lang#101824 (rustdoc: add test cases for turning ``[Vec<T>]`` into ``[`Vec<T>`]``) - rust-lang#101861 (Update stdarch) - rust-lang#101873 (Allow building `rust-analyzer-proc-macro-srv` as a standalone tool) - rust-lang#101918 (rustdoc: clean up CSS for All Items and All Crates lists) - rust-lang#101934 (Continue migration of CSS themes) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixes #101747
Added
try_find_layout_root
function to add a note forlayout_of
when query depth overflows. This would make the error in #101747 look like this:cc @semicoleon