Sema: improvements to error messages related to the handling of (error) values #19961
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Maybe I'm just being pedantic here (most likely) but I don't like how we're just telling the user here how to "suppress this error" by "assigning the value to '_'". I think it's better if we use the word "discard" here which I think is the official terminology and also tells the user what it actually means to "assign the value to '_'". Also, using the value would also be a way to "suppress the error". It is just one of the two options: discard or use.
For consistency.
Previously the error had a note suggesting to use `try`, `catch`, or `if`, even for error sets where none of those work. Instead, in case of an error set the way you can handle the error depends very much on the specific case. For example you might be in a `catch` where you are discarding or ignoring the error set capture value, in which case one way to handle the error might be to `return` the error. So, in that case, we do not attach that error note. Additionally, this makes the error tell you what kind of an error it is: is it an error set or an error union? This distinction is very relevant in how to handle the error.
Vexu
approved these changes
May 21, 2024
| if (non_optional_ty.zigTypeTag(mod) == .ErrorUnion) { | ||
| try sema.errNote(block, src, msg, "consider using 'try', 'catch', or 'if'", .{}); | ||
| } | ||
| break :msg msg; |
There was a problem hiding this comment.
Can you add a addDeclaredHereNote for non_optional_ty here to match how fail would add it?
There was a problem hiding this comment.
Done and added a test because this wasn't tested until now.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
reroll of #16461