Re: #ifdef vs #if for feature checks

2013-03-20 Thread Lubos Lunak 
On Tuesday 19 of March 2013, Thomas Arnhold wrote:
> On 19.03.2013 00:02, Norbert Thiebaud wrote:
> > otoh, #pragma once is supported by all the compiler we use isn't it ?
> > so if we are going to change it, why not use that instead.
>
> There are some discussions about that on the Internet. Most interesting:
> Some kind of benchmark comparison at
>
> http://tinodidriksen.com/2011/08/31/cpp-include-speed/
>
> Looks like header guards as we have them are the best solution on gcc,
> but the worst for MSVC and no combination would be acceptable compared
> to 'plain' header guard with gcc.

 That's so suspicious just from considering the idea that there could possibly 
be any noticeable difference. I can't quite imagine how broken the 
implementation of #pragma once would have to be to be slower than include 
guards (and presumably they're both implemented the same way). I tried with 
1 include files with GCC and the difference, if any, is just lost in the 
noise.

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

Re: #ifdef vs #if for feature checks [proof of concept]

2013-03-20 Thread Thomas Arnhold 

Hi,

https://gerrit.libreoffice.org/#/c/2875/

At least on Linux with

--without-java --enable-werror

works for me.

I only replaced guards like *_HXX_, *_HXX__ and *_HXX_INCLUDED which are 
not used at any other place. Those are ~8700 matches. Approx. 500 look 
like guards, but don't match the scheme and ~400 hxx headers don't have 
guards at all. So the replace of the 8700 matches should be a good estimate.


The script is some kind of weirdo, but it should demonstrate the 
difference between guard and pragma. I inserted #if 1 to ignore the 
#endif at the end of the file.


Files which RSC touches cannot be converted, because he complains about 
#pragma once. If you get RSC errors then some hrc or src includes a hxx 
with pragma once.


So the door is open for a comparison.

Happy benchmarking.

Thomas

rm guards-all.log
rm guards-all-count.log

# get a list of all guards from all hxx files (~8700)
# ZIPFILE_HXX
# ZIPFILE_HXX_
# ZIPFILE_HXX_INCLUDED
git grep -h '^\s*#\s*ifndef\s*.*_HXX\(_\|__\|_INCLUDED\)\?$' -- '*.hxx' | sed 
's/^\s*#\s*ifndef\s\+//g' | sort -u > guards-all.log

# those must not be in the list
# INCLUDED_COMPHELPER_IMPLBASE_VAR_HXX*

# omitted: those don't match to _HXX scheme (~500)
#git grep '^\s*#\s*ifndef\s*.*' -- '*.hxx' | grep -v '_HXX'

# omit those which are used in other files, too
# for this we count them (takes some time)


for i in `cat guards-all.log`; do
git grep -w $i > guards.tmp; # safe to save greps

# there has to be at least one define and one ifndef otherwise it's no 
guard
if [ `cat guards.tmp | grep "#\s*ifndef\s\+$i" | wc -l` == 1 ] && [ 
`cat guards.tmp | grep "#\s*define\s\+$i" | wc -l` == 1 ]; then
countfiles=`cat guards.tmp | cut -d':' -f1 | sort -u | wc -l`
countmatches=`cat guards.tmp | cut -d':' -f2- |
grep -v "#\s*ifndef\s\+$i" |
grep -v "#\s*define\s\+$i" |
grep -v '#\s*endif' |
grep -v "//\s*$i" |
wc -l`

### log it
# row 1: matches n files (should be one)
# row 2: k matches expect those guard lines (should be zero)
echo $countfiles $countmatches $i >> guards-all-count.log;

### substitute

# dirty fix: delete include guard and use #pragma once
# at the moment don't care about #endif -> insert #if 1

# TODO: insert pragma nicely right after the licence header
# one newline before and after, no more

# safe are those which are in one file and have no further 
matches than the defined above
if [ $countfiles == 1 ] && [ $countmatches == 0 ]; then
file=`cat guards.tmp | cut -d':' -f1 | sort -u`

echo $file; # for progress
# FIXME: doing this in one regex would be safer 
(CSV_CSV_ENV_HXX_HAD_NDEBUG)
sed -i "s/\s*\#\s*ifndef\s\+$i/#pragma once/" $file;
sed -i "s/\s*\#\s*define\s\+$i/#if 1/g" $file;
fi
fi;
done


### whitelist hrc and src included hxx files (rsc complies)

# omit guards in header files which are included in [sh]rc files
# fixed with git checking out the list of includes-hsrc.log at the moment

# get all includes from hrc and src files, only hxx files
for i in `find . -name *.hrc -or -name *.src | grep -v '/workdir/' | grep -v 
'/solver/'`; do grep '^\s*#\s*include\s*[<"]' $i | grep '\.hxx[">]'; done 
>includes-hsrc-all.log

# extract filename
cat includes-hsrc-all.log | sed 's/^\s*#\s*include\s*[<"]\([^">]*\)[">].*/\1/g' 
| sort -u > includes-hsrc.log

# reset them
for i in `cat includes-hsrc.log`; do find . -name `basename $i` | grep -v 
'/workdir/' | grep -v '/solver/'; done | xargs git checkout

# there are more
git checkout ./basic/inc/basic/sbxdef.hxx



## manual cross check
# there should be only added two lines with excatly these expressions
# git diff HEAD | grep '^+[^+]' | grep -v '^+#if 1$' | grep -v '^+#pragma once$'
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: #ifdef vs #if for feature checks

2013-03-20 Thread Michael Stahl 
On 20/03/13 12:41, Noel Grandin wrote:
> On 2013-03-20 12:16, Thomas Arnhold wrote:
>> Yes, but our internal RSC doesn't support #pragma once ;)
> 
> Maybe we could make that a "Medium Hack" project?

i've already filed https://bugs.freedesktop.org/show_bug.cgi?id=55324
about the CPP proliferation problem (of course rsc has its own
preprocessor implementation).

but iirc when all dialogs are converted to UI files the remaining uses
of rsc could be replaced with something more standard like gettext.


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

Re: #ifdef vs #if for feature checks

2013-03-20 Thread Noel Grandin 

On 2013-03-20 12:16, Thomas Arnhold wrote:

Yes, but our internal RSC doesn't support #pragma once ;)


Maybe we could make that a "Medium Hack" project?

Disclaimer: http://www.peralex.com/disclaimer.html


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

Re: #ifdef vs #if for feature checks

2013-03-20 Thread Thomas Arnhold 

Yes, but our internal RSC doesn't support #pragma once ;)

On 19.03.2013 07:53, vincent wrote:

HI,
I found some wiki about "#pragma once" compiler support
http://en.wikipedia.org/wiki/Pragma_once#Portability
http://en.wikipedia.org/wiki/Pragma_once#Portability
Compiler#pragma once
Clang Supported^[7]

Comeau C/C++ 
Supported^[8] 
Digital Mars C++

Supported^[9] 
GCC 
Supported^[10] 
Intel C++ Compiler
 Supported^[11]

Microsoft Visual C++
   Supported^[12]

C++Builder XE3

Supported^[13] 

http://stackoverflow.com/questions/10990488/how-to-include-header-files-more-clearly-in-c/10990521#10990521



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

Re: #ifdef vs #if for feature checks

2013-03-19 Thread Norbert Thiebaud 
On Tue, Mar 19, 2013 at 12:16 PM, Jan Holesovsky  wrote:
>
> That's a good point; so I think the person doing the script should check
> the build time of the entire LO with #pragma before pushing, but
> otherwise I wouldn't block the change just based on the benchmark - the
> benchmark might, or might not be relevant to our scenario :-)

