Re: Clean up OSL_ASSERT, DBG_ASSERT, etc.

On Tue, Jan 20, 2015 at 9:10 AM, Stephan Bergmann <sbergman@redhat.com>
wrote:
> [...]
> > I know some significant work has already been done in cleaning up
> > assertions. This in no way invalidates them at all.
> > In fact, changing assert to LO_ASSERT is trivial. The effort to
> > convert the OSL_ and DBG_ ones remains the same (with or without the
> > above proposal).
> >
> > However, LO_ASSERT will support the above advantages and give full
> > control over assertions in the project. The cost is very low (surely
> > searching and replacing assert with LO_ASSERT is not a high cost). The
> > variants will allow more fine-grained control and will make the
> > behavior more homogeneous.
> >
> > I hope you agree that this is an improvement over, and not a
> > competition with, the current effort to replace OSL_ and DBG_ versions
> > with assert.
>
>
> Over the years, we've seen lots of attempts come and go to devise
> sophisticated assertion/logging/debugging mechanisms, none of which lead
to
> a clean, consistent code base.  The current approach (assert, SAL_WARN,
> SAL_INFO) is deliberately simplistic.
>
> I would not say that you are not raising relevant points, but at this
point
> I would rather like to see fdo#43157 getting fixed, and not introduce yet
> another approach into the zoo.

I hear you, and understand the general reluctance.
I've made every effort to make the code as simple as possible (please do
checkout patch 13945).
At its core, I'm suggesting to wrap around assert() with something we can
control and is more powerful.
The other two versions (one always enabled and another that is enabled only
when OSL_DEBUG_LEVEL > 1) can be postponed.

That is, I'm suggesting using the proposed LO_ASSERT instead of assert()
and _optionally_ add a custom message and/or dump variables. At the very
least it will be an in-place replacement that gives us more control than
the system default. Initially, and by default, LO_ASSERT could log and
invoke assert(). Again, this wouldn't compete or distract from fdo#43157,
it'll only make it more fruitful.

I'd like to hear comments as to how to improve the patch to hone it into a
form that would get merged eventually. I'm willing to simplify if there is
concern that it's over-engineered. However I think LO_ASSERT(index < size,
"No pigeon at (%d), only (%d) holes.", index, size); is much more powerful
than assert(index < size); Although LO_ASSERT(index < size); is also
possible and isn't any less readable.