gsl::not_null can't be anymore zero cost abstraction.

[unixod]

Hi,

Some time ago, until the PR #831 was accepted, GSL had 3 configuration options:

• GSL_TERMINATE_ON_CONTRACT_VIOLATION: std::terminate will be called (default)
• GSL_THROW_ON_CONTRACT_VIOLATION: a gsl::fail_fast exception will be thrown
• GSL_UNENFORCED_ON_CONTRACT_VIOLATION: nothing happens

For some unclear reason, in #831 these options were removed.

Removal of those options led us to the following not good consequences:

• Expects(cond) can't be configured anymore: now its usage in different parts of GSL always implies runtime checks and calling std::terminate in case of failures, which in turn implies =>
• gsl::not_null allways checks in runtime for nullptr, which in turn implies =>
• gsl::not_null can't be configured to be zero cost abstraction anymore for example in release builds, , which in turn implies =>
• using gsl::not_null in hot parts of code may cause performance issues.

So, my proposal: let's return back the GSL_UNENFORCED_ON_CONTRACT_VIOLATION, or something else which will allow clients to make Expects(cond)/Ensures(cond) zero cost.

[gdr-at-ms]

@unixod Could you elaborate further on what you meant by Expects and Ensures being zero cost?

[unixod]

@unixod Could you elaborate further on what you meant by Expects and Ensures being zero cost?

Sure,

I think the easier way to explain my point is to show it in example: https://godbolt.org/z/grfjgv

The following is explanations.

We have a function:

#include <gsl/gsl>

int foo(gsl::not_null<int*> ptr)
{
    return *ptr;
}

and output of two compilation processes (both use -O3 and -DGSL_UNENFORCED_ON_CONTRACT_VIOLATION):

GSL 2.0.0	GSL 3.0.0
foo(gsl::not_null<int*>): mov eax, DWORD PTR [rdi] ret	foo(gsl::not_null<int*>): test rdi, rdi je .L7 mov eax, DWORD PTR [rdi] ret .L7: sub rsp, 8 call std::terminate()

The result assembly generated for old version of GSL is identical to accessing raw pointer (https://godbolt.org/z/hYtxg8), i.e. doesn't introduce any overhead, while new GSL leads to generation of additional runtime checks and branching instructions. I didn't find a way to eliminate these instructions for release builds.

I'd like to return to GSL the configuration option which would allow its clients to eliminate all redundant checks from release builds if it is necessary (as it is done for example for assert(s)). This possibility existed before #831, and had been removed after it.

[aforster]

This is very strange behaviour. As far as I can see there is no way to create a gsl::not_null with a nullpointer. So that means all accesses to get() are safe.

[jbonyun]

As a developer in a low-latency context, I'm firmly on the side of zero-cost, and it's frustrating that I will have to copy not_null into my own namespace and maintain my own version. Especially since zero-cost is one of the stated goals of the Guidelines that the gsl is supporting.

It looks like the zero-costness of not_null was also considered in #674 and #675. At the time it was decided that making it work in perfect safety with unique_ptr was more important than having zero-cost. I didn't follow all the details there, but if not_null<unique_ptr> is almost-but-not-quite-always-guaranteed-to-be-not-null, that seems like a small price to pay for making every other not_null zero-cost. Maybe the developers involved in those pull requests could comment? That's @annagrin and @neilmacintosh, I think.

Adding back the GSL_UNENFORCED_ON_CONTRACT_VIOLATION macro would be nice. Or triggering off of NDEBUG would also work well for me. Or a not_null-specific macro. Or really any way in which my release builds don't have to check a pointer on every use.

[runer112]

Hi,

Most of this discussion has been about GSL_UNENFORCED_ON_CONTRACT_VIOLATION being an important option as a zero-cost abstraction for time-critical code. If consideration is being made to add it back, I'd like to add that I believe it's important that GSL_THROW_ON_CONTRACT_VIOLATION be added back, as well.

I work on a software project that has many responsibilities in a safety-critical system. I'd really like to introduce GSL to the project for its features like not_null. Currently, such checks have to be written manually, but developers (myself included) are often lazy and don't, or simply don't recognize that a check is needed. Introducing not_null could more easily ensure that the necessary checks are done with little to no extra burden on developers.

However, the current GSL behavior of the entire process terminating due to a single error in logic is unacceptable. The proposed revival of skipping checking for errors, so if an error occurs it may "spread" and incidentally be caught at a later time, is also unacceptable. Instead, errors need to be detected early and quarantined to the smallest subsystem that introduced the error, which can then be reported, safely disabled, and potentially repaired/restarted. Exceptions are a great tool for implementing such behavior.

So if considering adding back GSL_UNENFORCED_ON_CONTRACT_VIOLATION, please include GSL_THROW_ON_CONTRACT_VIOLATION as well.

[hsutter]

@runer112 Thanks. Throwing contract violation handlers are a deeply discussed topic, and one key consideration is that if we allow throwing contract violation handlers then we could never put a contract on a noexcept function. That's generally viewed as not workable... whether or not a function can encounter runtime failures that a call site could programmatically fix (by sending an error code/exception to the call site) is independent of whether a function has a bug that put the program in an unexpected state that is generally needs to be fixed by a source code change (by reporting a contract violation to the human programmer at test time).

[runer112]

@hsutter Thanks for the (quick!) response.

I hadn't considered the interaction of contracts and noexcept. It seems that there's a lot more complication to this when thinking about it more deeply.

I think that what I'm really looking for regarding not-null can be seen in narrow. This takes the "wide" contract approach, specifying that if the input isn't exactly representable in the output type, an exception is thrown. This is exactly the type of contract validation I'd like to see for not-null. The only significant distinction is that the output type of narrow is tautologically guaranteed to hold a value that is exactly representable in the output type, while not-null needs a marker type to guarantee that the output value is not null. But otherwise, narrow and the desired not-null type and are fundamentally very similar.

To further draw on the parallel with narrow, it has a noexcept alternative, narrow_cast. These two functions serve to validate the same contract, but handle violations in different ways. And both have their place. I think it may equivalently be the case that there should be different not-null implementations to satisfy different contract validation needs. I'd leave it as an implementation question/choice whether to achieve this differentiation with tagged constructors, a template parameter on the type, or a separate type entirely.

This wide-contract approach to not-null checking may somewhat move it away from "compiler/language support library" and towards "general utility library." But if it fits closely enough (like narrow), I'd love to see it in GSL.

Late edit: It just occurred to me that what started out seemingly closely related to the initial issue (contract violation configuration defines) has diverged. Should this be forked off into its own issue?