Skip to content

[Guideline] Add do not divide by 0#132

Merged
felix91gr merged 18 commits into
Safety-Critical-Rust-Consortium:mainfrom
vapdrs:feature/do-not-divide-by-0
Aug 24, 2025
Merged

[Guideline] Add do not divide by 0#132
felix91gr merged 18 commits into
Safety-Critical-Rust-Consortium:mainfrom
vapdrs:feature/do-not-divide-by-0

Conversation

@vapdrs

@vapdrs vapdrs commented Jun 10, 2025

Copy link
Copy Markdown
Contributor

Closes #131

@netlify

netlify Bot commented Jun 10, 2025

Copy link
Copy Markdown

Deploy Preview for scrc-coding-guidelines ready!

Name Link
🔨 Latest commit 1076b36
🔍 Latest deploy log https://app.netlify.com/projects/scrc-coding-guidelines/deploys/689f3bf0b185860008a6b560
😎 Deploy Preview https://deploy-preview-132--scrc-coding-guidelines.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@felix91gr

felix91gr commented Jun 11, 2025

Copy link
Copy Markdown
Collaborator

Hi @vapdrs! I have already sent you a message in Zulip, but here seems like a better place to do so.

I come to add a couple of things to this guideline :)

  1. There is a Clippy lint (actually a couple of lints, but this one is the main one) that one can enable to catch these kinds of operations: arithmetic_side_effects. This is a fairly general lint that, if enabled using warn or deny, lints any arithmetic expression that either overflows or panics (division and modulo by 0 will do the latter).

  2. The aforementioned lint should indicate to the user that there are arithmetic operations in the stdlib that can be used to guarantee well-defined behavior. In the case of division by zero, it will suggest using operations such as checked_div, which outputs an Option<T> that the user must then handle properly. The None result indicates overflow, underflow or division by zero.

There are other such operations for division, such as checked_div_rem_euclid for when a remainder is desired.

And for other arithmetic operations, there are quite a few functions one can use to avoid Undefined Behavior: https://doc.rust-lang.org/std/?search=checked

  1. A complement to the two resources above is the NonZero type. One uses NonZero to enclose a value that is known, by construction, to not equal zero.

This combines rather well with Option, as in Option<NonZero>, since the compiler can do some memory layout optimization due to the fact that the value being enclosed by NonZero has one bit-pattern that is known to not be possible (the 000...000 pattern)

I will review the PR shortly :3

Comment thread src/coding-guidelines/expressions.rst Outdated
Comment thread src/coding-guidelines/expressions.rst Outdated

@PLeVasseur PLeVasseur left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for opening this up @vapdrs! Could you check the comments I left?

Comment thread src/coding-guidelines/expressions.rst Outdated
Comment thread src/coding-guidelines/expressions.rst Outdated
Comment thread src/coding-guidelines/expressions.rst Outdated
vapdrs added 2 commits June 24, 2025 17:23
As stated there is no compliant way to do this, so no example should be
present.
While the guideline does not strictly apply to this example, it is a
good suggestion for what to do instead.
@vapdrs
vapdrs requested review from PLeVasseur and felix91gr June 27, 2025 18:09

@PLeVasseur PLeVasseur left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for pulling this together @vapdrs! I left a few suggestions based on how conf.py works together with Sphinx Needs as well as a way to use our coding guidelines extension. Could you take a look?

Comment thread src/coding-guidelines/expressions.rst Outdated
Comment thread src/coding-guidelines/expressions.rst Outdated
Comment thread src/coding-guidelines/expressions.rst Outdated
Comment thread src/coding-guidelines/expressions.rst Outdated
Comment thread src/coding-guidelines/expressions.rst
@PLeVasseur

Copy link
Copy Markdown
Collaborator

I'm realizing now that perhaps the issue template having uppercase options may have been to blame for them being made uppercase in this PR. Was that the case for you @vapdrs?

I've opened #143 to fix this 👍

vapdrs and others added 4 commits July 14, 2025 16:33
Comment thread src/coding-guidelines/expressions.rst Outdated
Comment thread src/coding-guidelines/expressions.rst Outdated
Comment thread src/coding-guidelines/expressions.rst Outdated
vapdrs and others added 3 commits July 30, 2025 08:41
It is not recommended for normative language

Co-authored-by: Alex Eris Celeste née Gilding <alexg.nvfp@gmail.com>
Also moved to the Rationale.
Comment thread src/coding-guidelines/expressions.rst
Mandatory is a bit strong for panics as determined in today's meeting.
Comment thread src/coding-guidelines/expressions.rst Outdated
@felix91gr

Copy link
Copy Markdown
Collaborator

I haven't been able to give the examples a proper re-read just yet. Tis' been a low-battery week. Will do my best to have final feedback on those (also regarding #136)

@felix91gr

Copy link
Copy Markdown
Collaborator

@vapdrs the build is failing, but to no fault of your own. It's just something that happens every now and then when the FLS and our FLS lockfile diverge.

There should be a way to automate this for most PRs, but for right now... let's see. I'd usually ask you to run the command that fixes this, but for traceability purposes we actually want those changes to be their own Pull Requests.

Lemme fix this in main, and that should enable your changes to build again :)

@felix91gr felix91gr mentioned this pull request Aug 11, 2025
@felix91gr felix91gr added chapter: expressions decidability: undecidable A coding guideline which cannot be checked automatically labels Aug 11, 2025
@felix91gr

Copy link
Copy Markdown
Collaborator

@vapdrs thanks! I was gonna ping you here as well but you just did the thing ❤️

@felix91gr felix91gr self-assigned this Aug 11, 2025

@felix91gr felix91gr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think these are the last few touches I'd add.

Comment thread src/coding-guidelines/expressions.rst Outdated
Comment thread src/coding-guidelines/expressions.rst Outdated
Comment thread src/coding-guidelines/expressions.rst Outdated
Comment thread src/coding-guidelines/expressions.rst Outdated
vapdrs and others added 2 commits August 15, 2025 09:53
Co-authored-by: Félix Fischer <felix91gr@users.noreply.github.com>
Co-authored-by: Félix Fischer <felix91gr@users.noreply.github.com>
@felix91gr

Copy link
Copy Markdown
Collaborator

This one is done, as far as I'm concerned. Much like for #136, thank you @vapdrs for your work and for your patience ^^

I'll add it to the merge queue right away :)

@felix91gr
felix91gr enabled auto-merge August 24, 2025 00:53

@PLeVasseur PLeVasseur left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for seeing this through @vapdrs!

@felix91gr
felix91gr added this pull request to the merge queue Aug 24, 2025
Merged via the queue into Safety-Critical-Rust-Consortium:main with commit d695610 Aug 24, 2025
6 checks passed
@felix91gr felix91gr added the CERT C Issues or coding guidelines directly related to the CERT C Coding Guidelines label Feb 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CERT C Issues or coding guidelines directly related to the CERT C Coding Guidelines chapter: expressions decidability: undecidable A coding guideline which cannot be checked automatically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Coding Guideline]: Do not divide by 0

7 participants