actually the numbers seems to indicate a faster windows for a slower
gcc (and clang unchanged)
considering that windows perf is a bottleneck whereas linux is not..
it seems like a good trade-off
So no the performance impact should not outweigh the benefit here..

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

Re: #ifdef vs #if for feature checks

2013-03-19 Thread Jan Holesovsky 
Hi Thomas,

Thomas Arnhold píše v Út 19. 03. 2013 v 05:55 +0100:

> There are some discussions about that on the Internet. Most interesting: 
> Some kind of benchmark comparison at
> 
> http://tinodidriksen.com/2011/08/31/cpp-include-speed/
> 
> Looks like header guards as we have them are the best solution on gcc, 
> but the worst for MSVC and no combination would be acceptable compared 
> to 'plain' header guard with gcc.

That's a good point; so I think the person doing the script should check
the build time of the entire LO with #pragma before pushing, but
otherwise I wouldn't block the change just based on the benchmark - the
benchmark might, or might not be relevant to our scenario :-)

All the best,
Kendy

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

Re: #ifdef vs #if for feature checks

2013-03-19 Thread Norbert Thiebaud 
On Tue, Mar 19, 2013 at 10:25 AM, Christian Lohmaier
 wrote:
> Hi Tor, *,
>
> On Tue, Mar 19, 2013 at 6:51 AM, Tor Lillqvist  wrote:
>>> otoh, #pragma once is supported by all the compiler we use isn't it ?
>>
>> Excellent! Let's do that, unless somebody comes up with a
>> counter-argument in a week or so.
>
> This doesn't help when the same file is used from two different
> locations, let's say from solver and the module (should not happen,
> but...)

