http://d.puremagic.com/issues/show_bug.cgi?id=11188

           Summary: std.math.abs fails for shared, const or immutable
                    BigInt types
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: nobody@puremagic.com
        ReportedBy: joseph.wakeling@webdrake.net


--- Comment #0 from Joseph Rushton Wakeling <joseph.wakeling@webdrake.net> 2013-10-07 05:27:33 PDT ---
Created an attachment (id=1259)
Example code illustrating the problem with std.math.abs and shared, const or
immutable BigInts.

std.math.abs seems to work effectively with all mutable numeric types, including std.bigint.BigInt.

However, if BigInts are qualified as shared, const or immutable, std.math.abs will fail to compile.

Sample code attached illustrating the problem.  rdmd bigabs.d gives the errors:

------------------------------------------------------------------------------
bigabs.d(6): Error: template std.math.abs does not match any function template
declaration. Candidates are:
/opt/dmd/include/d2/std/math.d(303):        std.math.abs(Num)(Num x) if
(is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) && !(is(Num* :
const(ifloat*)) || is(Num* : const(idouble*)) || is(Num* : const(ireal*))))
/opt/dmd/include/d2/std/math.d(314):        std.math.abs(Num)(Num z) if
(is(Num* : const(cfloat*)) || is(Num* : const(cdouble*)) || is(Num* :
const(creal*)))
/opt/dmd/include/d2/std/math.d(322):        std.math.abs(Num)(Num y) if
(is(Num* : const(ifloat*)) || is(Num* : const(idouble*)) || is(Num* :
const(ireal*)))
bigabs.d(6): Error: template std.math.abs(Num)(Num x) if (is(typeof(Num.init >=
0)) && is(typeof(-Num.init)) && !(is(Num* : const(ifloat*)) || is(Num* :
const(idouble*)) || is(Num* : const(ireal*)))) cannot deduce template function
from argument types !()(shared(BigInt))
bigabs.d(14): Error: template instance bigabs.foo!(shared(BigInt)) error
instantiating
bigabs.d(6): Error: template std.math.abs does not match any function template
declaration. Candidates are:
/opt/dmd/include/d2/std/math.d(303):        std.math.abs(Num)(Num x) if
(is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) && !(is(Num* :
const(ifloat*)) || is(Num* : const(idouble*)) || is(Num* : const(ireal*))))
/opt/dmd/include/d2/std/math.d(314):        std.math.abs(Num)(Num z) if
(is(Num* : const(cfloat*)) || is(Num* : const(cdouble*)) || is(Num* :
const(creal*)))
/opt/dmd/include/d2/std/math.d(322):        std.math.abs(Num)(Num y) if
(is(Num* : const(ifloat*)) || is(Num* : const(idouble*)) || is(Num* :
const(ireal*)))
bigabs.d(6): Error: template std.math.abs(Num)(Num x) if (is(typeof(Num.init >=
0)) && is(typeof(-Num.init)) && !(is(Num* : const(ifloat*)) || is(Num* :
const(idouble*)) || is(Num* : const(ireal*)))) cannot deduce template function
from argument types !()(const(BigInt))
bigabs.d(15): Error: template instance bigabs.foo!(const(BigInt)) error
instantiating
bigabs.d(6): Error: template std.math.abs does not match any function template
declaration. Candidates are:
/opt/dmd/include/d2/std/math.d(303):        std.math.abs(Num)(Num x) if
(is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) && !(is(Num* :
const(ifloat*)) || is(Num* : const(idouble*)) || is(Num* : const(ireal*))))
/opt/dmd/include/d2/std/math.d(314):        std.math.abs(Num)(Num z) if
(is(Num* : const(cfloat*)) || is(Num* : const(cdouble*)) || is(Num* :
const(creal*)))
/opt/dmd/include/d2/std/math.d(322):        std.math.abs(Num)(Num y) if
(is(Num* : const(ifloat*)) || is(Num* : const(idouble*)) || is(Num* :
const(ireal*)))
bigabs.d(6): Error: template std.math.abs(Num)(Num x) if (is(typeof(Num.init >=
0)) && is(typeof(-Num.init)) && !(is(Num* : const(ifloat*)) || is(Num* :
const(idouble*)) || is(Num* : const(ireal*)))) cannot deduce template function
from argument types !()(immutable(BigInt))
bigabs.d(16): Error: template instance bigabs.foo!(immutable(BigInt)) error
instantiating
Failed: 'dmd' '-v' '-o-' 'bigabs.d' '-I.'
------------------------------------------------------------------------------

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11188


hsteoh@quickfur.ath.cx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hsteoh@quickfur.ath.cx


--- Comment #1 from hsteoh@quickfur.ath.cx 2013-10-10 11:20:06 PDT ---
Does std.math.abs even work for anything other than built-in types currently? I know it should, but I'm skeptical if it does.

In any case, it should take inout arguments since it doesn't (and shouldn't)
modify them. So this (in theory) should work:

