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


On 09/16/2012 01:57 AM, Norbert Thiebaud wrote:
in commit e9689e4fdcf876e7bcaf564e060a3512e0fe9ef3

you said:

Revert "saldllapi.h is really not included outside of sal itself"
This reverts commit 2dfe34ce0efef6ec0412130a32f755657710363d:

* "not every header needs to include sal/types.h"

* sal/config.h is the header to always include first (not sal/types.h)

that is a nice theory but:

n_th@tpa10 /lo/g_core/workdir/unxlngx6.pro/Dep (master)$ for f in
$(find /lo/g_core/sal/inc -type f) ; do echo "file $f : $(grep -R
"/${f/\/lo\/g_core\/sal\/inc\//}[^a-z]" * | wc -l)" ; done
...
file /lo/g_core/sal/inc/sal/config.h : 25887
...
file /lo/g_core/sal/inc/sal/types.h : 25873
...

so somehow somewhere there are 14 object that manage to include
config.h without including types.h out of 25887...

...which does not counter the claim that there are headers that need not include sal/types.h.

So what is the point of including <sal/config.h> _first_
and then have to include <sal/types.h>  which in turn include
sal/config.h (and other generally useful bits) anyway ?

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.

furthermore:
n_th@tpa10 /lo/g_core (master)$ git grep "sal/types.h" | wc -l
1323
n_th@tpa10 /lo/g_core (master)$ git grep "sal/config.h" | wc -l
659
n_th@tpa10 /lo/g_core (master)$

so not only sal/config.h must be included first is not really
enforced.. but type.h is actually much more popular...

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, 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, so enforcing it by modifying hundreds of files probably is not worth it right now.)

What I'm really aiming at, is to have a lo.h header (in solenv/inc for
instance) that one can/should really use in every _source_ (not
header) as a first include, to take care of most sal/osl/rtl
boilerplate includes...
[...]
having a common, mandatory header that is actually included in every
source and only in source of the product (above sal), would provide a
much better visibility over how sal-related stuff get included, and
much more flexibility and certainty when doing product wise changes...

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"?

in commit 659c6a363bf84bd2520042ba80bc507763b57b78
you said:

    For one, assert.h is designed to be includeable multiple times with changing
     NDEBUG settings, so it is not robust to include it early in
sal/macros.h with
     "correct" NDEBUG settings and potentially include it again later.

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.

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?

Stephan

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.