Skip to content

feat: add Step, Step instances, mixed sign tryFrom, some arithmetic#2

Open
abentkamp wants to merge 2 commits intomainfrom
step
Open

feat: add Step, Step instances, mixed sign tryFrom, some arithmetic#2
abentkamp wants to merge 2 commits intomainfrom
step

Conversation

@abentkamp
Copy link
Copy Markdown
Collaborator

This PR adds:

  • mixed sign try/try_from impls
  • iter.range.Step and its impls for integers
  • unchecked_X and checked_X_unsigned arithmetic functions

The Step impls are modeled after the official Rust implementation. In hindsight, maybe converting to mathematical integers and back would have been simpler?

@abentkamp abentkamp requested a review from maximebuyse May 6, 2026 09:06

macro_rules! step_identical_methods {
($Name:ty) => {
fn forward(start: Self, n: usize) -> Self {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

These functions should panic with with overflow, but with my implementaiton they panic duew to unwrap. In production builds, they should not panic at all, actually, but my implementation does.

use crate::option::Option;
use crate::result::Result;
/// See [`std::iter::Step`]
pub trait Step: Clone + PartialOrd<Self> + Sized {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

PartialOrd and Sized refer to core, here. I think that's correct?

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.

It depends, if we really need it that can be fine but ideally we would refer to the traits from core_models, otherwise we might end up having an incorrect order of items in the produced Lean

Comment thread core-models/src/core/convert.rs
Comment thread core-models/src/core/convert.rs Outdated
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.

2 participants