Skip to content

Add documentation for error E0208#98011

Merged
bors merged 1 commit intorust-lang:masterfrom
onlineSoftwareDevOK:long-error-explanation-e0208
Jun 13, 2022
Merged

Add documentation for error E0208#98011
bors merged 1 commit intorust-lang:masterfrom
onlineSoftwareDevOK:long-error-explanation-e0208

Conversation

@onlineSoftwareDevOK
Copy link
Contributor

@onlineSoftwareDevOK onlineSoftwareDevOK commented Jun 11, 2022

Related to #61137

@rust-highfive
Copy link
Contributor

rust-highfive commented Jun 11, 2022

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 11, 2022
@rust-highfive
Copy link
Contributor

rust-highfive commented Jun 11, 2022

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 11, 2022
GuillaumeGomez
GuillaumeGomez reviewed Jun 11, 2022
View reviewed changes
GuillaumeGomez
GuillaumeGomez reviewed Jun 11, 2022
View reviewed changes
GuillaumeGomez
GuillaumeGomez reviewed Jun 11, 2022
View reviewed changes
compiler/rustc_error_codes/src/error_codes/E0208.md Outdated
Copy link
Member

@GuillaumeGomez GuillaumeGomez Jun 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is really nothing more to be explained about this error code?

Copy link
Contributor Author

@onlineSoftwareDevOK onlineSoftwareDevOK Jun 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your feedback. What else should be explained about this error code?

Copy link
Member

@GuillaumeGomez GuillaumeGomez Jun 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to adopt this point of view: you don't know what this error code is about at all. What information does the reader need to understand why they got this error?

In the current case, you only show a table of variance, but that's far from enough to understand what's going on in here.

Copy link
Contributor Author

@onlineSoftwareDevOK onlineSoftwareDevOK Jun 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. A link to the following page would be enough? https://doc.rust-lang.org/nomicon/subtyping.html

Copy link
Contributor Author

@onlineSoftwareDevOK onlineSoftwareDevOK Jun 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have just realized it's already there

Copy link
Contributor Author

@onlineSoftwareDevOK onlineSoftwareDevOK Jun 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I felt the link to the documentation was enough @mbartlett21 What else can I add here?

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@onlineSoftwareDevOK
Copy link
Contributor Author

onlineSoftwareDevOK commented Jun 11, 2022

The job x86_64-gnu-llvm-12 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@GuillaumeGomez Can you explain why this failed, please? A similar sentence ("For more information...") was added in the following PR but it worked there #94449

compiler-errors
compiler-errors suggested changes Jun 11, 2022
View reviewed changes
Copy link
Contributor

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that an internal error needs error documentation.

The feature gate specifically says "the #[rustc_error] attribute is just used for rustc unit tests and will never be stable", and I don't think people working within the compiler need this documentation.

@onlineSoftwareDevOK
Copy link
Contributor Author

onlineSoftwareDevOK commented Jun 11, 2022

I don't think that an internal error needs error documentation.

The feature gate specifically says "the #[rustc_error] attribute is just used for rustc unit tests and will never be stable", and I don't think people working within the compiler need this documentation.

I prepared this PR because this error code is mentioned here #61137 (comment)

@compiler-errors
Copy link
Contributor

compiler-errors commented Jun 12, 2022 •
edited
Loading

@GuillaumeGomez, perhaps that one should also be marked off the list as unneeded? Edit: Same for E0640, E0711, E0717

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jun 12, 2022

I made the list based on the undocumented error codes but didn't make difference with internal ones. We can just add a comment like // internal error code behind it and not add a long error explanation. Same for the errors listed by @compiler-errors.

@onlineSoftwareDevOK
Copy link
Contributor Author

onlineSoftwareDevOK commented Jun 12, 2022

I made the list based on the undocumented error codes but didn't make difference with internal ones. We can just add a comment like // internal error code behind it and not add a long error explanation. Same for the errors listed by @compiler-errors.

Inside compiler/rustc_error_codes/src/error_codes.rs, right?

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jun 12, 2022

Yes. Something like this:

E0208, // internal error code

@onlineSoftwareDevOK onlineSoftwareDevOK force-pushed the long-error-explanation-e0208 branch from 557221e to 99672fb Compare June 12, 2022 23:00
@onlineSoftwareDevOK
Copy link
Contributor Author

onlineSoftwareDevOK commented Jun 12, 2022

Yes. Something like this:

E0208, // internal error code

Done

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jun 12, 2022

Thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 12, 2022

📌 Commit 99672fb has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 12, 2022
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jun 12, 2022
Closed
100 tasks
@compiler-errors compiler-errors mentioned this pull request Jun 13, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 13, 2022
@bors
Auto merge of rust-lang#98037 - compiler-errors:rollup-fbvy456, r=com…
c570ab5 
…piler-errors

Rollup of 3 pull requests

Successful merges:

 - rust-lang#97920 (Fix some test annotations)
 - rust-lang#97950 (Clarify `#[derive(PartialEq)]` on enums)
 - rust-lang#98011 (Add documentation for error E0208)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ac73b19 into rust-lang:master Jun 13, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

@compiler-errors compiler-errors Awaiting requested review from compiler-errors

Assignees

@lcnr lcnr

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.63.0

Development

Successfully merging this pull request may close these issues.

8 participants

@onlineSoftwareDevOK @rust-highfive @rust-log-analyzer @compiler-errors @GuillaumeGomez @bors @lcnr @rustbot