GNU bug report logs - #75648
Minor safety improvements to fns.c/eval.c

Previous Next

Full log

View this message in rfc822 format

From: Pip Cet <pipcet <at> protonmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Andrea Corallo <acorallo <at> gnu.org>, 75648 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>, Stefan Kangas <stefankangas <at> gmail.com>
Subject: bug#75648: Minor safety improvements to fns.c/eval.c
Date: Sat, 18 Jan 2025 17:10:02 +0000

"Michael Albinus" <michael.albinus <at> gmx.de> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> Hi,
>
>>> Patch 1 adds :crash tests to ert.el.
>>
>> Hmm... maybe this should be discussed separately, as it's a much
>> broader issue.  Can you provide the motivation, both for the new macro
>> (which is basically a one-liner), and for the new tag?
>
> This patch adds the :crash tag to ert.el. This is unfortune. Until now,
> there are no predefined tags in ert.el. Do we want to change this?

It's pretty clear you don't, so let's see what else we can do:
should-not-crash can print a message, but needs to somehow be able to
access the ert test for that message to make sense.  My naive idea is to
use a dynamic let-binding for a function to (fun)call to print a
message, would that work?  We can't do what ert-fail does (throws a
signal), IIUC, but we can make it look like an ert-fail call.

> If we need such a property for ert tests, we should use another
> mechanism. For example, another slot in the defstruct ert-test, like
> (should-not-crash nil). And your new macro should-not-crash sets this
> slot to non-nil in its scope. With this, you can even declare more
> precisely, which parts of your ert deftest shall be protected.

Hmm.  I don't see how that would work.  I'll have to study the code some
more.

> Furthermore, IIUC, your new macro succedds always if the form doesn't
> crash. So we have no information, whether the form returns nil, non-nil,
> or an error, in case it doesn't crash. I would still like to see the
> test result from should-not-crash in this case.

In this case, that's intentional: it's perfectly okay to do anything but
crash or infloop, and we really don't want to have to update the tests
for every behavior change.

I haven't thought much about the case where we want to assert both that
a form doesn't crash and which value it has.  My intuition is that it's
best to have separate tests.  Admittedly that's a little easier if
should-not-crash returns the value of the form.

Pip

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.