Skip to content

[Guideline] Add do not divide by 0 #132

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions src/coding-guidelines/expressions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,54 @@ Expressions
}

fn with_base(_: &Base) { ... }

.. guideline:: Do not divide by 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Asking a bit of a practical question to folks. Does the combination of:

  • mandatory
  • undecidable

put too great of a burden on reviewers / auditors or is this "just the way it is and should be" given the nature of writing safety-critical code?

Tagging @AlexCeleste and @rcseacord for their thoughts as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a bit of rationale for why I chose that combination. I used "mandatory" as a stand in for "very important" as a reason it goes above and beyond "required".

I would not want to mandate that no safety-critical programmer could never used unchecked arithmetic, the decidable subset version of this rule, but I do want to mandate that they do not take an action (divide by zero) that would lead to a panic.

Copy link
Collaborator

@felix91gr felix91gr Jul 15, 2025

Choose a reason for hiding this comment

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

Well, "do not divide by zero" is undecidable in the sense that statically knowing if the b in a / b is 0 is undecidable.

However, the guideline is not impossible to comply with since the user themselves can check (if b != 0 etc) but also because there are APIs that we can use for this, like the NonZero<T> struct and the checked_div and checked_rem functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is true, there are more clever ways to enforce this in Rust.

I would be fine mandating that any division must use checked functions, or have a NonZero<T> divisor (which elides any performance penalty from checking). This would then kick the can down the road, where as now your problem is don't use NonZero<T>::new_unchecked and cause UB.

Copy link
Collaborator

Choose a reason for hiding this comment

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

However, the guideline is not impossible to comply with since the user themselves can check

Definitely, I agree. Perhaps this is an instance of where we need to make use of the Exceptions section to list the sorts of suggestions you made. What do you two think?

like the NonZero<T>

TIL about NonZero<T>. Super cool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The rule already shouldn't apply to this, it is specified as applying to:

unsigned integer or two’s complement division

which is the "built-in" rules of the ArithmeticExpression. Using NonZero<T> goes through core::ops::Div::div which is already outside the scope of this guideline. I will clarify that to make it more explicit though.

Copy link
Collaborator

@felix91gr felix91gr Jul 19, 2025

Choose a reason for hiding this comment

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

This would then kick the can down the road, where as now your problem is don't use NonZero<T>::new_unchecked and cause UB.

Indeed. But then, new_unchecked is an unsafe function. So we've effectively eliminated all safe ways to create a division by zero, which I think is a good starting point.

If nothing else, using unsafe should feel like a red flag for something as simple as integer division.

:id: gui_kMbiWbn8Z6g5
:category: mandatory
:status: draft
:release: latest
:fls: fls_Q9dhNiICGIfr
:decidability: undecidable
:scope: system
:tags: numerics

This guideline applies when unsigned integer or two’s complement division is performed during the
evaluation of an `ArithmeticExpression
<https://rust-lang.github.io/fls/expressions.html#arithmetic-expressions>`_.

Note that this includes the evaluation of a `RemainderExpression
<https://rust-lang.github.io/fls/expressions.html#syntax_remainderexpression>`_, which uses unsigned integer or two's
complement division.

This rule does not apply to evaluation of a :std:`core::ops::Div` trait on types other than `integer
types <https://rust-lang.github.io/fls/types-and-traits.html#integer-types>`_. The use of
:std:`std::num::NonZero` or is therefore a recommended way to avoid the undecidability of this
guideline.

.. rationale::
:id: rat_h84NjY2tLSBW
:status: draft

Integer division by zero results in a panic, which is an abnormal program state and may terminate the process.

.. non_compliant_example::
:id: non_compl_ex_LLs3vY8aGz0F
:status: draft

When the division is performed, the right operand is evaluated to zero and the program panics.

.. code-block:: rust

let x = 0;
let x = 5 / x;

.. compliant_example::
:id: compl_ex_Ri9pP5Ch3kbb
:status: draft

There is no compliant way to perform integer division by zero. A checked division will prevent any
division by zero from happening. The programmer can then handle the returned :std:``std::option::Option``.

.. code-block:: rust

let x = 5u8.checked_div(0);