This program
import std.bigint;
import std.stdio;
void main()
{
auto b = BigInt(42);
writeln(b);
}
prints BigInt rather than 42. BigInt does not define a normal toString(). It looks like it declares a version of toString() which takes a delegate and format string in an attempt to have more control of what the string looks like. However, this is useless for cases where you need an actual toString() - particularly when functions which you have no control over call toString(). Normally, all types should define a toString() so that they can be printed, and BigInt doesn't do that.
So, BigInt should declare a normal toString() - presumably one which prints out the BigInt in decimal form.
Comment #1 by bearophile_hugs — 2010-11-17T23:10:31Z
This is a dupe of my 4122
The lack of a normally usable toString is not acceptable.
Comment #2 by clugdbug — 2010-11-18T00:03:42Z
That's not really the correct solution.
BigInt should act like an int. Specifically,
BigInt b;
writefln(" b = %d, %x", b, b);
should just work.
This issue cannot be resolved until the definition of toString() is changed.
void toString() is a fundamentally broken design. It's wrong on many levels.
Comment #3 by issues.dlang — 2010-11-18T00:24:35Z
void toString()? Normally, it's string toString(). The only problem that I'm aware of with regards to toString() design at the moment is the fact that it must be _exactly_ string toString() and can't be const or pure or whatnot.
I don't see why BigInt can't just have a normal toString() which returns a string representation of BigInt. Having a fancier toString() like it does now may be useful, but I don't see how it precludes having a normal toString().
Comment #4 by nfxjfg — 2010-11-18T00:56:03Z
(In reply to comment #2)
> void toString() is a fundamentally broken design. It's wrong on many levels.
But it allows more control over formatting and potentially reduces memory allocation. string toString() seems more broken to me: no control, forces you to do memory allocation. (Another broken design issue is that _all_ objects in D have a toString() method, even if it doesn't make sense, but that is off-topic here.)
What is worrying is that void toString is nowhere documented. Does std.format use it or what?
Comment #5 by nfxjfg — 2010-11-18T00:57:38Z
(In reply to comment #4)
> What is worrying is that void toString is nowhere documented. Does std.format
> use it or what?
Should be: is it supposed to use it? Of course it doesn't right now. If it should, the bug report is about std.format/writefln, and not std.bigint.
Comment #6 by schveiguy — 2010-11-18T05:10:23Z
(In reply to comment #2)
> That's not really the correct solution.
> BigInt should act like an int. Specifically,
>
> BigInt b;
> writefln(" b = %d, %x", b, b);
> should just work.
>
> This issue cannot be resolved until the definition of toString() is changed.
> void toString() is a fundamentally broken design. It's wrong on many levels.
So BigInt's aren't printable via writeln in protest? I guess I don't understand why you can't do this:
string toString()
{
string retval;
void sink(const(char)[] data) { retval ~= data; }
toString(&sink, null);
return retval;
}
It doesn't hurt/limit the current toString, does it? And then it makes bigints printable via writeln.
BTW, toString's delegate is not scope, so you are going to allocate a closure for that...
Comment #7 by clugdbug — 2010-11-18T05:58:20Z
(In reply to comment #6)
> (In reply to comment #2)
> So BigInt's aren't printable via writeln in protest?
Yes. I refuse to play any part in propagating the toString() abomination. BigInt will NEVER have a string toString() function. No frigging way.
This needs to be fixed in writefln/format.
Comment #8 by schveiguy — 2010-11-18T06:30:23Z
Pardon me for saying so, but that's too short-sighted.
first, it's not just a writeln/format change, it's a compiler change too. The compiler is the one who decides whether to store a function pointer to toString in the TypeInfo_Struct. Maybe you can help fix that...
Second, it's they way things currently work. It's like saying you refuse to have const functions because they should be inout, but inout doesn't work. When toString is fixed, then you can remove the crufty function, and nobody cares whether it was ever there or not. It looks to the outside like phobos is immature when it can't even print its own types, regardless of how inefficient it is.
Note that I 100% agree that the current system is crap, and needs to be completely redone similar to how you have implemented it, but it's not how it works now. Can't BigInt just play along and we can push for changes to the system without making the library look like a stubborn child?
BTW, when the system is changed, I wouldn't want it to be called toString, since string may have nothing to do with it. I'd call it something like format or output.
Comment #9 by clugdbug — 2010-11-18T08:42:45Z
(In reply to comment #8)
> Second, it's they way things currently work. It's like saying you refuse to
> have const functions because they should be inout, but inout doesn't work.
> When toString is fixed, then you can remove the crufty function, and nobody
> cares whether it was ever there or not. It looks to the outside like phobos is
> immature when it can't even print its own types, regardless of how inefficient
> it is.
If it was just a question of inefficiency, I would have implemented it. The issue is that it doesn't get the formatting string.
So
BigInt b;
writefln("%x %+d", b, b);
doesn't work, and cannot be made to work.
> Note that I 100% agree that the current system is crap, and needs to be
> completely redone similar to how you have implemented it, but it's not how it
> works now. Can't BigInt just play along and we can push for changes to the
> system without making the library look like a stubborn child?
No. The format string is absolutely fundamental to the implementation of outputting BigInt and BigFloat. It's not just "a crap implementation".
Comment #10 by schveiguy — 2010-11-18T10:20:08Z
(In reply to comment #9)
>
> If it was just a question of inefficiency, I would have implemented it. The
> issue is that it doesn't get the formatting string.
> So
> BigInt b;
> writefln("%x %+d", b, b);
>
> doesn't work, and cannot be made to work.
But what about writeln, or writefln("%s"...) ? Should those usages be excluded because you can't have custom formatting?
> No. The format string is absolutely fundamental to the implementation of
> outputting BigInt and BigFloat. It's not just "a crap implementation".
I disagree here. The huge problem I have with toString is the requirement for allocating immutable data that immediately gets thrown away. Supporting custom formatting is a nice feature but not having it does not severely impact users nearly as much as not having any output. In other words, if I want to print a BigInt in hex, and I get "BigInt", that is useless to me. If I get it in decimal, well at least I got something that can be used.
If the format string is so fundamental, then why does BigInt.toString support a null format argument?
Comment #11 by clugdbug — 2010-11-18T12:01:39Z
(In reply to comment #10)
> (In reply to comment #9)
> >
> > If it was just a question of inefficiency, I would have implemented it. The
> > issue is that it doesn't get the formatting string.
> > So
> > BigInt b;
> > writefln("%x %+d", b, b);
> >
> > doesn't work, and cannot be made to work.
>
> But what about writeln, or writefln("%s"...) ? Should those usages be excluded
> because you can't have custom formatting?
Yes.
Comment #12 by issues.dlang — 2010-11-18T12:31:36Z
So, you don't want writeln(bi); or to!string(bi) to work? I would think that ideally, BigInt would work exactly the same way that integers do.
to!string(i) -> decimal
writeln(i) -> decimal
writefln("%s", i) -> decimal
writefln("%d", i) -> decimal
writefln("%f", i) -> hex
So, ideally, BigInt would do the same:
to!string(bi) -> decimal
writeln(bi) -> decimal
writefln("%s", bi) -> decimal
writefln("%d", bi) -> decimal
writefln("%f", bi) -> hex
But you only want this?:
writefln("%d", bi) -> decimal
writefln("%f", bi) -> hex
Okay. I can see an argument for a better handling of toString() in general (I'd never even heard anyone complain about string toString() or discuss the possibility of a void toString() before the discussion on this bug report) and that to!string(), writeln(), writefln(), etc. need to be fixed to support a void toString() instead of string toString(), but I don't understand why you would refuse to allow for BigInt to be convertible to a string or printed as one without a format string. I don't see why it shouldn't act the same way that all of the built-in integral values do in this regard.
Comment #13 by issues.dlang — 2010-11-18T12:33:01Z
Whoops. I meant %x, not %f for hex. Sorry about that.
Comment #14 by clugdbug — 2010-11-18T13:39:08Z
(In reply to comment #12)
> So, you don't want writeln(bi); or to!string(bi) to work?
No, that's not what I meant.
> I would think that
> ideally, BigInt would work exactly the same way that integers do.
>
> to!string(i) -> decimal
> writeln(i) -> decimal
> writefln("%s", i) -> decimal
> writefln("%d", i) -> decimal
> writefln("%f", i) -> hex
>
>
> So, ideally, BigInt would do the same:
>
> to!string(bi) -> decimal
> writeln(bi) -> decimal
> writefln("%s", bi) -> decimal
> writefln("%d", bi) -> decimal
> writefln("%f", bi) -> hex
Yes. The thing I do *not* want is where it works for one case but not any of the others. I find that suggestion completely indefensible.
BTW, the void toString() was just a typo by me. I meant string toString().
Comment #15 by issues.dlang — 2010-11-18T14:32:17Z
Well, then I think that we agree on the desired eventual functionality. But I do think that in the interim, it would be a good idea to add string toString() to BigInt with the idea that it would be removed as soon as the fancier toString() stuff is sorted out. That way, at least _some_ of the desired behavior would work rather than forcing you to call the current toString() with a delegate just to get a string representation for a BigInt.
Comment #16 by clugdbug — 2010-11-19T01:27:43Z
This is a bug in format. Even the partial fix I made for bug 5237, makes BigInt print correctly, without any change to the BigInt code.
*** This issue has been marked as a duplicate of issue 5237 ***
Comment #17 by bearophile_hugs — 2010-11-19T07:14:29Z
(In reply to comment #16)
> This is a bug in format. Even the partial fix I made for bug 5237, makes BigInt
> print correctly, without any change to the BigInt code.
>
> *** This issue has been marked as a duplicate of issue 5237 ***
See also:
http://d.puremagic.com/issues/show_bug.cgi?id=4122