[Development] Disavowing the Lakos Rule for Q_ASSERT

[Volker Hilsheimer]

> On 27 Aug 2024, at 21:21, Thiago Macieira <thiago.macieira at intel.com> wrote:
> 
> On Tuesday 27 August 2024 10:29:05 GMT-7 Ville Voutilainen wrote:
>> On Tue, 27 Aug 2024 at 17:15, Thiago Macieira <thiago.macieira at intel.com>
> wrote:
>>> The point is that this putative mistake has no consequences, today. It
>>> remains to be seen whether it will with contracts, when those come. When
>>> contracts come, if those noexcept are a problem, then both libc++ and
>>> libstdc++ will deploy a solution, which should suffice for us too. Until
>>> then, I don't see a reason to deprive ourselves of any potential benefits
>>> that the noexcept might bring.
>> 
>> What benefits?
> 
> TBH, it's actually very little. But little is not zero.

TL;DR: Given that the tradeoff is between two marginally and somewhat speculative benefits, it seems to me that a pragmatic (as opposed to dogmatic) application of the Lakos rule in the cases where Q_ASSERT is used to verify preconditions is sensible for us. Which in our governance model translates to maintainer privilege. In other words: if Thiago believes that making something noexcept is correct because a try/catch on the call site would be the completely wrong tool to deal with the mistake that the Q_ASSERT guards against, then that’s acceptable (and doesn’t mean that it cannot be discussed passionately in code review).


We seem to have a tradeoff between two minor benefits: on the one hand, better performance and binary size by marking functions that assert preconditions as noexcept. On the other hand, the somewhat speculative hope that, by making such noexpect(false), we can in some future benefit from improved testability, support for different error handling paradigms coming from maybe-C++26 contracts, or otherwise. I’d also throw in that by making a function noexcept, we tell users of the API that they don’t have to bother with a try/catch block around that particular call, and that try/catch in those cases are the wrong tool to guard against “stupid” errors, whatever those are (perhaps: errors that could be prevented by static analysis).

Since Lakos is referenced in this thread, two quotes from the paper [1] that resulted in P3155R0 [2], aka “The Lakos Rule”:

On page 5 of [1]:

> What Are We Proposing
> In order to support effective testing, compilers should offer two ‘modes’ for handling noexcept violations.
> 
> i/ a “production” mode, the default, which guarantees to terminate the process rather than allow an exception to propagate past a noexcept exception specification.
> 
> ii/ a “testing” mode, which performs regular stack unwinding if an exception propagates beyond a noexcept exception specification. This would imply disabling any optimizations based on static analysis of exception specifications, while ensuring that the noexcept operator continues to see the same result.

On page 8 of [1]:

> Preferred additional recommendation
> If the core language can be amended to support a testing mode, we recommend the following guideline:
> Each operation whose behavior is such that it clearly cannot throw, when called with arguments satisfying function preconditions (and its own object state invariants), should be marked as unconditionally noexcept.

The core language, as of today, has not been amended to support a “testing mode” (where, as per proposal, an exception propagating past a noexcept function would not terminate). However, Q_ASSERT as a macro that expands to nothing in a “production” mode build, seems to me to be very similar in spirit: we don’t assert in a release build, so we can guarantee that we don’t throw an exception in code like the referenced

       static Qt::strong_ordering compareThreeWay_helper(const Iterator &lhs,
                                                         const Iterator &rhs) noexcept
       {
           Q_ASSERT(lhs.item.d == rhs.item.d);
           return Qt::compareThreeWay(lhs.item.i, rhs.item.i);
       }

This function, when called with arguments satisfying function preconditions, cannot throw. In the “test” mode, aka “debug build”, it does whatever we want Q_ASSERT to do.

Making it possible for us to test the effectiveness of asserts, esp when they are used to implement a “contract”, is a good goal. Throwing an exception would be one way to do that, but not the only one. Fork’ing, or using an assert handler, or defining Q_ASSERT to spit out a qCritical that makes the test fail (via QTest::failOnWarning), are all possible ways to do that today already.

One can argue that comparing iterators from two different lists resulting in undefined behavior is not an unexpected issue; the behavior of comparing two iterators over different ranges is undefined [3] (unless the respective comparison operator has an explicit noexcept specification). Does the same logic apply for calling std::vector::operator[] with an index out of bounds? Is it material to the principle that the index can come from somewhere else, and that the vector is probably long-lived and might have changed after the boundaries were asserted? It is well-defined that calling std::optional::value on a nullopt throws an exception, while it is not defined what operator[] does (but it will be, once a C++26 contract is added to it).

So, somewhat marginal technical benefits aside, what “noexcept” in a library API communicates to the caller of the API is whether or not they should consider exception handling as a tool to make their code more resilient. We do need to make sure that users can handle problems gracefully so that their program becomes resilient against unexpected issues. We already make it close to impossible to use C++ exception handling to that end (ref recent thread [4]).


Volker

[1] https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2011/n3248.pdf
[2] https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p3155r0.pdf
[3] https://www.open-std.org/jtc1/sc22/wg21/docs/lwg-closed.html#446 - open and pending because other things are evidently more important :)
[4] https://lists.qt-project.org/pipermail/development/2024-May/045252.html