bug#20066: 24.4; should-error and cl-assert

bug#20066: 24.4; should-error and cl-assert

[Leo Liu]

I just compiled emacs 25.0.50 from git and I have immediately noticed
the following difference:

1. (should-error (error "yes"))
2. (should-error (cl-assert nil t))

Test 1 succeeds but test 2 fails. In emacs 24.4, both tests succeed.
Could someone investigate what have caused this incompatibility?

Thanks,
Leo

bug#20066: 24.4; should-error and cl-assert

[Wolfgang Jenkner]

On Mon, Mar 09 2015, Leo Liu wrote:

> I just compiled emacs 25.0.50 from git and I have immediately noticed
> the following difference:
>
> 1. (should-error (error "yes"))
> 2. (should-error (cl-assert nil t))
>
> Test 1 succeeds but test 2 fails. In emacs 24.4, both tests succeed.
> Could someone investigate what have caused this incompatibility?

commit 61b4c22c6eba96718327a0d208a8492d8bad76e0
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date:   Sat Feb 14 00:46:29 2015 -0500

[...]
    (cl--assertion-failed): New function.

+(defun cl--assertion-failed (form &optional string sargs args)
+  (if debug-on-error
+      (debug `(cl-assertion-failed ,form ,string ,@sargs))


So, `debug' is explicitly called now, which circumvents the normal
call-chain: signal > maybe_call_debugger > call_debugger > debug

bug#20066: 24.4; should-error and cl-assert

[Leo Liu]

On 2015-03-09 16:06 +0800, Leo Liu wrote:
> I just compiled emacs 25.0.50 from git and I have immediately noticed
> the following difference:
>
> 1. (should-error (error "yes"))
> 2. (should-error (cl-assert nil t))
>
> Test 1 succeeds but test 2 fails. In emacs 24.4, both tests succeed.
> Could someone investigate what have caused this incompatibility?
>
> Thanks,
> Leo

Any comments on this bug?

bug#20066: 24.4; should-error and cl-assert

[Nicolas Richard]

Leo Liu <sdl.web@gmail.com> writes:
> Any comments on this bug?

Hi,

I can't seem to reproduce :

 $ src/emacs --version
GNU Emacs 25.0.50.1
Copyright (C) 2015 Free Software Foundation, Inc.
GNU Emacs comes with ABSOLUTELY NO WARRANTY.
You may redistribute copies of GNU Emacs
under the terms of the GNU General Public License.
For more information about these matters, see the file named COPYING.
 $ src/emacs --batch -l ert --eval '(should-error (cl-assert nil t))'; echo $?
0
 $ src/emacs --batch -l ert --eval '(should-error (error "yes"))'; echo $?
0
 $ src/emacs --batch -l ert --eval '(should-error nil)'; echo $?
Test failed: ((should-error nil) :form nil :value nil :fail-reason "did not signal an error")
255

-- 
Nicolas Richard

bug#20066: 24.4; should-error and cl-assert

[Nicolas Richard]

Nicolas Richard <theonewiththeevillook@yahoo.fr> writes:
>  $ src/emacs --version
> GNU Emacs 25.0.50.1

Forgot to mention I was at commit
7c691f32f78303750d29972a29dcc6754fae257a from a few days ago.

Nico.

bug#20066: 24.4; should-error and cl-assert

[Stefan Monnier]

>> 1. (should-error (error "yes"))
>> 2. (should-error (cl-assert nil t))
>> Test 1 succeeds but test 2 fails.

I think test 2 is incorrect.  It assumes that when cl-assert fails it
reacts by throwing a "normal" signal that can be caught like any other.
This is untrue for example when we decide to compile-away the assertions
(depending on speed ans safety settings).

cl-assert should be used to check impossible situations and an assertion
failure is just supposed to bring it to the user's attention to help
debug the code.  It is not meant to be caught&handled like a normal
signal.


        Stefan

bug#20066: 24.4; should-error and cl-assert

[Leo Liu]

On 2015-04-08 10:05 +0800, Stefan Monnier wrote:
> I think test 2 is incorrect.  It assumes that when cl-assert fails it
> reacts by throwing a "normal" signal that can be caught like any other.
> This is untrue for example when we decide to compile-away the assertions
> (depending on speed ans safety settings).
>
> cl-assert should be used to check impossible situations and an assertion
> failure is just supposed to bring it to the user's attention to help
> debug the code.  It is not meant to be caught&handled like a normal
> signal.

OK, that makes sense.

Leo