Skip to content

No grow in mp_set_int (2) #253

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants