tfm::format("%+d", 0) gives the wrong result on OSX

[c42f]

tfm::format("%+d", 0) gives the wrong result on OSX

[c42f]

Hmm, this is a known bug from 2007. Too bad it isn't fixed in whatever version of libstdc++ is used on OSX

http://gcc.gnu.org/ml/libstdc++/2007-11/msg00075.html
http://openbsd.7691.n7.nabble.com/Patch-to-fix-libstdc-std-showpos-td162922.html

[c42f]

Yup, this was fixed by the gcc guys all the way back in 2007:

http://gcc.gnu.org/viewcvs/trunk/libstdc%2B%2B-v3/include/bits/locale_facets.tcc?r1=130233&r2=130460&diff_format=h

[c42f]

Bah, the above link is out of date now, here's an updated one:

http://gcc.gnu.org/viewcvs/gcc/trunk/libstdc%2B%2B-v3/include/bits/locale_facets.tcc?r1=130459&r2=130460&diff_format=h

Which links to a diff as follows

--- trunk/libstdc++-v3/include/bits/locale_facets.tcc   2007/11/27 01:03:54 130459
+++ trunk/libstdc++-v3/include/bits/locale_facets.tcc   2007/11/27 01:59:41 130460
@@ -874,13 +874,13 @@
        if (__builtin_expect(__dec, true))
        {
            // Decimal.
-       if (__v > 0)
+       if (__v >= 0)
            {
                if (bool(__flags & ios_base::showpos)
                    && __gnu_cxx::__numeric_traits<_ValueT>::__is_signed)
                *--__cs = __lit[__num_base::_S_oplus], ++__len;
            }
-       else if (__v)
+       else
            *--__cs = __lit[__num_base::_S_ominus], ++__len;
        }
        else if (bool(__flags & ios_base::showbase) && __v)

[c42f]

Ok, this should be fixed with the commit above, but I'll need someone with an older OSX machine to say for sure. @lgritz would you be willing to give this a go? Just run a tinyformat "make test" and all tests should pass... I hope.

[lgritz]

Sure, I'll give it a try when I get a chance. May have to wait until later today or tomorrow, I've got a full day today.

[lgritz]

I hate to throw this back at you, Chris, but when I incorporated this latest tinyformat.h into OIIO, I get other errors related to how strutil.h declares 'Strutil::format' using TINYFORMAT_WRAP_FORMAT, and I'm not quite sure what to do to fix it.

[c42f]

This is probably because in OIIO we used more than 10 parameters so I had to rerun the cog code generated code which makes those horrible macros for the varags emulation.

Before we get into OIIO integration, I'd really just like to know whether the recent patch fixes the problems you were seeing on the tinyformat side. The tests in the tinyformat repo include a test for "%+d" so if you could just run those and confirm the fix then I'm happy to make a pull request to integrate the changes. (Clone the latest tinyformat and run "make test" - that should do it.)

[c42f]

Thanks for having a look by the way :)

I suppose I could up the default max number of parameters to tfm::format, since OIIO is already a case where 10 is too few.

[lgritz]

$ make
/Users/lg/bin/g++ -Wall --std=c++98 -DTINYFORMAT_NO_VARIADIC_TEMPLATES tinyformat_test.cpp -o tinyformat_test_cxx98
/Users/lg/bin/g++ -Wall --std=c++0x -DTINYFORMAT_USE_VARIADIC_TEMPLATES tinyformat_test.cpp -o tinyformat_test_cxx0x
cc1plus: error: unrecognized command line option "-std=c++0x"

But it does nonetheless make tinyformat_test_cxx98.

$ ./tinyformat_test_cxx98
(no output)

Does that mean it succeeded?

[lgritz]

I think I'm gonna try adding a few more C++98 variants and see if that fixes. Maybe you're right, and 10 is just not enough. Stay tuned.

[c42f]

Yep, that means it succeeded. The error is presumably because you're using clang and it no longer allows "c++0x" to refer to the c++11 standard, whereas the version of gcc I have still insists on it?

Sorry this is turning into a rigmarole, I thought it would be as simple as "git clone && make test"

[lgritz]

I made it work with OIIO by upping the number of supported parameters. I will send you a pull request.

[c42f]

Ok, fix confirmed, thanks Larry :)