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

mrb_int_mul_overflow is missing #3149

Closed
Asmod4n opened this Issue Apr 23, 2016 · 4 comments

Comments

Projects
None yet
3 participants
@Asmod4n
Contributor

Asmod4n commented Apr 23, 2016

Currently multiplications aren't checked for overflowing, there is __builtin_mul_overflow in GCC/Clang etc. I just don't know how to implement it in compilers which lack it.

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak Apr 23, 2016

Member

Is it available in C99?

Member

zzak commented Apr 23, 2016

Is it available in C99?

@Asmod4n

This comment has been minimized.

Show comment
Hide comment
@Asmod4n

Asmod4n Apr 23, 2016

Contributor

It would be mostly the same as https://github.com/mruby/mruby/blob/master/include/mruby/numeric.h#L40, so i guess its not in C99 too.

Contributor

Asmod4n commented Apr 23, 2016

It would be mostly the same as https://github.com/mruby/mruby/blob/master/include/mruby/numeric.h#L40, so i guess its not in C99 too.

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak Apr 23, 2016

Member

I'm not familiar enough with the builtin overflows, maybe matz can advise

Member

zzak commented Apr 23, 2016

I'm not familiar enough with the builtin overflows, maybe matz can advise

@cremno

This comment has been minimized.

Show comment
Hide comment
@cremno

cremno Apr 28, 2016

Contributor

I'll add it. The difficult part here is the fallback for compilers without these built-ins. It needs to be portable and fast. Of course, only to some degree. With 16/32 b mrb_int it's actually quite simple: 32/64 b multiplication + FIXABLE() (which btw. should be renamed to mrb_fixable or even mrb_fixable_p). But that's not an option with the 64b one…

Contributor

cremno commented Apr 28, 2016

I'll add it. The difficult part here is the fallback for compilers without these built-ins. It needs to be portable and fast. Of course, only to some degree. With 16/32 b mrb_int it's actually quite simple: 32/64 b multiplication + FIXABLE() (which btw. should be renamed to mrb_fixable or even mrb_fixable_p). But that's not an option with the 64b one…

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