If that was possible, wouldn't that be a bug to be fixed ? (and it is
becoming moot as we move away from delivering headers)

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

Re: #ifdef vs #if for feature checks

2013-03-19 Thread Michael Stahl 
On 19/03/13 16:25, Christian Lohmaier wrote:
> Hi Tor, *,
> 
> On Tue, Mar 19, 2013 at 6:51 AM, Tor Lillqvist  wrote:
>>> otoh, #pragma once is supported by all the compiler we use isn't it ?
>>
>> Excellent! Let's do that, unless somebody comes up with a
>> counter-argument in a week or so.
> 
> This doesn't help when the same file is used from two different
> locations, let's say from solver and the module (should not happen,
> but...)

we want to kill the "delivering" of headers anyway, so (with possible
exception of URE headers) this problem will go away if it even exists now.


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

Re: #ifdef vs #if for feature checks

2013-03-19 Thread Christian Lohmaier 
Hi Tor, *,

On Tue, Mar 19, 2013 at 6:51 AM, Tor Lillqvist  wrote:
>> otoh, #pragma once is supported by all the compiler we use isn't it ?
>
> Excellent! Let's do that, unless somebody comes up with a
> counter-argument in a week or so.

This doesn't help when the same file is used from two different
locations, let's say from solver and the module (should not happen,
but...)

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

Re: Re: #ifdef vs #if for feature checks

2013-03-19 Thread vincent 
HI,

I found some wiki about "#pragma once" compiler support
http://en.wikipedia.org/wiki/Pragma_once#Portability 

http://en.wikipedia.org/wiki/Pragma_once#Portability 
Compiler#pragma once
ClangSupported[7]
Comeau C/C++Supported[8]
Digital Mars C++Supported[9]
GCCSupported[10]
Intel C++ CompilerSupported[11]
Microsoft Visual C++Supported[12]
C++Builder XE3Supported[13]


http://stackoverflow.com/questions/10990488/how-to-include-header-files-more-clearly-in-c/10990521#10990521
 

Thanks,





vincent

From: Tor Lillqvist
Date: 2013-03-19 13:51
To: Norbert Thiebaud
CC: libreoffice; Lubos Lunak
Subject: Re: #ifdef vs #if for feature checks
> otoh, #pragma once is supported by all the compiler we use isn't it ?

Excellent! Let's do that, unless somebody comes up with a
counter-argument in a week or so.

--tml
___
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: #ifdef vs #if for feature checks

2013-03-19 Thread Thomas Arnhold 

On 19.03.2013 00:02, Norbert Thiebaud wrote:

On Mon, Mar 18, 2013 at 11:17 AM, Lubos Lunak  wrote:

