-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Added an explanation for the E0704 error. #59634
Conversation
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@eddyb I am trying to understand the error that Travis gives me, but I am failing to understand what went wrong. When I search the project for "E0704", the only things come up are:
Hope you can give me a pointer :) |
@eddyb Could you maybe respond to my previous questions? |
This comment has been minimized.
This comment has been minimized.
@DevQps tidy is complaining that the same error code appears twice, once in the placeholder list and once with a description. As you have a description, just remove line 443 and it will satisfy tidy. That list is only for error codes that do not have a description, but that we want to statically check for creation somewhere. You will also need to rebase due to a recently merged PR introducing conflicts. |
d0ba421
to
0156b56
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Need to run |
This comment has been minimized.
This comment has been minimized.
r? @estebank |
@DevQps great work! It seems like the code should work well now. Would you mind squashing your commits? I feel this can be merged then, gated on Travis being happy. |
This comment has been minimized.
This comment has been minimized.
@estebank I squashed them! I just saw that my IDE includes the description of each commit though. Hope that's not too much of a problem! Otherwise I will go try and fix it somehow :) EDIT: I don't understand what went wrong. I checked it on the playground and there it works: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=a15911ab678f31233b9d06fa62e47095 |
This comment has been minimized.
This comment has been minimized.
I'm not sure why it fails. @QuietMisdreavus, would you know what in rustdoc's world could cause the error we're seeing here? |
Rustdoc is probably wrapping that code in a main function, which hides the module. You probably need to add an empty |
This comment has been minimized.
This comment has been minimized.
@estebank I squashed them! Bors succeeded last time, so I guess it will be good this way as well. Thanks a lot for all your help. I sometimes miss the small parts here and there :) Glad you're here help me catch them. |
@bors r+ rollup |
📌 Commit 2be37ad has been approved by |
Added an explanation for the E0704 error. # Description Adds an explanation on the E0704 error. I tried to stick as closely to the message that the compiler generates. It's the first time I am fixing error messages here, so if there is something I did wrong or should improve, please let me know. closes rust-lang#55398
Rollup of 7 pull requests Successful merges: - #59634 (Added an explanation for the E0704 error.) - #60348 (move some functions from parser.rs to diagostics.rs) - #60385 (Emit metadata files earlier) - #60428 (Refactor `eval_body_using_ecx` so that it doesn't need to query for MIR) - #60437 (Ensure that drop order of `async fn` matches `fn` and that users cannot refer to generated arguments.) - #60439 (doc: Warn about possible zombie apocalypse) - #60452 (Remove Context and ContextKind) Failed merges: r? @ghost
Description
Adds an explanation on the E0704 error. I tried to stick as closely to the message that the compiler generates. It's the first time I am fixing error messages here, so if there is something I did wrong or should improve, please let me know.
closes #55398