Skip to content

No grow in mp_set_int (2) #253

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

Merged
merged 2 commits into from
May 12, 2019
Merged

No grow in mp_set_int (2) #253

merged 2 commits into from
May 12, 2019

Conversation

minad
Copy link
Member

@minad minad commented May 10, 2019

Based on #221, I tried to simplify some things. No padding to MP_PREC anymore. Ping @nijtmans

(Renamed from #252 to avoid interfering with CI)

@minad minad changed the title No grow in set int2 No grow in mp_set_int (2) May 10, 2019
@minad minad force-pushed the no_grow_in_set_int2 branch from f6a0035 to 00cbf17 Compare May 11, 2019 07:03
@minad minad requested review from nijtmans and czurnieden May 11, 2019 22:48
@minad minad force-pushed the no_grow_in_set_int2 branch from 00cbf17 to ba75541 Compare May 11, 2019 22:57
Copy link
Contributor

@czurnieden czurnieden left a comment

Choose a reason for hiding this comment

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

long long is not in c89.
But I had a hard time to get a warning, even with -std=c89. Only clang with its -Weverything uttered one with the following example.

#include <stdlib.h>
#include <stdio.h>
int main(void)
{
   long long a;
   a = 123LL;
   printf("%lld\n",a);
   exit(EXIT_SUCCESS);
}
2 warnings generated.
czurnieden ~/DIV_C_FILES$ clang -Weverything -std=c90 longlong.c -o longlong
longlong.c:7:4: warning: 'long long' is an extension when C99 mode is not enabled [-Wlong-long]
   long long a;
   ^
longlong.c:9:8: warning: 'long long' is an extension when C99 mode is not enabled [-Wlong-long]
   a = 123LL;
       ^
2 warnings generated.

The only one insisting on c89 is Debian, IIRC and they use GCC per default (or did they change that?) which I couldn't get to raise its voice. At least not with any of the default warnings (-W -Wall -Wextra), the -Wlong-long must be explicitly given.

Ignore the problem until somebody with a very old compiler complains?
It is for a measurement only, it can be replaced with an actual number for that compiler, so: yepp, ignore it.

@minad
Copy link
Member Author

minad commented May 12, 2019

@czurnieden Well, long long is already in the code base ;)

This PR needs careful checking, since it changes the allocation sizes (in particular no rounding to MP_PREC). But since we have valgrind in place I am pretty confident that things are alright.

@minad minad force-pushed the no_grow_in_set_int2 branch from ba75541 to 7c2740b Compare May 12, 2019 10:59
* mp_set_int* always return MP_OKAY
* remove return checks for mp_set_int*
* introduce MP_MIN_PREC
@minad minad force-pushed the no_grow_in_set_int2 branch from 7c2740b to 7365442 Compare May 12, 2019 11:04
Copy link
Collaborator

@nijtmans nijtmans left a comment

Choose a reason for hiding this comment

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

Looks perfect for me!

@nijtmans
Copy link
Collaborator

Thanks, @minad , for taking this further!

@minad
Copy link
Member Author

minad commented May 12, 2019

Thanks for taking a look!

@sjaeckel sjaeckel merged commit 1c94819 into develop May 12, 2019
@sjaeckel sjaeckel deleted the no_grow_in_set_int2 branch May 12, 2019 21:42
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.

4 participants