On Monday 18 of March 2013, Tor Lillqvist wrote:

Sounds great to me too; though of course I'd prefer to keep the diff
smaller and not replace all the header guards:

#ifndef INCLUDED_OOXML_FAST_CONTEXT_HANDLER_HXX
#define INCLUDED_OOXML_FAST_CONTEXT_HANDLER_HXX

...

Oh the other hand, it would be nice if the header guards were
consistently named... and didn't start with an underscore (such
identifiers are supposed to be reserved for the language and/or OS
implementation, unless I am mistaken). Somebody just needs to come up
with a consistent naming style and do it (with some nice script). (The
style of above example would be fine with me.)


  Yes, a script should be enough for this. And yes, "somebody" just needs to do
it :).


otoh, #pragma once is supported by all the compiler we use isn't it ?
so if we are going to change it, why not use that instead.


There are some discussions about that on the Internet. Most interesting: 
Some kind of benchmark comparison at


http://tinodidriksen.com/2011/08/31/cpp-include-speed/

Looks like header guards as we have them are the best solution on gcc, 
but the worst for MSVC and no combination would be acceptable compared 
to 'plain' header guard with gcc.


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

Re: #ifdef vs #if for feature checks

2013-03-18 Thread Tor Lillqvist 
> otoh, #pragma once is supported by all the compiler we use isn't it ?

Excellent! Let's do that, unless somebody comes up with a
counter-argument in a week or so.

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

Re: #ifdef vs #if for feature checks

2013-03-18 Thread Norbert Thiebaud 
On Mon, Mar 18, 2013 at 11:17 AM, Lubos Lunak  wrote:
> On Monday 18 of March 2013, Tor Lillqvist wrote:
>> > Sounds great to me too; though of course I'd prefer to keep the diff
>> > smaller and not replace all the header guards:
>> >
>> > #ifndef INCLUDED_OOXML_FAST_CONTEXT_HANDLER_HXX
>> > #define INCLUDED_OOXML_FAST_CONTEXT_HANDLER_HXX
...
>> Oh the other hand, it would be nice if the header guards were
>> consistently named... and didn't start with an underscore (such
>> identifiers are supposed to be reserved for the language and/or OS
>> implementation, unless I am mistaken). Somebody just needs to come up
>> with a consistent naming style and do it (with some nice script). (The
>> style of above example would be fine with me.)
>
>  Yes, a script should be enough for this. And yes, "somebody" just needs to do
> it :).

otoh, #pragma once is supported by all the compiler we use isn't it ?
so if we are going to change it, why not use that instead.

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

Re: #ifdef vs #if for feature checks

2013-03-18 Thread Lubos Lunak 
On Monday 18 of March 2013, Tor Lillqvist wrote:
> > Sounds great to me too; though of course I'd prefer to keep the diff
> > smaller and not replace all the header guards:
> >
> > #ifndef INCLUDED_OOXML_FAST_CONTEXT_HANDLER_HXX
> > #define INCLUDED_OOXML_FAST_CONTEXT_HANDLER_HXX

 This is not about include guards at all (there's nothing to set the value to 
0 for starters). This is only about #ifdef checks for values from 
config_xxx.hxx (and those that should eventually be converted to such ones).

 I have pushed commits adding -Wundef in order to detect such problems, and 
fixed various problems found by it.

 All the feature macros used in config_xxx.hxx headers now should be 
converted. I think it should be possible to do a compiler plugin to catch 
incorrect #ifdef usage later too.

> Oh the other hand, it would be nice if the header guards were
> consistently named... and didn't start with an underscore (such
> identifiers are supposed to be reserved for the language and/or OS
> implementation, unless I am mistaken). Somebody just needs to come up
> with a consistent naming style and do it (with some nice script). (The
> style of above example would be fine with me.)

 Yes, a script should be enough for this. And yes, "somebody" just needs to do 
it :). 

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

Re: #ifdef vs #if for feature checks

2013-03-18 Thread Norbert Thiebaud 
On Mon, Mar 18, 2013 at 4:42 AM, Michael Meeks  wrote:
>
> On Sat, 2013-03-16 at 21:18 -0500, Norbert Thiebaud wrote:
>> On Fri, Mar 15, 2013 at 10:58 AM, Lubos Lunak  wrote:
>> >  I'd like to propose that we convert our
>> >
>> > #ifdef HAVE_FOO
>> > to
>> > #if HAVE_FOO
>>
>>  +1
>
> Sounds great to me too; though of course I'd prefer to keep the diff
> smaller and not replace all the header guards:
>
> #ifndef INCLUDED_OOXML_FAST_CONTEXT_HANDLER_HXX
> #define INCLUDED_OOXML_FAST_CONTEXT_HANDLER_HXX

oh!, ny +1 was for HAVE_* and the like, not header guard...
iow for pre-compiler variable used to 'configure' things... not
mechanical ones like header guard.

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

Re: #ifdef vs #if for feature checks

2013-03-18 Thread Tor Lillqvist 
> Sounds great to me too; though of course I'd prefer to keep the diff
> smaller and not replace all the header guards:
>
> #ifndef INCLUDED_OOXML_FAST_CONTEXT_HANDLER_HXX
> #define INCLUDED_OOXML_FAST_CONTEXT_HANDLER_HXX

Oh the other hand, it would be nice if the header guards were
consistently named... and didn't start with an underscore (such
identifiers are supposed to be reserved for the language and/or OS
implementation, unless I am mistaken). Somebody just needs to come up
with a consistent naming style and do it (with some nice script). (The
style of above example would be fine with me.)

Currently many of the header guards have names that indicate an
obsolete location of the header file, like _SV_SALGDI_HXX even though
the file in question is not in any "sv" folder but in vcl/inc.

Yeah, I guess such cosmetic changes can be seen as totally pointless
and a waste of time... on the other hand, if a hundred developers all
get annoyed for a few seconds once a week when they see some such
"only cosmetic" inconsistency, it all adds up, too;) Style *is*
important to make the code nicer to look at.

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

Re: #ifdef vs #if for feature checks

2013-03-18 Thread Michael Meeks 

On Sat, 2013-03-16 at 21:18 -0500, Norbert Thiebaud wrote:
> On Fri, Mar 15, 2013 at 10:58 AM, Lubos Lunak  wrote:
> >  I'd like to propose that we convert our
> >
> > #ifdef HAVE_FOO
> > to
> > #if HAVE_FOO
>
>  +1

Sounds great to me too; though of course I'd prefer to keep the diff
smaller and not replace all the header guards:

#ifndef INCLUDED_OOXML_FAST_CONTEXT_HANDLER_HXX
#define INCLUDED_OOXML_FAST_CONTEXT_HANDLER_HXX

Which I suspect don't suffer this problem so badly. Given the
reasonably small number of #ifdef conditionals, I guess it'd be fine to
replace them before we've got 4.0.2 out in the wild too :-)

Thanks !

Michael.

-- 
michael.me...@suse.com  <><, Pseudo Engineer, itinerant idiot

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

Re: #ifdef vs #if for feature checks

2013-03-16 Thread Norbert Thiebaud 
On Fri, Mar 15, 2013 at 10:58 AM, Lubos Lunak  wrote:
>
>  Hello,
>
>  I'd like to propose that we convert our
>
> #ifdef HAVE_FOO
>
> to
>
> #if HAVE_FOO
>

 +1

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

Re: #ifdef vs #if for feature checks

2013-03-16 Thread Noel Grandin 
On Fri, Mar 15, 2013 at 5:58 PM, Lubos Lunak  wrote:

>
>  The problem with this change is that there still may be some cases where
> #ifdef is used, either because some system #defines work that way (so those
> would be valid but different), or people would still use #ifdef out of
> habbit
>

I like the general idea.
Perhaps we could build a small whitelist of the values for which #ifdef is
allowed?
There can't be very many of them.
Then the mechanical checker can become part of the toolkit we use to keep
the codebase sane.
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice