Date: prev next · Thread: first prev next last
2012 Archives by date, by thread · List index


On Mon, Sep 17, 2012 at 3:27 AM, Stephan Bergmann <sbergman@redhat.com> wrote:

The point of including sal/config.h first is to make sure there is a place
for code that needs to be seen very early in every compilation unit.  (A
good example is the "#ifdef sun" stuff at the end of sal/config.h that was
vital for compiling with Sun Studio.  Granted, for historical reasons there
is code in sal/config.h that would arguably better go someplace else.)

sal/types.h just happens to be a header that is needed in many compilation
units.

any compilation unit that use any scalar type sal_* woudl need it...
iow all of them practically...

cppuhelper/source/findofficepath.c
tools/source/misc/pathutils.cxx
extensions/source/nsplugin/source/so_env.cxx

are the only source I could find that include config.h and not
types.h... and it is not clear at all that it was deliberate.
which impacted 2 static library: libnpsoenv.a and libooopathutils.a

ucbhelper/source/client/contenbrooker.cxx was a false postive as the
content of if is excluded on non ANDROID
vcl/source/app/dbggui.cxx showed-up was a false positive since I did
not build in DBG_UTIL mode... ortherwise sal/types.h would have been
included



Yeah, the difference between theory and practice.  ;)  (Still, for one,
config.h probably /does/ effectively get included first most of the time,
thanks to the various headers that include it first thing,

Yep, actually most of sal's include get included indirectly via
sometime many layer of includes... but sure... always always I agree
config.h end-up being the first actual content parsed


and for another,
I think there currently is nothing in config.h that makes it extremely
problematic if it happens to not be included first thing into some
compilation unit

That is a flaw, when you want a heder to be first, you got to make
sure that you can't forget about it :-)


, so enforcing it by modifying hundreds of files probably is
not worth it right now.)

Well, it is like saying... we have thousand of warnings... so
contemplating -WError is not worth it...
that is a self-fullfilling prophecy...



I could understand the desire for some lo.h from the point of view of
performance (though the relevant sub-headers will probably manage to stay in
main memory anyway) or ease-of-maintenance, but what do you mean with
"flexibility and certainty"?

having a header that is  guaranteed to be included in every source
first (I mean source not header)

give the flexibility of doing fairly fancy stuff to allow for API
transitions, manage compatibility issue (like faking some api
transparently to the rest of the code etc...)
the 'certainty' comes from the property of the mandatory nature of the
header... and that is ensure by making sure that no source is going to
build without it...
for instance that can be achieved by banning any #include
<sal/types.h> or #<rtl/ustring.hxx> for isntance anywhere but in that
header...
that can be easily checked by script.
the absence of these header means that no non-trivail souce is goign
to compile if it forget to include "lo.h"
we can also script a verification that header do not included it and
that source do include it as a first include...

you are right... but that odd behavior of assert.h is actually a
reason to hunt down random #include of assert.h, especially in
headers, and centralize its use.


Why?  The includes of assert.h in headers pertain to uses of assert in
(inline) functions defined in those headers.  I see no need to complicate
things by building a protocol on top of plain Standard C assert.h here.

Just because the 'Standard' allow you to shoot yourself in the foot
does not mean it is a good idea.



for instance
sal/inc/osl/thread.hxx:#include <cassert>
sal/inc/rtl/strbuf.hxx:#include <cassert>
sal/inc/rtl/string.hxx:#include <cassert>
sal/inc/rtl/ustrbuf.hxx:#include <cassert>
sal/inc/rtl/ustring.hxx:#include <cassert>
sal/textenc/unichars.hxx:#include <cassert>

and, see above, these guys are included _a lot_ so assert.h get
defined/un-defined/redefined a lot too (just with the 6 include above
we are talking about 85K times. or somewhere liek 4 times per
source...
by centralyzing it at some choke point (sal/config.h if you want) and
hunting down random include of assert.h we actually improve things...


...measurably?

yes measurably 0.076875 milli-seconds per #include <assert.h> on my linux box.

or to put in another way: 0.15% of the total build time  (6 second
over 4 cpu, out of 17 minutes elapsed)

****
n_th@tpa10 ~ ()$ cat test_assert.c

#include <assert.h>

#ifdef MANY_ASSERT
#include "many_assert.h" // 80K include assert.h
#endif

int main(int argc, char* argv)
{
    assert(argc > 2);
    return argc;
}



n_th@tpa10 ~ ()$ time gcc test_assert.c

real    0m0.030s
user    0m0.016s
sys     0m0.013s
n_th@tpa10 ~ ()$ time gcc test_assert.c

real    0m0.030s
user    0m0.015s
sys     0m0.014s
n_th@tpa10 ~ ()$ time gcc test_assert.c

real    0m0.030s
user    0m0.021s
sys     0m0.008s
n_th@tpa10 ~ ()$ time gcc -DMANY_ASSERT test_assert.c

real    0m6.190s
user    0m5.612s
sys     0m0.574s
n_th@tpa10 ~ ()$ time gcc -DMANY_ASSERT test_assert.c

real    0m6.204s
user    0m5.612s
sys     0m0.588s
n_th@tpa10 ~ ()$ time gcc -DMANY_ASSERT test_assert.c

real    0m6.139s
user    0m5.562s
sys     0m0.572s
n_th@tpa10 ~ ()$


Norbert

Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.