www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 11188] New: std.math.abs fails for shared, const or immutable BigInt types

reply d-bugmail puremagic.com writes:
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: -------
Oct 07 2013
next sibling parent d-bugmail puremagic.com writes:
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: -------
Oct 10 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Oct 10 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Oct 10 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Oct 10 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Oct 10 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Nov 01 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Nov 02 2013
prev sibling parent d-bugmail puremagic.com writes:
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: -------
Nov 03 2013