These are chat archives for bluescarni/piranha

7th
Jun 2016
Srajan Garg
@srajangarg
Jun 07 2016 06:06
@bluescarni can you go ahead with the PR, I don't think I'm familiar enough with piranha right now. I'll surely look at the PR once you're done
Francesco Biscani
@bluescarni
Jun 07 2016 08:29
sure np
Francesco Biscani
@bluescarni
Jun 07 2016 09:01
@isuruf I am planning to start using C++14 in piranha eventually. I'd like to target initially at least GCC 4.9, but realistically the minimum requirement might slip up to GCC 5. Would this be a problem for symengine?
Isuru Fernando
@isuruf
Jun 07 2016 09:03
Nope, we'll update the gcc version for testing with piranha
Is -std=c++14 required?
Francesco Biscani
@bluescarni
Jun 07 2016 09:04
not at the moment. Piranha will just enable it if it is available, but no C++14 constructs are currently used in the code
but yes it will be required to be enabled
I think GCC 6 is the first to default to C++14
there might also be a bump up to the minimum cmake version
Isuru Fernando
@isuruf
Jun 07 2016 09:07
Right. I'll pick a commit and use that for now for testing and then update the testing when I have some time
Francesco Biscani
@bluescarni
Jun 07 2016 09:10
cheers.. I've been doing some reading about modern cmake practices and would like to tes them
Isuru Fernando
@isuruf
Jun 07 2016 09:11
Do you plan to make a release soon, or shall I just pick a git commit?
Francesco Biscani
@bluescarni
Jun 07 2016 09:30
there's no immediate plan at the moment
I am working on a couple of branches and waiting for the llvm apt situation to be resolved
it's just that I have been using C++14 on another project and it looks pretty neat :)
do you pull from piranh HEAD right now in symengine or did you pin a specific commit?
Isuru Fernando
@isuruf
Jun 07 2016 09:32
head right now, I'll pin a specific commit
Francesco Biscani
@bluescarni
Jun 07 2016 09:33
ok sounds good
I've been meaning to do releases for a while, but I need documentation done for that and it's been an awful problem without solution despite countless hours spent on it
I need a robust tool able to deal with and cross-reference sphinx/doxygen documentation, but none fits the bill
https://github.com/michaeljones/breathe is almost there but it chokes on the sort of C++ used in Piranha
Francesco Biscani
@bluescarni
Jun 07 2016 09:50
@srajangarg https://github.com/bluescarni/piranha/tree/term_noexcept_fix this works for me locally
(after disabling the term specialisation in symengine)
could you test it on your end?
Isuru Fernando
@isuruf
Jun 07 2016 10:17
@srajangarg, why not add noexcept to all the integer_class types? mpz_class and piranha::integer already has them
Srajan Garg
@srajangarg
Jun 07 2016 10:19
Okay I will try, will do this in the piranhapoly PR as the change is required because of that
what methods must be marked noexcept? ctor, dtor copy ctor? and
Francesco Biscani
@bluescarni
Jun 07 2016 10:23
move constructor and move assignment
unless noted, destructors are automatically assumed noexcept (but GCC < 4.8 does not get it right)
Srajan Garg
@srajangarg
Jun 07 2016 13:03
@isuruf mpz_class does not pass the checks
Srajan Garg
@srajangarg
Jun 07 2016 13:18
mpz_wrapper and fmpz_wrapper have been made to pass is_cf, all that remains is mpz_class
Srajan Garg
@srajangarg
Jun 07 2016 13:26
Is there a solution?
Isuru Fernando
@isuruf
Jun 07 2016 13:27
Hmm, did you include gmpxx.h yourself, or used the include from mp_class.h ?
Srajan Garg
@srajangarg
Jun 07 2016 13:29
just cmake -DINTGER_CLASS=gmpxx, i.e. the include from mp_class
Isuru Fernando
@isuruf
Jun 07 2016 13:30
Hmm, that's weird, move constructor of mpz_class in gmpxx.h is not marked noexcept
Srajan Garg
@srajangarg
Jun 07 2016 13:31
Why do we have both mpz_wrapper and mpz_class . Aren't both the same thing, a wrapper for mpz_t
Isuru Fernando
@isuruf
Jun 07 2016 13:33
@srajangarg, I think I answered your question in a chat with you about a week back, let me know if you find anything not clear
Srajan Garg
@srajangarg
Jun 07 2016 13:35
Right, found it thanks
Srajan Garg
@srajangarg
Jun 07 2016 13:44
So, we'll have to stick with the workaround for mpz_class I'm assuming
Isuru Fernando
@isuruf
Jun 07 2016 13:47
Yes, A recent thread on gmp-devel, mentioned marking it as noexcept since mpz_init can be marked nothrow due to recent changes
Francesco Biscani
@bluescarni
Jun 07 2016 13:47
@isuruf can you point me to this thread?
Francesco Biscani
@bluescarni
Jun 07 2016 13:48
thanks.. what recent changes are you referrinf to?
Isuru Fernando
@isuruf
Jun 07 2016 13:49
Lazy mpz allocation
I assumed it was recent
Francesco Biscani
@bluescarni
Jun 07 2016 13:49
thanks!
@isuruf are you aware of what this lazy allocation is about? any impact on user code?
Srajan Garg
@srajangarg
Jun 07 2016 13:55
So was there a follow up? Has it been made noexcept
*but not released
Isuru Fernando
@isuruf
Jun 07 2016 14:00
@bluescarni, No impact on user code, I guess. mpz_init method body has changed, I'm not sure what it means
@srajangarg, it's in gmp development repo, not released yet
Francesco Biscani
@bluescarni
Jun 07 2016 14:01
from the sound of it it looks like mpz_init now does not allocate any memory, and the memory is allocated on first use instead
Isuru Fernando
@isuruf
Jun 07 2016 14:11
@bluescarni, this change would affect mp_integer
Francesco Biscani
@bluescarni
Jun 07 2016 14:12
in which way?
ah right
Francesco Biscani
@bluescarni
Jun 07 2016 14:15
mhm I need to think about it
I guess that this is what happens when you rely on implementation details ;)
but I have a suspicion this will break quite a bit of code using GMP
Isuru Fernando
@isuruf
Jun 07 2016 14:18
Instead of one check, now you have to do 2 checks
You mean on piranha or other code?
Francesco Biscani
@bluescarni
Jun 07 2016 14:20
in general... my impression was that people working with GMP often go down to the implementation details level to get the most out of it
but maybe I am wrong
the problem is that mp_integer is relying on the _alloc member, which is shared between the dynamic and static ints
Isuru Fernando
@isuruf
Jun 07 2016 14:21
Yes, I know
Francesco Biscani
@bluescarni
Jun 07 2016 14:21
if now it is necessary to inspect another variable, that needs to be shared as well
Isuru Fernando
@isuruf
Jun 07 2016 14:22
_mp_d is shared and you have to check that it is not pointing to a dummy_limb
Francesco Biscani
@bluescarni
Jun 07 2016 14:23
_mp_size you mean?
Isuru Fernando
@isuruf
Jun 07 2016 14:25
No, _mp_d
Francesco Biscani
@bluescarni
Jun 07 2016 14:26
struct expected_mpz_struct_t
{
    mpz_alloc_t    _mp_alloc;
    mpz_size_t    _mp_size;
    ::mp_limb_t    *_mp_d;
};
And these are the members of static_integer:
    mpz_alloc_t    _mp_alloc;
    mpz_size_t    _mp_size;
    limbs_type    m_limbs;
Isuru Fernando
@isuruf
Jun 07 2016 14:29
m_limbs and _mp_d are both pointers right?
Francesco Biscani
@bluescarni
Jun 07 2016 14:30
no, limbs_type is an array
an std::array, but I think even with the plain array it would not matter
Isuru Fernando
@isuruf
Jun 07 2016 14:30
Yes, but both act as pointers
Francesco Biscani
@bluescarni
Jun 07 2016 14:31
to be shared they need to be the same type, and pointers and arrays are not I guess
Isuru Fernando
@isuruf
Jun 07 2016 14:31
In a std::union, you can access one type as another as if a reinterpret_cast right?
Francesco Biscani
@bluescarni
Jun 07 2016 14:32
no in general doing a reinterpret_cast on union members is undefined behaviour (in C++, not sure about C)
there is a special rule in the standard that if you have classes that share the same initial sequence of members you can access those members from either type
Isuru Fernando
@isuruf
Jun 07 2016 14:34
Now I realized, that won't work, because the alignment maybe different
Francesco Biscani
@bluescarni
Jun 07 2016 14:34
but they need to be identical types
Isuru Fernando
@isuruf
Jun 07 2016 14:40
Can you point me to where this special rule is mentioned?
Francesco Biscani
@bluescarni
Jun 07 2016 14:42
it's paraphrased here: http://en.cppreference.com/w/cpp/language/data_members I can find the standard section if you need
Two standard-layout non-union class types may have a common initial sequence of non-static data members and bit-fields (since C++14), for a sequence of one or more initial members (in order of declaration), if the members have layout-compatible types and if they are bit-fields, they have the same width (since C++14).
Isuru Fernando
@isuruf
Jun 07 2016 14:43
Thanks
Francesco Biscani
@bluescarni
Jun 07 2016 14:44
sure np
Srajan Garg
@srajangarg
Jun 07 2016 15:20
@bluescarni piranha/term_noexcept_fix is working fine! thanks. are you merging into master soon?
Francesco Biscani
@bluescarni
Jun 07 2016 18:26
@srajangarg good to hear! I need to run the test suite locally as travis is fubar, then I'll merge it
Francesco Biscani
@bluescarni
Jun 07 2016 18:36
@isuruf I think an easy solution to the lazy init problem is to have the static integer have an alloc size of -1, instead of 0... that means an alloc size of 2^32-1 or 2^64-1 depending on the platform, and it's guaranteed an mpz_t with that size is not possible
Francesco Biscani
@bluescarni
Jun 07 2016 19:01
@srajangarg I merged and pushed to master