hare-specification: Require assertion messages to be translation-compatible v1 PROPOSED

Drew DeVault: 1
 Require assertion messages to be translation-compatible

 1 files changed, 2 insertions(+), 1 deletions(-)

I think that it has too many footguns and it isn't that useful. Out of
nearly 4,000 asserts and aborts in the stdlib there were, what, six of
them which needed to be fixed following this change? And I've heard

Joe Finney <me@spxtr.net>

3 months ago

To be fair, this feature was only recently-ish added.

*several* people shooting themselves in this particular foot before. I'd
rather make the change to simplify it. IMO it's bad practice to spend
cycles and/or memory generating assertion failure messages anyway, since
in theory, you know, assertions aren't supposed to fail and it's a waste
of time and/or resources.

Bor Grošelj Simić <bgs@turminal.net>

3 months ago

Yeah I agree. This is part of why we need the builtin to work like
that - it is the only way to conditionally generate the error message
without resorting to if expressions.

The footgun here was that stack-allocated strings don't work with
assert/abort in tests. Are there any others, and would it be too
expensive to fix the test runner to save the message before the stack is
lost?

Is there a reason to not print stack traces when tests abort, regardless
of this patch?

IMO that's a broader problem with defer, not assert. You can replace
the assert with fmt::fatal and it will have the same issue. Your point
still stands though, the problem wouldn't happen with assert after your
patch.

I don't feel very strongly either way wrt this patch, so I'll check out
and let others chime in. It's probably good to note that this is a
breaking change in one of the commit messages.

[PATCH hare-specification] Require assertion messages to be translation-compatible Export this patch

Signed-off-by: Drew DeVault <sir@cmpwn.com>
---
 language/expressions.tex | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/language/expressions.tex b/language/expressions.tex
index 86b9c4d..e889877 100644
--- a/language/expressions.tex
+++ b/language/expressions.tex
@@ -614,7 +614,8 @@ result type of these forms is \terminal{void}.
 In the second \terminal{assert} form, and in the \terminal{abort} form if
 present, the final \nonterminal{expression} shall have type \terminal{str},
 which shall be provided to it as a type hint. The contents of the string shall
-be included in the diagnostic message.
+be included in the diagnostic message. The final \nonterminal{expression} shall
+utilize the \secref{Translation compatible expression subset}.
 
 \specsubsubitem
 In the \terminal{abort} form, the execution environment shall unconditionally
-- 
2.47.1

builds.sr.ht <builds@sr.ht>

3 months ago

hare-specification/patches/.build.yml: SUCCESS in 2m50s

[Require assertion messages to be translation-compatible][0] from [Drew DeVault][1]

[0]: https://lists.sr.ht/~sircmpwn/hare-dev/patches/56681
[1]: mailto:sir@cmpwn.com

✓ #1398535 SUCCESS hare-specification/patches/.build.yml https://builds.sr.ht/~sircmpwn/job/1398535

Joe Finney <me@spxtr.net>

3 months ago

This is a somewhat useful feature for table-based tests, where one
assert statement accounts for many test cases. Without this, you don't
know which test case triggered the assert.

I think it's all good if we add assert fns to fmt::

	fn assert(cond: bool, args: formattable...) void = {
		if (cond) {
			error(args);
			os::exit(255);
		};
	};

and similar for an assertf. Only problem is the keyword name "assert".
Any suggestions for a better name? "fatalif/fatalfif"?