Bug 17226 – Exception during the generation of an assert message hides AssertError

The following program fails an assertion. However, not the more important AssertError but a measly Exception is thrown instead: import std.string; void foo(int i) { // In this case a %s is forgotten but it could be any other trivial error. assert(i == 42, format("Bad parameter:", i)); } void main() { foo(43); } Output: std.format.FormatException@ [...] Orphan format arguments: args[0..1] I think this is a critical issue because the program could have caught the Exception and continue unawares of the most important AssertError. Ali

Full example of catching the exception and still going import std.string; import std.stdio; void foo(int i) { // In this case a %s is forgotten but it could be any other trivial error. assert(i == 42, format("Bad parameter:", i)); } void main() { try { foo(43); } catch (Exception) { writeln("something threw"); } writeln("This still runs despite being in undefined state"); } Yeah, this is really, really bad.

Ah, I'd just been wondering why an assert message eats its location information. Come on, come on :)

Not sure if this is a dmd error. I think this might be druntime related :-?

We also encountered this problem. We can't find out which statement is wrong, just get an error message.

Associated discussion about this: https://forum.dlang.org/thread/[email protected]

Just use compile-time format string checking:         assert(myAssumption, format!"%s doesn't work!"(blah)); This produces a compile error:         string s = format!"blah %d blah"(123.45); Problem solved.

To be specific, the issue here is that according to the language spec: > The first AssignExpression must evaluate to true. If it does not, an > Assert Failure has occurred and the program enters an Invalid State. > > [...] > > Undefined Behavior: Once in an Invalid State the behavior of the > continuing execution of the program is undefined. i.e., once the condition has been evaluated to false, continuing to execute is undefined *regardless* of what happens in the evaluation of the message. Perhaps the simplest way to fix this is to have assert(condition, message) evaluate the message *first*, so that Ali's example has behavior equivalent to the following code: --- import std.format; void foo(int i) { auto __msg = format("Bad parameter:", i); assert(i == 42, __msg); } void main() { foo(43); } --- This way, if the message expression throws, the assert's condition is never evaluated, and the program does not enter an invalid state.

(In reply to Paul Backus from comment #7) > Perhaps the simplest way to fix this is to have assert(condition, message) > evaluate the message *first* No. Changing the evaluation order would have a deep impact: * Side effects of evaluating the msg first might change the assert condition's outcome. * You don't want a potentially expensive msg expression to be evaluated unconditionally, i.e., including the regular case where the assertion holds. * The msg expression may depend on the state after a failed condition - e.g., `assert(x.trySomething(), x.getLastErrorMsg())` (horrible, I know...).

I think the "invalid state" ship has sailed here because a D program by default dumps backtrace to stderr. Can that always work? No, but I think we accept it as best-effort. I think the same applies to the assert message: A best-effort to show something meaningful. May work but it should not hide the actual issue.

(In reply to Paul Backus from comment #7) > if the message expression throws, the assert's condition is never > evaluated, and the program does not enter an invalid state. Wait a minute... :) It shouldn't be the assert expression itself that puts the program into invalid state. assert throws because the program is already in an invalid state. Removing the evaluation of the assert expression does not change that fact. If we accept that point, then even attempting to evaluate the assert expression is best-effort when the program is in an invalid state. That and my earlier observation about attempting to dump call stack proves we are already in a best-effort business when the program is in invalid state.

As Timon pointed out, requiring the evaluation of the assert msg to be nothrow is a limitation, I don’t think it is a major limitation. `assumeWontThrow` [1] is well-suited for this purpose. I don’t think that ```d import std.exception : assumeWontThrow; assert(condition, assumeWontThrow(potentially throwing expr)); ``` is too much to ask for. It documents clearly that the message-generating expression could throw. That way, an Exception thrown by the message-generating expression does not hide a bug. [1] https://dlang.org/phobos/std_exception.html#assumeWontThrow

What about creating a wrapper for format: ```d string assertFormat(Args...)(string fmt, Args args) { try { return format(fmt, args); } catch(Exception ex) { return "Error while evaluating assert message: " ~ ex.toString; } } // use like assert(i == 42, assertFormat("Something %s", functionThatThrows())); ``` This solves the problem, and also doesn't throw away any information. Plus it's not unpleasant to use. Yes, it's opt in. So what? It can just be a best practice recommendation.

(In reply to Steven Schveighoffer from comment #12) > What about creating a wrapper for format: > > ```d > string assertFormat(Args...)(string fmt, Args args) > { > try { > return format(fmt, args); > } catch(Exception ex) { > return "Error while evaluating assert message: " ~ ex.toString; > } > } > > > // use like > assert(i == 42, assertFormat("Something %s", functionThatThrows())); > ``` > > This solves the problem, and also doesn't throw away any information. Plus > it's not unpleasant to use. > > Yes, it's opt in. So what? It can just be a best practice recommendation. alias format = assertFormat; /* import std.format;//*/ auto foo(T)(T P) {  //assert(P == 43, format("T: %s", T.stringof));/* static assert(is(T == uint), format("T:", "int"));//*/ } > onlineapp.d(6): Error: static assert: "Error while evaluating assert message: FormatException@/dlang/dmd/linux/bin64/../../src/phobos/std/format/package.d(785): Orphan format arguments: args[0..1]" > onlineapp.d(13): instantiated from here: `foo!int` I think the problem is also with static. Because above is static assert I got at compile time. But I got such 4 errors message with the following library possibilities (without assertFormat): > /dlang/dmd/linux/bin64/../../src/phobos/std/exception.d(518): Error: uncaught CTFE exception `std.format.FormatException("Orphan format arguments: args[0..1]")` > onlineapp.d(6): called from here: `format("T:", "int")` > onlineapp.d(6): Error: static assert: __error > onlineapp.d(13): instantiated from here: `foo!int` //alias format = assertFormat; /* import std.format;//*/ auto foo(T)(T P) {  //assert(P == 43, format("T: %s", T.stringof));/* static assert(is(T == uint), format("T:", "int"));//*/ } import std.stdio; void main() { enum char D = 68; assertFormat("Hello %s and World", D).writeln; foo!int(42); } string assertFormat(Args...)(string fmt, Args args) { import std.format : _format = format; try { return _format(fmt, args); } catch(Exception ex) { enum m = "Error while evaluating assert message: "; return m ~ ex.toString; } } SDB@79

THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/17791 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB