-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add division by zero case to the CheckedDiv
comment
#16364
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
Conversation
Thank you for the patch! I'm guessing this came out of the reddit thread :) We've been marking failure as its own section, would you mind making to compliant? That'd look like this:
Make sense? |
The function doesn't fail, maybe I wasn't clear enough with the documentation – if that's the case, please tell me. Otherwise, is returning |
Actually, I guess in this case it's a small semantic difference. I can totally see it both ways. This function can't fail, exactly. Hrm. |
I was also under the impressions that the |
I'd also consider |
The current convention is to use A common convention is to use |
|
Should Checked* possibly return a Result then, since it's essentially "bad input" error? |
Theoretically yes, though all our current APIs that behave in this fashion ( Although there is also the argument that this isn't really an error, that it is in fact a valid result that indicates that the operation has no meaning, just like |
This is going offtopic. :) |
Right, this is a weird case. I think this patch is good enough for now, as it's clearly an improvement. |
internal: sync from downstream
No description provided.