Skip to content
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

Document when errors should have an associated error code #967

Merged
merged 2 commits into from
Nov 25, 2020

Conversation

camelid
Copy link
Member

@camelid camelid commented Nov 23, 2020

I don't know if there's an official guideline (that's why we need these docs!);
this is just what sounded reasonable to me.

r? @estebank

@jyn514
Copy link
Member

jyn514 commented Nov 23, 2020

Hmm, I don't know if this is a guideline so much as describing the current situation. I would say when in doubt, always add an error code (I've even made PRs adding error codes to errors that didn't have one before: rust-lang/rust#76143).

@camelid
Copy link
Member Author

camelid commented Nov 23, 2020

I don't know if there's an official guideline (that's why we need these docs!);
this is just what sounded reasonable to me.

^ @jyn514 That's what I was saying: this isn't some official guideline :)

@jyn514
Copy link
Member

jyn514 commented Nov 23, 2020

Right, yes, but I'm saying we shouldn't suggest it as a policy. This makes it seem like you shouldn't add error codes for parse errors, which I'm not sure is true.

@estebank
Copy link
Contributor

estebank commented Nov 24, 2020

@jyn514 parse errors in particular are tricky. I find that error codes work best when they are targeted and specific, meaning there is a small number of triggers for it and their description can give you enough context to fix the problem. There are some cases where the same error code has a bunch of solutions (E0277, E0308, for example), but for the most part that's fine. Parse errors in particular have a combination of things that we proactively handle (hey! missing semicolon here!) but the vast majority of them are kind of random and can be reached through multiple different kinds of unrelated mistakes. We could introduce a new generic error code for the general case of "E0666: parse error", and then take every case we proactively handle and give it its own error code. That would be fine, but I have instead focused on putting all the info that would be in the error code index into the error itself. When the index entry wouldn't have any more information, I don't bother creating a new code. Maybe that would be a good rule of thumb: give an error a code if the the index entry would give more info than the error itself which we haven't either because of verbosity or because we can't detect all the possible triggers.

@camelid I'm ok with the blurb you added, r=me if you don't want to expand it.

@jyn514
Copy link
Member

jyn514 commented Nov 24, 2020

Maybe that would be a good rule of thumb: give an error a code if the the index entry would give more info than the error itself which we haven't either because of verbosity or because we can't detect all the possible triggers.

Sounds great, thanks!

@camelid
Copy link
Member Author

camelid commented Nov 24, 2020

@estebank I changed the advice to something similar to what you suggested. Should be ready now!

@estebank
Copy link
Contributor

estebank commented Nov 25, 2020

@bors r+

Haha, wrong repo. 😅

@estebank estebank merged commit 5f7bd57 into rust-lang:master Nov 25, 2020
@camelid camelid deleted the when-to-have-an-error-code branch November 25, 2020 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants