-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
improve DiagnosticBuilder docs #63488
Conversation
r? @zackmdavis (rust_highfive has picked a reviewer for you, use r? to override) |
@@ -120,6 +120,9 @@ impl Diagnostic { | |||
} | |||
|
|||
/// Adds a span/label to be included in the resulting snippet. | |||
/// This label will be shown together with the original span/label used when creating the | |||
/// diagnostic, *not* a span added by one of the `span_*` methods. | |||
/// | |||
/// This is pushed onto the `MultiSpan` that was created when the | |||
/// diagnostic was first built. If you don't call this function at | |||
/// all, and you just supplied a `Span` to create the diagnostic, |
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.
I have no idea what this second paragraph here means. I left it because maybe it helps someone else? But it seems to only be helpful for people that actually know the internals of how diagnostics work. Maybe it even says what I am now also saying in the new sentence I added? I couldn't tell.^^
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.
If you called struct_span_err
, for example, you end up with an error with an underline under that span with no label. If you use span_label
on the DiagnosticBuilder
you can either set the label of the primary span or add more spans to the diagnostic. I believe this is what the comment is trying to convey.
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.
Okay, that wasn't really clear to me as I never heard of MultiSpan
. Now it says the same thing in two different ways then, maybe that helps...
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.
MultiSpan
is the only way of setting multiple primary spans, conceptually just Vec<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.
I see. I thought it would refer to how span_note
also adds a new span to the message, but that seems to be a different kind of "multiple spans".
r? @estebank |
r? @zackmdavis @bors r+ rollup |
📌 Commit fecf305 has been approved by |
@zackmdavis can you help answering the questions in the OP / in my comment above? |
Rollup of 11 pull requests Successful merges: - #62760 (Deduplicate error messages in `librsctc_mir`) - #62849 (typeck: Prohibit RPIT types that inherit lifetimes) - #63383 (`async fn` lifetime elision tests) - #63421 (Implement Clone, Display for ascii::EscapeDefault) - #63459 (syntax: account for CVarArgs being in the argument list.) - #63475 (Bring back suggestion for splitting `<-` into `< -`) - #63485 (ci: move mirrors to their standalone bucket) - #63486 (Document `From` trait for `BinaryHeap`) - #63488 (improve DiagnosticBuilder docs) - #63493 (Remove unneeded comment in src/libcore/hash/mod.rs) - #63499 (handle elision in async fn correctly) Failed merges: r? @ghost
Rollup of 10 pull requests Successful merges: - #62760 (Deduplicate error messages in `librsctc_mir`) - #62849 (typeck: Prohibit RPIT types that inherit lifetimes) - #63383 (`async fn` lifetime elision tests) - #63421 (Implement Clone, Display for ascii::EscapeDefault) - #63459 (syntax: account for CVarArgs being in the argument list.) - #63475 (Bring back suggestion for splitting `<-` into `< -`) - #63485 (ci: move mirrors to their standalone bucket) - #63486 (Document `From` trait for `BinaryHeap`) - #63488 (improve DiagnosticBuilder docs) - #63493 (Remove unneeded comment in src/libcore/hash/mod.rs) Failed merges: r? @ghost
Rollup of 17 pull requests Successful merges: - #62760 (Deduplicate error messages in `librsctc_mir`) - #62849 (typeck: Prohibit RPIT types that inherit lifetimes) - #63383 (`async fn` lifetime elision tests) - #63421 (Implement Clone, Display for ascii::EscapeDefault) - #63459 (syntax: account for CVarArgs being in the argument list.) - #63475 (Bring back suggestion for splitting `<-` into `< -`) - #63485 (ci: move mirrors to their standalone bucket) - #63486 (Document `From` trait for `BinaryHeap`) - #63488 (improve DiagnosticBuilder docs) - #63493 (Remove unneeded comment in src/libcore/hash/mod.rs) - #63499 (handle elision in async fn correctly) - #63501 (use `ParamName` to track in-scope lifetimes instead of Ident) - #63508 (Do not ICE when synthesizing spans falling inside unicode chars) - #63511 (ci: add a check for clock drift) - #63512 (Provide map_ok and map_err method for Poll<Option<Result<T, E>>>) - #63529 (RELEASES.md: ? is one of three Kleene operators) - #63530 (Fix typo in error message.) Failed merges: r? @ghost
Rollup of 17 pull requests Successful merges: - #62760 (Deduplicate error messages in `librsctc_mir`) - #62849 (typeck: Prohibit RPIT types that inherit lifetimes) - #63383 (`async fn` lifetime elision tests) - #63421 (Implement Clone, Display for ascii::EscapeDefault) - #63459 (syntax: account for CVarArgs being in the argument list.) - #63475 (Bring back suggestion for splitting `<-` into `< -`) - #63485 (ci: move mirrors to their standalone bucket) - #63486 (Document `From` trait for `BinaryHeap`) - #63488 (improve DiagnosticBuilder docs) - #63493 (Remove unneeded comment in src/libcore/hash/mod.rs) - #63499 (handle elision in async fn correctly) - #63501 (use `ParamName` to track in-scope lifetimes instead of Ident) - #63508 (Do not ICE when synthesizing spans falling inside unicode chars) - #63511 (ci: add a check for clock drift) - #63512 (Provide map_ok and map_err method for Poll<Option<Result<T, E>>>) - #63529 (RELEASES.md: ? is one of three Kleene operators) - #63530 (Fix typo in error message.) Failed merges: r? @ghost
Cc @estebank @oli-obk
Is there any way to do something like
span_note
but with a label attached to the span? I thought.span_note().span_label()
would do it, but no, that does not work.