sc/source/core/data/dociter.cxx: really suspicious use of ! in condition

2015-02-18 Thread Matteo Casalin
Hi all,
while looking at 
http://cgit.freedesktop.org/libreoffice/core/commit/?id=a5ab0e3a8b1cb7c06072229e1c4d956eb81fa002
 I noticed a really suspicious use of ! on the same line that was modified:

 ! nMinNextEnd  nRow

Based on the 'n' prefix and the related usage of ! in conditionals on integers, 
I would say that that should be !(nMinNextEnd  nRow) or nMinNextEnd = nRow

Sadly I don't have time to propose a patch in gerrit for review. Does anybody 
have a clue on what should go there, how to test the results and, if needed, 
patch the code?

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


Re: sc/source/core/data/dociter.cxx: really suspicious use of ! in condition

2015-02-18 Thread Stephan Bergmann

On 02/18/2015 11:51 AM, Matteo Casalin wrote:

 while looking at 
http://cgit.freedesktop.org/libreoffice/core/commit/?id=a5ab0e3a8b1cb7c06072229e1c4d956eb81fa002
 I noticed a really suspicious use of ! on the same line that was modified:

 ! nMinNextEnd  nRow


mistakes like this (which appears to already have been fixed) are 
automatically found by loplugin:implicitboolconversion

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


Re: sc/source/core/data/dociter.cxx: really suspicious use of ! in condition

2015-02-18 Thread Stephan Bergmann

On 02/18/2015 03:57 PM, Matteo Casalin wrote:

That code was already there - could it be that for some reason the plugin was 
used for that file but did not detect that instance?


Sorry for being cryptic.  My remark was merely meant as a shameless plug 
to advertise the usefulness of those plugins to a wider developer 
audience.  (You need to actively do something to benefit from those 
plugins, cf. 
https://wiki.documentfoundation.org/Development/Clang#Additional_compilation_warnings.)


(In this particular case, the mistake was only originally introduced a 
few hours earlier, so likely no build with plugins enabled happened to 
stumble over it before it got fixed.)

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


Re: sc/source/core/data/dociter.cxx: really suspicious use of ! in condition

2015-02-18 Thread Matteo Casalin
Hi Stephan,
thanks for looking into this.
That code was already there - could it be that for some reason the plugin was 
used for that file but did not detect that instance?

Kind regards
Matteo

On Wed, 18 Feb 2015 12:53:34 +0100
Stephan Bergmann sberg...@redhat.com wrote:

 On 02/18/2015 11:51 AM, Matteo Casalin wrote:
   while looking at 
  http://cgit.freedesktop.org/libreoffice/core/commit/?id=a5ab0e3a8b1cb7c06072229e1c4d956eb81fa002
   I noticed a really suspicious use of ! on the same line that was modified:
 
   ! nMinNextEnd  nRow
 
 mistakes like this (which appears to already have been fixed) are 
 automatically found by loplugin:implicitboolconversion
 ___
 LibreOffice mailing list
 LibreOffice@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/libreoffice
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: sc/source/core/data/dociter.cxx: really suspicious use of ! in condition

2015-02-18 Thread Németh László
Hi,

2015-02-18 17:06 GMT+01:00 Stephan Bergmann sberg...@redhat.com:

 (In this particular case, the mistake was only originally introduced a few
 hours earlier, so likely no build with plugins enabled happened to stumble
 over it before it got fixed.)

In this particular case, really a useful compiler warning/error
detected the problem (and I was very glad of it :):

Box name: iOS-ARM@29
 Branch: MASTER
 starttime: 1424212524
 Machine: Darwin sniff.local 14.1.0 Darwin Kernel Version 14.1.0: Mon
Dec 22 23:10:38 PST 2014; root:xnu-2782.10.72~2/RELEASE_X86_64 x86_64
 Configured with: --with-parallelism=1
--with-distro=LibreOfficeiOS
--enable-symbols
--enable-werror
--disable-odk
--with-external-tar=/Users/tml/lo/core/src

 Commits since the last success:

  core 
  a5ab0e3  tdf#89436 handle skipping empty rows

...

[build CXX] sc/source/core/data/dociter.cxx
/Users/tml/lo/master-ios-arm/sc/source/core/data/dociter.cxx:2324:44:
error: logical not is only applied to the left hand side of this
comparison [-Werror,-Wlogical-not-parentheses]
if ( !bRowEmpty  bRepeatedRow  ! nMinNextEnd  nRow ) //
use the data of the previous row
   ^ ~
/Users/tml/lo/master-ios-arm/sc/source/core/data/dociter.cxx:2324:44:
note: add parentheses after the '!' to evaluate the comparison first
if ( !bRowEmpty  bRepeatedRow  ! nMinNextEnd  nRow ) //
use the data of the previous row
   ^
 ( )
/Users/tml/lo/master-ios-arm/sc/source/core/data/dociter.cxx:2324:44:
note: add parentheses around left hand side expression to silence this
warning
if ( !bRowEmpty  bRepeatedRow  ! nMinNextEnd  nRow ) //
use the data of the previous row
   ^
   ()
1 error generated.
make[1]: *** 
[/Users/tml/lo/master-ios-arm/workdir/CxxObject/sc/source/core/data/dociter.o]
Error 1
make: *** [build] Error 2)

Best regards,
Laszlo
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice