Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Implement operator overloading and improve GMP #342

Closed
wants to merge 11 commits into from

6 participants

@nikic
Owner
@lstrojny

Cool idea!

nikic added some commits
@nikic nikic Improve GMP performance
a) Allow to specify an initial value when creating a GMP object
b) Allow creating the object into an existing zval rather than
   doing a alloc, copy, dtor cycle
1234aca
@nikic nikic More perf improvement 9ba5739
@nikic nikic Add compare object handler (rather than going through do_operation) 208442f
@nikic nikic Move std_compare code back to compare_function 4f9b114
@nikic nikic A bit more cleanup
 * gmp_mod and gmp_div_r no longer return a long result if the
   modulus is long. Now a GMP instance is always return (which is
   of course castable to a long).
 * Due to the above change gmp_div_r no longer returns incorrect
   results in some rounding modes with a long exponent.
 * If an invalid rounding mode is passed a warning is thrown rather
   than silently returning NULL.
2ca3543
@nikic nikic Remove various indirections
Rather than allocating mpz_t structures and storing them in mpz_t*
pointers directly work with the mpz_t structures (without additional
allocation). mpz_t is already a (pseudo) indirection.

Also now uses zval*s everywhere rather than zval**s.
efbbf20
@nikic nikic Remove compare_objects_t back to compare_t 05ae4b1
@weltling

On windows ext/gmp tests pass so far. Let me know if anything else on QA can be done. All the best :)

@smalyshev
Owner

@nikic wasn't this already merged? if so, this pull should be closed.

@php-pulls
Collaborator

Comment on behalf of nikic at php.net:

@smalyshev Yup, thanks for the notice :)

@php-pulls php-pulls closed this
@pilif

I'm really sorry to hop into the discussion like this (I didn't find a way to leave a quick comment on the RFC page), but the decision to base GMP on objects (as seen in proposal B on https://wiki.php.net/rfc/operator_overloading_gmp) feels really dangerous to me as objects are always passed by reference in PHP. Especially when combined with operator overloading, this could have unexpected results.


$a = gmp_init(42);
$b = $a;
$a = $a * 2;
assert($a > $b); // will fail because $b points to the same object as $b

This is fundamentally different from what people are usually getting in PHP when they work with traditional numbers.

The resource thing pre-patch is bad too, so unfortunately I don't have a solution for the issue. It's just that the new behavior feels dangerous to me.

Really sorry if this is the wrong place to post something, but the real place wasn't apparent to me.

@nikic
Owner

@pilif In PHP everything is always passed by value, object values just happen to exhibit "reference-like" behavior, but only in the same way that resources exhibit reference-like behavior. It's not the same as real references though, which is why your above example will work as expected. $a and $b will be distinct objects after the $a=$a*2 line.

@lstrojny

This is what C# has structs for. @nikic is there a way to change assignment behavior as copy on assignment would indeed be more logical.

@nikic
Owner

@lstrojny Not sure I understand the issue. As already said objects exhibit the same behavior as references, so there is no BC break here. Overloaded operators will also always result in a new object and not modify the old one. This also applies for compund assignment operators, for example with $b = $a; $a *= 2 only the value of $a is changed, not the value of $b.

Generally the GMP functions always return new instances, only exceptions being gmp_setbit / clrbit. Those are the only functions where the instance is actually modified, so it's the only place where the reference-like behavior of objects could make a difference. But as already said, resources have exactly the same behavior there.

@smalyshev
Owner

@pliff I'm not sure I understand why $a * 2 would return the same object. Did you check it really happens? I was under impression $a * 2 produces completely new object, if it is not so it certainly has to be fixed. But I don't see anything like this happening in my tests. Could you show a short script together if expected and actual output that demonstrates the problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 12, 2013
  1. @nikic
  2. @nikic

    Improve GMP performance

    nikic authored
    a) Allow to specify an initial value when creating a GMP object
    b) Allow creating the object into an existing zval rather than
       doing a alloc, copy, dtor cycle
  3. @nikic

    More perf improvement

    nikic authored
Commits on May 13, 2013
  1. @nikic
  2. @nikic
  3. @nikic

    A bit more cleanup

    nikic authored
     * gmp_mod and gmp_div_r no longer return a long result if the
       modulus is long. Now a GMP instance is always return (which is
       of course castable to a long).
     * Due to the above change gmp_div_r no longer returns incorrect
       results in some rounding modes with a long exponent.
     * If an invalid rounding mode is passed a warning is thrown rather
       than silently returning NULL.
Commits on May 14, 2013
  1. @nikic

    Remove various indirections

    nikic authored
    Rather than allocating mpz_t structures and storing them in mpz_t*
    pointers directly work with the mpz_t structures (without additional
    allocation). mpz_t is already a (pseudo) indirection.
    
    Also now uses zval*s everywhere rather than zval**s.
  2. @nikic
Commits on Jun 7, 2013
  1. @nikic
  2. @nikic

    Add support for shifts on GMP objects

    nikic authored
    >> behaves as an arithmetic shift, i.e. -42 >> 2 == -11
  3. @nikic
Something went wrong with that request. Please try again.