-
Notifications
You must be signed in to change notification settings - Fork 13.3k
fixes rust-lang#56766 #59041
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fixes rust-lang#56766 #59041
Conversation
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
r? @estebank |
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 you add a test to src/test/ui/parser/doc-inside-trait-item.rs
:
trait User{
fn test();
/// empty doc
//~^ ERROR found a documentation comment that doesn't document anything
}
fn main() {}
and run ./x.py test src/test/ui --bless
and add the new test and its corresponding generated .stderr
file?
err.help("doc comments must come before what they document, maybe a \ | ||
comment was intended with `//`?", | ||
); | ||
err.emit(); |
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.
For everything between 6636 and 6644 you can replace it with self.span_fatal_err(self.prev_span, Error::UselessDocComment).emit();
, like in
rust/src/libsyntax/parse/parser.rs
Line 5290 in fcfc796
s.span_fatal_err(s.prev_span, Error::UselessDocComment).emit(); |
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.
@estebank This is the reason I used struct_span_err_with_code
. Do we not need that now?
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.
(indeed, this does not seem like it needs to be a fatal error; I concur with @saleemjaffer's choice to use a non-fatal error here.)
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
ping from triage @saleemjaffer you need to address the failing test |
This was fixed. All tests seem to be passing. |
@estebank this is ready for a review |
Ping from triage, @estebank, we eagerly await your review! |
@bors r? pnkfelix |
@bors r+ |
📌 Commit d258481 has been approved by |
r? @pnkfelix |
…er_error_msg, r=pnkfelix fixes rust-lang#56766 fixes rust-lang#56766
Apologies for the unresponsiveness over the past week. I've been sporadically online. |
…er_error_msg, r=pnkfelix fixes rust-lang#56766 fixes rust-lang#56766
Rollup of 7 pull requests Successful merges: - #58507 (Add a -Z time option which prints only passes which runs once) - #58919 (Suggest using anonymous lifetime in `impl Trait` return) - #59041 (fixes #56766) - #59586 (Fixed URL in cargotest::TEST_REPOS) - #59595 (Update rustc-guide submodule) - #59601 (Fix small typo) - #59603 (stabilize ptr::hash) Failed merges: r? @ghost
fixes #56766