Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Generic exp for pow and checked_pow#300

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
mtshr wants to merge3 commits intorust-num:master
base:master
Choose a base branch
Loading
frommtshr:generic-exp-for-pow

Conversation

@mtshr
Copy link

@mtshrmtshr commentedNov 25, 2023
edited
Loading

Due to theexp beingusize, the current implementation has been inconsistent with core implementation ofpow andchecked_pow takingu32 forexp. This has also inhibited implementation of somePow<u*> for primitive integers.

This pull request generalizesexp to use unsigned primitive integer for the sake of the implementation of lackingPow<u*> and more importantly, I guess, the consistency with corepow andchecked_pow utilizingu32, for the use of architecture dependently sized type, namelyusize here, for arithmetic function does not sound reasonable.

Although this change should bea minor change asusize does implementsPrimInt + Unsigned, please note that the existing crate depending on the functions in question may come to be in need of adding some type notation, as {integer} is assumed to bei32.

num_traits::pow(7,7);// ngnum_traits::pow(7,7usize);// ok

(prim_int $t:ty) =>{
pow_impl!($t,u8);
pow_impl!($t,u16);
pow_impl!($t,u32,u32, <$t>::pow);
Copy link
Author

Choose a reason for hiding this comment

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

Utilizes the default implementation of pow for exp with u32

if exp ==0{
pubfnpow<T,U>(mutbase:T,mutexp:U) ->T
where
T:Clone +One,
Copy link
Author

Choose a reason for hiding this comment

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

I removedT: Mul<T, Output = T> becauseOne needs it.

@cuviper
Copy link
Member

It is an unfortunate historical difference, but I think the{integer} inference loss is probably not worth it when the more flexiblePow<RHS> already exists. Do you need an equivalentCheckedPow?

@mtshr
Copy link
Author

mtshr commentedNov 29, 2023
edited
Loading

Thank you for taking a look.

I think the{integer} inference loss is probably not worth it when the more flexiblePow<RHS> already exists.

I am sorry that my English ability is not sufficient, but am I right in interpreting you think it is too much to changeexp: usize to general one and therefore are not in favor of the change of the pull request? What I give more importance here is actually not the fact that the user can implement their ownpow withPow<RHS>, but the reusability of the library's existing code and mitigating the concern about type size. After implementingClone (and preferablyCopy for the case I am describing at the moment considering the cost) +One for their own struct, the user will be able to just callnum_traits::pow without writing their ownpow. Also for example, forWrapping<T> or something similar, the user can reasonably take exp >= 2^32, but always having to takeusize's actual size into account leads to implementing their ownpow and in the end the user cannot benefit much frompub pow.

Do you need an equivalentCheckedPow?

I know others may have a interest in one (ref#254) and I am not against the idea to add one. I consider making a pull request once the idea of generalizing exp is accepted, for I do not want to yield another FIXME.

// FIXME: these should be possible
// pow_impl!($t, u16);
// pow_impl!($t, u32);
// pow_impl!($t, u64);

Maybe I should have created an issue first. I apologize if discussing the idea here is quite inappropriate.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mtshr@cuviper

[8]ページ先頭

©2009-2025 Movatter.jp