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

[Norbert Thiebaud]

On Mon, Sep 17, 2012 at 11:48 AM, Stephan Bergmann <sbergman at redhat.com> wrote:
> On 09/17/2012 06:25 PM, Michael Stahl wrote:
>>
>> i don't really think the performance benefit is that important; but
>> including it in a central place would give a consistency benefit in
>> whether assert is enabled, which sounds good to me...
>
>
> ...but why try and fight the Standard here?  I see no practical benefit.
>
> We appear to have a handful of places in the code that fiddle with NDEBUG,
> for apparently dubious historical reasons.  We should clean them up.

Agrred
>
> But why should anybody coming to this project not be able to expect the
> assert mechanism to be working as advertised by the Standard?

in order to have such behavior, he would have to add another  #include
<cassert> somewhere...
This is a matter, then, of code review and checking with him why he
really need that...
as long as this is done in a source code... after the includes for
that source code, the fiddling would have limited scope (only that
particular source)
and quite frankly It is very unlikely that that would be a legitimate case.


> (After the
> above clean up, I assume there'll be no code left around that would fiddle
> with NDEBUG.  But if there were, it would likely be for some reason.)

I agree that it would be best not to fiddle with NDEBUG in the code,
and any such residual use need to be investigated to see how to do
that best...

Note, that I did change my mind and agree to use NDEBUG as a trigger
to do assert in release when we choose to... doing that doesn't rely
in when assert.h is included
but it does rely on NDEBUG not being use (abused?) for orthogonal issues...

Note: in this thread, in the course of the discussion we are
conflating different issues:

1/ how to have a way to have assert() aborting in release code
2/ the benefit of having a standard product-wise include. that is
included in every source (and never in a header)... iow a sal/config.h
but for libreoffice, not sal, and actually enforced with stricter
semantics..

for 1/
I accept your solution to do that at the gbuild level by defining or
not NDEBUG. the caveat is that place where NDEBUG is fiddled with
and/or used for other purpose than assert.h control, need to be
audited and possibly adjusted to avoid conflicts.
Note that this can only be an issue when --enable-assert-always-abort is on...

for 2/
I propose to create a file 'lo.h', in solenv/inc/ for now... and start
to bring all source code in conformance... I would have that lo.h
include sal/config.h and other sal+osl+rtl header that are extremely
commons (like ustring.hxx). and start to clean-up such include in the
rest of the code. note: my priority is to bring some order to the
includes, performance is not the main motivation).
Having such a header and its use actually enforced give a nice
platform to do pretty fancy stuff, and to avoid the all to common
re-invention of the wheel wrt to convenience macros.
Yes it is a lot of grease-monkey work... but hey I'm used to that...
and I don't mind.. it's like doing Sudoku to me :-)

Norbert