Bug 11188 – std.math.abs fails for shared BigInt type

Created attachment 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.' ------------------------------------------------------------------------------

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; }

(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?

(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!

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.

(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.

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.

(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.

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.

Seems fixed or partially fixed: https://github.com/D-Programming-Language/phobos/compare/cc52695e4a15...9a93d82e9924

Only valid for shared nowadays import std.bigint, std.math, std.typetuple; auto foo(T)() { T n = -3; return std.math.abs(n); } void main() { foreach (T; TypeTuple!(byte, short, int, long, BigInt)) { assert(foo!T() == 3); // works assert(foo!(shared(T)) == 3); // fails for BigInt } } The problem is that operator overloads don't accept shared void main() { import std.bigint; shared(BigInt) i = 0; assert(i >= 0); }

I think concurrency support is unreasonable here (and lacks rationale and use case). How it would work? Create a mutex for every BigInt?

(In reply to anonymous4 from comment #11) > I think concurrency support is unreasonable here (and lacks rationale and > use case). How it would work? Create a mutex for every BigInt? BigInt internals are immutable, and abs takes its argument by value, so no mutex is necessary. For anyone who wants to fix this, the problem is not actually in std.math.abs, but in BigInt - its operator overloads are not shared-aware.

Indeed, but it's still big and shallow mutable. How would you avoid tearing? It suggests different data layout with increased pressure on GC.

True - access to the BigInt would need to be protected somehow, and letting abs(shared BigInt) simply compile without warning signals to the user that it's perfectly safe, while tearing could still happen (unsynced ptr/length for the data field, or wrong sign). The way shared works in D today, it's simply the wrong choice. Thanks for enlightening me! :) Closing this issue as wontfix because while important parts of the original issue are fixed, the current name indicates shared only, and forcing the user to cast (and thus hopefully think about locking or otherwise limiting access).