Skip to content

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

Merged
iliekturtles merged 1 commit into
iliekturtles:masterfrom
adamreichold:const-zero
Apr 14, 2021
Merged

RFC: Add a zero constant to quantities based on primitive types.#250
iliekturtles merged 1 commit into
iliekturtles:masterfrom
adamreichold:const-zero

Conversation

@adamreichold

@adamreichold adamreichold commented Apr 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

iliekturtles
iliekturtles reviewed Apr 12, 2021
View reviewed changes
Comment thread src/storage_types.rs Outdated
Comment thread src/quantity.rs Outdated
iliekturtles
iliekturtles reviewed Apr 13, 2021
View reviewed changes

@iliekturtles iliekturtles left a comment

Copy link
Copy Markdown
Owner

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.

Comment thread src/storage_types.rs Outdated
iliekturtles
iliekturtles reviewed Apr 14, 2021
View reviewed changes

@iliekturtles iliekturtles left a comment

Copy link
Copy Markdown
Owner

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?

Comment thread src/lib.rs Outdated
Comment thread src/lib.rs Outdated
Comment thread src/lib.rs
@adamreichold

adamreichold commented Apr 14, 2021

Copy link
Copy Markdown
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

iliekturtles commented Apr 14, 2021

Copy link
Copy Markdown
Owner

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

@iliekturtles iliekturtles mentioned this pull request Apr 14, 2021
Open
@adamreichold adamreichold deleted the const-zero branch April 14, 2021 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iliekturtles iliekturtles iliekturtles left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add zero constants/methods for each quantity

2 participants

@adamreichold @iliekturtles