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

2013-01-22 Thread Caolán McNamara
On Sat, 2013-01-19 at 11:28 +0100, Lubos Lunak wrote:
 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
 
  Or am I missing something there?

Yeah, IIRC I just wanted to see if there were more cases of bools being
streamed out. Should have tweaked the stream class instead.

C.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice



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

2013-01-21 Thread Stephan Bergmann

On 01/19/2013 03:12 PM, Michael Stahl wrote:

On 19/01/13 11:28, Lubos Lunak wrote:

  -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


Some of the funniness is likely due to 
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56067 Removal of -Wsynth 
from doc didn't remove example.



added more overloads to silence it, but I rather wonder why we have this
explicitly enabled at all.

  My hypothesis is like this:
- the idea behind the warning is just nonsense (who cares to what integer type
the value is promoted)


overflow for unsigned integral types is defined by the C and C++
standards, while overflow for signed types is explicitly undefined.
some implementations therefore assume that it does not occur and may
remove tests on signed integers from the code that could only evaluate
to true in case of an overflow in order to improve the all-important
SPEC benchmark scores.

-Wsign-promo is specifically about overload resolution, i'm not sure if
that overflow problem would be relevant in this case but this is C++ so
i'm never sure about anything :)


No, undefined behavior due to signed overflow is not relevant here.  For 
the integral promotion involved in overload resolution, no overflow can 
happen.  (And the overflow that can happen with integral conversion, say 
a truncation from long to int, is implementation-defined rather than 
undefined, btw.)


Stephan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


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

2013-01-21 Thread Stephan Bergmann

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

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


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

2013-01-19 Thread Lubos Lunak
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.

 My hypothesis is like this:
- the idea behind the warning is just nonsense (who cares to what integer type 
the value is promoted)
- 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

 Or am I missing something there?

-- 
 Lubos Lunak
 l.lu...@suse.cz
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


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

2013-01-19 Thread Michael Stahl
On 19/01/13 11:28, Lubos Lunak wrote:
  -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.
 
  My hypothesis is like this:
 - the idea behind the warning is just nonsense (who cares to what integer 
 type 
 the value is promoted)

overflow for unsigned integral types is defined by the C and C++
standards, while overflow for signed types is explicitly undefined.
some implementations therefore assume that it does not occur and may
remove tests on signed integers from the code that could only evaluate
to true in case of an overflow in order to improve the all-important
SPEC benchmark scores.

-Wsign-promo is specifically about overload resolution, i'm not sure if
that overflow problem would be relevant in this case but this is C++ so
i'm never sure about anything :)

 - 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
 
  Or am I missing something there?

maybe there are more overloaded identifiers that simply haven't been
called with wrongly promoted parameters yet?


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice