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

Add division by zero case to the CheckedDiv comment #16364

Closed
wants to merge 1 commit into from

Conversation

tbu-
Copy link
Contributor

@tbu- tbu- commented Aug 8, 2014

No description provided.

@steveklabnik
Copy link
Member

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:

/// Performs division that's checked against failure.
///
/// # Failure
///
/// If dividing by zero is attempted, this will return `None`.
///
/// If the result would overflow or underflow, this will wrap around.

Make sense?

@tbu-
Copy link
Contributor Author

tbu- commented Aug 8, 2014

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 None considered failure now?

@steveklabnik
Copy link
Member

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.

@reem
Copy link
Contributor

reem commented Aug 8, 2014

I was also under the impressions that the # Failure section was for literal calls to fail!() and that when a function returns None that is not failure per se. I think having an explicit marker for actual task failure is very important because it's not annotated in the type that it's even possible - when a function returns None should be a different section.

@steveklabnik
Copy link
Member

I'd also consider # Failure to be like things that return Result as well. Option may be too far.

@aturon
Copy link
Member

aturon commented Aug 8, 2014

The current convention is to use # Failure for actual calls to fail!. This is a perfect example of why fail is poorly named :-)

A common convention is to use # Errors when producing a Result, see http://static.rust-lang.org/doc/master/std/io/trait.Writer.html#tymethod.write

@lilyball
Copy link
Contributor

lilyball commented Aug 8, 2014

# Failure definitely means task failure. So that's explicit calls to fail!(), to assert!(), and to other functions that may fail.

@Gankra
Copy link
Contributor

Gankra commented Aug 8, 2014

Should Checked* possibly return a Result then, since it's essentially "bad input" error?

@lilyball
Copy link
Contributor

lilyball commented Aug 8, 2014

Theoretically yes, though all our current APIs that behave in this fashion (Checked*, ToPrimitive, etc) all use Option.

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 NaN in floating-point calculations. In other words, nothing actually went wrong.

@tbu-
Copy link
Contributor Author

tbu- commented Aug 8, 2014

This is going offtopic. :)

@steveklabnik
Copy link
Member

Right, this is a weird case.

I think this patch is good enough for now, as it's clearly an improvement.

@bors bors closed this Aug 19, 2014
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2024
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.

None yet

8 participants