inout(Num) abs(Num)(inout(Num) x) if ( ... ) {
    return (x >= 0) ? x : -x;
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11188



--- Comment #2 from Joseph Rushton Wakeling <joseph.wakeling@webdrake.net> 2013-10-10 11:23:53 PDT ---
(In reply to comment #1)
> Does std.math.abs even work for anything other than built-in types currently? I know it should, but I'm skeptical if it does.

It works for regular BigInt, it's just when you qualify that with shared, const or immutable that it fails.

> In any case, it should take inout arguments since it doesn't (and shouldn't)
> modify them. So this (in theory) should work:
> 
> inout(Num) abs(Num)(inout(Num) x) if ( ... ) {
>     return (x >= 0) ? x : -x;
> }

I'll give that a go and if it works, send a patch to Phobos.  Thanks for the thought.  Incidentally, shouldn't "in" be sufficient there for the input?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11188



--- Comment #3 from hsteoh@quickfur.ath.cx 2013-10-10 11:36:03 PDT ---
(In reply to comment #2)
> I'll give that a go and if it works, send a patch to Phobos.  Thanks for the thought.  Incidentally, shouldn't "in" be sufficient there for the input?

In this case, you need inout, because you want to propagate the incoming qualifiers to the output: if x>=0, then you're returning x again, so the return type needs to carry whatever qualifiers x had on entering the function. Everything implicitly converts to const, of course, but it would suck if you returned const and the caller passed in unqual, and now after calling abs he can't modify the BigInt anymore!

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11188



--- Comment #4 from Joseph Rushton Wakeling <joseph.wakeling@webdrake.net> 2013-10-10 11:47:51 PDT ---
Tweaking std.math.abs to accept an inout input results in the following error when I added a regular BigInt to the unittests:

    assert(abs(BigInt(-234567)) == 234567);

-------------------------------
std/math.d(311): Error: function std.bigint.BigInt.opCmp (ref const(BigInt) y)
const is not callable using argument types (int) inout
std/math.d(311): Error: mutable method std.bigint.BigInt.opUnary!"-".opUnary is
not callable using a inout object
std/math.d(341): Error: template instance std.math.abs!(BigInt) error
instantiating
-------------------------------

So I'd guess in turn std.bigint.BigInt.opCmp (and other BigInt methods) ought
to require (or at least have available) inout input.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11188



--- Comment #5 from Joseph Rushton Wakeling <joseph.wakeling@webdrake.net> 2013-10-10 17:33:40 PDT ---
(In reply to comment #3)
> In this case, you need inout, because you want to propagate the incoming qualifiers to the output: if x>=0, then you're returning x again, so the return type needs to carry whatever qualifiers x had on entering the function. Everything implicitly converts to const, of course, but it would suck if you returned const and the caller passed in unqual, and now after calling abs he can't modify the BigInt anymore!

I wasn't talking about returning const, but about the _input_ to the function.

I'm not actually certain you do always want to propagate the qualifiers to the output -- I may want to be able to do:

    immutable BigInt x = -345678;
    BigInt xAbs = abs(x);

... or is inout sensitive to things like this?  In any case, bear in mind that abs doesn't always guaranteed to return the same type -- if you give it (say) a cfloat or an ifloat, it'll return a real.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11188


Simen Kjaeraas <simen.kjaras@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull
                 CC|                            |simen.kjaras@gmail.com


--- Comment #6 from Simen Kjaeraas <simen.kjaras@gmail.com> 2013-11-01 18:35:01 PDT ---
https://github.com/D-Programming-Language/phobos/pull/1679

This pull solves some of the problems in this issue. Shared is more complicated, so will have to wait.

It may be that the problems of shared are better fixed elsewhere - pure functions on a type that's implicitly castable to immutable seem like they should be callable with shared parameters, but I might be wrong.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11188



--- Comment #7 from Joseph Rushton Wakeling <joseph.wakeling@webdrake.net> 2013-11-02 04:55:32 PDT ---
(In reply to comment #6)
> This pull solves some of the problems in this issue. Shared is more complicated, so will have to wait.

Thanks very much Simen -- there are some aspects of your fix that I would not have thought of myself, but now that I've seen them, they are obvious things to watch out for, so I really appreciate not only the solution but also the insight gained thereby :-)

I agree that shared is a bigger issue than just BigInt and any fix for it needs to wait for shared itself to be properly sorted out.  See also: https://github.com/D-Programming-Language/phobos/pull/1616#issuecomment-25865836

... for earlier discussion.

I'll give your code a test and see how it plays with std.rational, which was the original project from which awareness of this issue arose.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11188



--- Comment #8 from Simen Kjaeraas <simen.kjaras@gmail.com> 2013-11-03 13:25:17 PST ---
Adding this overload to std.math is a workaround for the problems with shared:

Num abs(Num)(shared Num x) @safe pure nothrow
    if (is(typeof(abs(Num.init))) && is(typeof({Num n = x;})))
{
    Num tmp = x;
    return abs(tmp);
}


However, given that shared is currently rather ill-specified, and possibly will change dramatically relatively soon (let's hope so), and that this is a workaround for a single function only, I feel it's better to leave the workaround out for the time being, and wait for a real solution.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------