CppCoreGuidelines icon indicating copy to clipboard operation
CppCoreGuidelines copied to clipboard

Error Handling with Core Guidelines GSL Expects, Ensures, and narrow_cast

Open pwm1234 opened this issue 5 years ago • 11 comments

I am trying to follow the Cpp Core Guidelines and use GSL where appropriate. In particular, I would like to use Expects and Ensures for pre and post-conditions, as well as span, and narrow_cast, but the error handling is not robust and does not provide any diagnostics. When encountering errors GSL simply calls terminate leaving the tester with no clues as to what caused the termination.

So my question is: How does one use GSL and keep code robust in the presence of errors? Or more simply, how to use GSL and get error diagnostics prior to termination?

(I posted this same question on StackOverflow, https://stackoverflow.com/questions/63350483/error-handling-with-core-guidelines-gsl-expects-ensures-and-narrow-cast, but it has not received any feedback.)

I appreciate any help.

pwm1234 avatar Aug 12 '20 20:08 pwm1234

That question was brought up in some form at https://github.com/microsoft/GSL. I think the answer was as seen in this comment https://github.com/microsoft/GSL/pull/831#discussion_r359992776, which can be summarized as "setting your own termination handler to provide your own diagnostics".

JohelEGP avatar Aug 12 '20 23:08 JohelEGP

This issue sounds like a duplicate of #1561

shaneasd avatar Aug 13 '20 01:08 shaneasd

Certainly my question is similar to #1561 and some of the discussion on GSL, but I think this question is also different. I am not debating the pros/cons of using exception handling or not. I am simply asking if I have Expects(i < j) and i is not less than j. how can I get a message about the values of i and j before termination? Setting up my own termination handler does not address my need, as all I can do in a termination handler is print a message that termination will happen next. I do not know how to print any reason for the termination. Perhaps this is a duplicate of #1561 but I simply do not understand the language nuances well enough to apply that discussion to my question.

And if this question is that hard to answer, then how are non-expert C++ programmers to effectively use GSL? When I say this, I am not trying to be contrary or provacative. I appreciate the effort that has gone into these guidelines and GSL, but I honestly do not understand how a non-expert C++ programmer is supposed to follow the guideline of use Expects to check preconditions when a violated Expects call simply results in termination with no clue as to why termination occurred.

pwm1234 avatar Aug 13 '20 04:08 pwm1234

I mislinked the comment: https://github.com/microsoft/GSL/pull/831#discussion_r359992776.

In the original discussion I refer to (which I can't find and is not the linked comment), I remember a recommendation to print the stack trace.

JohelEGP avatar Aug 13 '20 05:08 JohelEGP

Please also compare https://github.com/microsoft/GSL/pull/761.

beinhaerter avatar Aug 13 '20 15:08 beinhaerter

So perhaps my question is a follow-on to 761, in which @hsutter said:

C++ Core Guidelines editors' call: This seems to be asking for a debugging aid that can be expensive in release mode. It could be added in debug mode only, and then only for projects that are not supposed to change their dependencies between debug and release modes (which seems undesirable because it limits use of GSL in such projects). Better would be to use the debugger to view the information if an exception is thrown, such as by setting a breakpoint in the implementation of narrow.

That response might have been reasonable for that merge request, which did involve bringing a lot of baggage to handle a narrowing error.

But my question is: so how do I use GSL in production software and provide reasonable feedback when a termination error has occurred? And certainly, "using the debugger" is not a viable answer. But neither is a silent termination nor a not silent termination that simply says "a termination error has occurred." At the very least, if I have Expects(i < j), I would like to be able to show a stacktrace; ideally I would like to include information about the values of i and j. Without some way to intercept the termination, I do not see the practicality of using these GSL constructs, which seems out of line with the Core Guidelines that seems intent on being practical.

Again, note that I am not casting stones at GSL. I am not a language expert and am not trying to debate exception versus termination. I am simply trying to understand how I can use GSL but avoid a silent and unhelpful termination when a debugger is not (often) available.

pwm1234 avatar Aug 13 '20 22:08 pwm1234

I think this touches on the general argument if contract checks are a pure development and maybe security feature, or more generally a way to harden your code against bugs.

I think the short answer is that Ensures/Expects from the GSL are currently little more than glorified asserts, that don't give you what you want. Maybe gsl could learn something from the implementation of the CHECK macro in Catch2, which can actually decompose simple expressions like CHECK(a == b) and then print the values for a and b and not just "a == b evaluates to false".

Technical aspects aside, I somewhat question the value of showing failed assertions to the end user. It is very likely that they have no meaning whatsoever to them. At most, a user might want to know what part of the program cause the crash.

MikeGitb avatar Aug 14 '20 09:08 MikeGitb

We agree that showing failed assertions to the end user is not valuable. But having something (a stacktrace with a message) that the user can send back, is infinitely better than simply "your software just crashed."

But even if contract checks are a pure development, wouldn't development be aided by having some context for the reason for termination? I am using MSVS 2019, and an Expects failure is only helpful when running in the debugger. I cannot attach a Debugger to a failed contract check. And this is especially bad for some applications or library code where debug mode is simply too slow to use. (Debug can sometimes be two orders of magnitude slower, so I often develop using Release with symbols instead of Debug.)

Yes, if Catch2 and Boost.Test and ... can provide useful information about the failed predicate why can't GSL?

Let me again say I am not trying to force upon GSL something it is not meant to do. I am trying to understand how, if at all, I should use GSL in my delivered software. And the discussion (and @hsutter comment about using the debugger and setting a breakpoint) seems to indicate that GSL is meant as a development library and should not remain in production code, which I believe is the first side of the general argument (pure development versus general hardening). If that is the case, then how does one remove GSL from production code? It seems strange that a library as capable as GSL is meant to be ifdef'ed out at the end of development.

pwm1234 avatar Aug 14 '20 14:08 pwm1234

And this is especially bad for some applications or library code where debug mode is simply too slow to use. (Debug can sometimes be two orders of magnitude slower, so I often develop using Release with symbols instead of Debug.)

Didn't you just give yourself the answer: You can absolutely run your system under a debugger, even if compiled in release mode. A stacktrace won't give you anything beyond what the debugger gives you.

It seems strange that a library as capable as GSL is meant to be ifdef'ed out at the end of development.

GSL is definetly not meant do be ifdefed out. But there is an ongoing discussion around contract checks in particular (not only the macros Ensures and Expects from the gsl, but also the language proposal), if they should be deactivated in release mode, just like asserts.

MikeGitb avatar Aug 14 '20 14:08 MikeGitb

But I am trying to get away from manually running in the debugger and use more automated unit and functional tests, which are not run in the debugger. (Plus, most of the time, the interesting errors happen when you are not expecting them.) Is there a way on windows and MSVS 2019 to automate running a command line app or test in the debugger? If so, then, yes, you have a valid point.

pwm1234 avatar Aug 14 '20 14:08 pwm1234

Perhaps the easiest way of getting better diagnostics for precondition/postcondition checks would be to implement them with assert(), for which the standard states the following requirements (emphasis mine):

If NDEBUG is not defined, then assert checks if its argument (which must have scalar type) compares equal to zero. If it does, assert outputs implementation-specific diagnostic information on the standard error output and calls std::abort. The diagnostic information is required to include the text of expression, as well as the values of the standard macros __FILE__, __LINE__, and the standard variable __func__ (since C++11).

I implemented that for gsl-lite with the gsl_CONFIG_CONTRACT_VIOLATION_ASSERTS configuration option in https://github.com/gsl-lite/gsl-lite/pull/294. Preferably, I'd make this the default behavior, although unfortunately it wouldn't fully comply with the Core Guidelines, cf. https://github.com/isocpp/CppCoreGuidelines/issues/1512#issuecomment-540729216.

mbeutel avatar Mar 29 '21 10:03 mbeutel