https://bugs.freedesktop.org/show_bug.cgi?id=88309
Bug ID: 88309
Summary: Standardize, cleanup, and improve Assertions
Product: LibreOffice
Version: 4.5.0.0.alpha0+ Master
Hardware: All
OS: All
Status: UNCONFIRMED
Severity: enhancement
Priority: medium
Component: Libreoffice
Assignee: [email protected]
Reporter: [email protected]
Asserting, when used carefully, is a very useful tool to document and debug
software.
There are numerous different assertion functions/macros used in the Libreoffice
codebase, each with its own behavior and implementation. This is problematic in
its own right, as there is no obvious way to know what to expect when a certain
precondition is broken (by new code, modified code, or by a new external
input,) which is vital for the developer who plans to work on an issue.
Furthermore, reading and modifying the code is made more difficult by this
unnecessary variations.
This is a proposal to improve the situation by carefully designing an assertion
API that is standard to the project, cleaner in specs, and provides more
support to the development team than the current available breed.
A quick grep returns the following assertion functions (those containing
case-insensitive "assert"): assert, DBG_ASSERT, OSL_ASSERT, DBG_BF_ASSERT. This
is excluding CPPUNIT_ASSERT and assertions in 3rdparty libraries (which are out
of scope).
I know there has been a push to use the system assert, which at least on
Windows aborts, which has the nice side-effect that it can't be ignored.
However this behavior can, and indeed should, be defined by the
project-specific assert and furthermore be made configurable by the developer
at build time.
Names here are suggestive, not necessarily required to be verbatim (I know
naming can be contentious). However I do believe that a different name should
be used than the existing ones to help in tracking the conversion progress,
avoid collisions etc.
Four types of assertions:
1) Static assertion: Compile-time assertion that fails the build if violated,
otherwise compiles into nop in all configurations.
LO_ASSERT_STATIC(EXP);
2) Permanent assertion: Runtime assertion that is never compiled out of the
code, even in releases (see below). These are very cheap, but critical, checks.
Typically trivial bounds or invariant checks. The overhead of such an assertion
should be <5% (typically 2%) of the function they are in.
LO_ASSERT_REL(EXP);
3) Debug assertion: Runtime assertion that validates pre- and post-conditions,
including some checks that require some nominal amount of computation. These
are the typical debug assertions and should have an overhead of 5-20%. Enabled
for non-Release builds only.
LO_ASSERT(EXP);
4) Sanity/Validation assertion: Extensive runtime assertions that validate an
algorithm by executing extensive checks and recomputations. These checks have a
too high a cost to be enabled in a build used by even the wider development
community. They are best enabled when debugging a certain algorithm or tracking
down a particularly nasty issue. The overhead of these assertions are >20%.
Enabled only when LO_USE_ASSERT_CHECKS is defined.
LO_ASSERT_DEBUG(EXP);
Each type will come in a version that takes an optional second argument which
is a custom message (other versions that take variable arguments to print upon
failure may also be provided).
Failed assertions will not take any action on their own, rather, a handler
function will be called. For a start, this handler can be defined by configure,
but a better approach is to wrap it in a static library to avoid rebuilding the
complete code-base when a different strategy is necessary to track an issue. In
the latter case the static library is rebuilt when the assertion handler is
changed and only the binaries are linked.
There will be 5 standard handler provided which can be chosen at compile time.
Developers may implement their own handlers if necessary (possibly a future
extension, not immediately necessary).
1) FailLog: Prints to stderr and a log file (optional). (Ideally will include
the stack-trace as well.)
2) FailAbort: Calls FailLog then terminate (which might invoke CrashRep when
functional).
3) FailBreak: Calls FailLog then breaks into the debugger (typically by issuing
'int 3' on x86/x64). (This might not be useful on non-Windows platforms.)
4) FailSpin: Calls FailLog then spins (with short sleeps) to allow for hooking
a debugger.
5) FailThrow: Calls FailLog then throws std::logic_error. (This assumes there
are handlers, or a debugger is set to break on C++ exceptions.)
When a proper CrashRep is implemented, FailLog will dump the stack-trace as
well.
LO_ASSERT_REL would nominally FailLog and potentially FailAbort (to be
decided).
Once the support code is created, this is solidly an easy hack, one that
beginners should be encouraged to work on.
Some ideas are influenced by Bloomberg's BSL library (see
https://github.com/bloomberg/bde/blob/master/groups/bsl/bsls/bsls_assert.h).
--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Libreoffice-bugs mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
