printf %g sometimes unnecessarily adds trailing space

[garrison]

The printf %g specifier will at times output an additional, unnecessary trailing space:

julia> @sprintf "%g" 1
"1 "
julia> @sprintf "%.8g" 23
"23 "
julia> @sprintf "%.2g" 23
"23"

CC @dfannius (#11941)

[dfannius]

Thanks for the report; I'll check it out this weekend if no one has already.

[mason-bially]

If it's alright, I'm working on code coverage for printf.jl and have gotten pretty familiar with the printf code lately, I could take it. I actually noticed this bug as well and was going to check if it was supposed to do that the next time I worked on it.

[StefanKarpinski]

Thanks both for testing this and working on it.

[dfannius]

@mason-bially Go for it! Be warned, the %g code is kinda hairy (largely because %e and %f try to handle everything at macro-expansion time and %g can't).

[mason-bially]

So I found the problem, but... g% has some other weirdness to it (tested in 0.4 as well).

julia> @sprintf("%#.0g", 42)
"4.200000e+01"
c-> "4.e+01"

julia> @sprintf("%.0g", 42)
"4.2e+01"
c-> "4e+01"

julia> @sprintf("%.0g", 42.3)
"4.23e+01"
c-> "4e+01"

Also, ok, can we talk about this module?

• First, what is DIGITS, why is it a constant global in another module _that we are writing too_ from expanded macros. And why are we using it that way? If we are going to do that we really need like threadlocal bindings so we can start fixing things like this before they become problems in future threading code.
• Second, I really don't see how this is supposed to be faster. Edit: I had a long rant here, but basically, is there any documentation on how this optimization strategy is supposed to work out? Because I think dumping an Array, of known max size, from memory using type hints to the output stream is probably faster than using IOBuffer the way we are. But on the other hand I'm still new to optimization in Julia so I might be missing some magic.

[dfannius]

1. Issues are cheap; can you make a new one with the three new bugs (one of them is included twice) and point it at this conversation for reference? Then we can mark this resolved once Fixing original issue #14331 by cleaning up print_fixed_point #14348 is accepted and it won't be in some half-open state. I think putting all three bugs in one issue is fine; at least two of them and maybe three look like the same thing. I can try to take a look at them this weekend unless you grab them first again.

2. There should probably be yet another issue called "printf.jl is a mess". Obviously it was written long ago and some of the design decisions may not be the same ones that would be made today, either because some of the tradeoffs have changed or simply because people's minds have. I do agree that the whole global DIGITS thing seems problematic in the face of threads.

[mason-bially]

Sure I can do that. I think they are all the same bug, I'll even fix them (I'm still in the middle of improving code coverage for this module.

But I'll also create a new topic for the printf.jl thing.

I do agree that the whole global DIGITS thing seems problematic in the face of threads.

I would say in general it seems problematic but that's me.

[StefanKarpinski]

The global array thing is definitely not workable with threads impending (was not a concern in Jan 2012) and moreover is just kind of ugly, but at the time I wrote that code was the only way I could get adequate performance. Printf definitely needs an overhaul as I've described elsewhere. If someone feels like taking a crack at it, that would be great. High-level advice:

• All of the time in formatted integer printing is integer decoding or output system calls. Therefore most of the compile-time code generation in our printf is fairly pointless.
• A design where each printf call is reduced to a series of static prints and calls to per-format-specifier functions that take arguments describing how to format would probably work and be just as fast.

[mason-bially]

I'll have the increased code coverage for the module this weekend, maybe add a couple perf tests. And I'd be willing to rewrite this module.

However my goal in doing so is so that it can be removed from Base and stuffed in some sort "compatibility for c" utility library in the future. Currently that's impossible because of it's dependencies. And failing that, at least it won't be a pain to look at.

[garrison]

The examples I gave above have all been fixed on 0.5 (most likely due to ddd0f05).