in reply to Re^5: Puzzled by value of $overload::ops{binary}
in thread Puzzled by value of $overload::ops{binary}

I'd add some debugging checks in overload_add_eq to see what state the SVs are in when they arrive in it. Also, as a matter of good practice, I would keep all the SV-mangling stuff in the XS function, and just pass to your C function the mpz_t (or a pointer) values. If that meant the C functions you've shown here go away, then so be it.

Replies are listed 'Best First'.
Re^7: Puzzled by value of $overload::ops{binary}
by syphilis (Archbishop) on Jun 26, 2024 at 14:28 UTC
    I keep looking at this piece of overload.pm documentation: 
        *    *Assignments*

             +=  -=  *=  /=  %=  **=  <<=  >>=  x=  .=
             &=  |=  ^=  &.=  |.=  ^.=

         [snip]

         The subroutine for the assignment variant of an operator is
         required only to return the result of the operation.

    [download]
    That seems to me to be asserting that these operations (unlike '++' and '--') do require a value to be returned.
    And it also looks to me that overload.pm is somehow enforcing that policy.

    This following rewrite of the simplified overload_add_eq C and XS functions works fine: 
    void overload_add_eq(pTHX_ SV * a, SV * b, SV * third) {
     dXSARGS;
     PERL_UNUSED_ARG(third);
     mpz_add_ui(*(INT2PTR(mpz_t *, SvIVX(SvRV(a)))), *(INT2PTR(mpz_t *
+, SvIVX(SvRV(a)))), SvUV(b));
     XSRETURN(1);
}

    [download]
    and 
    void
overload_add_eq (a, b, third)
    SV *    a
    SV *    b
    SV *    third
        CODE:
        overload_add_eq (aTHX_ a, b, third);
        XSRETURN(1);

    [download]
    But I wonder if that achieves anything significant over the implementation that has been working fine for the last umpteen years.

    Anyway, I don't want to get too bogged down in this - though it's certainly interesting enough to encourage a bit more investigation.

    Cheers,
    Rob
[reply]
[d/l]
[select]
      It definitely doesn't look right that you're doing 
           dXSARGS;
/* ... */
     XSRETURN(1);

      [download]
      in your alleged C function, which should either mutate things and let the XS function do all the Perl stack stuff, or return a value but strictly in C-land.
[reply]
[d/l]
        I think that the overloading procedure views '+=' as an assignment operation ($x = $x + $addon), not as an inplace mutation ($x += $addon).
        The former requires a value to be returned; the latter does not.
        At least, it's behaving as though that's the case.

        That's why that documentation snippet I posted goes on to say: 
                 The subroutine for the assignment variant of an operator is
         required only to return the result of the operation. It is
         permitted to change the value of its operand (this is safe be
+cause
         Perl calls the copy constructor first), but this is optional 
+since
         Perl assigns the returned value to the left-hand operand anyw
+ay.

        [download]
        Note that it's specifically stating that it's "permitted" (but not mandatory) "to change the value of its operand ".
        If it was being deemed to be an inplace mutation, then it would have to change the value of its operand.

        AFAICS, there aren't many modules that overload '+=', but Math::BigFloat does: 
          '+='    =>      sub { $_[0] -> badd($_[1]); },

        [download]
        which I manually altered to: 
          '+='    =>      sub { $_[0] -> badd($_[1]); 42 },

        [download]
        I then get: 
        D:\>perl -MDevel::Peek -MMath::BigFloat -le "$x = Math::BigFloat->new(
+20); $x += 100; print $x; Dump $x;"
42
SV = PVIV(0x1bc99137430) at 0x1bc9913a450
  REFCNT = 1
  FLAGS = (IOK,pIOK,pPOK)
  IV = 42
  PV = 0x1bc9b44fbe0 "42"\0
  CUR = 2
  LEN = 16

        [download]
        In that pure perl environment it's clearly the returned result, not the result of the inplace addition, that's being seen.

        I'm now feeling more confident that the need for '+=' overloading to return the new value is intended (though unexpected) behaviour.

        Cheers,
        Rob
[reply]
[d/l]
[select]

In Section Seekers of Perl Wisdom