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

RFC: Add a zero constant to quantities based on primitive types. #250

Merged
merged 1 commit into from
Apr 14, 2021
Merged

RFC: Add a zero constant to quantities based on primitive types. #250

merged 1 commit into from
Apr 14, 2021

Conversation

adamreichold
Copy link
Contributor

@adamreichold adamreichold commented Apr 10, 2021

The public helper trait is a bit cumbersome but it appears to be the most straight-forward way to construct a zero value in const context.

An alternative that avoid the intermediate trait this would be to implement something like

impl<U> $quantity<U, $type>
where
    U: __system::Units<$type> + ?Sized,
{
    /// Constant representing the zero value.
    pub const ZERO: Self = Self {
        dimension: $crate::lib::marker::PhantomData,
        units: $crate::lib::marker::PhantomData,
        value: 0 as $type,
    };
}

for the same set of primitive types.

Fixes #26

src/storage_types.rs Outdated Show resolved Hide resolved
src/quantity.rs Outdated Show resolved Hide resolved
Copy link
Owner

@iliekturtles iliekturtles left a comment

Choose a reason for hiding this comment

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

Thanks so much for the PR! I didn't intend to do a full review yesterday morning and completely forgot to thank you.

src/storage_types.rs Outdated Show resolved Hide resolved
Copy link
Owner

@iliekturtles iliekturtles left a comment

Choose a reason for hiding this comment

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

Couple of minor things and I think we're good to merge.

Thoughts about the proposed new_in_base_units. Just push that into the const fn issue or create a new issue?

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@adamreichold
Copy link
Contributor Author

Thoughts about the proposed new_in_base_units. Just push that into the const fn issue or create a new issue?

I think a new issue is the better approach because our goal should probably be to have Quantity::new be a const fn even though this is currently still blocked on the trait bounds as well as const eval for floating point math. We could use a new issue to e.g. discuss adding nightly only features or macro-based workarounds.

@iliekturtles iliekturtles merged commit f0aa415 into iliekturtles:master Apr 14, 2021
@iliekturtles
Copy link
Owner

Thanks so much for this PR. I'll create a new issue to continue efforts for new / new_from_base_units

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.

Add zero constants/methods for each quantity
2 participants