Re: sal/config.g sal/types.h and sal/osl/rtl includes in general

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