Re: -Wsign-promo (Re: [BUILD FAILS DEBUG MODE] with test_strings_valuex.cxx)

On 01/19/2013 11:28 AM, Lubos Lunak wrote:
> On Friday 18 of January 2013, julien2412 wrote:
> > Hello,
> >
> > On pc Debian x86-64 with master sources after having runned "make clean",
> > I've got this:
> > /home/julien/compile-libreoffice/libo/sal/qa/rtl/strings/test_strings_value
> > x.cxx: In instantiation of ‘void testInt() [with T = rtl::OUString]’:
> > /home/julien/compile-libreoffice/libo/sal/qa/rtl/strings/test_strings_value
> > x.cxx:77:28: required from here
> > /home/julien/compile-libreoffice/libo/sal/qa/rtl/strings/test_strings_value
> > x.cxx:48:5: error: passing ‘unsigned char’ chooses ‘int’ over ‘unsigned int’
> > [-Werror=sign-promo]
>
>   -Wsign-promo is a rather pointless warning these days (the section in the gcc
> manpage is a funny read and not only because it talks about Cfront). I've
> added more overloads to silence it, but I rather wonder why we have this
> explicitly enabled at all.

I removed the explicitly enabled -Wsign-promo with 
<http://cgit.freedesktop.org/libreoffice/core/commit/?id=488823a140217e393298bc83e75084041a85ed45> 
"Remove -Wsign-promo," mainly as a quick way to silence the warnings 
that started to pop up after 
<http://cgit.freedesktop.org/libreoffice/core/commit/?id=63bcb139b941a2eff1b5ad367046bca067e7d1f8> 
"Replaced O[U]String::valueOf( static_cast<> ) with O[U]String::number()."

That also allowed to revert 
<http://cgit.freedesktop.org/libreoffice/core/commit/?id=1efe9a15f86a7a9dc08b57fd1dd12336522ba515> 
"avoid -Wsign-promo warnings" again with 
<http://cgit.freedesktop.org/libreoffice/core/commit/?id=2ae6e77881028a287610f13d10f4c37242ff464b> 
'Revert "avoid -Wsign-promo warnings.'"

The explicit -Wsign-promo had originally been added with 
<http://cgit.freedesktop.org/libreoffice/core/commit/?id=e8bbb76827dd7a0e30d7d1db34a812a84d85f390> 
"ensure correct export size type in stream operation," apparently to 
detect an SvStream::operator<< call with a bool argument that would pick 
the "int" overload instead of the "unsigned char" (aka sal_Bool) one 
(which would have happened to trigger the -Wsign-promo warning "by 
luck," given that sal_Bool is unsigned char and not signed char, say). 
I assume that <https://gerrit.libreoffice.org/#/c/1798/> "Detect 
SvStream::operator<< calls with bool args w/o using -Wsign-promo" would 
be a better fix for that (at least, a local "make check" kept succeeding 
with that patch applied here).

Stephan

> - but it incidentally triggers when passing bool to SvStream, because it
> doesn't have any overload for operator<<(bool), and int is chosen over
> unsigned char AKA sal_Bool , so Caolan added it in
> e8bbb76827dd7a0e30d7d1db34a812a84d85f390
> - if SvStream gets overload for bool, the warning can be dumped

Ach, I failed to read Lubos' mail carefully until I now quote it for my 
reply, and all the stuff I write above has already been said...